TEC-49: booking-completion screen + active-job handoff (Phase 8)#18
Merged
Conversation
Adds the /booking-completion route and drives the active-job surface to redirect completed bookings there instead of rendering a stale view. - New route: apps/product-app/app/booking-completion.js - New state: booking-completion-route-state (loading/error/empty/loaded) - New presenter: booking-completion-presenter (completion summary, payment, invoice, reviews, warnings, dispute/review actions) - New actions: booking-completion-screen-actions (load, submitReview, submitDispute) - active-job-route-state: adds 'handoff' status on completed booking - active-job.js: router.replace to /booking-completion on handoff state - Tests: route-state, presenter, screen-actions, handoff coverage in active-job-route-state tests (31 files, 192 tests pass) Co-Authored-By: Paperclip <noreply@paperclip.ing>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 3 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a booking-completion feature and wiring: active-job route can hand off completed bookings to a new booking-completion route; implements route state, data loading/actions, presenter, screen UI, and tests for the new flow and handoff condition. Error handling and navigation behaviors were also introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActiveJobRoute as Active Job Route
participant BookingCompletion as Booking Completion Route
participant API as Platform API
participant Presenter as Presenter
User->>ActiveJobRoute: Open active job
ActiveJobRoute->>ActiveJobRoute: resolveActiveJobRouteState()
ActiveJobRoute->>API: loadBookingContinuation / booking data
API-->>ActiveJobRoute: booking (status='completed')
ActiveJobRoute->>ActiveJobRoute: Detect handoff (status === 'completed')
ActiveJobRoute->>BookingCompletion: router.replace /booking-completion?bookingId=...
BookingCompletion->>BookingCompletion: resolveBookingCompletionRouteState()
BookingCompletion->>API: loadBookingCompletion(bookingId, sessionToken)
API-->>BookingCompletion: booking (+ maybe payment)
alt booking completed
BookingCompletion->>API: fetch invoice
BookingCompletion->>API: fetch reviews
API-->>BookingCompletion: invoice, reviews
BookingCompletion->>Presenter: presentBookingCompletion(...)
Presenter-->>BookingCompletion: viewModel
BookingCompletion->>User: render BookingCompletionScreen(viewModel)
else not completed
BookingCompletion->>User: show empty/pending message
end
User->>BookingCompletion: submit review / open dispute
BookingCompletion->>API: submit review or submit dispute
API-->>BookingCompletion: success / error
BookingCompletion->>BookingCompletion: refresh state (load)
BookingCompletion->>User: update UI (feedback)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 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 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: 6
🧹 Nitpick comments (2)
apps/product-app/src/features/booking/booking-completion-presenter.ts (1)
29-31: Consider centralizing money formatting used across presenters.
formatMoneyduplicates the same formatter pattern already present inapps/product-app/src/features/booking/active-job-presenter.ts(context snippet line range 105-108). A shared utility would reduce drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/booking/booking-completion-presenter.ts` around lines 29 - 31, Extract the duplicate formatMoney function into a shared utility (e.g., export function formatMoney(amountCents: number, currency: string): string) and replace the local implementations in booking-completion-presenter (function formatMoney) and active-job-presenter with imports from that utility; update both files to import and call the shared formatMoney so formatting logic is centralized and no duplicate function remains.apps/product-app/src/features/booking/booking-completion-screen-actions.test.ts (1)
95-121: Strengthen this test by asserting no downstream lookups were called.Right now it checks outputs only; it doesn’t prove
/payment,/invoice, and/reviewswere skipped.💡 Suggested test hardening
it('returns non-completed booking without invoice/review lookups', async () => { + const requestedPaths: string[] = []; const fetchMock: typeof fetch = async (url) => { + requestedPaths.push(url.toString()); if (url.toString().includes('/payment')) { return { ok: false, status: 404 } as Response; } @@ expect(result.errorMessage).toBeUndefined(); expect(result.booking?.status).toBe('accepted'); expect(result.invoice).toBeUndefined(); expect(result.reviews).toEqual([]); + expect(requestedPaths.some((p) => p.includes('/payment'))).toBe(false); + expect(requestedPaths.some((p) => p.includes('/invoice'))).toBe(false); + expect(requestedPaths.some((p) => p.includes('/reviews'))).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/booking/booking-completion-screen-actions.test.ts` around lines 95 - 121, The test should assert that downstream endpoints were not invoked: replace the inline fetchMock with a spyable mock (e.g., const fetchMock = jest.fn(async (url) => { ... })) and keep the same behavior for the base booking fetch; then after calling loadBookingCompletion({ sessionToken: 'tok', bookingId: 'bk-2' }, fetchMock) assert via fetchMock.mock.calls (or .toHaveBeenCalledWith) that URLs containing '/payment', '/invoice', and '/reviews' were not requested. Reference the test "returns non-completed booking without invoice/review lookups" and the loadBookingCompletion function when making this 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 `@apps/product-app/app/active-job.js`:
- Around line 38-45: The promise chain handling nextState lacks a rejection
handler, so if the promise rejects the UI can remain stuck loading; add a
.catch(...) after the .then(...) that handles errors by calling setScreenState
with an error/failure state (or otherwise toggling loading off) and logging the
error (e.g., using console.error or existing logger), ensuring you still
preserve the existing router.replace logic for 'handoff' and the
setScreenState(nextState) path; refer to the nextState, router.replace, and
setScreenState identifiers when locating the code to modify.
In `@apps/product-app/app/booking-completion.js`:
- Around line 79-130: Both handlers allow duplicate non-idempotent POSTs because
they don't guard against re-entry; update handleSubmitReview and
handleOpenDispute to early-return if a submit is already in progress (check the
corresponding reviewState.status or disputeState.status for 'submitting' or use
an in-flight ref flag) before setting status to 'submitting', and mark/clear
that flag/state around the async submitBookingCompletionReview /
submitBookingCompletionDispute promise so additional taps are ignored until the
request completes (also ensure the UI receives the submitting flag so
BookingCompletionScreen can disable controls).
- Around line 67-77: The render currently performs side effects when
sessionToken is missing (calling signOut() and router.replace('/auth')) which
can run multiple times; move that logic into a useEffect that watches the
resolved session token (result of resolveSessionToken(session)) and performs
signOut() + router.replace('/auth') only when token becomes falsy, and remove
the duplicate calls from the existing load callback so auth cleanup/redirect is
consolidated in that single effect; update any references to
session/sessionToken in the effect closure and ensure the effect has the
appropriate dependency array to avoid repeated triggers.
In `@apps/product-app/src/features/booking/booking-completion-route-state.ts`:
- Around line 27-64: resolveBookingCompletionRouteState currently only handles
returned errorMessage but not thrown exceptions from loadImpl/presentImpl; wrap
the await loadImpl(...) and the presentImpl(...) call in try/catch blocks (or a
single try surrounding both) inside resolveBookingCompletionRouteState so any
rejection or thrown error is caught and you return { status: 'error',
errorMessage: String(err) } (include contextual text if desired); reference
loadImpl/loadBookingCompletion and presentImpl/presentBookingCompletion so the
caught errors from those implementations are mapped to the route error state
instead of letting the promise reject.
In `@apps/product-app/src/features/booking/booking-completion-screen-actions.ts`:
- Around line 21-37: The union type LoadBookingCompletionResult must carry the
latest dispute and the loader loadBookingCompletion must fetch and return it:
add a latestDispute?: DisputeRecord (or null) field to the success branch of the
LoadBookingCompletionResult union and a latestDispute?: undefined (or absent) on
the error branch so booking-completion-presenter.ts can consume it; update the
loadBookingCompletion function (and any helper that builds the result in the
same file) to query the latest dispute for the booking (using your existing
dispute data access utilities) after loading booking/payment/invoice/reviews and
include that dispute in the returned result; also update any other code paths in
this file that construct LoadBookingCompletionResult (the other return sites
mentioned) to populate latestDispute consistently.
In `@apps/product-app/src/features/booking/booking-completion-screen.js`:
- Around line 108-123: Add an "in-flight" guard and UI disabled state around the
submission handlers to prevent duplicate requests: introduce a boolean state
(e.g., isSubmitting) used by onSubmitReview (and the other write-action handler
that corresponds to the second button at lines ~175-189) to early-return if
already true, set isSubmitting = true at the start of the async submit and reset
it in finally, and pass disabled/adjust styling (e.g., reduced opacity or
pointerEvents='none') to the TouchableOpacity(s) (referencing
testID="booking-completion-submit-review" and the other submit button testID) so
taps are ignored while isSubmitting is true. Ensure both the onSubmitReview
function and the second submit handler perform the same guard and state updates.
---
Nitpick comments:
In `@apps/product-app/src/features/booking/booking-completion-presenter.ts`:
- Around line 29-31: Extract the duplicate formatMoney function into a shared
utility (e.g., export function formatMoney(amountCents: number, currency:
string): string) and replace the local implementations in
booking-completion-presenter (function formatMoney) and active-job-presenter
with imports from that utility; update both files to import and call the shared
formatMoney so formatting logic is centralized and no duplicate function
remains.
In
`@apps/product-app/src/features/booking/booking-completion-screen-actions.test.ts`:
- Around line 95-121: The test should assert that downstream endpoints were not
invoked: replace the inline fetchMock with a spyable mock (e.g., const fetchMock
= jest.fn(async (url) => { ... })) and keep the same behavior for the base
booking fetch; then after calling loadBookingCompletion({ sessionToken: 'tok',
bookingId: 'bk-2' }, fetchMock) assert via fetchMock.mock.calls (or
.toHaveBeenCalledWith) that URLs containing '/payment', '/invoice', and
'/reviews' were not requested. Reference the test "returns non-completed booking
without invoice/review lookups" and the loadBookingCompletion function when
making this change.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6f950ab-c3a7-4167-871a-2bb9cc85bd54
📒 Files selected for processing (11)
apps/product-app/app/active-job.jsapps/product-app/app/booking-completion.jsapps/product-app/src/features/booking/active-job-route-state.test.tsapps/product-app/src/features/booking/active-job-route-state.tsapps/product-app/src/features/booking/booking-completion-presenter.test.tsapps/product-app/src/features/booking/booking-completion-presenter.tsapps/product-app/src/features/booking/booking-completion-route-state.test.tsapps/product-app/src/features/booking/booking-completion-route-state.tsapps/product-app/src/features/booking/booking-completion-screen-actions.test.tsapps/product-app/src/features/booking/booking-completion-screen-actions.tsapps/product-app/src/features/booking/booking-completion-screen.js
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 6 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 6 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)
apps/product-app/src/features/booking/booking-completion-screen-actions.ts (1)
279-291:⚠️ Potential issue | 🟠 Major
latestDisputeis still never populated.The success shape now advertises
latestDispute, but this loader always returnsundefinedhere. AftersubmitBookingCompletionDispute()succeeds and the screen reloads, the completion state falls back to the "no dispute" path, so dispute status/history and CTA gating cannot be rebuilt correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/booking/booking-completion-screen-actions.ts` around lines 279 - 291, The loader currently returns latestDispute as undefined; fix it by fetching and parsing the booking dispute before returning. Use createGetBookingDisputeRequest(input.sessionToken, input.bookingId) to call the GET dispute endpoint, await the response, then call parseDispute(response) (or the appropriate parser) to set latestDispute; ensure you handle missing endpoints/errors gracefully (fallback to undefined) and place this logic where latestDispute is declared in booking-completion-screen-actions.ts so that after submitBookingCompletionDispute() succeeds and the screen reloads the dispute state is reconstructed.
🧹 Nitpick comments (1)
apps/product-app/src/features/booking/booking-completion-screen.js (1)
80-80: Consider using design tokens for hardcoded colors.Several hex values are hardcoded throughout the component while design tokens from
@quickwerk/uiare already imported. For consistency and easier theming:
Hardcoded Semantic purpose Suggestion #E2E8F0,#CBD5E1Borders, inactive states colors.borderor similar#F8FAFCInput backgrounds colors.inputBackground#B91C1CDanger/dispute button colors.danger#FEFCE8,#FDE68A,#854D0EWarning banner colors.warning*variantsIf these semantic colors don't exist in the token set, consider extending
@quickwerk/uito include them.Also applies to: 100-100, 149-149, 189-189, 203-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/booking/booking-completion-screen.js` at line 80, Several UI color hex literals in booking-completion-screen.js (e.g., backgroundColor: value <= reviewRating ? colors.primary : '#E2E8F0' and other occurrences at lines noted) should be replaced with design tokens from the imported colors object; update each hardcoded value (`#E2E8F0`, `#CBD5E1`, `#F8FAFC`, `#B91C1C`, `#FEFCE8`, `#FDE68A`, `#854D0E`) to semantic tokens such as colors.border, colors.inputBackground, colors.danger, colors.warning (or appropriate variants) in the relevant JSX/style blocks (look for the backgroundColor/border/color usages and conditional for reviewRating), and if a required semantic token does not exist in `@quickwerk/ui`, add/extend that token there and then reference it from this component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/product-app/src/features/booking/booking-completion-screen-actions.ts`:
- Around line 264-270: The code only warns when all reviews are invalid; change
the condition to detect any discarded entries by comparing payload.length to
reviews.length (i.e., if payload.length > reviews.length) and push a warning
like 'Some reviews missing required fields.' so that when parseReview drops one
or more entries the route logs a warning; update the block handling reviews (the
mapping/filter using parseReview and the warningMessages array) to use this new
condition while keeping the ReviewRecord type guard.
- Around line 93-117: The current mapping for invoice['lineItems'] silently
drops malformed rows which can leave subtotalCents/totalCents inconsistent with
displayed rows; modify the logic around the lineItems mapping in
booking-completion-screen-actions.ts so that you validate all items first (using
invoice['lineItems'] and the same type checks for description, quantity,
unitAmountCents, totalAmountCents) and if any item fails validation return null
(reject the whole invoice) instead of filtering out invalid items; update the
code that constructs lineItems (and any helper like InvoiceRecord['lineItems'])
to perform an all-valid check (or set a flag during map) and bail out by
returning null when a malformed item is detected.
In `@apps/product-app/src/features/booking/booking-completion-screen.js`:
- Around line 202-223: The current JSX uses the raw string values as React keys
in the map callbacks for viewModel.warningMessages and viewModel.statusHistory
(the map over warningMessages -> key={message} and map over statusHistory ->
key={entry}), which can collide for duplicate strings; update those map
callbacks to use the array index (or a combination of the index and the value)
as the key by using the second map parameter (e.g., (message, i) and (entry, i))
and changing the key to a stable unique token like `${message}-${i}` or
`${entry}-${i}` to avoid React key collisions while rendering.
---
Duplicate comments:
In `@apps/product-app/src/features/booking/booking-completion-screen-actions.ts`:
- Around line 279-291: The loader currently returns latestDispute as undefined;
fix it by fetching and parsing the booking dispute before returning. Use
createGetBookingDisputeRequest(input.sessionToken, input.bookingId) to call the
GET dispute endpoint, await the response, then call parseDispute(response) (or
the appropriate parser) to set latestDispute; ensure you handle missing
endpoints/errors gracefully (fallback to undefined) and place this logic where
latestDispute is declared in booking-completion-screen-actions.ts so that after
submitBookingCompletionDispute() succeeds and the screen reloads the dispute
state is reconstructed.
---
Nitpick comments:
In `@apps/product-app/src/features/booking/booking-completion-screen.js`:
- Line 80: Several UI color hex literals in booking-completion-screen.js (e.g.,
backgroundColor: value <= reviewRating ? colors.primary : '#E2E8F0' and other
occurrences at lines noted) should be replaced with design tokens from the
imported colors object; update each hardcoded value (`#E2E8F0`, `#CBD5E1`, `#F8FAFC`,
`#B91C1C`, `#FEFCE8`, `#FDE68A`, `#854D0E`) to semantic tokens such as colors.border,
colors.inputBackground, colors.danger, colors.warning (or appropriate variants)
in the relevant JSX/style blocks (look for the backgroundColor/border/color
usages and conditional for reviewRating), and if a required semantic token does
not exist in `@quickwerk/ui`, add/extend that token there and then reference it
from this component.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74b963af-b684-48fc-97a4-d3447295e6dc
📒 Files selected for processing (5)
apps/product-app/app/active-job.jsapps/product-app/app/booking-completion.jsapps/product-app/src/features/booking/booking-completion-route-state.tsapps/product-app/src/features/booking/booking-completion-screen-actions.tsapps/product-app/src/features/booking/booking-completion-screen.js
✅ Files skipped from review due to trivial changes (1)
- apps/product-app/app/booking-completion.js
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/product-app/app/active-job.js
- apps/product-app/src/features/booking/booking-completion-route-state.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 2 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
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 8 delivers the
/booking-completionproduct-app surface and wires the active-job screen to redirect completed bookings there automatically.apps/product-app/app/booking-completion.js— full completion summary with payment, invoice, status history, review/dispute CTAsbooking-completion-route-state.ts—loading | error | empty | loadedlifecycle backed by real platform-api endpointsbooking-completion-presenter.ts— maps completion data to a view model with warnings, payment/invoice readouts, and bounded action availabilitybooking-completion-screen-actions.ts—loadCompletedBooking,submitReview,submitDisputeactive-job-route-state.tsgains ahandoffstatus;active-job.jscallsrouter.replaceto/booking-completionon first load when booking iscompletedTest plan
pnpm --filter @quickwerk/product-app typecheck— passespnpm --filter @quickwerk/product-app test— 31 test files, 192 tests passingReferences
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Behavior Changes
Bug Fixes