UQComputingSociety / uqcsbot-discord

:mortar_board: UQCSbot: Our friendly little Discord bot
https://discord.uqcs.org
MIT License
20 stars 20 forks source link

Fix admin/committee ping permissions, fix timezone issues #119

Closed LimaoC closed 1 year ago

LimaoC commented 1 year ago

This should fix an issue with members not being recognised as administrators (also it now checks for the "Committee" role rather than administrator perms). The datetime/timezone issues should hopefully also be fixed now too.

andrewj-brown commented 1 year ago

As far as perms go, and this is purely personal preference, I'd like bot stuff to be based off actual specific permissions, not role names - i.e. the starboard perm checks aren't "do you have the committee role", they're "do you have Manage Server / Manage Messages". I'd suggest tying this to the "Mention @\everyone" permission, rather than searching for a named committee role.

LimaoC commented 1 year ago

Making the perms role-based was an arbitrary choice so I'm happy to change them to be based on guild permissions.

AFAIK manage_guild permissions are given to the infra role as well, but I do think there are cases where having commands be committee-only are necessary (e.g. drawing advent of code winners). I suppose in that case we can tie those commands to some other permission that's only given to committee.

andrewj-brown commented 1 year ago

Making the perms role-based was an arbitrary choice so I'm happy to change them to be based on guild permissions.

* Bypassing reminder limit will require `manage_guild` permissions

* Mentioning other users/roles in reminders will require `mention_everyone` permissions

AFAIK manage_guild permissions are given to the infra role as well, but I do think there are cases where having commands be committee-only are necessary (e.g. drawing advent of code winners). I suppose in that case we can tie those commands to some other permission that's only given to committee.

Yep, manage_guild is the only perm that Infra currently has over @\everyone (IIRC, I can't actually check, on account of "don't have manage_roles to confirm").

The breakdown of bot perms (pre-this-PR) is:

As far as a generic, committee-but-not-infra perm goes (to use for the reminder limit in this PR, or drawing AoC winners), I'd suggest manage_events.

LimaoC commented 1 year ago

Sounds good to me