Setup guards for check-in #414
Conversation
WalkthroughAdded optional Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
![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)
src/api/routes/tickets.ts (1)
529-529: Consider refactoring the message construction for better readability.The audit log message uses conditional string concatenation with template literals, which works but could be more maintainable.
Apply this diff to improve readability:
- message: `checked in ticket of type "${request.body.type}" ${request.body.type === "merch" ? `purchased by email ${request.body.email}.` : "."}${headerReason ? `\nUser-provided context: "${headerReason}"` : ""}`, + message: [ + `checked in ticket of type "${request.body.type}"`, + request.body.type === "merch" ? `purchased by email ${request.body.email}` : "", + headerReason ? `User-provided context: "${headerReason}"` : "", + ] + .filter(Boolean) + .join("\n"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/api/routes/tickets.ts(2 hunks)src/ui/pages/tickets/ViewTickets.page.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests
- GitHub Check: Build Application
🔇 Additional comments (7)
src/api/routes/tickets.ts (3)
395-400: LGTM! Proper use of Zod v4 optional header schema.The optional header schema for audit context is well-defined using Zod v4 syntax with
.meta()for description.
533-538: LGTM! Explicit return ensures audit log completes before response.Moving the response to after audit log creation with an explicit return ensures logging completes before the handler terminates, which aligns with the PR objective.
522-532: Confirm audit log table is deployed in us-east-2 (or your application's default region).The code change routes audit log writes from
UsEast1DynamoClient(us-east-1) tofastify.dynamoClient, which usesgenericConfig.AwsRegion(defaults to us-east-2 viaprocess.env.AWS_REGION ?? "us-east-2"). This is consistent with the intent stated in tickets.ts lines 113–114—audit logs should reside in the application's default region, not with legacy ticket data stuck in us-east-1. Verify that the "infra-core-api-audit-log" table is actually deployed in that target region to avoid write failures.src/ui/pages/tickets/ViewTickets.page.tsx (4)
80-86: LGTM! Well-structured state for confirmation modal.The modal state variables are clearly named and appropriately typed.
125-137: LGTM! Clean modal lifecycle handlers.The open and close handlers properly initialize and reset modal state.
145-153: LGTM! Robust email validation with case-insensitive comparison.The email validation correctly handles case differences and whitespace, providing a clear error message when validation fails.
340-413: LGTM! Well-designed confirmation modal with clear warnings.The modal implementation effectively:
- Displays a prominent warning about irreversibility
- Shows all relevant purchase details
- Requires exact email confirmation
- Provides clear action buttons with appropriate states
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ui/pages/tickets/ViewTickets.page.tsx (1)
185-232: Critical: API payload still doesn't match backend schema for ticket type.This issue was previously flagged but remains unaddressed. The API request always sends
stripePifields (lines 205-207), but the backend schema expects different fields based on ticket type:
- Merch:
{ type: "merch", email: string, stripePi: string }✓- Ticket:
{ type: "ticket", ticketId: string }✗ (currently sendingstripePi)This will cause validation errors when trying to manually fulfill ticket-type purchases.
Apply this diff to conditionally construct the correct payload:
try { - const response = await api.post( - `/api/v1/tickets/checkIn`, - { - type: ticketToFulfill.type, - email: ticketToFulfill.purchaserData.email, - stripePi: ticketToFulfill.ticketId, - }, - { - headers: { - "x-auditlog-context": "Manually marked as fulfilled.", - }, - }, - ); + const payload = + ticketToFulfill.type === "merch" + ? { + type: "merch" as const, + email: ticketToFulfill.purchaserData.email, + stripePi: ticketToFulfill.ticketId, + } + : { + type: "ticket" as const, + ticketId: ticketToFulfill.ticketId, + }; + + const response = await api.post(`/api/v1/tickets/checkIn`, payload, { + headers: { + "x-auditlog-context": "Manually marked as fulfilled.", + }, + }); if (!response.data.valid) {
🧹 Nitpick comments (1)
src/ui/pages/tickets/ViewTickets.page.tsx (1)
81-89: Minor inconsistency in countdown initialization.Line 89 initializes
countdownto3, but it should beWAIT_BEFORE_FULFILLING_SECSfor consistency. While the value is reset when the modal opens (line 94), using a consistent initial value improves code clarity.Apply this diff:
- const [countdown, setCountdown] = useState(3); + const [countdown, setCountdown] = useState(WAIT_BEFORE_FULFILLING_SECS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/ui/pages/tickets/ViewTickets.page.tsx(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests
- GitHub Check: Build Application
🔇 Additional comments (6)
src/ui/pages/tickets/ViewTickets.page.tsx (6)
67-67: LGTM!The 15-second guard is a good security measure to prevent accidental fulfillment.
91-130: LGTM! Solid countdown implementation with visibility guards.The countdown logic correctly prevents users from bypassing the wait time by switching tabs or hiding the page. The visibility change handler and the interval guard work together effectively.
Minor note: Line 99 sets the countdown to
WAIT_BEFORE_FULFILLING_SECS + 1when the page is hidden, adding a 1-second penalty. This appears intentional but consider documenting this behavior if it's a deliberate security measure.
169-183: LGTM!Modal handlers correctly manage state transitions and reset logic.
234-236: LGTM!Correctly updated to open the confirmation modal instead of immediately fulfilling.
386-461: LGTM! Well-designed confirmation modal with good UX.The modal implementation includes:
- Clear warning alert about irreversible action
- Purchase details for verification
- Email confirmation with validation
- Countdown guard to prevent accidental clicks
- Good accessibility with autofocus on input
Mantine's Modal component uses portals by default, so the placement within the component tree doesn't affect rendering behavior.
20-20: No issues found—the import path is valid.The import
import * as z from "zod/v4";is an officially supported subpath export from Zod that remains available. Zod exposes subpaths like "zod/v4" for incremental migration. This pattern is used consistently across 50+ files in the codebase and requires no changes.Likely an incorrect or invalid review comment.
Summary by CodeRabbit