-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/bulk refunds #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2487092
3965b20
b2a03b3
3f973ce
f0e0a64
8bb5113
46e5c3c
cbfb135
1c1b942
593e0c2
bc002a7
5f39b55
66b7f7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| import { eq, inArray } from "drizzle-orm" | ||
| import { and, eq, inArray, like } from "drizzle-orm" | ||
|
|
||
| import { Inject, Injectable } from "@nestjs/common" | ||
|
|
||
| import { | ||
| DrizzleService, | ||
| payment, | ||
| PaymentRowWithDetails, | ||
| PaymentWithPlayerDetails, | ||
| player, | ||
| refund, | ||
| registrationFee, | ||
| registrationSlot, | ||
|
|
@@ -70,6 +72,62 @@ export class PaymentsRepository { | |
| } | ||
| } | ||
|
|
||
| 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, | ||
| }, | ||
| ], | ||
| }) | ||
| } | ||
|
Comment on lines
+75
to
+125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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; The cardinality allows a single payment to have multiple 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| return Array.from(paymentsMap.values()) | ||
| } | ||
|
|
||
| async createPayment(data: PaymentInsert): Promise<number> { | ||
| const [result] = await this.drizzle.db.insert(payment).values(data) | ||
| return Number(result.insertId) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { Observable, Subject } from "rxjs" | ||
|
|
||
| import { Injectable, Logger } from "@nestjs/common" | ||
| import { BulkRefundProgressEvent, BulkRefundResponse } from "@repo/domain/types" | ||
|
|
||
| const PROGRESS_CLEANUP_MS = 5 * 60 * 1000 // 5 minutes | ||
|
|
||
| @Injectable() | ||
| export class BulkRefundProgressTracker { | ||
| private readonly logger = new Logger(BulkRefundProgressTracker.name) | ||
|
|
||
| private readonly activeOperations = new Map<number, Subject<BulkRefundProgressEvent>>() | ||
|
|
||
| /** | ||
| * Start tracking a new bulk refund operation and return the progress subject | ||
| */ | ||
| 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) | ||
|
|
||
|
Comment on lines
+17
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 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 |
||
| return subject | ||
| } | ||
|
|
||
| /** | ||
| * Get the progress observable for an active operation | ||
| */ | ||
| getProgressObservable(eventId: number): Observable<BulkRefundProgressEvent> | null { | ||
| return this.activeOperations.get(eventId)?.asObservable() ?? null | ||
| } | ||
|
|
||
| /** | ||
| * Emit progress update for a payment being processed | ||
| */ | ||
| 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, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Mark operation as complete with final results | ||
| */ | ||
| 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, | ||
| }) | ||
| } | ||
|
|
||
| // Cleanup after short delay to allow final event to be sent | ||
| setTimeout(() => { | ||
| this.cleanupOperation(eventId) | ||
| }, 1000) | ||
| } | ||
|
|
||
| /** | ||
| * Mark operation as failed with error | ||
| */ | ||
| errorOperation(eventId: number, error: string): void { | ||
| const subject = this.activeOperations.get(eventId) | ||
| if (subject) { | ||
| subject.next({ | ||
| status: "error", | ||
| current: 0, | ||
| total: 0, | ||
| error, | ||
| }) | ||
| } | ||
|
|
||
| // Cleanup after short delay | ||
| setTimeout(() => { | ||
| this.cleanupOperation(eventId) | ||
| }, 1000) | ||
| } | ||
|
|
||
| /** | ||
| * Check if an operation is currently active | ||
| */ | ||
| isOperationActive(eventId: number): boolean { | ||
| return this.activeOperations.has(eventId) | ||
| } | ||
|
|
||
| /** | ||
| * Clean up resources for an operation | ||
| */ | ||
| private cleanupOperation(eventId: number): void { | ||
| 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}`) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Force cleanup of all operations (for testing/shutdown) | ||
| */ | ||
| cleanupAll(): void { | ||
| for (const eventId of this.activeOperations.keys()) { | ||
| this.cleanupOperation(eventId) | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: finleysg/bhmc-admin
Length of output: 597
🏁 Script executed:
Repository: finleysg/bhmc-admin
Length of output: 17426
🏁 Script executed:
Repository: finleysg/bhmc-admin
Length of output: 2877
🏁 Script executed:
Repository: finleysg/bhmc-admin
Length of output: 2861
🏁 Script executed:
Repository: finleysg/bhmc-admin
Length of output: 2181
🏁 Script executed:
Repository: finleysg/bhmc-admin
Length of output: 1553
Replace hardcoded issuerId with authenticated admin identity from request context.
Hardcoding
issuerId = 1breaks auditability—all refunds are incorrectly attributed to user ID 1 regardless of who issued them. Inject@Req() req: AuthenticatedRequestand usereq.user.idinstead. This same issue exists at line 195 in theprocessRefundsmethod. The pattern is established elsewhere in the codebase (user-registration.controller.ts, user-payments.controller.ts).🤖 Prompt for AI Agents