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

refactors user schema #77

Merged
merged 54 commits into from
Sep 11, 2024
Merged

refactors user schema #77

merged 54 commits into from
Sep 11, 2024

Conversation

xwilson03
Copy link
Collaborator

@xwilson03 xwilson03 commented Aug 3, 2024

Description:

Reorganizes and renames user data tables into a more extensible format for future feature additions.

Closes #76

Reviewers:

Review Guide:

  • Examine schema changes, compare those with what we've discussed in meetings using the dbdiagram link.
    • Note that checkinTimestamp was originally removed from the dbdiagram during our meetings; will this still be made into an event?
  • Examine rh-24 database to see migration in effect; ensure migration is safe to run on hackkit db.
  • Examine changelog and comment on any causes of concern.
  • (PENDING: blocked by migration review) Examine latest Vercel deployment to verify capabilities of modified endpoints.

Changelog:

Renames:

  • Renames tables to de-obfuscate contents and usage
    • users -> userCommonData
    • registrationData -> userHackerData
  • Renames certain columns to shorten them
    • userCommonData:
      • discordUsername -> discord
      • registrationComplete -> isFullyRegistered
      • createdAt -> signupTime
      • hasSearchableProfile -> isSearchable
      • rsvp -> isRSVPed
      • approved -> isApproved
    • userHackerData
      • shortID -> schoolID
      • acceptedMLHCodeOfConduct -> hasAcceptedMLHCoC
      • sharedDataWithMLH -> hasSharedDataWithMLH
      • wantsToReceiveMLHEmails -> isEmailable

Moved Columns:

  • profileData -> userCommonData

    • Columns: discordUsername, pronouns, bio, skills, profilePhoto
    • Reason: Profile data needs to be stored for all users.
  • userHackerData -> userCommonData

    • Columns: age, gender, race, ethnicity, shirtSize, dietRestrictions, accommodationNote
    • Reason: These fields are applicable to every user regardless of role
  • userCommonData -> userHackerData

    • Columns: group, teamID, points
    • Reason: These fields are only applicable to hackers.

Other:

  • Updates UserWithAllData interface to reflect schema changes
    • Should be phased out and replaced when role and user data coalescion are implemented
  • Updates references to moved and renamed fields across codebase
    • Removes unused imports and swaps some instances of hackerTag with clerkID in searches in affected files
    • Changes Admin Scanner Action to set checkinTimestamp instead of checkedIn boolean
  • Moves relation userCommonData.team to userHackerData
  • Adds relation from userCommonData to userHackerData via clerkID
  • Visually reorganizes user data tables in schema file
  • Removes redundant qualifiers for userHackerData.clerkID
  • Adds and applies data-safe migration script
  • Patches /admin/users/ AccountInfo component to handle missing Clerk data
  • Slightly refactors /admin/scanner/ and /actions/teams/
  • Formats many files to pass pipeline

@xwilson03 xwilson03 self-assigned this Aug 3, 2024
Copy link

vercel bot commented Aug 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hack-kit-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 11, 2024 2:23pm

package.json Outdated Show resolved Hide resolved
packages/db/schema.ts Outdated Show resolved Hide resolved
christianhelp
christianhelp previously approved these changes Sep 6, 2024
Copy link
Collaborator

@christianhelp christianhelp left a comment

Choose a reason for hiding this comment

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

Please check the small messages I left, but this is outstanding work. Thanks so much for this :)

@christianhelp
Copy link
Collaborator

Also, have you tested any sort of registering or queries on the db that you were given? Would like to know at least a few things were tried after the changes were made before we test on our db.

@xwilson03
Copy link
Collaborator Author

xwilson03 commented Sep 7, 2024

Also, have you tested any sort of registering or queries on the db that you were given? Would like to know at least a few things were tried after the changes were made before we test on our db.

@christianhelp It's been a little while, but iirc I was able to register under the new schema, and I didn't encounter any broken endpoints during manual testing.

christianhelp
christianhelp previously approved these changes Sep 8, 2024
Copy link
Collaborator

@christianhelp christianhelp left a comment

Choose a reason for hiding this comment

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

Great work. Budget approved ‼️

Lermatroid
Lermatroid previously approved these changes Sep 11, 2024
Copy link
Member

@Lermatroid Lermatroid left a comment

Choose a reason for hiding this comment

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

lgtm

@christianhelp christianhelp merged commit f2e80ed into dev Sep 11, 2024
3 checks passed
@christianhelp christianhelp deleted the feat/refactor-user-schema branch September 16, 2024 18:06
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.

Refactor User Data Schema
3 participants