chore: Integrate confirmation booking audit #89
Conversation
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
Code Review by Qodo
1. acceptedBookings.oldStatus hardcoded
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+411
to
+416
| acceptedBookings = [ | ||
| { | ||
| oldStatus: BookingStatus.ACCEPTED, | ||
| uid: booking.uid, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
1. acceptedbookings.oldstatus hardcoded 📘 Rule violation ✧ Quality
• acceptedBookings is built with oldStatus: BookingStatus.ACCEPTED, which does not represent the real previous booking status for a newly-confirmed booking. • This produces incorrect audit metadata (no meaningful old→new transition), undermining the ability to reconstruct what actually happened. • This violates the requirement that audit trails include clear action context and outcome details.
Agent prompt
## Issue description
`acceptedBookings.oldStatus` is hardcoded to `BookingStatus.ACCEPTED`, which makes audit metadata incorrect (old status will not reflect the actual previous status).
## Issue Context
Audit logs should allow reconstructing the state transition (e.g., PENDING → ACCEPTED). The current code produces misleading audit trails.
## Fix Focus Areas
- packages/features/bookings/lib/handleConfirmation.ts[411-416]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+432
to
+447
| const updatedRecurringBookings = await prisma.booking.findMany({ | ||
| where: { | ||
| uid: { | ||
| in: unconfirmedRecurringBookings.map((booking) => booking.uid), | ||
| }, | ||
| }, | ||
| select: { | ||
| uid: true, | ||
| status: true, | ||
| }, | ||
| }); | ||
|
|
||
| rejectedBookings = updatedRecurringBookings.map((recurringBooking) => ({ | ||
| uid: recurringBooking.uid, | ||
| oldStatus: recurringBooking.status, | ||
| })); |
There was a problem hiding this comment.
2. Recurring rejection oldstatus wrong 📘 Rule violation ✧ Quality
• In the recurring rejection flow, oldStatus is derived from updatedRecurringBookings after updateMany sets all statuses to REJECTED. • This makes the audit data record REJECTED → REJECTED instead of capturing the real prior status (typically PENDING). • This breaks audit trail accuracy and impairs incident/compliance investigations.
Agent prompt
## Issue description
For recurring booking rejection, `oldStatus` is taken from rows re-read after the update, so it reflects the new `REJECTED` status instead of the previous status.
## Issue Context
Accurate audit trails require capturing the true state transition.
## Fix Focus Areas
- packages/trpc/server/routers/viewer/bookings/confirm.handler.ts[409-447]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
48
to
+50
| const { booking, user: userWithCredentials, evt, eventType } = await getBooking(bookingId); | ||
| const apps = eventTypeAppMetadataOptionalSchema.parse(eventType?.metadata?.apps); | ||
| const actor = getAppActor({ appSlug, bookingId, apps }); |
There was a problem hiding this comment.
3. Uncaught zod parse failure 📘 Rule violation ⛯ Reliability
• eventTypeAppMetadataOptionalSchema.parse(eventType?.metadata?.apps) can throw if stored metadata is malformed, and this failure is not handled. • A thrown Zod error can abort the payment success flow and webhook processing instead of degrading gracefully (e.g., falling back to a slug-based actor). • This violates the requirement to handle edge cases and provide robust error handling at failure points.
Agent prompt
## Issue description
`eventTypeAppMetadataOptionalSchema.parse(...)` can throw and fail webhook/payment handling when metadata is malformed.
## Issue Context
This is a critical flow; failures should degrade gracefully and provide actionable internal logs.
## Fix Focus Areas
- packages/app-store/_utils/payments/handlePaymentSuccess.ts[48-51]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+36
to
+42
| log.warn( | ||
| `Missing credentialId for app, using appSlug fallback`, | ||
| safeStringify({ | ||
| bookingId, | ||
| appSlug, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
4. getappactor logs unstructured json 📘 Rule violation ✧ Quality
• The warning log passes safeStringify(...) (a string) rather than structured fields, which may reduce log queryability and auditing effectiveness. • The compliance requirement expects structured logs suitable for monitoring/auditing; stringified JSON often prevents indexing on fields like bookingId and appSlug. • Consider emitting structured metadata as an object instead of a pre-stringified payload.
Agent prompt
## Issue description
The logger call uses `safeStringify(...)` which produces a string payload instead of structured fields.
## Issue Context
Structured logging improves auditability and supports compliance monitoring without relying on text parsing.
## Fix Focus Areas
- packages/app-store/_utils/getAppActor.ts[36-42]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
40
to
42
| type ConfirmOptions = { | ||
| ctx: { | ||
| user: Pick< |
There was a problem hiding this comment.
6. Trpc confirm test breaks 🐞 Bug ⛯ Reliability
• packages/trpc/server/routers/viewer/bookings/confirm.handler.test.ts still calls
confirmHandler with the old shape (no traceContext, no actor, no actionSource).
• Root vitest config runs all **/*.{test,spec}.* files by default, so this will fail CI.
• The PR updates other confirm tests, but this one remains incompatible with the new handler
contract.
Agent prompt
### Issue description
An existing vitest test file still calls `confirmHandler` without the newly required `ctx.traceContext` and `input.actor/actionSource`, which will fail CI.
### Issue Context
Root vitest config includes all `**/*.{test,spec}.*` files by default.
### Fix Focus Areas
- packages/trpc/server/routers/viewer/bookings/confirm.handler.test.ts[98-110]
- packages/trpc/server/routers/viewer/bookings/confirm.handler.ts[40-49]
- vitest.config.mts[20-32]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
19
to
+37
| const fieldsSchemaV1 = z.object({ | ||
| rejectionReason: StringChangeSchema, | ||
| status: StringChangeSchema, | ||
| rejectionReason: z.string().nullable(), | ||
| status: BookingStatusChangeSchema, | ||
| }); | ||
|
|
||
| export class RejectedAuditActionService implements IAuditActionService { | ||
| readonly VERSION = 1; | ||
| public static readonly TYPE = "REJECTED" as const; | ||
| private static dataSchemaV1 = z.object({ | ||
| version: z.literal(1), | ||
| fields: fieldsSchemaV1, | ||
| }); | ||
| private static fieldsSchemaV1 = fieldsSchemaV1; | ||
| public static readonly latestFieldsSchema = fieldsSchemaV1; | ||
| // Union of all versions | ||
| public static readonly storedDataSchema = RejectedAuditActionService.dataSchemaV1; | ||
| // Union of all versions | ||
| public static readonly storedFieldsSchema = RejectedAuditActionService.fieldsSchemaV1; | ||
| private helper: AuditActionServiceHelper< | ||
| typeof RejectedAuditActionService.latestFieldsSchema, | ||
| typeof RejectedAuditActionService.storedDataSchema | ||
| >; | ||
| readonly VERSION = 1; | ||
| public static readonly TYPE = "REJECTED" as const; | ||
| private static dataSchemaV1 = z.object({ | ||
| version: z.literal(1), | ||
| fields: fieldsSchemaV1, | ||
| }); | ||
| private static fieldsSchemaV1 = fieldsSchemaV1; | ||
| public static readonly latestFieldsSchema = fieldsSchemaV1; | ||
| // Union of all versions | ||
| public static readonly storedDataSchema = RejectedAuditActionService.dataSchemaV1; | ||
| // Union of all versions | ||
| public static readonly storedFieldsSchema = RejectedAuditActionService.fieldsSchemaV1; | ||
| private helper: AuditActionServiceHelper<typeof RejectedAuditActionService.latestFieldsSchema, typeof RejectedAuditActionService.storedDataSchema>; |
There was a problem hiding this comment.
7. Rejected audit schema break 🐞 Bug ⛯ Reliability
• RejectedAuditActionService changes the persisted fields shape but keeps VERSION = 1, so historical REJECTED audit records may no longer parse. • BookingAuditViewerService parses stored BookingAudit.data using the action service schema; a schema mismatch will throw and can break audit-log retrieval. • Because BookingAudit.data is persisted JSON, schema evolution needs versioning and migration/union parsing.
Agent prompt
### Issue description
`RejectedAuditActionService` changes the persisted fields schema but keeps `VERSION = 1`, risking inability to parse existing persisted REJECTED audit records.
### Issue Context
Audit records are persisted in `BookingAudit.data` (JSON) and parsed at read time by `BookingAuditViewerService` via `actionService.parseStored(log.data)`.
### Fix Focus Areas
- packages/features/booking-audit/lib/actions/RejectedAuditActionService.ts[19-77]
- packages/features/booking-audit/lib/service/BookingAuditViewerService.ts[143-153]
- packages/prisma/schema.prisma[2799-2836]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Benchmark PR from qodo-benchmark#714