Require invitation flow for existing users in competition contexts#373
Require invitation flow for existing users in competition contexts#373zacjones93 wants to merge 1 commit intomainfrom
Conversation
…t auto-add When a teammate already had a WODsmith account, inviteUserToTeamInternal silently added them to the team without sending an email or creating an invitation. This meant they were never notified about their competition registration and couldn't complete required registration actions (questions, waivers). Now, existing users in a competition context go through the same invitation flow as new users: an invitation is created, the email is sent, and the acceptance flow handles team membership and registration requirements. https://claude.ai/code/session_0195edp7LRhJY25VXrFhNzMr
WalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview DeployedURL: https://wodsmith-app-pr-373.zacjones93.workers.dev
This comment is automatically updated on each push to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/wodsmith-start/src/server/registration.ts (1)
211-233: Add regression coverage for the new branch split.This is the behavior this PR is changing, so it needs explicit tests for both cases: existing user +
competitionContextshould create an invitation and not a membership, while existing user withoutcompetitionContextshould create the membership and return early.As per coding guidelines,
Add exactly one //@lat: [[section-id]] comment (or #@lat: for Python) next to each relevant test to reference its test spec, not at the top of the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/server/registration.ts` around lines 211 - 233, Add two regression tests covering the new branch split in registration: (1) for an existing user with competitionContext=true assert that an invitation is created (no team_membership row), the function returns invitationSent=true (or appropriate shape), and updateAllSessionsOfUser is NOT called; (2) for an existing user with competitionContext=false assert that a team_membership row is inserted, the function returns early with userJoined=true and invitationSent=false, and updateAllSessionsOfUser is called. In each test, create a pre-existing user and team, call the registration function (the code path around db.insert(teamMembershipTable).values and updateAllSessionsOfUser), assert DB state and return values, and add a single // `@lat`: [[section-id]] comment immediately next to each test to reference its spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/wodsmith-start/src/server/registration.ts`:
- Around line 206-210: The current flow calls registerForCompetition() then
sendCompetitionTeamInviteEmail(), which allows email failures to leave persisted
team/membership/registration rows; wrap the multi-write registration logic in a
db.transaction() (use Drizzle ORM's db.transaction) inside
registerForCompetition()/the registration path so team, captain membership, and
registration are written atomically, then send or enqueue the invite email only
after the transaction commits; alternatively, if you prefer minimal changes,
catch errors from sendCompetitionTeamInviteEmail() here (where
competitionContext is handled), log them and continue (or queue a retry) so
delivery failures do not roll back or expose a partial persisted registration.
---
Nitpick comments:
In `@apps/wodsmith-start/src/server/registration.ts`:
- Around line 211-233: Add two regression tests covering the new branch split in
registration: (1) for an existing user with competitionContext=true assert that
an invitation is created (no team_membership row), the function returns
invitationSent=true (or appropriate shape), and updateAllSessionsOfUser is NOT
called; (2) for an existing user with competitionContext=false assert that a
team_membership row is inserted, the function returns early with userJoined=true
and invitationSent=false, and updateAllSessionsOfUser is called. In each test,
create a pre-existing user and team, call the registration function (the code
path around db.insert(teamMembershipTable).values and updateAllSessionsOfUser),
assert DB state and return values, and add a single // `@lat`: [[section-id]]
comment immediately next to each test to reference its spec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95aaaef4-9cd9-4425-8293-f0d8164690a6
📒 Files selected for processing (1)
apps/wodsmith-start/src/server/registration.ts
| if (competitionContext) { | ||
| // Get competition to find the competition_event team | ||
| const competition = await db.query.competitionsTable.findFirst({ | ||
| where: eq(competitionsTable.id, competitionContext.competitionId), | ||
| // For competition invites, always create an invitation and send email | ||
| // so the user is notified and can complete registration requirements | ||
| // (questions, waivers). The acceptance flow handles adding them to the team. | ||
| // Fall through to the invitation creation path below. |
There was a problem hiding this comment.
Don’t let invite-email failures unwind an already-persisted registration.
Routing existing users through the invite path here means a failure in sendCompetitionTeamInviteEmail() now bubbles out after registerForCompetition() has already inserted the team, captain membership, and registration rows. That leaves a partial registration behind and makes retries hit the duplicate team/registration guards. Catch delivery failures here or move the write set into a transaction and send/queue the email after commit.
As per coding guidelines, Use db.transaction() for multiple writes that need to be atomic in Drizzle ORM.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/server/registration.ts` around lines 206 - 210, The
current flow calls registerForCompetition() then
sendCompetitionTeamInviteEmail(), which allows email failures to leave persisted
team/membership/registration rows; wrap the multi-write registration logic in a
db.transaction() (use Drizzle ORM's db.transaction) inside
registerForCompetition()/the registration path so team, captain membership, and
registration are written atomically, then send or enqueue the invite email only
after the transaction commits; alternatively, if you prefer minimal changes,
catch errors from sendCompetitionTeamInviteEmail() here (where
competitionContext is handled), log them and continue (or queue a retry) so
delivery failures do not roll back or expose a partial persisted registration.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/server/registration.ts">
<violation number="1" location="apps/wodsmith-start/src/server/registration.ts:207">
P2: Existing users in competition contexts now fall through to the invitation-creation + email-sending path. If `sendCompetitionTeamInviteEmail()` throws after the invitation record (and any prior registration rows) have already been committed, the result is a partial state that cannot be retried cleanly because duplicate guards will reject the next attempt. Wrap the invitation insert and email send in a `db.transaction()`, or catch email delivery errors so they don't unwind the already-persisted data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Get competition to find the competition_event team | ||
| const competition = await db.query.competitionsTable.findFirst({ | ||
| where: eq(competitionsTable.id, competitionContext.competitionId), | ||
| // For competition invites, always create an invitation and send email |
There was a problem hiding this comment.
P2: Existing users in competition contexts now fall through to the invitation-creation + email-sending path. If sendCompetitionTeamInviteEmail() throws after the invitation record (and any prior registration rows) have already been committed, the result is a partial state that cannot be retried cleanly because duplicate guards will reject the next attempt. Wrap the invitation insert and email send in a db.transaction(), or catch email delivery errors so they don't unwind the already-persisted data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server/registration.ts, line 207:
<comment>Existing users in competition contexts now fall through to the invitation-creation + email-sending path. If `sendCompetitionTeamInviteEmail()` throws after the invitation record (and any prior registration rows) have already been committed, the result is a partial state that cannot be retried cleanly because duplicate guards will reject the next attempt. Wrap the invitation insert and email send in a `db.transaction()`, or catch email delivery errors so they don't unwind the already-persisted data.</comment>
<file context>
@@ -203,51 +203,34 @@ async function inviteUserToTeamInternal({
- // Get competition to find the competition_event team
- const competition = await db.query.competitionsTable.findFirst({
- where: eq(competitionsTable.id, competitionContext.competitionId),
+ // For competition invites, always create an invitation and send email
+ // so the user is notified and can complete registration requirements
+ // (questions, waivers). The acceptance flow handles adding them to the team.
</file context>
2 participants
Summary
Modified the user invitation logic to ensure existing users invited to competition teams go through the invitation acceptance flow rather than being added directly. This allows them to complete required registration steps (questions, waivers) before joining the team.
Key Changes
Conditional logic for existing users: Split the flow based on context:
Removed automatic team membership: Eliminated the direct insertion into
teamMembershipTablefor existing users in competition contexts, along with the associated logic for adding them to thecompetition_eventteamSimplified return path: Non-competition invites now return immediately after adding the user, avoiding unnecessary invitation creation
Implementation Details
https://claude.ai/code/session_0195edp7LRhJY25VXrFhNzMr
Summary by cubic
Require invitation acceptance for existing users when invited to competition teams, so they receive an email and complete registration (questions, waivers) before joining. Non-competition invites still add existing users immediately.
competition_eventteam.Written for commit ceb2e23. Summary will update on new commits.
Summary by CodeRabbit