feat: complete webhook and add auto assign commands - #260
Conversation
| ) | ||
| @app_commands.describe( | ||
| event_id="UUID of event", | ||
| hacker_role="Discord role to assign to hackers" |
There was a problem hiding this comment.
please change hacker_role to role
| DISCORD_TOKEN= No newline at end of file | ||
| DISCORD_TOKEN= | ||
| API_KEY= | ||
| GEMINI_API_KEY= |
There was a problem hiding this comment.
i mean cmon man this shouldnt be part of our codebase at all...
if you use AI please make sure to clean stuff up before making commits
There was a problem hiding this comment.
Do you mean the gemini api key? thats for the thread summarizations
There was a problem hiding this comment.
Yes. .env.example is used to show environment variables needed for the project to run during development. Gemini should not be required for stuff to run or to develop. You can keep it in your own env file for your own needs, I just don't think there's a need to show it as a required parameter
There was a problem hiding this comment.
Holy hell I was not thinking... Apologies
There was a problem hiding this comment.
Holy hell I was not thinking... Apologies
| } | ||
| } | ||
|
|
||
| func (s *DiscordService) GetUserEventRoleByDiscordID(ctx context.Context, discordID string) (*uuid.UUID, *sqlc.EventRoleType, error) { |
There was a problem hiding this comment.
I think it would be better to use a function called GetUserEventRoleByDiscordIDAndEventId, and when you make the api request it should be from a route like GET /events/{eventId}/discord/{discordId}
event_roles are associated with a specific event, so that needs to be included in the query. Users can be part of multiple events, namely if they attend multiple swamphacks hackathons
| SELECT event_id, role | ||
| FROM event_roles | ||
| WHERE user_id = $1 | ||
| ORDER BY assigned_at DESC |
There was a problem hiding this comment.
this gets only the event_role from the oldest event that a user has joined. you should be passing in an event_role with the user_id to get this. see other comment
|
Hi please resolve conflicts :) |
| -- name: DeleteAccount :exec | ||
| DELETE FROM auth.accounts | ||
| WHERE provider_id = $1 AND account_id = $2; | ||
|
|
There was a problem hiding this comment.
if this and its repo function isn't being used anywhere it should be removed, but since we are low on time im okay with doing that in a refactor later
|
|
||
| r.Get("/", api.Handlers.Event.GetEventByID) | ||
| r.Get("/role", api.Handlers.Event.GetEventRole) | ||
| r.Get("/discord/{discordId}", api.Handlers.Discord.GetUserEventRoleByDiscordIDAndEventId) |
There was a problem hiding this comment.
In a later refactor, we should probably move this to GET /discord/account/{discordId}/ under the Discord Routes section
There was a problem hiding this comment.
should probably add the gemini token back in, my bad for not thinking :)
| if response.status == 200: | ||
| data = await response.json() | ||
| event_role = data.get("role") | ||
| discord_role_name = "Hacker" |
There was a problem hiding this comment.
should probably add these to env but hardcoding is fine for now
h1divp
left a comment
h1divp
left a comment
There was a problem hiding this comment.
just make sure to delete the 39mb api/api binary file and you're good to merge
h1divp
left a comment
h1divp
left a comment
There was a problem hiding this comment.
sqlc issue needs to be fixed, delete large bin file, then merge is OK
3 participants
Description
Linked Jira Ticket
Type of Change
Checklist
Additional Notes