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

Use Twitch User Id as the primary key for the users table #24

Merged
merged 3 commits into from
Dec 8, 2024

Conversation

sophearahsp
Copy link
Contributor

@sophearahsp sophearahsp commented Nov 26, 2024

#20

Changes

  • Created another column twitchUserId and made it the primary key of users. Kept the username column since I think it could still be useful
  • Created automatic migration stuff with npx drizzle-kit generate. I also added a line in the migration script to delete all existing users because twitchUserId is required and I didn't want to just generate a random id to fill in the blank
  • Did my best to update the api code to use twitchUserId instead of username but I didn't go through and check every endpoint ngl. If I missed something it should be an easy fix though

@changesbyjames
Copy link
Collaborator

hey @sophearahsp, thank you so much for this pr! I've used your changes (I think you did get all of the places in the api where the username is referenced!) to do a middleground option where we store the username & twitch ID like you implemented but also have the primary key be an ID that we control. this means we've got a little more flexibility if we ever want to use something else to login and if need to anonymise the research data, we can do that a little more easily.

again, thank you for your work! let me know if you want to make any changes or not and then I'll complete!

ps. as we're still early on in the project and the migration to change a primary key from a string -> number throws up a ton of errors, I've just reset the migrations from scratch so you'll need to drop your database locally!

@sophearahsp
Copy link
Contributor Author

having the primary key be an ID we control makes sense to me! The only other thought I have is that it might be a little bit better to use uuid instead of serialized IDs for participants? But I feel like it's probably fine either way

@changesbyjames
Copy link
Collaborator

very fair point! i think I'm happy with it being a serial number ID, I normally only reach for a UUID for things that are going to have a ton of rows or if there would be security concerns from enumeration attacks. i think we're fine on both here!

@sophearahsp
Copy link
Contributor Author

ok awesome sounds good!

@changesbyjames changesbyjames merged commit bc314ad into alveusgg:main Dec 8, 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