Skip to content

Ensure player auth emails are unique by game#894

Merged
tudddorrr merged 1 commit intodevelopfrom
unique-player-auth-emails
Mar 23, 2026
Merged

Ensure player auth emails are unique by game#894
tudddorrr merged 1 commit intodevelopfrom
unique-player-auth-emails

Conversation

@tudddorrr
Copy link
Copy Markdown
Collaborator

No description provided.

@tudddorrr tudddorrr added the enhancement New feature or request label Mar 23, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Mar 23, 2026

PR Review: Ensure player auth emails are unique by game

The feature logic is sound and tests are comprehensive. One issue worth addressing:


🟡 Data Integrity

No database-level unique constraint backing the application check

isEmailTakenForGame performs a findOne check before setting the email, but there is no migration adding a composite unique constraint on (email, game_id) to the player_auth table. Under concurrent load, two simultaneous requests with the same email could both pass the findOne check before either commits, resulting in duplicate emails slipping through.

The application-level guard is correct, but it needs a DB constraint to be reliable. A migration should add:

ALTER TABLE player_auth ADD UNIQUE KEY uq_player_auth_email_game (email, game_id);

Without this, the EMAIL_TAKEN check is best-effort rather than guaranteed.


No other issues found

  • Error handling and activity logging are consistent with the existing codebase
  • excludePlayer is correctly used in toggle-verification.ts and correctly omitted in change-email.ts (same-email is already caught earlier by the NEW_EMAIL_MATCHES_CURRENT_EMAIL check on line 58)
  • Test coverage is thorough across all three routes

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.32%. Comparing base (60f1d42) to head (58da98d).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #894      +/-   ##
===========================================
+ Coverage    97.29%   97.32%   +0.02%     
===========================================
  Files          403      403              
  Lines         6481     6496      +15     
  Branches       844      848       +4     
===========================================
+ Hits          6306     6322      +16     
  Misses          90       90              
+ Partials        85       84       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tudddorrr tudddorrr merged commit e291d69 into develop Mar 23, 2026
10 checks passed
@tudddorrr tudddorrr deleted the unique-player-auth-emails branch March 23, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant