Skip to content
This repository has been archived by the owner on Jun 28, 2023. It is now read-only.

Add MinimumPowerLevel option to clients #357

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Half-Shot
Copy link
Contributor

This change adds an option to specify the minimum PL level of the inviting user for the bot to remain joined to the room. This change will only affect services which accept invites to rooms, so services like GitHub need a different change.

Copy link
Member

@jaywink jaywink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments. Some documentation updates would be needed as well imho.

return
}
var pl = plContent.Users[event.Sender.String()]
if pl == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this is wrong - the user could have a PL of 0 set explicitly, and afaict in that case the PL should not revert to UsersDefault. It should revert to UsersDefault if the user has not explicitly been set any power level.

However I'm not sure in this context does plContent.Users have all users or just those who have a PL set? If the latter, wont this whole thing crash since there will be nothing in the map for that user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some learning of go, and doing aMap["foo"] will return two values. The first is the value, and the second is whether the value existed in the map.

So in this case, I can do var pl, userExists and if userExists is false, we fall back to the default PL.

clients/clients.go Show resolved Hide resolved
clients/clients.go Show resolved Hide resolved
@Half-Shot Half-Shot self-assigned this Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants