feat(phase3-slice1): booking completion + payment domain event + upload URL stub#10
Merged
imKXNNY merged 6 commits intoApr 1, 2026
Merged
Conversation
…ad URL stub Milestone 3a — POST /api/v1/bookings/:id/complete - BookingStatus extended with 'completed' - completeAcceptedBooking() in booking repository (in-memory + postgres) - BookingsService.completeBooking() — role guard, status transition, payment capture - BookingsService.getBookingPayment() — fetch payment by bookingId - GET /api/v1/bookings/:id/payment endpoint - PaymentCapturedDomainEvent emitted and logged as structured breadcrumb Milestone 3a — Payments domain - PaymentsModule: PaymentRepository interface + InMemoryPaymentRepository - PaymentsService: capturePaymentForBooking() (idempotent), getPaymentByBookingId() - BookingsModule imports PaymentsModule Milestone 3b — POST /api/v1/providers/me/verification/upload-url - UploadUrlRecord domain type added to @quickwerk/domain - UploadUrlRepository interface + InMemoryUploadUrlRepository (stub presigned URL, 15min TTL) - ProvidersService.requestUploadUrl() — provider-only role guard - ProvidersController endpoint wired and tested Background workers - payment-captured.worker.ts stub (mirrors booking-accepted pattern, DLQ + retry) - payment-captured.worker.test.ts (8 tests) API client + app actions - bookingApiRoutes.complete, bookingApiRoutes.payment - providerApiRoutes.requestUploadUrl - createCompleteBookingRequest, createGetBookingPaymentRequest, createRequestUploadUrlRequest - booking-state: 'completed' added to status union - booking-screen-actions: completeBookingRequest action - provider-screen-actions: requestVerificationUploadUrl action Tests (all passing: 322 total across all packages) - bookings.service.complete.test.ts — 9 tests - payments.service.test.ts — 4 tests - providers.service.upload-url.test.ts — 4 tests - booking-screen-actions-complete.test.ts — 5 tests Smoke script - scripts/smoke/phase3-slice1-complete-upload-smoke.sh — 13 assertions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds booking completion with server-side payment capture, a provider presigned upload‑URL flow, payments domain/repositories/services, payment-captured worker and DLQ logic, new controller endpoints and DI wiring, tests, and a Slice‑1 end‑to‑end smoke script. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Product App
participant API as Platform API
participant PaymentSvc as PaymentsService
participant BookingRepo as Booking Repository
participant EventPub as Event Publisher
Client->>API: POST /api/v1/bookings/{id}/complete (Bearer)
API->>API: resolve session, correlationId
API->>PaymentSvc: capturePaymentForBooking(bookingId,...)
PaymentSvc->>PaymentSvc: payments.createPayment(...)
PaymentSvc-->>API: PaymentRecord {status: captured}
API->>BookingRepo: completeAcceptedBooking(bookingId,...)
BookingRepo-->>API: BookingRecord {status: completed}
API->>EventPub: publishPaymentCaptured(payment.captured event)
EventPub-->>API: ack
API-->>Client: 200 { booking, payment }
sequenceDiagram
participant Provider as Provider App
participant API as Platform API
participant UploadRepo as Upload URL Repository
Provider->>API: POST /api/v1/providers/me/verification/upload-url (Bearer, filename, mimeType)
API->>API: resolve session, validate provider role
API->>UploadRepo: createUploadUrl(providerUserId, filename, mimeType, now)
UploadRepo-->>API: UploadUrlRecord { uploadId, presignedUrl, expiresAt, filename, mimeType }
API-->>Provider: 201 Created { uploadId, presignedUrl, expiresAt, filename, mimeType }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 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: 8
🧹 Nitpick comments (6)
services/platform-api/src/providers/providers.module.ts (1)
6-7: Remove duplicate provider registration for upload URL repository.
InMemoryUploadUrlRepositoryanduploadUrlRepositoryProviderare both registered in the providers array. Keep only the token-bound provider to avoid redundant DI bindings and potential multi-instance issues. The service injects by token (UPLOAD_URL_REPOSITORY), not by class.♻️ Proposed cleanup
-import { InMemoryUploadUrlRepository } from './infrastructure/in-memory-upload-url.repository'; import { uploadUrlRepositoryProvider } from './infrastructure/upload-url-repository.provider'; @@ providers: [ ProvidersService, providerVerificationRepositoryProvider, providerProfileRepositoryProvider, - InMemoryUploadUrlRepository, uploadUrlRepositoryProvider, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/providers/providers.module.ts` around lines 6 - 7, Remove the duplicate provider registration in the module providers array: keep the token-bound provider export uploadUrlRepositoryProvider and remove the direct class InMemoryUploadUrlRepository so the DI binding uses the UPLOAD_URL_REPOSITORY token only; update the providers array to include uploadUrlRepositoryProvider (and any other token providers) and delete the InMemoryUploadUrlRepository entry to avoid multi-instance bindings when injecting by UPLOAD_URL_REPOSITORY.packages/domain/src/index.ts (1)
135-145: FieldcapturedAtis always required but status may not becaptured.
PaymentRecord.capturedAtis a required field, butstatuscan be'pending' | 'authorized' | 'failed' | 'refunded'in addition to'captured'. For non-captured payments, this field name is semantically misleading and may cause confusion.Consider renaming to
createdAtor making it optional (capturedAt?: string) when the payment isn't incapturedstatus.💡 Option A: Rename to createdAt
export type PaymentRecord = { paymentId: string; bookingId: string; customerUserId: string; providerUserId: string; amountCents: number; currency: string; status: PaymentStatus; - capturedAt: string; + createdAt: string; correlationId: string; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/domain/src/index.ts` around lines 135 - 145, PaymentRecord currently requires capturedAt even when status (PaymentStatus) may be 'pending'|'authorized'|'failed'|'refunded'; update the type so the timestamp makes sense: either rename capturedAt to createdAt across the codebase (type PaymentRecord and all usages) to represent the creation time, or make capturedAt optional (capturedAt?: string) and ensure callers only set/read it when status === 'captured' (update validation/constructors and any code that assumes it always exists). Reference the PaymentRecord type, capturedAt field, and PaymentStatus usages when making the change.services/platform-api/src/bookings/bookings.service.complete.test.ts (1)
96-113: Consider adding a test for cross-provider completion attempt.The PR objectives mention "cross-provider attempts return 409", but there's no test verifying that a different provider attempting to complete a booking accepted by another provider receives a 409 conflict response. This is an important idempotency/authorization edge case.
💡 Suggested test case
it('returns 409 when different provider attempts to complete booking accepted by another provider', async () => { const { service } = createService(); const customer = createSession('customer', 'customer-1'); const providerA = createSession('provider', 'provider-a'); const providerB = createSession('provider', 'provider-b'); const created = await service.createBooking(customer, { requestedService: 'Plumbing' }); expect(created.ok).toBe(true); if (!created.ok) return; await service.acceptBooking(providerA, created.booking.bookingId); const result = await service.completeBooking(providerB, created.booking.bookingId); expect(result.ok).toBe(false); if (!result.ok) expect(result.statusCode).toBe(409); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/bookings/bookings.service.complete.test.ts` around lines 96 - 113, Add a new test that verifies a different provider cannot complete a booking accepted by another provider: use createService() and createSession('provider-a') / createSession('provider-b'), call service.createBooking(...) then service.acceptBooking(providerA, bookingId) and then call service.completeBooking(providerB, bookingId) and assert the result is a failure with statusCode 409 (i.e., result.ok is false and result.statusCode === 409) to ensure cross-provider completion returns Conflict; reference the existing helpers createService, createSession and the service methods createBooking, acceptBooking, completeBooking when adding the test.scripts/smoke/phase3-slice1-complete-upload-smoke.sh (2)
76-80: JSON extraction withsedis fragile.The
extractfunction usessedregex to parse JSON, which can fail with escaped quotes, nested objects, or multiline values. Consider usingjqfor more robust parsing, as it's already commonly available in CI environments.💡 Suggested jq-based extract function
extract() { local json="$1" local field="$2" - echo "$json" | sed -n "s/.*\"${field}\":\"\([^\"]*\)\".*/\1/p" | head -1 + echo "$json" | jq -r ".$field // empty" 2>/dev/null || echo "" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/smoke/phase3-slice1-complete-upload-smoke.sh` around lines 76 - 80, The current extract function uses sed which breaks on escaped quotes, nested JSON, and multiline values; update the extract function to parse JSON with jq instead: accept the same parameters (json, field), check for jq availability, and run the JSON through jq -r --arg field "$field" '.[$field] // empty' (or equivalent) to reliably extract the value; if jq is not available in the environment add a lightweight fallback (e.g., a python -c one-liner) that safely decodes JSON, and ensure the function returns an empty string on missing keys to preserve existing behavior.
160-164: Consider adding cross-provider completion smoke test.The PR objectives mention "cross-provider attempts return 409", but this smoke test only validates that a customer cannot complete (403). Adding a test where a second provider attempts to complete a booking accepted by the first provider would increase coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/smoke/phase3-slice1-complete-upload-smoke.sh` around lines 160 - 164, Add a cross-provider completion smoke test after the customer 403 check: ensure the booking has been accepted by the first provider (call req_status POST "/api/v1/bookings/${BOOKING_ID}/accept" with PROVIDER1_TOKEN and assert success), then simulate a different provider attempting to complete the same booking (call req_status POST "/api/v1/bookings/${BOOKING_ID}/complete" with PROVIDER2_TOKEN) and assert_status that this returns "409" to verify cross-provider conflict handling; use the existing helper functions req_status and assert_status and the existing BOOKING_ID variable and token variables (PROVIDER1_TOKEN, PROVIDER2_TOKEN) to locate where to insert these steps.services/platform-api/src/providers/providers.service.ts (1)
466-516: Consider validatingfilenameandmimeTypeinputs.The method accepts
filenameandmimeTypewithout validation. Empty strings or invalid MIME types could be passed through to the repository. Other methods in this service (e.g.,upsertProfile) validate required fields likedisplayName.💡 Suggested input validation
async requestUploadUrl( session: AuthSession, input: { filename: string; mimeType: string }, context?: { correlationId?: string }, ): Promise< | { ok: false; statusCode: 400 | 403; error: string } | { ok: true; statusCode: 201; uploadUrl: UploadUrlRecord } > { const correlationId = context?.correlationId ?? 'corr-missing'; + + const filename = input.filename?.trim(); + const mimeType = input.mimeType?.trim(); + + if (!filename || !mimeType) { + logStructuredBreadcrumb({ + event: 'provider.upload-url.request', + correlationId, + status: 'failed', + details: { reason: 'invalid-input', actorUserId: session.userId }, + }); + return { ok: false, statusCode: 400, error: 'filename and mimeType are required.' }; + } if (session.role !== 'provider') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/providers/providers.service.ts` around lines 466 - 516, The requestUploadUrl handler (requestUploadUrl) currently passes input.filename and input.mimeType straight to uploadUrls.createUploadUrl; add validation to reject empty or malformed values before calling createUploadUrl: check filename is a non-empty string (and optionally length/character rules) and mimeType is a non-empty valid MIME type (use a simple regex or a shared validation helper if available), and if validation fails return a 400-style response (ok: false, statusCode: 400, error: "...") and log the failure with logStructuredBreadcrumb (event: 'provider.upload-url.request', status: 'failed', details: { reason: 'invalid-input', ... }) so invalid inputs never reach uploadUrls.createUploadUrl or the repository.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agent/plans/phase3-payments-payouts-kickoff-2026-03-31.md:
- Line 180: The file references the wrong smoke script name: update the
reference `phase3-payment-upload-smoke.sh` to match the actual script added
`phase3-slice1-complete-upload-smoke.sh` so the doc aligns with the PR
objectives; edit the line that currently lists "Smoke script:
`phase3-payment-upload-smoke.sh`" and replace the filename with
`phase3-slice1-complete-upload-smoke.sh`.
- Line 4: Update the status line "**Status:** PLANNING — Phase 2 complete (PR
`#9`), Phase 3 not yet started" to reflect that Phase 3 Slice 1 has been delivered
(e.g., "**Status:** IN PROGRESS — Phase 3 Slice 1 delivered (PR #<number>)" or
similar), ensuring the exact status string in
.agent/plans/phase3-payments-payouts-kickoff-2026-03-31.md is replaced so the
file accurately reflects the current implementation state.
In `@apps/admin-web/next-env.d.ts`:
- Line 3: Remove the manual import statement 'import
"./.next/dev/types/routes.d.ts";' from the auto-generated next-env.d.ts and do
not commit manual edits to that file; instead either (A) delete the import and
revert next-env.d.ts to the generated default and add next-env.d.ts to
.gitignore so Next manages it, or (B) if you actually want typed routes, enable
typedRoutes: true in next.config.ts and then regenerate the Next files so the
proper routes typings are produced—do not keep a manual import in next-env.d.ts.
In `@services/platform-api/src/bookings/bookings.service.ts`:
- Around line 455-482: A PaymentCapturedDomainEvent is built
(paymentCapturedEvent) and a breadcrumb is logged (logStructuredBreadcrumb with
event 'payment.captured.domain-event.emit') but the event is never published
because BookingDomainEventPublisher only exposes publishBookingAccepted() and
publishBookingDeclined(); add proper publishing to avoid the misleading log.
Update the BookingDomainEventPublisher interface to include
publishPaymentCaptured(paymentCapturedEvent: PaymentCapturedDomainEvent) (or add
a separate PaymentDomainEventPublisher with publishPaymentCaptured) and then
call that publisher where paymentCapturedEvent is constructed; also change the
breadcrumb to reflect success only after the publish promise resolves or log
failure on publish error.
In
`@services/platform-api/src/bookings/infrastructure/postgres-booking.repository.ts`:
- Around line 269-276: The call to insertStatusEvent is using toStatus:
'completed' which violates the booking_status_history enum constraint (migration
0006 allows only 'submitted','accepted','declined'), so either change the
inserted status to one of the allowed values (e.g., set toStatus: 'accepted' in
the code path that calls insertStatusEvent) or update the database migration to
add 'completed' to the enum; locate the call in postgress-booking.repository.ts
(insertStatusEvent invocation) and fix the status value or create a new
migration that alters the enum to include 'completed' to resolve the constraint
violation.
In `@services/platform-api/src/payments/domain/payment.repository.ts`:
- Around line 13-15: CreatePaymentResult currently includes an unused failure
branch { ok: false; reason: 'duplicate-booking' }; update the type by either
removing that union member if duplicate cases are handled by returning { ok:
true; payment, replayed: true } (remove the { ok: false; reason:
'duplicate-booking' } variant from CreatePaymentResult) or add a comment/docs on
CreatePaymentResult explaining when { ok: false; reason: 'duplicate-booking' }
would be returned in future implementations and why it differs from the replayed
true path so callers know which behavior to expect.
In `@services/platform-api/src/payments/payments.service.ts`:
- Around line 20-35: The breadcrumb currently logs status: 'succeeded'
unconditionally before checking the capture result; update the logic in
payments.service.ts around logStructuredBreadcrumb and the result handling so
the breadcrumb reflects the real outcome: either move the
logStructuredBreadcrumb call to after the if (!result.ok) check (so it runs only
on success), or compute status based on result.ok (e.g., status: result.ok ?
'succeeded' : 'failed') and include result.reason when failed; reference the
existing symbols logStructuredBreadcrumb, result, and input to locate and adjust
the code.
In `@services/platform-api/src/providers/providers.service.upload-url.test.ts`:
- Around line 86-99: The TTL assertion in the test using only `before` is too
tight and causes flakiness; after calling `service.requestUploadUrl` (the code
that reads `result.uploadUrl.expiresAt`) capture an `after = Date.now()` then
compute a lower bound using `before + 15min` and an upper bound using `after +
15min` (add a small slack like ±1s) and assert that `expiresMs` falls between
those bounds so the test tolerates execution latency.
---
Nitpick comments:
In `@packages/domain/src/index.ts`:
- Around line 135-145: PaymentRecord currently requires capturedAt even when
status (PaymentStatus) may be 'pending'|'authorized'|'failed'|'refunded'; update
the type so the timestamp makes sense: either rename capturedAt to createdAt
across the codebase (type PaymentRecord and all usages) to represent the
creation time, or make capturedAt optional (capturedAt?: string) and ensure
callers only set/read it when status === 'captured' (update
validation/constructors and any code that assumes it always exists). Reference
the PaymentRecord type, capturedAt field, and PaymentStatus usages when making
the change.
In `@scripts/smoke/phase3-slice1-complete-upload-smoke.sh`:
- Around line 76-80: The current extract function uses sed which breaks on
escaped quotes, nested JSON, and multiline values; update the extract function
to parse JSON with jq instead: accept the same parameters (json, field), check
for jq availability, and run the JSON through jq -r --arg field "$field"
'.[$field] // empty' (or equivalent) to reliably extract the value; if jq is not
available in the environment add a lightweight fallback (e.g., a python -c
one-liner) that safely decodes JSON, and ensure the function returns an empty
string on missing keys to preserve existing behavior.
- Around line 160-164: Add a cross-provider completion smoke test after the
customer 403 check: ensure the booking has been accepted by the first provider
(call req_status POST "/api/v1/bookings/${BOOKING_ID}/accept" with
PROVIDER1_TOKEN and assert success), then simulate a different provider
attempting to complete the same booking (call req_status POST
"/api/v1/bookings/${BOOKING_ID}/complete" with PROVIDER2_TOKEN) and
assert_status that this returns "409" to verify cross-provider conflict
handling; use the existing helper functions req_status and assert_status and the
existing BOOKING_ID variable and token variables (PROVIDER1_TOKEN,
PROVIDER2_TOKEN) to locate where to insert these steps.
In `@services/platform-api/src/bookings/bookings.service.complete.test.ts`:
- Around line 96-113: Add a new test that verifies a different provider cannot
complete a booking accepted by another provider: use createService() and
createSession('provider-a') / createSession('provider-b'), call
service.createBooking(...) then service.acceptBooking(providerA, bookingId) and
then call service.completeBooking(providerB, bookingId) and assert the result is
a failure with statusCode 409 (i.e., result.ok is false and result.statusCode
=== 409) to ensure cross-provider completion returns Conflict; reference the
existing helpers createService, createSession and the service methods
createBooking, acceptBooking, completeBooking when adding the test.
In `@services/platform-api/src/providers/providers.module.ts`:
- Around line 6-7: Remove the duplicate provider registration in the module
providers array: keep the token-bound provider export
uploadUrlRepositoryProvider and remove the direct class
InMemoryUploadUrlRepository so the DI binding uses the UPLOAD_URL_REPOSITORY
token only; update the providers array to include uploadUrlRepositoryProvider
(and any other token providers) and delete the InMemoryUploadUrlRepository entry
to avoid multi-instance bindings when injecting by UPLOAD_URL_REPOSITORY.
In `@services/platform-api/src/providers/providers.service.ts`:
- Around line 466-516: The requestUploadUrl handler (requestUploadUrl) currently
passes input.filename and input.mimeType straight to uploadUrls.createUploadUrl;
add validation to reject empty or malformed values before calling
createUploadUrl: check filename is a non-empty string (and optionally
length/character rules) and mimeType is a non-empty valid MIME type (use a
simple regex or a shared validation helper if available), and if validation
fails return a 400-style response (ok: false, statusCode: 400, error: "...") and
log the failure with logStructuredBreadcrumb (event:
'provider.upload-url.request', status: 'failed', details: { reason:
'invalid-input', ... }) so invalid inputs never reach uploadUrls.createUploadUrl
or the repository.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c9feba1-07dc-4bc9-9359-a7062cc274ab
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (37)
.agent/plans/phase3-payments-payouts-kickoff-2026-03-31.mdapps/admin-web/next-env.d.tsapps/admin-web/package.jsonapps/product-app/src/features/booking/booking-screen-actions-complete.test.tsapps/product-app/src/features/booking/booking-screen-actions.tsapps/product-app/src/features/booking/booking-state.tsapps/product-app/src/features/provider/provider-screen-actions.tspackages/api-client/src/index.tspackages/domain/src/index.tsscripts/smoke/phase3-slice1-complete-upload-smoke.shservices/background-workers/src/workers/payment-captured.worker.test.tsservices/background-workers/src/workers/payment-captured.worker.tsservices/platform-api/src/bookings/bookings.controller.tsservices/platform-api/src/bookings/bookings.module.tsservices/platform-api/src/bookings/bookings.service.complete.test.tsservices/platform-api/src/bookings/bookings.service.test.tsservices/platform-api/src/bookings/bookings.service.tsservices/platform-api/src/bookings/domain/booking.repository.tsservices/platform-api/src/bookings/infrastructure/in-memory-booking.repository.tsservices/platform-api/src/bookings/infrastructure/postgres-booking.repository.tsservices/platform-api/src/payments/domain/payment.repository.tsservices/platform-api/src/payments/infrastructure/in-memory-payment.repository.tsservices/platform-api/src/payments/infrastructure/payment-repository.provider.tsservices/platform-api/src/payments/payments.module.tsservices/platform-api/src/payments/payments.service.test.tsservices/platform-api/src/payments/payments.service.tsservices/platform-api/src/providers/domain/upload-url.repository.tsservices/platform-api/src/providers/infrastructure/in-memory-upload-url.repository.tsservices/platform-api/src/providers/infrastructure/upload-url-repository.provider.tsservices/platform-api/src/providers/providers.controller.tsservices/platform-api/src/providers/providers.module.tsservices/platform-api/src/providers/providers.service.discovery.test.tsservices/platform-api/src/providers/providers.service.profile.test.tsservices/platform-api/src/providers/providers.service.provider-detail.test.tsservices/platform-api/src/providers/providers.service.test.tsservices/platform-api/src/providers/providers.service.tsservices/platform-api/src/providers/providers.service.upload-url.test.ts
Comment on lines
+455
to
+482
| const paymentCapturedEvent: PaymentCapturedDomainEvent = { | ||
| eventName: 'payment.captured', | ||
| eventId: randomUUID(), | ||
| occurredAt: completedAt, | ||
| correlationId, | ||
| replayed: completed.replayed, | ||
| payment: { | ||
| paymentId: payment.paymentId, | ||
| bookingId: payment.bookingId, | ||
| customerUserId: payment.customerUserId, | ||
| providerUserId: payment.providerUserId, | ||
| amountCents: payment.amountCents, | ||
| currency: payment.currency, | ||
| status: 'captured', | ||
| }, | ||
| }; | ||
|
|
||
| logStructuredBreadcrumb({ | ||
| event: 'payment.captured.domain-event.emit', | ||
| correlationId, | ||
| status: 'succeeded', | ||
| details: { | ||
| eventId: paymentCapturedEvent.eventId, | ||
| paymentId: payment.paymentId, | ||
| bookingId, | ||
| replayed: completed.replayed, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the domain event publisher interface and implementations
rg -nA 10 'BookingDomainEventPublisher' --type tsRepository: MarkoAtc/QuickWerk
Length of output: 1778
🏁 Script executed:
#!/bin/bash
# First, let's locate the bookings.service.ts file and examine the code in question
find . -name "bookings.service.ts" -type fRepository: MarkoAtc/QuickWerk
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Search for BookingDomainEventPublisher to understand its interface
rg -n "BookingDomainEventPublisher" --type ts -A 5Repository: MarkoAtc/QuickWerk
Length of output: 11203
🏁 Script executed:
#!/bin/bash
# Search for publishPaymentCaptured or similar payment-related publish methods
rg -n "publishPayment" --type tsRepository: MarkoAtc/QuickWerk
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for publish methods in domain event publisher to understand available methods
rg -n "publish.*\(" --type ts | grep -i "domain\|event" | head -20Repository: MarkoAtc/QuickWerk
Length of output: 2015
🏁 Script executed:
#!/bin/bash
# Read the actual code at lines 455-482 to see context
sed -n '450,490p' services/platform-api/src/bookings/bookings.service.tsRepository: MarkoAtc/QuickWerk
Length of output: 1199
🏁 Script executed:
#!/bin/bash
# Search for PaymentCapturedDomainEvent definition
rg -n "PaymentCapturedDomainEvent" --type ts -B 2 -A 5Repository: MarkoAtc/QuickWerk
Length of output: 5975
🏁 Script executed:
#!/bin/bash
# Check if there are any calls to publish payment events
rg -n "payment.*captured" --type ts -i | head -20Repository: MarkoAtc/QuickWerk
Length of output: 2188
🏁 Script executed:
#!/bin/bash
# Look for the domain-event.publisher.ts file to see full interface
cat services/platform-api/src/orchestration/domain-event.publisher.tsRepository: MarkoAtc/QuickWerk
Length of output: 449
🏁 Script executed:
#!/bin/bash
# Check if there's a separate payment event publisher or another way to publish payment events
rg -n "PaymentDomainEventPublisher|payment.*publisher" --type ts -iRepository: MarkoAtc/QuickWerk
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check what other services inject for event publishing - look for similar patterns
rg -n "publishBookingAccepted\|publishBookingDeclined" --type ts -B 3 -A 3 | head -40Repository: MarkoAtc/QuickWerk
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Find all usages of publishBookingAccepted and publishBookingDeclined
rg -n "publishBookingAccepted\|publishBookingDeclined" --type tsRepository: MarkoAtc/QuickWerk
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the complete context around line 240 and 355 to see the pattern
sed -n '235,245p' services/platform-api/src/bookings/bookings.service.ts
sed -n '350,360p' services/platform-api/src/bookings/bookings.service.tsRepository: MarkoAtc/QuickWerk
Length of output: 634
PaymentCapturedDomainEvent is constructed but never published, and the breadcrumb is misleading.
The paymentCapturedEvent object is created (lines 455-470) and a breadcrumb logs 'payment.captured.domain-event.emit' with status: 'succeeded' (line 473), but there is no actual call to publish the event. This means the payment-captured worker will never receive the event to process, and the log falsely claims an emit succeeded when none occurred.
The issue is that BookingDomainEventPublisher interface only supports publishBookingAccepted() and publishBookingDeclined() — there is no publishPaymentCaptured() method. The interface must be extended to support payment events, or an alternative event publishing mechanism needs to be used for payment domain events (separate from booking events).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform-api/src/bookings/bookings.service.ts` around lines 455 -
482, A PaymentCapturedDomainEvent is built (paymentCapturedEvent) and a
breadcrumb is logged (logStructuredBreadcrumb with event
'payment.captured.domain-event.emit') but the event is never published because
BookingDomainEventPublisher only exposes publishBookingAccepted() and
publishBookingDeclined(); add proper publishing to avoid the misleading log.
Update the BookingDomainEventPublisher interface to include
publishPaymentCaptured(paymentCapturedEvent: PaymentCapturedDomainEvent) (or add
a separate PaymentDomainEventPublisher with publishPaymentCaptured) and then
call that publisher where paymentCapturedEvent is constructed; also change the
breadcrumb to reflect success only after the publish promise resolves or log
failure on publish error.
Comment on lines
+269
to
+276
| await this.insertStatusEvent(client, { | ||
| bookingId: input.bookingId, | ||
| changedAt: input.completedAt, | ||
| fromStatus: 'accepted', | ||
| toStatus: 'completed', | ||
| actorRole: input.actorRole, | ||
| actorUserId: input.actorUserId, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CHECK constraints on booking_status_history table
rg -n "CHECK.*to_status|CHECK.*from_status" --type sqlRepository: MarkoAtc/QuickWerk
Length of output: 631
🏁 Script executed:
# First, find the actual code at lines 269-276 in the repository
cat -n services/platform-api/src/bookings/infrastructure/postgres-booking.repository.ts | sed -n '269,276p'Repository: MarkoAtc/QuickWerk
Length of output: 385
🏁 Script executed:
# Search for all migrations in order and see if any add 'completed' status
find services/platform-api/migrations -name "*.sql" -type f | sort | head -20Repository: MarkoAtc/QuickWerk
Length of output: 480
🏁 Script executed:
# Search for 'completed' in migration files to see if it's ever added to constraints
rg -n "completed" services/platform-api/migrations/ -A 2 -B 2Repository: MarkoAtc/QuickWerk
Length of output: 44
🏁 Script executed:
# Search for declineSubmittedBooking method to verify the 'declined' claim
rg -n "declineSubmittedBooking" services/platform-api/src/bookings/infrastructure/postgres-booking.repository.ts -A 10Repository: MarkoAtc/QuickWerk
Length of output: 648
🏁 Script executed:
# Check the full content of migration 0006 to understand the current constraints
cat services/platform-api/migrations/0006_booking_declined_support.sqlRepository: MarkoAtc/QuickWerk
Length of output: 1684
🏁 Script executed:
# Check if there are any migrations after 0006
ls -la services/platform-api/migrations/ | tail -20Repository: MarkoAtc/QuickWerk
Length of output: 747
🏁 Script executed:
# Get full declineSubmittedBooking method to see what status it writes
rg -n "declineSubmittedBooking" services/platform-api/src/bookings/infrastructure/postgres-booking.repository.ts -A 30Repository: MarkoAtc/QuickWerk
Length of output: 1369
🏁 Script executed:
# Search for all status writes in the repository file to understand what statuses are being used
rg -n "toStatus:" services/platform-api/src/bookings/infrastructure/postgres-booking.repository.tsRepository: MarkoAtc/QuickWerk
Length of output: 233
The toStatus: 'completed' value violates database constraints and will fail at runtime.
The insertStatusEvent call writes toStatus: 'completed', but the booking_status_history table constraints only allow 'submitted', 'accepted', and 'declined' (per migration 0006). This INSERT will fail with a constraint violation at runtime.
The claim about 'declined' status in declineSubmittedBooking is not applicable—migration 0006 has already added 'declined' to the allowed constraint values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform-api/src/bookings/infrastructure/postgres-booking.repository.ts`
around lines 269 - 276, The call to insertStatusEvent is using toStatus:
'completed' which violates the booking_status_history enum constraint (migration
0006 allows only 'submitted','accepted','declined'), so either change the
inserted status to one of the allowed values (e.g., set toStatus: 'accepted' in
the code path that calls insertStatusEvent) or update the database migration to
add 'completed' to the enum; locate the call in postgress-booking.repository.ts
(insertStatusEvent invocation) and fix the status value or create a new
migration that alters the enum to include 'completed' to resolve the constraint
violation.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 12 file(s) based on 8 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 12 file(s) based on 8 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
services/platform-api/src/bookings/bookings.service.ts (1)
474-484: Drop the secondpayment.captured.domain-event.emitbreadcrumb.
publishPaymentCaptured()already goes throughLoggingBookingDomainEventPublisher.publishPaymentCaptured(), which emits this same event name. Keeping another emit log here will double-count publishes and make the trace noisy.Suggested cleanup
- logStructuredBreadcrumb({ - event: 'payment.captured.domain-event.emit', - correlationId, - status: 'succeeded', - details: { - eventId: paymentCapturedEvent.eventId, - paymentId: payment.paymentId, - bookingId, - replayed: completed.replayed, - }, - });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/bookings/bookings.service.ts` around lines 474 - 484, Remove the duplicate breadcrumb emit by deleting the logStructuredBreadcrumb call that logs event 'payment.captured.domain-event.emit' in the current function; rely on LoggingBookingDomainEventPublisher.publishPaymentCaptured (and its wrapper publishPaymentCaptured()) to emit that breadcrumb so you don’t double-count publishes or create noisy traces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform-api/src/bookings/bookings.service.ts`:
- Around line 405-453: The code currently calls bookings.completeAcceptedBooking
(committing the booking to completed) before calling
paymentsService.capturePaymentForBooking, which can throw and leave a terminal
booking without a payment; change the flow so payment capture occurs before
finalizing the booking or add a compensating rollback if capture fails.
Specifically, either call paymentsService.capturePaymentForBooking using
completedAt/bookingId/customerUserId/providerUserId first and only call
bookings.completeAcceptedBooking after a successful capture, or, if you must
finalize first, implement a rollback via the bookings API (e.g., a new
revert/transition method or bookings.completeAcceptedBooking with an undo path)
that resets status back to the prior state on capture failure and surface a 500
with consistent state handling; ensure errors from capture are caught and logged
with correlationId and actorUserId (session.userId).
- Around line 523-525: The current guard only verifies customer ownership
(checks session.role === 'customer' against booking.customerUserId) and lets any
provider read payments; update the access check in the bookings service method
that returns payments so both participants are validated: if session.role ===
'customer' require booking.customerUserId === session.userId, and if
session.role === 'provider' require booking.providerUserId === session.userId;
otherwise return the same 403 error object. Ensure you reference the existing
variables booking, session, booking.customerUserId and booking.providerUserId
and preserve existing behavior for other roles (e.g., admin) if applicable.
- Around line 445-460: The payment.captured event is using completed.replayed
instead of the replay flag returned by capturePaymentForBooking, which mislabels
first successful payment events; update the PaymentCapturedDomainEvent
construction in bookings.service.ts to set replayed to the replay flag returned
by capturePaymentForBooking (use the payment.replayed/value from the const {
payment } = await this.paymentsService.capturePaymentForBooking(...) call) so
the event reflects the payment operation's actual replay status.
In `@services/platform-api/src/orchestration/relay-domain-event.publisher.ts`:
- Around line 195-200: The publishPaymentCaptured method currently only calls
loggingPublisher.publishPaymentCaptured and bypasses the relay/retry/DLQ path,
preventing services/background-workers/src/workers/payment-captured.worker.ts
from ever running; update publishPaymentCaptured (in
relay-domain-event.publisher) to use the same relay/publish path used by the
accepted/declined publishers (e.g., invoke the relay/emit method or the shared
publishViaRelay helper used by publishPaymentAccepted/publishPaymentDeclined) so
the event is enqueued for the background worker and goes through retry/DLQ
handling instead of only being logged.
---
Nitpick comments:
In `@services/platform-api/src/bookings/bookings.service.ts`:
- Around line 474-484: Remove the duplicate breadcrumb emit by deleting the
logStructuredBreadcrumb call that logs event
'payment.captured.domain-event.emit' in the current function; rely on
LoggingBookingDomainEventPublisher.publishPaymentCaptured (and its wrapper
publishPaymentCaptured()) to emit that breadcrumb so you don’t double-count
publishes or create noisy traces.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd7dca96-7479-4dd2-8231-db0eaf42366b
📒 Files selected for processing (12)
.agent/plans/phase3-payments-payouts-kickoff-2026-03-31.md.gitignoreapps/admin-web/next-env.d.tsservices/platform-api/src/bookings/bookings.service.complete.test.tsservices/platform-api/src/bookings/bookings.service.test.tsservices/platform-api/src/bookings/bookings.service.tsservices/platform-api/src/orchestration/domain-event.publisher.tsservices/platform-api/src/orchestration/logging-domain-event.publisher.tsservices/platform-api/src/orchestration/relay-domain-event.publisher.tsservices/platform-api/src/payments/domain/payment.repository.tsservices/platform-api/src/payments/payments.service.tsservices/platform-api/src/providers/providers.service.upload-url.test.ts
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- apps/admin-web/next-env.d.ts
- services/platform-api/src/payments/domain/payment.repository.ts
- .agent/plans/phase3-payments-payouts-kickoff-2026-03-31.md
🚧 Files skipped from review as they are similar to previous changes (3)
- services/platform-api/src/bookings/bookings.service.test.ts
- services/platform-api/src/providers/providers.service.upload-url.test.ts
- services/platform-api/src/bookings/bookings.service.complete.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
services/platform-api/src/bookings/bookings.service.ts (1)
426-438:⚠️ Potential issue | 🟠 MajorUse the capture result’s replay flag, not
payment.replayed.Line 437 reads
captureResult.payment.replayed, but replay is returned at the operation level (captureResult.replayed). This can mislabel first-vs-replay emits.Suggested fix
- paymentReplayed = captureResult.payment.replayed ?? false; + paymentReplayed = captureResult.replayed;#!/bin/bash # Verify replay flag location and current usage rg -n -C3 "capturePaymentForBooking\\(|replayed" services/platform-api/src/payments/payments.service.ts services/platform-api/src/bookings/bookings.service.ts --type ts rg -n -C3 "type PaymentRecord|interface PaymentRecord" --type ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/bookings/bookings.service.ts` around lines 426 - 438, captureResult.replayed (operation-level) is the correct replay flag — update the booking completion code that calls paymentsService.capturePaymentForBooking so it uses captureResult.replayed instead of captureResult.payment.replayed; specifically, after assigning payment = captureResult.payment, set paymentReplayed = captureResult.replayed ?? false (or equivalent) and remove reliance on captureResult.payment.replayed; check symbols capturePaymentForBooking, captureResult, payment, and paymentReplayed to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/background-workers/src/workers/index.ts`:
- Around line 19-26: The workerPipelines constant is missing the
payment-captured pipeline; update the exported workerPipelines array (the const
named workerPipelines) to include the string 'payment-captured-orchestration'
alongside the existing 'booking-accepted-orchestration' and
'booking-declined-orchestration' entries so the payment-captured worker is
registered and monitored; preserve the as const typing and ordering consistent
with related booking pipelines.
In `@services/platform-api/src/bookings/bookings.service.ts`:
- Around line 407-434: The code captures payment via
paymentsService.capturePaymentForBooking before verifying the booking can
transition to completed; move the transition eligibility check so you validate
bookingToComplete's current state (e.g., status/accepted flag or a helper like
canTransitionToCompleted) and return the 409 error/logging path if not eligible
before calling capturePaymentForBooking; apply the same ordering for the
replay/alternate branch around the capture logic (the blocks referencing
captureResult/paymentReplayed and the subsequent 467-495 flow) so no payment is
created for invalid transitions.
- Around line 379-381: The return type for completeBooking is missing the 500
error status; update the Promise return union in the completeBooking signature
so the error branch includes 500 (i.e., change statusCode: 403 | 404 | 409 to
403 | 404 | 409 | 500) to match the actual return where a payment capture
failure returns { ok: false, statusCode: 500, error: string }; ensure the type
change references the existing variant that currently uses
ReturnType<BookingsService['serializeRecord']> and PaymentRecord so the
successful branch remains unchanged.
---
Duplicate comments:
In `@services/platform-api/src/bookings/bookings.service.ts`:
- Around line 426-438: captureResult.replayed (operation-level) is the correct
replay flag — update the booking completion code that calls
paymentsService.capturePaymentForBooking so it uses captureResult.replayed
instead of captureResult.payment.replayed; specifically, after assigning payment
= captureResult.payment, set paymentReplayed = captureResult.replayed ?? false
(or equivalent) and remove reliance on captureResult.payment.replayed; check
symbols capturePaymentForBooking, captureResult, payment, and paymentReplayed to
locate the change.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8cf55ff-1df5-448c-8037-6bd07fbaccf6
📒 Files selected for processing (3)
services/background-workers/src/workers/index.tsservices/platform-api/src/bookings/bookings.service.tsservices/platform-api/src/orchestration/relay-domain-event.publisher.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
services/platform-api/src/bookings/bookings.service.ts (2)
447-458:⚠️ Potential issue | 🟠 MajorUse
captureResult.replayedas the source of truth.
capturePaymentForBooking()returns the idempotency flag next topayment. ReadingcaptureResult.payment.replayedhere will mislabel thepayment.capturedevent/logs wheneverPaymentRecorddoes not carry that field.Suggested fix
payment = captureResult.payment; - paymentReplayed = captureResult.payment.replayed ?? false; + paymentReplayed = captureResult.replayed;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/bookings/bookings.service.ts` around lines 447 - 458, The code currently sets paymentReplayed from captureResult.payment.replayed which can be missing on PaymentRecord; instead use the idempotency flag returned on the outer result. After assigning payment = captureResult.payment, set paymentReplayed = captureResult.replayed ?? false (i.e., read the replay flag from captureResult rather than captureResult.payment) so functions like capturePaymentForBooking and variables payment and paymentReplayed use the correct source of truth.
421-455:⚠️ Potential issue | 🔴 CriticalPreserve repository replay/conflict semantics before capturing payment.
This preflight only accepts
accepted, so same-provider retries on an alreadycompletedbooking now return 409 instead of the expected replayed success. It also still lets a different provider reachcapturePaymentForBooking()for another provider’s accepted booking beforecompleteAcceptedBooking()rejects the transition.Suggested fix
- // Validate booking can transition to completed before capturing payment - if (bookingToComplete.status !== 'accepted') { + // Match repository transition semantics before capturing payment + const isSameProvider = bookingToComplete.providerUserId === session.userId; + const isReplay = bookingToComplete.status === 'completed' && isSameProvider; + + if (bookingToComplete.providerUserId && !isSameProvider) { + logStructuredBreadcrumb({ + event: 'booking.complete.write', + correlationId, + status: 'failed', + details: { + reason: 'transition-conflict', + bookingId, + actorUserId: session.userId, + currentStatus: bookingToComplete.status, + }, + }); + + return { + ok: false, + statusCode: 409, + error: `Booking cannot transition from ${bookingToComplete.status} to completed.`, + }; + } + + if (bookingToComplete.status !== 'accepted' && !isReplay) { logStructuredBreadcrumb({ event: 'booking.complete.write', correlationId, status: 'failed',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/bookings/bookings.service.ts` around lines 421 - 455, The current preflight rejects already-completed bookings causing retries to return 409 instead of a replay success and allows a different provider to capture before the transition check; to fix, call this.paymentsService.capturePaymentForBooking(...) before enforcing the bookingToComplete.status check so capture handles replay semantics first (inspect captureResult.replayed and return success if true), then validate transition (bookingToComplete.status === 'accepted') and provider identity (compare bookingToComplete.providerUserId with session.userId and the providerUserId passed) before calling completeAcceptedBooking(...); use the existing symbols bookingToComplete.status, captureResult.replayed, this.paymentsService.capturePaymentForBooking, bookingId, bookingToComplete.providerUserId, session.userId, and completeAcceptedBooking to locate and reorder/add these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/background-workers/src/index.ts`:
- Around line 8-10: The payment-captured consumer exported as
consumePaymentCapturedAttempt (along with buildPaymentCapturedWorkerEnvelope and
markPaymentCapturedDlq) is not being registered, so wire it into the consumer
registration routine where booking.accepted/booking.declined are registered: add
a registration call that registers the "payment.captured" subject using
buildPaymentCapturedWorkerEnvelope for the envelope creation,
consumePaymentCapturedAttempt as the handler, and markPaymentCapturedDlq as the
DLQ handler (mirror the existing booking.* registration pattern so subscription
options and error handling match).
---
Duplicate comments:
In `@services/platform-api/src/bookings/bookings.service.ts`:
- Around line 447-458: The code currently sets paymentReplayed from
captureResult.payment.replayed which can be missing on PaymentRecord; instead
use the idempotency flag returned on the outer result. After assigning payment =
captureResult.payment, set paymentReplayed = captureResult.replayed ?? false
(i.e., read the replay flag from captureResult rather than
captureResult.payment) so functions like capturePaymentForBooking and variables
payment and paymentReplayed use the correct source of truth.
- Around line 421-455: The current preflight rejects already-completed bookings
causing retries to return 409 instead of a replay success and allows a different
provider to capture before the transition check; to fix, call
this.paymentsService.capturePaymentForBooking(...) before enforcing the
bookingToComplete.status check so capture handles replay semantics first
(inspect captureResult.replayed and return success if true), then validate
transition (bookingToComplete.status === 'accepted') and provider identity
(compare bookingToComplete.providerUserId with session.userId and the
providerUserId passed) before calling completeAcceptedBooking(...); use the
existing symbols bookingToComplete.status, captureResult.replayed,
this.paymentsService.capturePaymentForBooking, bookingId,
bookingToComplete.providerUserId, session.userId, and completeAcceptedBooking to
locate and reorder/add these checks.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 550b2ca8-53f6-4536-bab2-d3f61228fd73
📒 Files selected for processing (3)
services/background-workers/src/index.tsservices/background-workers/src/workers/index.tsservices/platform-api/src/bookings/bookings.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- services/background-workers/src/workers/index.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This was referenced Apr 1, 2026
This was referenced Apr 13, 2026
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
Phase 3 Slice 1 delivers two vertical features closing Milestones 3a and 3b:
PaymentCapturedDomainEventis emittedBoth slices follow the exact same repository → service → controller → API client → app actions → tests pattern established in Phase 2.
What's in this PR
Milestone 3a — Booking Completion + Payment
Domain / packages
packages/domain:PaymentStatus,PaymentRecord,PaymentCapturedDomainEventtypesBookingStatusextended with'completed'CompleteAcceptedBookingInput/CompleteAcceptedBookingResulttypesBooking repository
completeAcceptedBooking()— in-memory + postgres implementationsreplayed: true(200), cross-provider → 409Payments module (new:
services/platform-api/src/payments/)PaymentRepositoryinterface +InMemoryPaymentRepositoryPaymentsService.capturePaymentForBooking()— idempotent payment capturePaymentsService.getPaymentByBookingId()PaymentsModuleexported and imported byBookingsModuleEndpoints
POST /api/v1/bookings/:id/complete— provider-only; transitions booking, captures payment, emits domain eventGET /api/v1/bookings/:id/payment— returns payment record for booking ownerBackground worker
payment-captured.worker.ts— stub with deterministic exponential backoff + DLQ (mirrors booking-accepted pattern)Milestone 3b — Verification Document Upload URL
Domain / packages
packages/domain:UploadUrlRecordtypeUpload URL repository (new in providers module)
UploadUrlRepositoryinterface +InMemoryUploadUrlRepositoryEndpoint
POST /api/v1/providers/me/verification/upload-url— provider-only, returns{ uploadId, presignedUrl, expiresAt, filename, mimeType }API Client + App Actions
createCompleteBookingRequest,createGetBookingPaymentRequest,createRequestUploadUrlRequestbooking-state.ts:'completed'added to status unionbooking-screen-actions.ts:completeBookingRequestprovider-screen-actions.ts:requestVerificationUploadUrlTest coverage
bookings.service.complete.test.tspayments.service.test.tsproviders.service.upload-url.test.tsbooking-screen-actions-complete.test.tspayment-captured.worker.test.tsTypeScript strict mode: ✅ clean across all 12 packages
Smoke script
scripts/smoke/phase3-slice1-complete-upload-smoke.sh— 13 assertions covering:Architecture notes
completeBooking()— no relay queue in Slice 1. Thepayment-captured.worker.tsis a stub that mirrorsbooking-accepted.worker.ts. Real async dispatch is Phase 3 Slice 2+.https://storage.stub/uploads/...). Real S3/LocalStack integration is Phase 3 Slice 2.amountCents: 0for now — real pricing model is Phase 3 Slice 2+.Phase 3 Slice 1 exit criteria
POST /api/v1/bookings/:id/completetransitions booking tocompletedand creates payment recordGET /api/v1/bookings/:id/paymentreturns payment recordPaymentCapturedDomainEventemitted as structured breadcrumbPOST /api/v1/providers/me/verification/upload-urlreturns stub presigned URL🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
Chores