TEC-25: QuickWerk Phase 4 operator dispute lifecycle (end-to-end)#13
Merged
Conversation
Implement operator dispute flow (start-review, resolve, close) across domain contract, API client, platform API, persistence adapters, admin queue reducers/actions, tests, and smoke script. Co-Authored-By: Paperclip <noreply@paperclip.ing>
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:
📝 WalkthroughWalkthroughAdds a full disputes lifecycle: DB migration and Postgres repository, repo resolver, service/controller operator transitions (startReview/resolve/close) with validation and guards, API client routes, frontend optimistic transition flows and state helpers, tests across layers, and a smoke script. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Admin Web UI
participant Store as State Management
participant API as Backend API
participant Service as DisputesService
participant Repo as DisputeRepository
participant DB as PostgreSQL
rect rgba(76, 175, 80, 0.5)
Note over Frontend,DB: Optimistic Dispute Transition Flow
end
Frontend->>Store: submitDisputeTransition(disputeId, 'resolve')
activate Store
Store->>Store: beginOptimisticDisputeTransition()
Store-->>Frontend: Updated state (transitioning)
deactivate Store
rect rgba(33, 150, 243, 0.5)
Note over Frontend,DB: API Request Phase
end
Frontend->>API: PATCH /api/v1/disputes/:disputeId/resolve {resolutionNote}
activate API
API->>Service: resolveDispute(session, disputeId, note)
activate Service
Service->>Repo: transitionStatus({nextStatus:'resolved', allowedCurrentStatuses:[...]})
activate Repo
Repo->>DB: SELECT ... FOR UPDATE / UPDATE
Repo-->>Service: {ok: true, dispute}
deactivate Repo
Service-->>API: {ok: true, dispute}
deactivate Service
API-->>Frontend: 200 OK, dispute
deactivate API
rect rgba(156, 39, 176, 0.5)
Note over Frontend,DB: Success Path
end
Frontend->>Store: applyDisputeTransitionSuccess(dispute)
activate Store
Store-->>Frontend: Updated state (done / dispute removed)
deactivate Store
sequenceDiagram
participant Frontend as Admin Web UI
participant Store as State Management
participant API as Backend API
participant Service as DisputesService
participant Repo as DisputeRepository
rect rgba(244, 67, 54, 0.5)
Note over Frontend,Repo: Error/Rollback Flow
end
Frontend->>Store: submitDisputeTransition(disputeId, 'startReview')
activate Store
Store->>Store: beginOptimisticDisputeTransition()
Store-->>Frontend: Updated state (under-review optimistic)
deactivate Store
Frontend->>API: PATCH /api/v1/disputes/:disputeId/start-review
activate API
API->>Service: startReviewDispute(session, disputeId)
activate Service
Service->>Repo: transitionStatus({nextStatus:'under-review', allowedCurrentStatuses:[...]})
activate Repo
Repo-->>Service: {ok:false, reason:'transition-conflict'}
deactivate Repo
Service-->>API: {ok:false, statusCode:409, error}
deactivate Service
API-->>Frontend: 409 Conflict
deactivate API
Frontend->>Store: rollbackDisputeTransition(previousState, error)
activate Store
Store-->>Frontend: Restored state with queueAction error
deactivate Store
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 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: 5
🧹 Nitpick comments (5)
services/platform-api/src/disputes/disputes.service.test.ts (1)
205-286: Consider adding aclosetransition test for full lifecycle coverage.The suite validates
startReviewandresolve, but notclose. Adding one close-path test would complete coverage for all operator actions introduced in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/disputes/disputes.service.test.ts` around lines 205 - 286, Add a new spec in the "operator transitions" describe block that covers the full lifecycle including the close transition: create service/bookings, submit a dispute via submitDispute, transition it to under-review with startReviewDispute, resolve it with resolveDispute, then call closeDispute as an operator and assert the final status becomes "closed", resolvedAt remains set, resolutionNote retained, and that getPendingDisputes no longer returns the dispute; also include role/guard checks (e.g., ensure non-operator cannot close) to mirror the existing tests' patterns using the same helper functions (submitDispute, startReviewDispute, resolveDispute, closeDispute, getPendingDisputes).services/platform-api/src/disputes/infrastructure/dispute-repository.provider.test.ts (1)
21-34: Add fail-fast coverage for missingDATABASE_URLin postgres mode.Current tests validate selection, but not the required config guard. Add one negative case so provider behavior is locked down.
✅ Suggested test addition
describe('resolveDisputeRepository', () => { + it('throws when postgres mode is selected without DATABASE_URL', () => { + const inMemoryRepository = new InMemoryDisputeRepository(); + + expect(() => + resolveDisputeRepository({ + inMemoryRepository, + postgresClient: new PostgresClient(), + env: { + PERSISTENCE_MODE: 'postgres', + }, + }), + ).toThrowError(/DATABASE_URL/i); + }); + it('returns postgres repository in postgres mode', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/disputes/infrastructure/dispute-repository.provider.test.ts` around lines 21 - 34, Add a negative test case to ensure resolveDisputeRepository fails fast when env.PERSISTENCE_MODE is 'postgres' but env.DATABASE_URL is missing; update the test file to call resolveDisputeRepository with inMemoryRepository, a PostgresClient instance, and env.PERSISTENCE_MODE set to 'postgres' but without DATABASE_URL (or set it to empty/undefined) and assert that it throws/returns an error (or behaves as your provider's guard does). Specifically reference resolveDisputeRepository, PERSISTENCE_MODE, DATABASE_URL, InMemoryDisputeRepository, PostgresClient, and PostgresDisputeRepository so the test mirrors the existing positive case but expects the guard to trigger.services/platform-api/src/persistence/postgres-mode.integration.test.ts (1)
147-167: Extend integration test to assert replay and conflict semantics.The test currently verifies only the happy path. Add replay/conflict checks to validate parity with the in-memory contract and catch regression in transition rules.
✅ Suggested assertions to add
expect(resolveTransition.ok).toBe(true); if (!resolveTransition.ok) return; expect(resolveTransition.dispute.status).toBe('resolved'); expect(resolveTransition.dispute.resolutionNote).toContain('integration test'); + + const replayResolve = await disputeRepository.transitionStatus({ + disputeId, + allowedCurrentStatuses: ['under-review'], + nextStatus: 'resolved', + resolvedAt: new Date().toISOString(), + resolutionNote: 'Resolved in integration test.', + }); + expect(replayResolve.ok).toBe(true); + if (replayResolve.ok) { + expect(replayResolve.replayed).toBe(true); + } + + const invalidClose = await disputeRepository.transitionStatus({ + disputeId, + allowedCurrentStatuses: ['under-review'], + nextStatus: 'closed', + }); + expect(invalidClose.ok).toBe(false); + if (!invalidClose.ok) { + expect(invalidClose.reason).toBe('transition-conflict'); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform-api/src/persistence/postgres-mode.integration.test.ts` around lines 147 - 167, After the happy-path assertions for reviewTransition and resolveTransition, add replay and conflict checks: call disputeRepository.transitionStatus again with the same parameters used for reviewTransition and assert the repeated call indicates a replay/no-op (expect ok to be false and dispute status remains 'under-review'); then call transitionStatus with disputeId but with an incorrect allowedCurrentStatuses (e.g., ['open'] after it moved) and assert that this returns a conflict (expect ok to be false and the dispute status remains unchanged); use the existing variables disputeId, reviewTransition, resolveTransition and the transitionStatus method names to locate where to insert these assertions.apps/admin-web/src/features/disputes/dispute-queue-actions.test.ts (1)
84-96: Assert the request path in these transition tests.Both stubs accept any URL, so a regression in the
/resolveor/start-reviewroute mapping would still pass. Verifying the expected path here would make the new coverage exercise the client wiring, not just the reducer behavior.Also applies to: 113-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/features/disputes/dispute-queue-actions.test.ts` around lines 84 - 96, The fetch stub (fetchImpl) in the dispute transition tests only asserts the HTTP method and ignores the request URL, so add an assertion that the incoming _url matches the expected route for the action under test (e.g., the resolve test should assert the path includes the dispute id and "/resolve", and likewise the start-review test should assert the path includes "/start-review"). Update the fetchImpl used in those tests (the one referencing makeDispute('d-1')) to call expect(_url).toBe(...) or expect(_url).toMatch(...) with the exact API route you expect, and apply the same URL assertion change to the other stub used around lines 113-118 so both transition tests validate the client routing as well as method and response.apps/admin-web/src/features/disputes/dispute-queue-state.ts (1)
3-9: Reuse the domain transition helpers for optimistic updates.This file now hardcodes both the action union and the action → status mapping even though
packages/domain/src/index.ts:312-327already owns those rules. Importing the domain types/helpers here would keep the optimistic UI in lockstep with the backend and avoid showing impossible transient states if the workflow changes.Also applies to: 26-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/features/disputes/dispute-queue-state.ts` around lines 3 - 9, Replace the hardcoded DisputeQueueActionType and DisputeQueueAction definitions with the canonical types/helpers from the domain package: remove DisputeQueueActionType and the local DisputeQueueAction union and import the domain-owned action type and transition helper (the types/helpers referenced in packages/domain that drive action→status transitions) and use them for optimistic UI state and mapping; update any code that previously relied on DisputeQueueActionType/DisputeQueueAction to use the imported domain type names and the domain transition helper so the optimistic states follow the backend-owned rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api-client/src/index.ts`:
- Around line 347-356: The helper createResolveDisputeRequest currently requires
body: { resolutionNote: string } while the API allows resolutionNote to be
optional; change the body type to { resolutionNote?: string } and ensure any
code using createResolveDisputeRequest still works with an optional
resolutionNote (no other signature changes needed), keeping the returned object
shape and const assertions the same.
In `@services/platform-api/src/disputes/disputes.service.test.ts`:
- Around line 157-177: The test currently doesn't verify that startReviewDispute
actually succeeded or that an "under-review" dispute is returned; update the
test to assert the startReviewDispute call (the result of
service.startReviewDispute) is ok (throw or expect) and then tighten the final
assertions on service.getPendingDisputes to require that at least one dispute
has status === 'under-review' rather than only checking every dispute is 'open'
|| 'under-review'; reference submitDispute, startReviewDispute, and
getPendingDisputes to locate the calls and ensure the operator session is used
for the review action.
In `@services/platform-api/src/disputes/disputes.service.ts`:
- Around line 154-161: The current code calls this.disputes.findByStatus twice
(producing openDisputes and underReviewDisputes) and then merges/sorts them,
which can race and double-count if a status changes mid-flight; change to a
single repository read that queries both statuses at once (e.g., add/use a
method like this.disputes.findByStatuses(['open','under-review'], { orderBy:
'createdAt' }) or a single find({ status: IN(...) , order: 'createdAt' })) and
replace the Promise.all + merge/sort logic with that single call so the DB
returns a consistent ordered snapshot. Ensure you update the Disputes repository
(method name used above) and call site (replace openDisputes/underReviewDisputes
and the combined sort) accordingly.
- Around line 174-195: The service calls .trim() on resolutionNote without
runtime type checks, causing a TypeError if a non-string value is passed; update
resolveDispute and closeDispute to first verify typeof resolutionNote ===
'string' (for resolveDispute reject non-strings with a 400 and message like
"resolutionNote must be a string and non-empty") and for closeDispute treat
non-string/undefined as null or return 400 for explicit non-string inputs per
your API contract, then call .trim() only after confirming it's a string and
pass the trimmed string (or null) into transitionDispute (reference functions:
resolveDispute, closeDispute, transitionDispute).
In
`@services/platform-api/src/disputes/infrastructure/postgres-dispute.repository.ts`:
- Around line 26-67: The save method currently ignores the query result so
INSERT ... ON CONFLICT DO NOTHING can silently fail; update the save(dispute:
DisputeRecord) implementation to inspect the result from
this.postgresClient.query (check result.rowCount) and if rowCount === 0 return {
ok: false } (or alternatively fetch and return the existing dispute row) instead
of always returning { ok: true }; reference the save function and the
postgresClient.query call and ensure the caller in disputes.service.ts can
detect insertion failure via saved.ok.
---
Nitpick comments:
In `@apps/admin-web/src/features/disputes/dispute-queue-actions.test.ts`:
- Around line 84-96: The fetch stub (fetchImpl) in the dispute transition tests
only asserts the HTTP method and ignores the request URL, so add an assertion
that the incoming _url matches the expected route for the action under test
(e.g., the resolve test should assert the path includes the dispute id and
"/resolve", and likewise the start-review test should assert the path includes
"/start-review"). Update the fetchImpl used in those tests (the one referencing
makeDispute('d-1')) to call expect(_url).toBe(...) or expect(_url).toMatch(...)
with the exact API route you expect, and apply the same URL assertion change to
the other stub used around lines 113-118 so both transition tests validate the
client routing as well as method and response.
In `@apps/admin-web/src/features/disputes/dispute-queue-state.ts`:
- Around line 3-9: Replace the hardcoded DisputeQueueActionType and
DisputeQueueAction definitions with the canonical types/helpers from the domain
package: remove DisputeQueueActionType and the local DisputeQueueAction union
and import the domain-owned action type and transition helper (the types/helpers
referenced in packages/domain that drive action→status transitions) and use them
for optimistic UI state and mapping; update any code that previously relied on
DisputeQueueActionType/DisputeQueueAction to use the imported domain type names
and the domain transition helper so the optimistic states follow the
backend-owned rules.
In `@services/platform-api/src/disputes/disputes.service.test.ts`:
- Around line 205-286: Add a new spec in the "operator transitions" describe
block that covers the full lifecycle including the close transition: create
service/bookings, submit a dispute via submitDispute, transition it to
under-review with startReviewDispute, resolve it with resolveDispute, then call
closeDispute as an operator and assert the final status becomes "closed",
resolvedAt remains set, resolutionNote retained, and that getPendingDisputes no
longer returns the dispute; also include role/guard checks (e.g., ensure
non-operator cannot close) to mirror the existing tests' patterns using the same
helper functions (submitDispute, startReviewDispute, resolveDispute,
closeDispute, getPendingDisputes).
In
`@services/platform-api/src/disputes/infrastructure/dispute-repository.provider.test.ts`:
- Around line 21-34: Add a negative test case to ensure resolveDisputeRepository
fails fast when env.PERSISTENCE_MODE is 'postgres' but env.DATABASE_URL is
missing; update the test file to call resolveDisputeRepository with
inMemoryRepository, a PostgresClient instance, and env.PERSISTENCE_MODE set to
'postgres' but without DATABASE_URL (or set it to empty/undefined) and assert
that it throws/returns an error (or behaves as your provider's guard does).
Specifically reference resolveDisputeRepository, PERSISTENCE_MODE, DATABASE_URL,
InMemoryDisputeRepository, PostgresClient, and PostgresDisputeRepository so the
test mirrors the existing positive case but expects the guard to trigger.
In `@services/platform-api/src/persistence/postgres-mode.integration.test.ts`:
- Around line 147-167: After the happy-path assertions for reviewTransition and
resolveTransition, add replay and conflict checks: call
disputeRepository.transitionStatus again with the same parameters used for
reviewTransition and assert the repeated call indicates a replay/no-op (expect
ok to be false and dispute status remains 'under-review'); then call
transitionStatus with disputeId but with an incorrect allowedCurrentStatuses
(e.g., ['open'] after it moved) and assert that this returns a conflict (expect
ok to be false and the dispute status remains unchanged); use the existing
variables disputeId, reviewTransition, resolveTransition and the
transitionStatus method names to locate where to insert these assertions.
🪄 Autofix (Beta)
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: d1d8389d-6621-4c80-8523-95d84ca12dec
📒 Files selected for processing (21)
apps/admin-web/src/features/disputes/dispute-queue-actions.test.tsapps/admin-web/src/features/disputes/dispute-queue-actions.tsapps/admin-web/src/features/disputes/dispute-queue-state.test.tsapps/admin-web/src/features/disputes/dispute-queue-state.tspackages/api-client/src/index.tspackages/domain/src/index.tsscripts/smoke/operator-dispute-flow-smoke.shservices/platform-api/migrations/0007_disputes_lifecycle.sqlservices/platform-api/migrations/README.mdservices/platform-api/src/disputes/disputes.controller.tsservices/platform-api/src/disputes/disputes.module.tsservices/platform-api/src/disputes/disputes.service.test.tsservices/platform-api/src/disputes/disputes.service.tsservices/platform-api/src/disputes/domain/dispute.repository.tsservices/platform-api/src/disputes/infrastructure/dispute-repository.provider.test.tsservices/platform-api/src/disputes/infrastructure/dispute-repository.provider.tsservices/platform-api/src/disputes/infrastructure/in-memory-dispute.repository.test.tsservices/platform-api/src/disputes/infrastructure/in-memory-dispute.repository.tsservices/platform-api/src/disputes/infrastructure/postgres-dispute.repository.tsservices/platform-api/src/modules/index.tsservices/platform-api/src/persistence/postgres-mode.integration.test.ts
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/admin-web/src/features/disputes/dispute-queue-state.ts`:
- Around line 30-52: The reducer beginOptimisticDisputeTransition must
early-return instead of entering a transitioning queueAction when the target
dispute is missing, the action is invalid for the dispute's current status, or
another transition is already in-flight; update beginOptimisticDisputeTransition
to: 1) check state.status === 'loaded' (keep existing), 2) return state if
state.queueAction?.status === 'transitioning', 3) find the target dispute by
disputeId from state.disputes and return state if not found, 4) compute
nextStatus via disputeOperatorActionTransitions[actionType] and verify the
action is valid for the dispute's current status (e.g., mapping exists and
nextStatus differs from dispute.status); only then produce the new disputes
array with the single dispute's status set to nextStatus and set queueAction to
{ status: 'transitioning', disputeId, actionType } — otherwise return the
original state.
In `@services/platform-api/src/disputes/disputes.controller.ts`:
- Around line 23-25: The ResolveDisputeBody type currently declares
resolutionNote as optional but the service (resolve method in
disputes.service.ts) rejects missing notes; update the type alias
ResolveDisputeBody so resolutionNote is required (change resolutionNote?:
unknown to resolutionNote: unknown) and ensure any usages (e.g., the resolve
endpoint handler in disputes.controller.ts that accepts ResolveDisputeBody)
reflect this stricter contract; leave close as optional if present.
In
`@services/platform-api/src/disputes/infrastructure/postgres-dispute.repository.ts`:
- Around line 230-239: The short-circuit that returns replayed when
current.status === input.nextStatus should also verify the resolution fields;
instead of immediately returning {replayed: true} in the block that checks
current.status === input.nextStatus, compute the intended
nextResolvedAt/nextResolutionNote (using input.resolvedAt/input.resolutionNote
falling back to current.resolved_at/current.resolution_note as the diff shows)
and only treat it as a replay if nextResolvedAt === current.resolved_at and
nextResolutionNote === current.resolution_note; if the status matches but either
resolution note or resolvedAt would change, return a conflict (e.g., the same
shape as the existing transition-conflict { ok: false, reason:
'transition-conflict', currentStatus: current.status }) so callers get a 409
instead of a silent replay—apply this change in the method in
postgres-dispute.repository.ts where current, input.nextStatus,
input.resolutionNote, and input.resolvedAt are used and mapDisputeRow is
returned for successful replays.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f980746-7b54-45b4-96c6-9cc4de2e98a9
📒 Files selected for processing (11)
apps/admin-web/src/features/disputes/dispute-queue-actions.test.tsapps/admin-web/src/features/disputes/dispute-queue-state.tspackages/api-client/src/index.tsservices/platform-api/src/disputes/disputes.controller.tsservices/platform-api/src/disputes/disputes.service.test.tsservices/platform-api/src/disputes/disputes.service.tsservices/platform-api/src/disputes/domain/dispute.repository.tsservices/platform-api/src/disputes/infrastructure/dispute-repository.provider.test.tsservices/platform-api/src/disputes/infrastructure/in-memory-dispute.repository.tsservices/platform-api/src/disputes/infrastructure/postgres-dispute.repository.tsservices/platform-api/src/persistence/postgres-mode.integration.test.ts
✅ Files skipped from review due to trivial changes (1)
- services/platform-api/src/disputes/disputes.service.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- services/platform-api/src/disputes/infrastructure/dispute-repository.provider.test.ts
- apps/admin-web/src/features/disputes/dispute-queue-actions.test.ts
- services/platform-api/src/persistence/postgres-mode.integration.test.ts
- services/platform-api/src/disputes/domain/dispute.repository.ts
- services/platform-api/src/disputes/infrastructure/in-memory-dispute.repository.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 5 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 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 (1)
apps/admin-web/src/features/disputes/dispute-queue-state.ts (1)
1-5:⚠️ Potential issue | 🟠 MajorStill missing allowed-from-status validation before optimistic transition.
At Line 48-50, checking only
nextStatus !== dispute.statusis insufficient; invalid actions (for example,closefromopen) can still entertransitioningand cause phantom optimistic updates.Suggested fix
import { + canApplyDisputeOperatorAction, disputeOperatorActionTransitions, type DisputeOperatorActionType, type DisputeRecord, } from '@quickwerk/domain'; @@ const nextStatus = disputeOperatorActionTransitions[actionType]; - if (!nextStatus || nextStatus === dispute.status) { + if (!nextStatus || !canApplyDisputeOperatorAction(dispute.status, actionType) || nextStatus === dispute.status) { return state; }Also applies to: 48-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/features/disputes/dispute-queue-state.ts` around lines 1 - 5, The optimistic transition is being applied without verifying the action is allowed from the current dispute status; before setting a dispute to "transitioning" (the logic around disputeOperatorActionTransitions and the optimistic update path that compares nextStatus !== dispute.status), validate that the current dispute.status is included in disputeOperatorActionTransitions[action].from (or the appropriate allowed-from set) for the given DisputeOperatorActionType/DisputeRecord; if the action is not allowed from the current status, abort the optimistic state change and surface an error instead of flipping to transitioning. Ensure the check references disputeOperatorActionTransitions and DisputeOperatorActionType so invalid actions (e.g., close from open) cannot produce phantom optimistic updates.
🤖 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/disputes/infrastructure/postgres-dispute.repository.ts`:
- Around line 230-241: The check in postgres-dispute.repository.ts treats
resolvedAt as part of replay identity and can mark identical retries as
'transition-conflict'; update the logic in the block where current.status ===
input.nextStatus to ignore resolvedAt differences (or treat resolvedAt as
server-assigned) when deciding replayed vs conflict: compare resolutionNote
(normalize types, e.g., stringify/ISO when comparing current.resolved_at and
input.resolvedAt if you do compare), but return { ok: true, dispute:
mapDisputeRow(current), replayed: true } when the intent (nextStatus and
resolutionNote) matches, and only treat a true conflict when the action/status
or resolutionNote differ; reference variables: current.resolved_at,
input.resolvedAt, input.resolutionNote, input.nextStatus, and function
mapDisputeRow.
---
Duplicate comments:
In `@apps/admin-web/src/features/disputes/dispute-queue-state.ts`:
- Around line 1-5: The optimistic transition is being applied without verifying
the action is allowed from the current dispute status; before setting a dispute
to "transitioning" (the logic around disputeOperatorActionTransitions and the
optimistic update path that compares nextStatus !== dispute.status), validate
that the current dispute.status is included in
disputeOperatorActionTransitions[action].from (or the appropriate allowed-from
set) for the given DisputeOperatorActionType/DisputeRecord; if the action is not
allowed from the current status, abort the optimistic state change and surface
an error instead of flipping to transitioning. Ensure the check references
disputeOperatorActionTransitions and DisputeOperatorActionType so invalid
actions (e.g., close from open) cannot produce phantom optimistic updates.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7829bc5-d572-46c2-b0c8-2f5d5b93fb95
📒 Files selected for processing (5)
apps/admin-web/next-env.d.tsapps/admin-web/src/features/disputes/dispute-queue-state.tsapps/admin-web/tsconfig.jsonservices/platform-api/src/disputes/disputes.controller.tsservices/platform-api/src/disputes/infrastructure/postgres-dispute.repository.ts
✅ Files skipped from review due to trivial changes (2)
- apps/admin-web/next-env.d.ts
- apps/admin-web/tsconfig.json
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>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
services/platform-api/src/disputes/infrastructure/postgres-dispute.repository.ts (1)
76-85: Extract the repeated dispute projection into a shared constant.The same
id::text, booking_id::text, ... resolution_notelist is duplicated across seven queries. The next column change will be easy to miss in one path and breakmapDisputeRowonly there; a shared projection/helper would keep these reads in lockstep.Also applies to: 102-111, 128-137, 150-159, 172-181, 208-217, 257-266
🤖 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/disputes/infrastructure/postgres-dispute.repository.ts`:
- Around line 233-245: The in-memory repository's same-status path must match
Postgres behavior: in InMemoryDisputeRepository (method that handles status
transitions, e.g., the update/transition method currently returning a replay on
same-status) change the logic so when current.status === input.nextStatus you
normalize and compare resolution notes (treat undefined and null as equivalent)
and if they match return a replay ({ ok: true, dispute: ..., replayed: true }),
but if they differ return { ok: false, reason: 'transition-conflict',
currentStatus: current.status }; alternatively factor this compare/decision into
shared helper logic (e.g., a new function compareTransitionIntent) and call it
from both InMemoryDisputeRepository and PostgresDisputeRepository to avoid
duplication.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fec5760-e0ae-42a5-9747-1c61b37e6596
📒 Files selected for processing (1)
services/platform-api/src/disputes/infrastructure/postgres-dispute.repository.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>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/disputes/infrastructure/in-memory-dispute.repository.ts`:
- Around line 16-18: The in-memory repository is returning references to the
internal DisputeRecord objects; update the repository methods (findById,
findByOrderId, listByAccountId, listByIds and any other listed-return methods)
to return defensive copies (deep clones) of records instead of the original
objects, and ensure save and transitionStatus store a clone of incoming records
rather than the caller's object. Use a safe deep-clone strategy supported by the
runtime (e.g., structuredClone if available, otherwise JSON deep-copy or a
utility) and apply it when reading from the internal Map (clone single records
returned by findById/findByOrderId, clone arrays returned by
listByAccountId/listByIds, etc.) and when putting/updating entries in
save/transitionStatus so repository state cannot be mutated by external callers.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b9f32d5-05cc-4d95-b28a-167deebd21be
📒 Files selected for processing (1)
services/platform-api/src/disputes/infrastructure/in-memory-dispute.repository.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 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
Validation
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation