Open
Conversation
Added BulkRefundPaymentPreview, BulkRefundPreview, BulkRefundResult, BulkRefundResponse, BulkRefundProgressEvent interfaces to refund.ts. Types exported via existing barrel exports. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…by event - Added PaymentWithPlayerDetails type to database types - Added findConfirmedPaymentsByEventWithDetails() to PaymentsRepository - Joins payment → registrationFee → registrationSlot → player - Filters by eventId, confirmed=1, paymentCode LIKE 'pi_%' - Groups results by paymentId with player name Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Calls findConfirmedPaymentsByEventWithDetails, filters to payments with paid fees (isPaid=1), builds BulkRefundPreview with per-payment details. Payments with no paid fees increment skippedCount. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
New BulkRefundProgressTracker class for SSE streaming: - startTracking(eventId) creates Subject, returns it - getProgressObservable(eventId) returns Observable if running - emitProgress(eventId, current, total, playerName) sends processing event - completeOperation(eventId, result) sends complete event, cleans up - errorOperation(eventId, error) sends error event, cleans up - Auto-cleanup after 5min timeout Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Returns Observable immediately, spawns async background op to process refunds one-by-one with progress tracking. Continues on individual payment failure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add GET :eventId/bulk-refund-preview endpoint that returns BulkRefundPreview. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add @sse :eventId/bulk-refund endpoint for streaming bulk refund progress. Checks for existing operation, returns Observable piped through map() to JSON format. Follows golfgenius.controller.ts pattern. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
GET /api/registration/bulk-refund-preview?eventId={id}
Proxies to NestJS /admin-registration/{eventId}/bulk-refund-preview
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- GET /api/registration/bulk-refund?eventId=X - Uses fetchSSEWithAuth() to proxy SSE stream - Follows same pattern as bulk-refund-preview route Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- apps/web/app/events/[eventId]/refunds/page.tsx: page with preview table - Fetches preview on mount, shows player name/fee count/refund amount - Shows total refund amount, skipped count, Refund All button Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- useReducer for state (phase, current, total, playerName, error, result) - EventSource to /api/registration/bulk-refund?eventId=X - progress bar w/ current/total, current player name - complete phase shows refundedCount, failedCount, totalRefundAmount - lists per-payment failures w/ error messages - closes EventSource on complete/error Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
WalkthroughThis pull request implements a complete bulk refund feature enabling administrators to preview and process batch refunds across multiple payments within an event. The implementation spans domain type definitions, database repository queries, a real-time progress tracking service, REST and SSE API endpoints, web proxy routes, and a streaming React UI component. Changes
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![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: 7
🤖 Fix all issues with AI agents
In `@apps/api/src/registration/controllers/admin-registration.controller.ts`:
- Around line 219-221: Replace the hardcoded issuerId = 1 with the authenticated
admin ID from the request: inject `@Req`() req: AuthenticatedRequest into the
controller methods and use req.user.id when calling
refundService.processBulkRefundsStream and refundService.processRefunds (the two
locations where issuerId is currently set). Update the parameter list for the
methods that create the observable/handle refunds to accept Req and pass
req.user.id into the refundService calls so refunds are attributed to the actual
authenticated admin.
In `@apps/api/src/registration/repositories/payments.repository.ts`:
- Around line 75-125: The current findConfirmedPaymentsByEventWithDetails logic
writes a single playerName per payment in paymentsMap but a payment can have
fees for multiple players; update the model and grouping to preserve per-fee
player info or collect distinct player names: modify the
PaymentWithPlayerDetails type and the loop in
findConfirmedPaymentsByEventWithDetails so each fee entry includes
playerFirstName/playerLastName (or playerName) OR add a players: string[] (or
players: {firstName,lastName}[]) on the payment and, when iterating results,
either push a fee object that includes the
row.playerFirstName/row.playerLastName or maintain a Set to collect unique
player names and assign players array before setting/returning the map values
(adjust paymentsMap handling and the initial object creation accordingly).
In `@apps/api/src/registration/services/bulk-refund-progress-tracker.ts`:
- Around line 17-29: The current startTracking creates an unrefreshable timeout
that can kill long-running operations; change the activeOperations map to store
an object { subject, timeoutId } instead of just Subject, create the cleanup
timer in startTracking using PROGRESS_CLEANUP_MS and save its id, then update
emitProgress to clear and rearm (reset) that timer on every emission, and ensure
cleanupOperation (and any completion/error methods like
completeTracking/failTracking) clear the timer and remove the entry so the
timeout is not left running.
In `@apps/api/src/registration/services/refund.service.ts`:
- Around line 154-172: The preview calculation in getBulkRefundPreview currently
sums raw parseFloat(f.amount) causing mismatch with processRefunds which rounds
each fee to cents; update getBulkRefundPreview so paidFees are summed the same
way as processRefunds (either round each fee to cents with
Math.round(parseFloat(f.amount) * 100) / 100 before summing, or sum integer
cents then divide by 100) and ensure refundAmount and totalRefundAmount use that
rounded-to-cents value so preview and actual refunds match; modify the logic
around paidFees/refundAmount/totalRefundAmount in getBulkRefundPreview to mirror
processRefunds' rounding behavior.
In `@apps/web/app/api/registration/bulk-refund-preview/route.ts`:
- Around line 4-12: The GET handler uses
request.nextUrl.searchParams.get("eventId") and directly interpolates it into
backendPath (/admin-registration/${eventId}/bulk-refund-preview), which allows
path injection; update the GET function to validate eventId before use by either
enforcing it matches a positive integer (e.g., /^\d+$/) or by URL-encoding it,
return 400 if invalid, and only then build backendPath and call fetchWithAuth
with that safe value (reference identifiers: GET, eventId, backendPath,
fetchWithAuth).
In `@apps/web/app/api/registration/bulk-refund/route.ts`:
- Around line 5-15: The code uses request.nextUrl.searchParams.get("eventId")
and directly interpolates it into backendPath in GET; validate that eventId is a
positive integer (e.g., via /^\d+$/ or Number.isInteger(parseInt(eventId)) &&
parseInt(eventId) > 0) before calling fetchSSEWithAuth (or alternatively
encodeURIComponent), and return a 400 NextResponse if validation fails; update
the GET handler and the usage of backendPath
`/admin-registration/${eventId}/bulk-refund` accordingly (referencing the GET
function and the backendPath interpolation).
In `@apps/web/app/events/`[eventId]/refunds/page.tsx:
- Around line 3-4: Import useRef and create a ref (e.g., refundEventSourceRef)
to hold the EventSource instance, update handleRefundAll to close any existing
refundEventSourceRef.current before creating a new EventSource and store the new
instance there, ensure existing onmessage/onerror handlers still close and null
out refundEventSourceRef.current on completion/error, and add a useEffect
cleanup that closes refundEventSourceRef.current on component unmount to prevent
lingering SSE connections.
🧹 Nitpick comments (1)
apps/web/app/events/[eventId]/refunds/page.tsx (1)
33-65: Add a default case in the reducer for type safety and defensive programming.The reducer is currently exhaustive for all
RefundActioncases, but lacks a default branch. With TypeScript strict mode enabled (strict: truein tsconfig), adding an explicit default case improves code robustness and prevents unexpected behavior if the action union is extended in the future.Suggested fix
case "ERROR": return { ...state, phase: "error", error: action.error, } + default: + return state } }
Comment on lines
+219
to
+221
| // Start new bulk refund and return progress stream | ||
| const issuerId = 1 // TODO: change issuer to a string | ||
| const observable = this.refundService.processBulkRefundsStream(eventId, issuerId) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's verify the file exists and examine the specific lines
if [ -f "apps/api/src/registration/controllers/admin-registration.controller.ts" ]; then
echo "=== File content around lines 219-221 ==="
sed -n '210,230p' apps/api/src/registration/controllers/admin-registration.controller.ts
else
echo "File not found"
fiRepository: finleysg/bhmc-admin
Length of output: 597
🏁 Script executed:
#!/bin/bash
# Search for issuerId usage patterns in the api directory
echo "=== issuerId usage in API code ==="
rg -n "issuerId" apps/api/src --context 2
echo ""
echo "=== Admin identity/user context patterns ==="
rg -n "req\.user|request\.user|@Req\(" apps/api/src/registration/controllers -A 2 -B 2Repository: finleysg/bhmc-admin
Length of output: 17426
🏁 Script executed:
#!/bin/bash
# Search for refundService.processBulkRefundsStream to understand the method signature
echo "=== processBulkRefundsStream method signature ==="
rg -n "processBulkRefundsStream" apps/api/src -B 5 -A 10Repository: finleysg/bhmc-admin
Length of output: 2877
🏁 Script executed:
#!/bin/bash
# Look for other admin-related endpoints to find the pattern
echo "=== Admin context patterns in other controllers ==="
rg -n "@Controller|@Post|@Get" apps/api/src/registration/controllers/admin-registration.controller.ts -A 3Repository: finleysg/bhmc-admin
Length of output: 2861
🏁 Script executed:
#!/bin/bash
# Quick check on the method signature for getUser/extracting admin identity
echo "=== Checking AuthenticatedRequest type ==="
rg -n "interface AuthenticatedRequest|type AuthenticatedRequest" apps/api/src -A 5
echo ""
echo "=== Checking for admin identity extraction patterns ==="
rg -n "req\.user\.id" apps/api/src/registration/controllers -B 2 -A 1Repository: finleysg/bhmc-admin
Length of output: 2181
🏁 Script executed:
#!/bin/bash
# Check if there's a request object available in the controller context
echo "=== Full admin-registration.controller.ts first 50 lines ==="
head -50 apps/api/src/registration/controllers/admin-registration.controller.tsRepository: finleysg/bhmc-admin
Length of output: 1553
Replace hardcoded issuerId with authenticated admin identity from request context.
Hardcoding issuerId = 1 breaks auditability—all refunds are incorrectly attributed to user ID 1 regardless of who issued them. Inject @Req() req: AuthenticatedRequest and use req.user.id instead. This same issue exists at line 195 in the processRefunds method. The pattern is established elsewhere in the codebase (user-registration.controller.ts, user-payments.controller.ts).
🤖 Prompt for AI Agents
In `@apps/api/src/registration/controllers/admin-registration.controller.ts`
around lines 219 - 221, Replace the hardcoded issuerId = 1 with the
authenticated admin ID from the request: inject `@Req`() req: AuthenticatedRequest
into the controller methods and use req.user.id when calling
refundService.processBulkRefundsStream and refundService.processRefunds (the two
locations where issuerId is currently set). Update the parameter list for the
methods that create the observable/handle refunds to accept Req and pass
req.user.id into the refundService calls so refunds are attributed to the actual
authenticated admin.
Comment on lines
+75
to
+125
| async findConfirmedPaymentsByEventWithDetails( | ||
| eventId: number, | ||
| ): Promise<PaymentWithPlayerDetails[]> { | ||
| const results = await this.drizzle.db | ||
| .select({ | ||
| paymentId: payment.id, | ||
| paymentCode: payment.paymentCode, | ||
| paymentAmount: payment.paymentAmount, | ||
| playerFirstName: player.firstName, | ||
| playerLastName: player.lastName, | ||
| registrationFeeId: registrationFee.id, | ||
| feeAmount: registrationFee.amount, | ||
| isPaid: registrationFee.isPaid, | ||
| }) | ||
| .from(payment) | ||
| .innerJoin(registrationFee, eq(payment.id, registrationFee.paymentId)) | ||
| .innerJoin(registrationSlot, eq(registrationFee.registrationSlotId, registrationSlot.id)) | ||
| .innerJoin(player, eq(registrationSlot.playerId, player.id)) | ||
| .where( | ||
| and( | ||
| eq(payment.eventId, eventId), | ||
| eq(payment.confirmed, 1), | ||
| like(payment.paymentCode, "pi_%"), | ||
| ), | ||
| ) | ||
|
|
||
| // Group by payment ID | ||
| const paymentsMap = new Map<number, PaymentWithPlayerDetails>() | ||
| for (const row of results) { | ||
| const existing = paymentsMap.get(row.paymentId) | ||
| if (existing) { | ||
| existing.fees.push({ | ||
| registrationFeeId: row.registrationFeeId, | ||
| amount: row.feeAmount, | ||
| isPaid: row.isPaid, | ||
| }) | ||
| } else { | ||
| paymentsMap.set(row.paymentId, { | ||
| paymentId: row.paymentId, | ||
| paymentCode: row.paymentCode, | ||
| paymentAmount: row.paymentAmount, | ||
| playerName: `${row.playerFirstName} ${row.playerLastName}`, | ||
| fees: [ | ||
| { | ||
| registrationFeeId: row.registrationFeeId, | ||
| amount: row.feeAmount, | ||
| isPaid: row.isPaid, | ||
| }, | ||
| ], | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find schema definitions
fd -type f -name '*.ts' apps/api/src/database | head -20Repository: finleysg/bhmc-admin
Length of output: 233
🏁 Script executed:
# Search for schema definitions with payment, registrationFee, and relevant tables
rg -n "export const payment|export const registrationFee|export const registrationSlot|export const player" apps/api/src --type ts -A 10 | head -100Repository: finleysg/bhmc-admin
Length of output: 8491
🏁 Script executed:
# Look for the payments.repository.ts file and examine its context
cat -n apps/api/src/registration/repositories/payments.repository.ts | head -150Repository: finleysg/bhmc-admin
Length of output: 5261
🏁 Script executed:
# Search for how payments are created/inserted to understand cardinality
rg -n "paymentId|registrationFee.*insert|payment.*insert" apps/api/src/registration -g '*.ts' -B 2 -A 5 | head -100Repository: finleysg/bhmc-admin
Length of output: 8531
🏁 Script executed:
# Confirm the schema for registrationSlot and its FK to player
cat -n apps/api/src/database/schema/registration.schema.ts | sed -n '100,150p'Repository: finleysg/bhmc-admin
Length of output: 2021
🏁 Script executed:
# Check registrationFee schema for paymentId and registrationSlotId
cat -n apps/api/src/database/schema/registration.schema.ts | sed -n '125,145p'Repository: finleysg/bhmc-admin
Length of output: 882
Payment may span multiple players; playerName field will only reflect the first player.
The cardinality allows a single payment to have multiple registrationFee records pointing to different registrationSlots (each with its own player). The grouping logic stores only the first player's name per payment, but the fees array can include fees from other players. Either return an array of distinct player names or attach playerName to each individual fee.
🤖 Prompt for AI Agents
In `@apps/api/src/registration/repositories/payments.repository.ts` around lines
75 - 125, The current findConfirmedPaymentsByEventWithDetails logic writes a
single playerName per payment in paymentsMap but a payment can have fees for
multiple players; update the model and grouping to preserve per-fee player info
or collect distinct player names: modify the PaymentWithPlayerDetails type and
the loop in findConfirmedPaymentsByEventWithDetails so each fee entry includes
playerFirstName/playerLastName (or playerName) OR add a players: string[] (or
players: {firstName,lastName}[]) on the payment and, when iterating results,
either push a fee object that includes the
row.playerFirstName/row.playerLastName or maintain a Set to collect unique
player names and assign players array before setting/returning the map values
(adjust paymentsMap handling and the initial object creation accordingly).
Comment on lines
+17
to
+29
| startTracking(eventId: number): Subject<BulkRefundProgressEvent> { | ||
| if (this.activeOperations.has(eventId)) { | ||
| throw new Error(`Bulk refund operation already in progress for event ${eventId}`) | ||
| } | ||
|
|
||
| const subject = new Subject<BulkRefundProgressEvent>() | ||
| this.activeOperations.set(eventId, subject) | ||
|
|
||
| // Auto-cleanup after timeout | ||
| setTimeout(() => { | ||
| this.cleanupOperation(eventId) | ||
| }, PROGRESS_CLEANUP_MS) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and read the bulk-refund-progress-tracker.ts file
fd "bulk-refund-progress-tracker.ts" --type f | head -1 | xargs cat -nRepository: finleysg/bhmc-admin
Length of output: 4005
🏁 Script executed:
#!/bin/bash
# Also check for PROGRESS_CLEANUP_MS constant definition
rg "PROGRESS_CLEANUP_MS" --type ts -B 2 -A 2Repository: finleysg/bhmc-admin
Length of output: 986
🏁 Script executed:
#!/bin/bash
# Search for any PRD or documentation files related to bulk refund
fd -e "md" -e "json" -e "txt" | xargs grep -l "bulk.refund\|bulk-refund" 2>/dev/null | head -5Repository: finleysg/bhmc-admin
Length of output: 89
🏁 Script executed:
#!/bin/bash
cat plans/bulk-refund-prd.json | jq '.[] | select(.property | contains("duration") or contains("timeout") or contains("batch") or contains("limit"))' 2>/dev/null || cat plans/bulk-refund-prd.json | head -100Repository: finleysg/bhmc-admin
Length of output: 5687
🏁 Script executed:
#!/bin/bash
# Get the rest of the PRD file to check for batch size or duration limits
tail -n +100 plans/bulk-refund-prd.json | head -200Repository: finleysg/bhmc-admin
Length of output: 6041
🏁 Script executed:
#!/bin/bash
# Check if there are any batch size limits or expected operation duration comments in the code
rg "batch|limit|duration|timeout|refund.*timeout|refund.*delay" apps/api/src/registration --type ts -i -B 2 -A 2Repository: finleysg/bhmc-admin
Length of output: 14044
🏁 Script executed:
#!/bin/bash
# Check the refund service to see how payments are processed
fd "refund.service.ts" --type f | head -1 | xargs wc -lRepository: finleysg/bhmc-admin
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Check the refund service implementation to understand processing model
cat -n apps/api/src/registration/services/refund.service.ts | sed -n '1,50p'Repository: finleysg/bhmc-admin
Length of output: 2312
🏁 Script executed:
#!/bin/bash
# Look for the processBulkRefundsStream method specifically
rg -A 30 "processBulkRefundsStream" apps/api/src/registration/services/refund.service.tsRepository: finleysg/bhmc-admin
Length of output: 1176
🏁 Script executed:
#!/bin/bash
# Check if there are any batch size limits documented or enforced
rg "MAX.*PAYMENT|MAX.*BATCH|batch.*size" apps/api/src/registration -i -B 2 -A 2Repository: finleysg/bhmc-admin
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check the rest of processBulkRefundsStream to see completion flow
rg -A 60 "processBulkRefundsStream" apps/api/src/registration/services/refund.service.ts | tail -40Repository: finleysg/bhmc-admin
Length of output: 1407
🏁 Script executed:
#!/bin/bash
# Check for TypeScript strict mode compliance and any type safety issues in the file
head -20 apps/api/src/registration/services/bulk-refund-progress-tracker.tsRepository: finleysg/bhmc-admin
Length of output: 802
🏁 Script executed:
#!/bin/bash
# Check tsconfig for strict mode settings
rg "strict" apps/api/tsconfig.jsonRepository: finleysg/bhmc-admin
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Find tsconfig files
fd "tsconfig" --type f | grep -E "api|root" | head -3Repository: finleysg/bhmc-admin
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Check for any usage of 'any' type in the bulk-refund-progress-tracker file
rg ": any" apps/api/src/registration/services/bulk-refund-progress-tracker.tsRepository: finleysg/bhmc-admin
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check tsconfig strict mode
cat apps/api/tsconfig.json | grep -A 5 '"strict"'Repository: finleysg/bhmc-admin
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Find tsconfig and check strict mode
find . -name "tsconfig*.json" -type f | head -5 | xargs grep -l "strict" 2>/dev/nullRepository: finleysg/bhmc-admin
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Check the root tsconfig
cat tsconfig.json 2>/dev/null | head -30Repository: finleysg/bhmc-admin
Length of output: 572
Unconditional 5-minute cleanup timeout can terminate long-running bulk refund operations.
The startTracking method sets a fixed 5-minute timeout without any reset mechanism. Since emitProgress does not refresh the timer, events with many payments could exceed the timeout mid-stream, causing subscribers to miss the final progress/completion events. With no batch size limits documented in the PRD, this becomes a real concern for large events.
Implement a refreshable cleanup timer that resets on each progress emission and is explicitly cleared on completion or error. See suggested fix below.
🧹 Suggested fix (refreshable cleanup timer)
export class BulkRefundProgressTracker {
private readonly logger = new Logger(BulkRefundProgressTracker.name)
private readonly activeOperations = new Map<number, Subject<BulkRefundProgressEvent>>()
+ private readonly cleanupTimers = new Map<number, ReturnType<typeof setTimeout>>()
+
+ private scheduleCleanup(eventId: number, delayMs = PROGRESS_CLEANUP_MS): void {
+ const existing = this.cleanupTimers.get(eventId)
+ if (existing) {
+ clearTimeout(existing)
+ }
+ this.cleanupTimers.set(
+ eventId,
+ setTimeout(() => this.cleanupOperation(eventId), delayMs),
+ )
+ }
startTracking(eventId: number): Subject<BulkRefundProgressEvent> {
if (this.activeOperations.has(eventId)) {
throw new Error(`Bulk refund operation already in progress for event ${eventId}`)
}
const subject = new Subject<BulkRefundProgressEvent>()
this.activeOperations.set(eventId, subject)
- setTimeout(() => {
- this.cleanupOperation(eventId)
- }, PROGRESS_CLEANUP_MS)
+ this.scheduleCleanup(eventId)
return subject
}
emitProgress(eventId: number, current: number, total: number, playerName: string): void {
const subject = this.activeOperations.get(eventId)
if (subject) {
subject.next({
status: "processing",
current,
total,
playerName,
})
+ this.scheduleCleanup(eventId)
}
}
completeOperation(eventId: number, result: BulkRefundResponse): void {
const subject = this.activeOperations.get(eventId)
if (subject) {
subject.next({
status: "complete",
current: result.refundedCount + result.failedCount,
total: result.refundedCount + result.failedCount + result.skippedCount,
result,
})
}
- setTimeout(() => {
- this.cleanupOperation(eventId)
- }, 1000)
+ this.scheduleCleanup(eventId, 1000)
}
errorOperation(eventId: number, error: string): void {
const subject = this.activeOperations.get(eventId)
if (subject) {
subject.next({
status: "error",
current: 0,
total: 0,
error,
})
}
- setTimeout(() => {
- this.cleanupOperation(eventId)
- }, 1000)
+ this.scheduleCleanup(eventId, 1000)
}
private cleanupOperation(eventId: number): void {
+ const timer = this.cleanupTimers.get(eventId)
+ if (timer) {
+ clearTimeout(timer)
+ this.cleanupTimers.delete(eventId)
+ }
const subject = this.activeOperations.get(eventId)
if (subject) {
subject.complete()
this.activeOperations.delete(eventId)
this.logger.debug(`Cleaned up bulk refund operation for event ${eventId}`)
}
}Also applies to: lines 43–53, 58–73, 78–93, 105–112
🤖 Prompt for AI Agents
In `@apps/api/src/registration/services/bulk-refund-progress-tracker.ts` around
lines 17 - 29, The current startTracking creates an unrefreshable timeout that
can kill long-running operations; change the activeOperations map to store an
object { subject, timeoutId } instead of just Subject, create the cleanup timer
in startTracking using PROGRESS_CLEANUP_MS and save its id, then update
emitProgress to clear and rearm (reset) that timer on every emission, and ensure
cleanupOperation (and any completion/error methods like
completeTracking/failTracking) clear the timer and remove the entry so the
timeout is not left running.
Comment on lines
+154
to
+172
| for (const p of confirmedPayments) { | ||
| const paidFees = p.fees.filter((f) => f.isPaid === 1) | ||
| if (paidFees.length === 0) { | ||
| skippedCount++ | ||
| continue | ||
| } | ||
|
|
||
| const refundAmount = paidFees.reduce((sum, f) => sum + parseFloat(f.amount), 0) | ||
| payments.push({ | ||
| paymentId: p.paymentId, | ||
| playerName: p.playerName, | ||
| feeCount: paidFees.length, | ||
| refundAmount, | ||
| registrationFeeIds: paidFees.map((f) => f.registrationFeeId), | ||
| }) | ||
| } | ||
|
|
||
| const totalRefundAmount = payments.reduce((sum, p) => sum + p.refundAmount, 0) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PaymentWithPlayerDetails type definition
rg -n "PaymentWithPlayerDetails" apps/api/src/registration/ -A 5 -B 2
# Search for the refund service functions
rg -n "getBulkRefundPreview|processRefunds" apps/api/src/registration/ -A 20 -B 2
# Search for isPaid field usage across the registration module
rg -n "isPaid" apps/api/src/registration/ -B 2 -A 2Repository: finleysg/bhmc-admin
Length of output: 19887
🏁 Script executed:
#!/bin/bash
# Get a broader view of the refund.service.ts file structure
wc -l apps/api/src/registration/services/refund.service.ts
head -100 apps/api/src/registration/services/refund.service.tsRepository: finleysg/bhmc-admin
Length of output: 3494
🏁 Script executed:
#!/bin/bash
# Search for type definitions related to fees and payments
rg -n "interface.*Fee|type.*Fee|interface.*Payment|type.*Payment" apps/api/src/registration/ -A 3Repository: finleysg/bhmc-admin
Length of output: 2608
🏁 Script executed:
#!/bin/bash
# Find the findPaymentWithDetailsById method implementation
rg -n "findPaymentWithDetailsById" apps/api/src/registration/repositories/ -A 30Repository: finleysg/bhmc-admin
Length of output: 3042
🏁 Script executed:
#!/bin/bash
# Check the schema/database definition for registrationFee to understand isPaid type
fd -e ts -e sql "schema|database" apps/ packages/ | head -20
rg -n "registrationFee\s*=" apps/database/ packages/database/ 2>/dev/null | head -20Repository: finleysg/bhmc-admin
Length of output: 536
🏁 Script executed:
#!/bin/bash
# Check the registration schema to see isPaid field definition
cat apps/api/src/database/schema/registration.schema.ts | head -200Repository: finleysg/bhmc-admin
Length of output: 5270
🏁 Script executed:
#!/bin/bash
# Search for RegistrationFeeRow type definition
rg -n "type RegistrationFeeRow|interface RegistrationFeeRow" apps/ packages/ -A 15Repository: finleysg/bhmc-admin
Length of output: 4168
🏁 Script executed:
#!/bin/bash
# Check PaymentRowWithDetails and PaymentWithPlayerDetails types
rg -n "PaymentRowWithDetails|PaymentWithPlayerDetails" apps/api/src/database/types.ts -A 5Repository: finleysg/bhmc-admin
Length of output: 351
🏁 Script executed:
#!/bin/bash
# Look at the actual refund.service.ts file more completely to see both methods
sed -n '145,175p' apps/api/src/registration/services/refund.service.ts
sed -n '34,60p' apps/api/src/registration/services/refund.service.tsRepository: finleysg/bhmc-admin
Length of output: 1921
Fix rounding inconsistency between preview and actual refund calculations.
getBulkRefundPreview sums floating-point amounts directly (parseFloat(f.amount)), while processRefunds rounds each amount to cents before summing (Math.round(parseFloat(fee.amount) * 100)). This causes the preview total to diverge from the actual refund amount when fee amounts have more than 2 decimal places.
For example, with a fee of $10.555:
- Preview: 10.555
- Actual: 10.56
Align both methods to round to cents consistently.
🤖 Prompt for AI Agents
In `@apps/api/src/registration/services/refund.service.ts` around lines 154 - 172,
The preview calculation in getBulkRefundPreview currently sums raw
parseFloat(f.amount) causing mismatch with processRefunds which rounds each fee
to cents; update getBulkRefundPreview so paidFees are summed the same way as
processRefunds (either round each fee to cents with
Math.round(parseFloat(f.amount) * 100) / 100 before summing, or sum integer
cents then divide by 100) and ensure refundAmount and totalRefundAmount use that
rounded-to-cents value so preview and actual refunds match; modify the logic
around paidFees/refundAmount/totalRefundAmount in getBulkRefundPreview to mirror
processRefunds' rounding behavior.
Comment on lines
+4
to
+12
| export async function GET(request: NextRequest) { | ||
| const eventId = request.nextUrl.searchParams.get("eventId") | ||
|
|
||
| if (!eventId) { | ||
| return NextResponse.json({ error: "eventId is required" }, { status: 400 }) | ||
| } | ||
|
|
||
| const backendPath = `/admin-registration/${eventId}/bulk-refund-preview` | ||
| return fetchWithAuth({ request, backendPath }) |
Contributor
There was a problem hiding this comment.
Validate eventId before interpolating into backend path.
A crafted value containing / or .. can alter the backend URL. Enforce a positive integer (or encode) before building the path.
🔧 Suggested fix
export async function GET(request: NextRequest) {
- const eventId = request.nextUrl.searchParams.get("eventId")
+ const eventIdParam = request.nextUrl.searchParams.get("eventId")
+ const eventId = Number(eventIdParam)
- if (!eventId) {
- return NextResponse.json({ error: "eventId is required" }, { status: 400 })
+ if (!eventIdParam || !Number.isInteger(eventId) || eventId <= 0) {
+ return NextResponse.json({ error: "eventId must be a positive integer" }, { status: 400 })
}
const backendPath = `/admin-registration/${eventId}/bulk-refund-preview`
return fetchWithAuth({ request, backendPath })
}🤖 Prompt for AI Agents
In `@apps/web/app/api/registration/bulk-refund-preview/route.ts` around lines 4 -
12, The GET handler uses request.nextUrl.searchParams.get("eventId") and
directly interpolates it into backendPath
(/admin-registration/${eventId}/bulk-refund-preview), which allows path
injection; update the GET function to validate eventId before use by either
enforcing it matches a positive integer (e.g., /^\d+$/) or by URL-encoding it,
return 400 if invalid, and only then build backendPath and call fetchWithAuth
with that safe value (reference identifiers: GET, eventId, backendPath,
fetchWithAuth).
Comment on lines
+5
to
+15
| export async function GET(request: NextRequest) { | ||
| const eventId = request.nextUrl.searchParams.get("eventId") | ||
|
|
||
| if (!eventId) { | ||
| return NextResponse.json({ error: "eventId is required" }, { status: 400 }) | ||
| } | ||
|
|
||
| return fetchSSEWithAuth({ | ||
| request, | ||
| backendPath: `/admin-registration/${eventId}/bulk-refund`, | ||
| }) |
Contributor
There was a problem hiding this comment.
Validate eventId before interpolating into backend path.
A crafted value containing / or .. can alter the backend URL. Enforce a positive integer (or encode) before building the path.
🔧 Suggested fix
export async function GET(request: NextRequest) {
- const eventId = request.nextUrl.searchParams.get("eventId")
+ const eventIdParam = request.nextUrl.searchParams.get("eventId")
+ const eventId = Number(eventIdParam)
- if (!eventId) {
- return NextResponse.json({ error: "eventId is required" }, { status: 400 })
+ if (!eventIdParam || !Number.isInteger(eventId) || eventId <= 0) {
+ return NextResponse.json({ error: "eventId must be a positive integer" }, { status: 400 })
}
return fetchSSEWithAuth({
request,
backendPath: `/admin-registration/${eventId}/bulk-refund`,
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function GET(request: NextRequest) { | |
| const eventId = request.nextUrl.searchParams.get("eventId") | |
| if (!eventId) { | |
| return NextResponse.json({ error: "eventId is required" }, { status: 400 }) | |
| } | |
| return fetchSSEWithAuth({ | |
| request, | |
| backendPath: `/admin-registration/${eventId}/bulk-refund`, | |
| }) | |
| export async function GET(request: NextRequest) { | |
| const eventIdParam = request.nextUrl.searchParams.get("eventId") | |
| const eventId = Number(eventIdParam) | |
| if (!eventIdParam || !Number.isInteger(eventId) || eventId <= 0) { | |
| return NextResponse.json({ error: "eventId must be a positive integer" }, { status: 400 }) | |
| } | |
| return fetchSSEWithAuth({ | |
| request, | |
| backendPath: `/admin-registration/${eventId}/bulk-refund`, | |
| }) | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/app/api/registration/bulk-refund/route.ts` around lines 5 - 15, The
code uses request.nextUrl.searchParams.get("eventId") and directly interpolates
it into backendPath in GET; validate that eventId is a positive integer (e.g.,
via /^\d+$/ or Number.isInteger(parseInt(eventId)) && parseInt(eventId) > 0)
before calling fetchSSEWithAuth (or alternatively encodeURIComponent), and
return a 400 NextResponse if validation fails; update the GET handler and the
usage of backendPath `/admin-registration/${eventId}/bulk-refund` accordingly
(referencing the GET function and the backendPath interpolation).
Comment on lines
+3
to
+4
| import { useEffect, useReducer, useState } from "react" | ||
| import { useParams, useRouter } from "next/navigation" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and read it
echo "=== Checking file existence ==="
ls -la apps/web/app/events/[eventId]/refunds/page.tsx 2>/dev/null || echo "File not found"
# List the directory to find the correct filename
echo -e "\n=== Listing refunds directory ==="
ls -la apps/web/app/events/ 2>/dev/null | head -20
fd "refunds" apps/web/app/events/ -type f 2>/dev/nullRepository: finleysg/bhmc-admin
Length of output: 453
🏁 Script executed:
#!/bin/bash
# Search for EventSource usage in the web app
echo "=== EventSource usage in apps/web ==="
rg -n "EventSource" -S apps/web --type ts --type tsx -C 3Repository: finleysg/bhmc-admin
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Read the file to see the actual implementation
echo "=== Reading refunds/page.tsx ==="
cat -n apps/web/app/events/[eventId]/refunds/page.tsxRepository: finleysg/bhmc-admin
Length of output: 12152
🏁 Script executed:
#!/bin/bash
# Search for EventSource usage without file type filter
echo "=== EventSource usage in apps/web ==="
rg -n "EventSource" apps/web -C 3Repository: finleysg/bhmc-admin
Length of output: 1495
Add EventSource cleanup on component unmount to prevent lingering SSE connections.
The EventSource created in handleRefundAll() is only closed when specific server events arrive (completion, error, or parse error). If the user navigates away before these events occur, the connection persists unnecessarily. Track the EventSource instance in a ref and close it on unmount; also close any existing connection before opening a new one.
Add useRef to the imports and implement cleanup:
Suggested fix
-import { useEffect, useReducer, useState } from "react"
+import { useEffect, useReducer, useRef, useState } from "react"Then add the cleanup effect and ref tracking in the component:
const [refundState, dispatch] = useReducer(refundReducer, initialRefundState)
+const eventSourceRef = useRef<EventSource | null>(null)
+
+useEffect(() => {
+ return () => {
+ eventSourceRef.current?.close()
+ eventSourceRef.current = null
+ }
+}, [])And in handleRefundAll():
dispatch({ type: "START_PROCESSING", total: preview.payments.length })
+eventSourceRef.current?.close()
const eventSource = new EventSource(`/api/registration/bulk-refund?eventId=${eventId}`)
+eventSourceRef.current = eventSource🤖 Prompt for AI Agents
In `@apps/web/app/events/`[eventId]/refunds/page.tsx around lines 3 - 4, Import
useRef and create a ref (e.g., refundEventSourceRef) to hold the EventSource
instance, update handleRefundAll to close any existing
refundEventSourceRef.current before creating a new EventSource and store the new
instance there, ensure existing onmessage/onerror handlers still close and null
out refundEventSourceRef.current on completion/error, and add a useEffect
cleanup that closes refundEventSourceRef.current on component unmount to prevent
lingering SSE connections.
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.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.