fix: validate teammate emails before payment to prevent orphaned charges#283
Open
theianjones wants to merge 4 commits intomainfrom
Open
fix: validate teammate emails before payment to prevent orphaned charges#283theianjones wants to merge 4 commits intomainfrom
theianjones wants to merge 4 commits intomainfrom
Conversation
Moved teammate email validation from registerForCompetition() to initiateRegistrationPaymentFn() to occur BEFORE creating Stripe checkout session. This prevents users from being charged when they enter invalid teammate data (e.g., their own email as teammate). Previously, users would complete payment in Stripe, then validation would fail during post-payment processing, resulting in succeeded payments with failed registrations. Changes: - Add teammate email validation in initiateRegistrationPaymentFn (before payment) - Check teammate email is different from user's email (case-insensitive) - Check teammate not already registered for competition - Check teammate not already invited to another team - Add comprehensive test coverage for validation logic Resolves 3 orphaned payments totaling $65.83: - pi_3T1oxgPZIommwrdT0kxWMlDQ ($31.20) - Leona Sandoval - pi_3T0RL0PZIommwrdT2HUf4XyK ($31.20) - Heff Doidge - pi_3Sn7VgPZIommwrdT3cUJM6Pc ($3.43) - Test account Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
WalkthroughThis PR removes registrationCount data propagation from UI components and route loaders, eliminates registration statistics badges, and adds pre-payment teammate email validation to the registration flow with comprehensive error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant RegFn as initiateRegistrationPaymentFn
participant DB as Database
participant Stripe as Stripe
User->>RegFn: Submit registration with teammates
activate RegFn
RegFn->>DB: Get current user email
DB-->>RegFn: User email
loop For each teammate
RegFn->>RegFn: Validate email ≠ user email
alt Email matches user
RegFn-->>User: Error: Invalid teammate
end
RegFn->>DB: Check if teammate registered<br/>for competition
DB-->>RegFn: Registration status
alt Already registered
RegFn-->>User: Error: Duplicate registration
end
RegFn->>DB: Get all registrations<br/>for competition
DB-->>RegFn: Registrations list
RegFn->>RegFn: Parse & check pending<br/>teammates for duplicates
alt Found as pending teammate
RegFn-->>User: Error: Already invited
end
end
RegFn->>Stripe: Create checkout session
Stripe-->>RegFn: Session created
RegFn-->>User: Redirect to Stripe
deactivate RegFn
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (all 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/test/server-fns/registration-fns.test.ts">
<violation number="1" location="apps/wodsmith-start/test/server-fns/registration-fns.test.ts:751">
P2: `vi.mock` is hoisted in Vitest, so this mock runs before `mockStripeSession` is defined and can throw or leave Stripe unmocked. Move the Stripe mock to top-level with `vi.hoisted`/inline literals, or use `vi.doMock` with a dynamic import so the mock can access local variables.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| status: 'open', | ||
| } | ||
|
|
||
| vi.mock('@/lib/stripe', () => ({ |
Contributor
There was a problem hiding this comment.
P2: vi.mock is hoisted in Vitest, so this mock runs before mockStripeSession is defined and can throw or leave Stripe unmocked. Move the Stripe mock to top-level with vi.hoisted/inline literals, or use vi.doMock with a dynamic import so the mock can access local variables.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/test/server-fns/registration-fns.test.ts, line 751:
<comment>`vi.mock` is hoisted in Vitest, so this mock runs before `mockStripeSession` is defined and can throw or leave Stripe unmocked. Move the Stripe mock to top-level with `vi.hoisted`/inline literals, or use `vi.doMock` with a dynamic import so the mock can access local variables.</comment>
<file context>
@@ -511,4 +523,267 @@ describe('registration-fns', () => {
+ status: 'open',
+ }
+
+ vi.mock('@/lib/stripe', () => ({
+ getStripe: vi.fn(() => ({
+ checkout: {
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/wodsmith-start/src/components/event-details-content.tsx (1)
19-19:registrationCountis unused in this component's logic.The
registrationCountfield is declared in theDivisionWithDetailsinterface but not referenced anywhere inevent-details-content.tsx. However, removing it from the interface would break other components that depend on it (e.g.,event-details-form.tsx,organizer-division-item.tsx). If the intention is to clean up the local component, consider whether this field should be kept for consistency across the shared type, or refactor to extract a component-specific variant of the interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/components/event-details-content.tsx` at line 19, The DivisionWithDetails field registrationCount is declared but unused in event-details-content.tsx; either remove usage locally by creating a component-specific type without registrationCount or keep the field on the shared interface for consistency—update the component to use the new local type (e.g., create DivisionSummary or DivisionWithDetailsLocal) or leave DivisionWithDetails unchanged so other files (event-details-form.tsx, organizer-division-item.tsx) continue to compile; ensure any prop typings in event-details-content.tsx reference the chosen type and adjust imports accordingly.apps/wodsmith-start/test/server-fns/registration-fns.test.ts (1)
68-77: Remove unusedparseCompetitionSettingsfrom mock.
parseCompetitionSettingsis mocked (line 74-76) butinitiateRegistrationPaymentFndoesn't import or use it—onlygetDivisionSpotsAvailableFnis called. Remove it to keep the mock focused on actual dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/test/server-fns/registration-fns.test.ts` around lines 68 - 77, The test mock for '@/server-fns/competition-divisions-fns' includes an unused parseCompetitionSettings export; remove parseCompetitionSettings from the mock so only the actually used getDivisionSpotsAvailableFn remains, keeping the stub focused on real dependencies called by initiateRegistrationPaymentFn; update the vi.mock block to only provide getDivisionSpotsAvailableFn (mockResolvedValue { isFull: false, spotsAvailable: 100 }).
🤖 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-fns/registration-fns.ts`:
- Around line 265-348: Move the call to
db.query.competitionRegistrationsTable.findMany that populates allRegistrations
out of the for (const teammate ...) loop so it runs once before iterating
teammates; then tighten the catch around JSON.parse only so that only JSON parse
errors are swallowed (allow other runtime errors to propagate) by
catching/parsing pendingTeammates per reg and validating p.email exists before
calling p.email.toLowerCase(); update references in this block
(allRegistrations, pendingTeammates, the teammate loop, and the try/catch)
accordingly.
- Around line 292-294: The teammate lookup uses a lowercased email
(teammateEmail) but signup stores the original casing (auth-fns.ts uses email:
data.email), causing mismatches on case-sensitive DBs; fix by normalizing on
write: update the signup flow in auth-fns.ts to store email:
data.email.toLowerCase() (and any other places that create/update user.email) so
db.query.userTable.findFirst({ where: eq(userTable.email, teammateEmail) }) will
match reliably, and consider adding a DB/validation note to enforce lowercase
for user.email going forward.
In `@apps/wodsmith-start/test/server-fns/registration-fns.test.ts`:
- Around line 707-787: The test uses a try/catch and negative string assertions
which allow unrelated failures to pass and places vi.mock('@/lib/stripe', ...)
inside the test body (which Vitest hoists globally); fix by moving the Stripe
mock to the module-level mocks (so it is declared alongside other vi.mock calls)
returning mockStripeSession, then change the test to assert a positive outcome
from initiateRegistrationPaymentFn (call the function without try/catch and
either await its resolved value and assert it contains the mockStripeSession.id
or url, or assert that the mocked getStripe().checkout.sessions.create was
called with expected args). Ensure you reference initiateRegistrationPaymentFn
and the mocked getStripe()/checkout.sessions.create when adding these
assertions.
---
Nitpick comments:
In `@apps/wodsmith-start/src/components/event-details-content.tsx`:
- Line 19: The DivisionWithDetails field registrationCount is declared but
unused in event-details-content.tsx; either remove usage locally by creating a
component-specific type without registrationCount or keep the field on the
shared interface for consistency—update the component to use the new local type
(e.g., create DivisionSummary or DivisionWithDetailsLocal) or leave
DivisionWithDetails unchanged so other files (event-details-form.tsx,
organizer-division-item.tsx) continue to compile; ensure any prop typings in
event-details-content.tsx reference the chosen type and adjust imports
accordingly.
In `@apps/wodsmith-start/test/server-fns/registration-fns.test.ts`:
- Around line 68-77: The test mock for '@/server-fns/competition-divisions-fns'
includes an unused parseCompetitionSettings export; remove
parseCompetitionSettings from the mock so only the actually used
getDivisionSpotsAvailableFn remains, keeping the stub focused on real
dependencies called by initiateRegistrationPaymentFn; update the vi.mock block
to only provide getDivisionSpotsAvailableFn (mockResolvedValue { isFull: false,
spotsAvailable: 100 }).
Comment on lines
+265
to
+348
| // 3.7. Validate teammate emails BEFORE payment | ||
| // This prevents charging users for invalid team registrations | ||
| if (input.teammates && input.teammates.length > 0) { | ||
| // Get current user for email comparison | ||
| const currentUser = await db.query.userTable.findFirst({ | ||
| where: eq(userTable.id, userId), | ||
| columns: { email: true }, | ||
| }) | ||
|
|
||
| if (!currentUser?.email) { | ||
| throw new Error("User email not found") | ||
| } | ||
|
|
||
| const currentUserEmail = currentUser.email.toLowerCase() | ||
|
|
||
| // Check each teammate | ||
| for (const teammate of input.teammates) { | ||
| const teammateEmail = teammate.email.toLowerCase() | ||
|
|
||
| // CRITICAL: Check if teammate email is the same as the user's own email | ||
| if (teammateEmail === currentUserEmail) { | ||
| throw new Error( | ||
| `${teammate.email} is your own email. Please enter a different teammate's email.`, | ||
| ) | ||
| } | ||
|
|
||
| // Check if teammate is already registered for this competition | ||
| const teammateUser = await db.query.userTable.findFirst({ | ||
| where: eq(userTable.email, teammateEmail), | ||
| }) | ||
|
|
||
| if (teammateUser) { | ||
| const teammateReg = | ||
| await db.query.competitionRegistrationsTable.findFirst({ | ||
| where: and( | ||
| eq(competitionRegistrationsTable.eventId, input.competitionId), | ||
| eq(competitionRegistrationsTable.userId, teammateUser.id), | ||
| ), | ||
| }) | ||
|
|
||
| if (teammateReg) { | ||
| throw new Error( | ||
| `${teammate.email} is already registered for this competition`, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // Check if email is already in pendingTeammates of another registration | ||
| const allRegistrations = | ||
| await db.query.competitionRegistrationsTable.findMany({ | ||
| where: eq( | ||
| competitionRegistrationsTable.eventId, | ||
| input.competitionId, | ||
| ), | ||
| }) | ||
|
|
||
| for (const reg of allRegistrations) { | ||
| if (!reg.pendingTeammates) continue | ||
|
|
||
| try { | ||
| const pending = JSON.parse(reg.pendingTeammates) as Array<{ | ||
| email: string | ||
| }> | ||
| const isPending = pending.some( | ||
| (p) => p.email.toLowerCase() === teammateEmail, | ||
| ) | ||
|
|
||
| if (isPending) { | ||
| throw new Error( | ||
| `${teammate.email} has already been invited to another team for this competition`, | ||
| ) | ||
| } | ||
| } catch (e) { | ||
| // Re-throw our custom errors, ignore JSON parse errors | ||
| if ( | ||
| e instanceof Error && | ||
| e.message.includes("already been invited") | ||
| ) { | ||
| throw e | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Move allRegistrations query outside the teammate loop and tighten the catch block.
Two issues in this validation block:
-
N+1 query (lines 313-319): The
findManyfor all competition registrations is inside thefor (const teammate …)loop (line 281). Every teammate triggers an identical query. Hoist it above the loop so it runs once. -
Fragile catch block (lines 337-345): The
catchonly re-throws errors whose message includes"already been invited", silently swallowing every other runtime error. If anypendingTeammaterecord has anullemail,p.email.toLowerCase()will throw aTypeErrorthat gets silently eaten, causing the validation to pass when it shouldn't. Prefer inverting the logic: let all errors propagate except known-safe JSON parse failures.
Proposed fix
// 3.7. Validate teammate emails BEFORE payment
if (input.teammates && input.teammates.length > 0) {
const currentUser = await db.query.userTable.findFirst({
where: eq(userTable.id, userId),
columns: { email: true },
})
if (!currentUser?.email) {
throw new Error("User email not found")
}
const currentUserEmail = currentUser.email.toLowerCase()
+ // Fetch all registrations for this competition ONCE (for pendingTeammates check)
+ const allRegistrations =
+ await db.query.competitionRegistrationsTable.findMany({
+ where: eq(
+ competitionRegistrationsTable.eventId,
+ input.competitionId,
+ ),
+ })
+
for (const teammate of input.teammates) {
const teammateEmail = teammate.email.toLowerCase()
if (teammateEmail === currentUserEmail) {
throw new Error(
`${teammate.email} is your own email. Please enter a different teammate's email.`,
)
}
const teammateUser = await db.query.userTable.findFirst({
where: eq(userTable.email, teammateEmail),
})
if (teammateUser) {
const teammateReg =
await db.query.competitionRegistrationsTable.findFirst({
where: and(
eq(competitionRegistrationsTable.eventId, input.competitionId),
eq(competitionRegistrationsTable.userId, teammateUser.id),
),
})
if (teammateReg) {
throw new Error(
`${teammate.email} is already registered for this competition`,
)
}
}
- const allRegistrations =
- await db.query.competitionRegistrationsTable.findMany({
- where: eq(
- competitionRegistrationsTable.eventId,
- input.competitionId,
- ),
- })
-
for (const reg of allRegistrations) {
if (!reg.pendingTeammates) continue
- try {
- const pending = JSON.parse(reg.pendingTeammates) as Array<{
- email: string
- }>
- const isPending = pending.some(
- (p) => p.email.toLowerCase() === teammateEmail,
- )
-
- if (isPending) {
- throw new Error(
- `${teammate.email} has already been invited to another team for this competition`,
- )
- }
- } catch (e) {
- if (
- e instanceof Error &&
- e.message.includes("already been invited")
- ) {
- throw e
- }
- }
+ let pending: Array<{ email: string }>
+ try {
+ pending = JSON.parse(reg.pendingTeammates) as Array<{ email: string }>
+ } catch {
+ // Skip registrations with corrupted JSON
+ continue
+ }
+
+ const isPending = pending.some(
+ (p) => p.email?.toLowerCase() === teammateEmail,
+ )
+
+ if (isPending) {
+ throw new Error(
+ `${teammate.email} has already been invited to another team for this competition`,
+ )
+ }
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/server-fns/registration-fns.ts` around lines 265 -
348, Move the call to db.query.competitionRegistrationsTable.findMany that
populates allRegistrations out of the for (const teammate ...) loop so it runs
once before iterating teammates; then tighten the catch around JSON.parse only
so that only JSON parse errors are swallowed (allow other runtime errors to
propagate) by catching/parsing pendingTeammates per reg and validating p.email
exists before calling p.email.toLowerCase(); update references in this block
(allRegistrations, pendingTeammates, the teammate loop, and the try/catch)
accordingly.
Comment on lines
+292
to
+294
| const teammateUser = await db.query.userTable.findFirst({ | ||
| where: eq(userTable.email, teammateEmail), | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the schema definition for userTable to understand how emails are stored
find . -type f -name "*.ts" -path "*/db/*" -o -path "*/schema*" | head -20Repository: wodsmith/thewodapp
Length of output: 973
🏁 Script executed:
# Search for the userTable schema definition
rg -n "userTable|email" --type=ts -g '*schema*' -g '*db*' -B2 -A2 | head -100Repository: wodsmith/thewodapp
Length of output: 7351
🏁 Script executed:
# Check for email normalization patterns throughout the codebase
rg -n "toLowerCase|lower_case|LOWER\(" --type=ts -g '!node_modules' -g '!*.test.*' -C3Repository: wodsmith/thewodapp
Length of output: 50374
🏁 Script executed:
# Look at the registration-fns file to see the context and how emails are being handled
cat -n apps/wodsmith-start/src/server-fns/registration-fns.ts | head -350 | tail -100Repository: wodsmith/thewodapp
Length of output: 3602
🏁 Script executed:
# Find userTable schema definition
rg -n "userTable\s*=" --type=ts -A20 | grep -A20 "userTable\s*=" | head -100Repository: wodsmith/thewodapp
Length of output: 5622
🏁 Script executed:
# Look for user insertion code to verify emails are lowercased
rg -n "insert.*userTable|\.values\(" --type=ts -B2 -A8 apps/wodsmith-start/src/server-fns/auth-fns.ts | head -150Repository: wodsmith/thewodapp
Length of output: 1016
🏁 Script executed:
# Check if there's any email normalization in the schema itself
cat -n apps/wodsmith-start/src/db/schemas/teams.ts | grep -A5 -B5 "email"Repository: wodsmith/thewodapp
Length of output: 984
🏁 Script executed:
# Check the signup form to see if emails are normalized before submission
rg -n "email.*toLowerCase|sign.*up|form.*email" --type=tsx apps/wodsmith-start/src/routes -B3 -A3 | head -120Repository: wodsmith/thewodapp
Length of output: 89
🏁 Script executed:
# Check auth-fns more carefully to see if email is lowercased BEFORE insertion
cat -n apps/wodsmith-start/src/server-fns/auth-fns.ts | sed -n '170,210p'Repository: wodsmith/thewodapp
Length of output: 1514
🏁 Script executed:
# Check if there's any validation or normalization middleware for email input
rg -n "data\.email|email.*=.*toLowerCase|normalizeEmail" --type=ts apps/wodsmith-start/src/server-fns/auth-fns.ts -B2 -A2 | head -80Repository: wodsmith/thewodapp
Length of output: 1903
🏁 Script executed:
# Look for signup form validation
rg -n "email.*schema|parseAsync|zod|email.*validate" --type=ts apps/wodsmith-start/src/server-fns/auth-fns.ts -B5 -A5 | head -100Repository: wodsmith/thewodapp
Length of output: 439
🏁 Script executed:
# Check if there's any database collation setting or LOWER() usage in queries
rg -n "COLLATE|collation|LOWER\(" --type=ts apps/wodsmith-start/src/db | head -50Repository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
# Look at the exact signup schema validation
cat -n apps/wodsmith-start/src/server-fns/auth-fns.ts | sed -n '1,90p'Repository: wodsmith/thewodapp
Length of output: 3502
🏁 Script executed:
# Check the signup schema definition
cat -n apps/wodsmith-start/src/schemas/auth.schema.tsRepository: wodsmith/thewodapp
Length of output: 2512
🏁 Script executed:
# Also check if there's email normalization happening in any middleware or input validation
rg -n "email.*toLowerCase|\.toLowerCase\(\)" --type=ts apps/wodsmith-start/src/schemas -B2 -A2Repository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
# Check the complete signup flow - from schema validation through storage
cat -n apps/wodsmith-start/src/server-fns/auth-fns.ts | sed -n '155,210p'Repository: wodsmith/thewodapp
Length of output: 2072
🏁 Script executed:
# Search for any transform or normalize happening in the schema
rg -n "\.transform|\.toLowerCase|\.toUpperCase" --type=ts apps/wodsmith-start/src/schemas/auth.schema.tsRepository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
# Check if there's global email normalization happening before schema parsing
rg -n "signUpSchema\.parse|signUpFn" --type=ts apps/wodsmith-start/src/server-fns/auth-fns.ts -B3 -A3Repository: wodsmith/thewodapp
Length of output: 339
Email normalization is inconsistent: signup stores original casing but lookups use lowercase.
Emails are stored with original casing during signup (line 203 in auth-fns.ts: email: data.email), but the teammate lookup on line 293 queries with a lowercased value. If the database uses case-sensitive collation, a user registered as Teammate@Example.com won't be found when querying with teammate@example.com. Normalize all emails to lowercase during signup to ensure consistent lookups throughout the system.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/server-fns/registration-fns.ts` around lines 292 -
294, The teammate lookup uses a lowercased email (teammateEmail) but signup
stores the original casing (auth-fns.ts uses email: data.email), causing
mismatches on case-sensitive DBs; fix by normalizing on write: update the signup
flow in auth-fns.ts to store email: data.email.toLowerCase() (and any other
places that create/update user.email) so db.query.userTable.findFirst({ where:
eq(userTable.email, teammateEmail) }) will match reliably, and consider adding a
DB/validation note to enforce lowercase for user.email going forward.
Comment on lines
+707
to
+787
| it('should allow registration with valid teammate email', async () => { | ||
| // Mock all required dependencies for successful registration | ||
| const mockDivision = { | ||
| id: testDivisionId, | ||
| label: 'RX', | ||
| teamSize: 2, | ||
| } | ||
|
|
||
| const mockOrganizingTeam = { | ||
| id: 'organizing-team-123', | ||
| stripeAccountStatus: 'VERIFIED', | ||
| stripeConnectedAccountId: 'acct_test123', | ||
| organizerFeePercentage: null, | ||
| organizerFeeFixed: null, | ||
| } | ||
|
|
||
| // Mock database responses | ||
| mockDb.query.scalingLevelsTable = { | ||
| findFirst: vi.fn().mockResolvedValue(mockDivision), | ||
| findMany: vi.fn().mockResolvedValue([mockDivision]), | ||
| } | ||
|
|
||
| mockDb.query.teamTable = { | ||
| findFirst: vi.fn().mockResolvedValue(mockOrganizingTeam), | ||
| findMany: vi.fn().mockResolvedValue([]), | ||
| } | ||
|
|
||
| // Mock competition divisions for fee lookup | ||
| mockDb.query.competitionDivisionsTable = { | ||
| findFirst: vi.fn().mockResolvedValue({ | ||
| competitionId: testCompetitionId, | ||
| divisionId: testDivisionId, | ||
| feeCents: 5000, | ||
| }), | ||
| findMany: vi.fn().mockResolvedValue([]), | ||
| } | ||
|
|
||
| // Mock Stripe | ||
| const mockStripeSession = { | ||
| id: 'cs_test123', | ||
| url: 'https://checkout.stripe.com/test', | ||
| status: 'open', | ||
| } | ||
|
|
||
| vi.mock('@/lib/stripe', () => ({ | ||
| getStripe: vi.fn(() => ({ | ||
| checkout: { | ||
| sessions: { | ||
| create: vi.fn().mockResolvedValue(mockStripeSession), | ||
| }, | ||
| }, | ||
| })), | ||
| })) | ||
|
|
||
| // This should NOT throw - teammate email is different from captain | ||
| // Note: The actual call might still fail due to other validations or mocks, | ||
| // but it should pass the teammate email validation | ||
| try { | ||
| await initiateRegistrationPaymentFn({ | ||
| data: { | ||
| competitionId: testCompetitionId, | ||
| divisionId: testDivisionId, | ||
| teamName: 'Test Team', | ||
| teammates: [ | ||
| { | ||
| email: 'different@example.com', // Different from captain@example.com | ||
| firstName: 'Jane', | ||
| lastName: 'Doe', | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
| } catch (error) { | ||
| // If it throws, it should NOT be about the teammate email | ||
| if (error instanceof Error) { | ||
| expect(error.message).not.toContain('is your own email') | ||
| expect(error.message).not.toContain('already registered') | ||
| expect(error.message).not.toContain('already been invited') | ||
| } | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Weak assertion: the "valid teammate" test can't distinguish success from unrelated failures.
The try/catch pattern (lines 764-786) asserts only that the error message doesn't contain specific teammate-validation strings. If the function fails for any other reason (as it likely will given incomplete mocks), the test still passes. This doesn't actually verify that valid teammates proceed through the flow.
Additionally, vi.mock('@/lib/stripe', …) on line 751 is inside a test body. Vitest hoists vi.mock calls to the top of the file, so this mock applies globally — it isn't scoped to this test and could interfere with tests added later.
Consider moving the Stripe mock to the module level (alongside the other vi.mock calls) and asserting a positive outcome (e.g., that the Stripe checkout session creation was called) rather than using a catch-all try/catch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/test/server-fns/registration-fns.test.ts` around lines
707 - 787, The test uses a try/catch and negative string assertions which allow
unrelated failures to pass and places vi.mock('@/lib/stripe', ...) inside the
test body (which Vitest hoists globally); fix by moving the Stripe mock to the
module-level mocks (so it is declared alongside other vi.mock calls) returning
mockStripeSession, then change the test to assert a positive outcome from
initiateRegistrationPaymentFn (call the function without try/catch and either
await its resolved value and assert it contains the mockStripeSession.id or url,
or assert that the mocked getStripe().checkout.sessions.create was called with
expected args). Ensure you reference initiateRegistrationPaymentFn and the
mocked getStripe()/checkout.sessions.create when adding these assertions.
…e-email-validation-before-payment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Users were being charged for team registrations even when their teammate data was invalid (e.g., entering their own email as a teammate). This resulted in succeeded Stripe payments with failed registrations.
Root Cause
Teammate validation happened after payment in
registerForCompetition(), which runs during webhook processing after Stripe checkout succeeds.Flow (broken):
registerForCompetition()Impact
Solution
Move teammate email validation to happen before creating Stripe checkout session in
initiateRegistrationPaymentFn().Flow (fixed):
Changes
Core Fix
apps/wodsmith-start/src/server-fns/registration-fns.tsTests
apps/wodsmith-start/test/server-fns/registration-fns.test.tsAffected Customers
Options: Get correct teammate email or issue refund
Test Plan
🤖 Generated with Claude Code
Summary by cubic
Validates teammate emails before starting Stripe checkout to prevent orphaned charges. Also removes registration/division counts from the UI and simplifies spot hints.
Bug Fixes
Refactors
Written for commit a94c279. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests