Bare Minimum Admin Features#13
Conversation
* Feat: add lint for PRs * Feat: workflow - lint * Fix: Ruff linter checks * Fix: Format with ruff * Remove duplicate load_env (fixed by sync call in main) * Fix: workflow name - autofix support
…o create new event, queues claim tickets into channel specified during event creation
|
Fix lint + typecheck please 🙏 |
| "eventId;year": event_year_key, | ||
| "eventId": event_name, | ||
| "eventRoleId": role.id, | ||
| "mentorRoleId": role.id, |
There was a problem hiding this comment.
Should refer to mentor_role, not role. This worked while testing because everyone had the admin role so there was no difference
| @app_commands.command(name="admin", description="Test bot responsiveness") | ||
| async def overview(self, interaction: discord.Interaction): | ||
| """Simple ping command to test if the bot is responding.""" | ||
| await interaction.response.send_modal(ConfigCreateModal(None)) |
|
|
||
| try: | ||
| await db.create(event_item, TICKET_EVENTS_TABLE) | ||
| except Exception: |
There was a problem hiding this comment.
This print statement is not helpful to us and also provides no feedback to the user. You should send an ephemeral failure message and also log the actual error
except Exception as e:
print("could not save event", e)
await interactions ... # send message| async def get_claim_channel_id(eventId: str, year: int) -> int | None: | ||
| """Checks biztechTicketsEvents table for event's claim channelId""" | ||
| try: | ||
| event_item = await db.get_one_custom({ |
There was a problem hiding this comment.
query could succeed but if the key doesnt exist in the table then it will return a None object (of which what you've written below cannot call pythons _get_item() implementation
| event_item = await db.get_one_custom({ | ||
| "TableName": TICKET_EVENTS_TABLE, | ||
| "Key": { | ||
| "eventId": {"S": eventId}, |
There was a problem hiding this comment.
have you already created the table? I would probably like you to recreate it to be preferable since the tables are different (eventID is normally how the things are capitalized)
| "eventId;year": {"S": f"{eventId};{year}"} | ||
| } | ||
| }) | ||
| return int(event_item["claimChannelId"]["N"]) |
There was a problem hiding this comment.
once again if it returns an empty item (which it could) this will cause the bot to panic/restart
| from botocore.exceptions import ClientError | ||
|
|
||
| from lib.constants import TICKETS_TABLE | ||
| from lib.constants import TICKET_EVENTS_TABLE |
…ded None checks, deleted unused imports
1fa5940 to
513191c
Compare
…eatures Mergin elijahs code
elijahzhao24
left a comment
elijahzhao24
left a comment
There was a problem hiding this comment.
This current Pr only allows for admins to set a custom channel for the incoming tickets. I think we should hold off for now merging this until the rest of the functionality is finished.
I think there is some other flows that should be changed in this command. Category, channel, and role creation should be handled on the server side, and not first manually by the client, and then passed to the server. Not only does this make this cooked if there is a typo, but it just introduces more error as you are maintaining two sources of truth on creation, when it could just all be handled on the server side. This should also include creating the roles, and allowing mentor roles in certain channels.
(Note: doing this would also still work with your current flow, as you would only create a new category or channel if it doesn't already exist)
In addition, this command should for setting up what channels you can use the bot in. By default we can just block use of the bot unless your an admin on any channel, but on the new event creation, we will allow the use of the bot on those channels.
Make sure to fully integrate your changes as well so there is not two sources of truth, as in the /delete flow, we don't currently query your db for the incoming ticket channel id.
| return | ||
|
|
||
| ticket = update_response.get("Attributes", {}) | ||
| # db.update_db() returns attributes directly, not a DynamoDB response envelope. |
3 participants
Still need to implement:
Demo Video:
https://imgur.com/a/lDbFxDj