Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Bot Admin-only functionality #12

Closed
wants to merge 17 commits into from

Conversation

someyuck
Copy link

This PR resolves 4 tasks out of the 6 listed in the issue.

  1. Read discord IDs of admins that must be set in .env file.
  2. Add a check function that checks if a given member ID is a bot admin.
  3. Make backend_info an admin command.
  4. Make query/role admin-able commands so that admins can run it outside the "academic server" constraint.

I've also added docstrings for the functions defined, and updated the module docstring with those functions.

@someyuck someyuck changed the title Implemennt Bot Admin-only functionality Implement Bot Admin-only functionality Apr 13, 2024
bot/main.py Outdated

if server_config is None:
return False
if not is_bot_admin(str(user_id)):
raise CheckFailedException("is_academic_or_bot_admin")
return server_config.get("is_academic", False)
Copy link
Member

Choose a reason for hiding this comment

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

The is_bot_admin block will not run in this codepath (when server_config is not None), it would be needed here as well

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes, I'll fix this.

bot/main.py Outdated
@@ -45,12 +53,22 @@
MONGO_DATABASE = os.getenv("MONGO_DATABASE")
MONGO_URI = os.getenv("MONGO_URI")
BASE_URL = os.getenv("BASE_URL")
BOT_ADMINS = os.getenv("BOT_ADMINS").split(",")
Copy link
Member

Choose a reason for hiding this comment

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

Just a preference, I'd like to store this as a list of ints instead. So that checks are more convenient and we don't have to convert id to str everytime

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I'll add that.

if (
isinstance(error, commands.CheckFailure)
and isinstance(error.original, CheckFailedException)
and error.original.check_name == "is_author_bot_admin"
Copy link
Member

Choose a reason for hiding this comment

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

In my testing, this codepath isn't being triggered. So the thing is the .original attribute does not contain the original error raised.

@ankith26 ankith26 force-pushed the master branch 2 times, most recently from 8eb9229 to dd96fe2 Compare May 13, 2024 12:20
@someyuck someyuck closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants