Skip to content

escrow indexer queries#186

Open
Lobito8 wants to merge 1 commit intoStarShopCr:mainfrom
Lobito8:feat/escrow_indexer_queries
Open

escrow indexer queries#186
Lobito8 wants to merge 1 commit intoStarShopCr:mainfrom
Lobito8:feat/escrow_indexer_queries

Conversation

@Lobito8
Copy link

@Lobito8 Lobito8 commented Sep 27, 2025

🚀 StarShop Pull Request – Issue #10 (Escrow Indexer Queries)

Mark with an x all the checkboxes that apply (like [x]).

✅ Checklist

  • Closes Escrow Indexer Queries #164
  • Added tests (e2e: escrow listing, balances, validation)
  • Run tests (attach screenshot of npm run test:e2e)
  • Run formatting (npm run format / npm run lint:fix)
  • Evidence attached (add screenshots below)
  • Commented the code (service + controller additions)

📌 Type of Change

  • Documentation (README: Escrow Indexer Queries section)
  • Bug fix
  • Enhancement (new escrow querying + balances polling support)
  • Breaking change

📝 Changes Description

Implemented backend support for seller dashboard live escrow data:

  1. Endpoints:
    • GET /api/v1/escrows (all escrows for authenticated user as buyer or seller)
    • GET /api/v1/escrows?role=buyer|seller (role-filtered)
    • POST /api/v1/escrows/balances { ids: string[] } (batch balances polling)
  2. Added DTOs: GetEscrowsQueryDto, MultipleEscrowBalancesDto.
  3. Service logic: computed releasedAmount & remainingAmount from approved milestones.
  4. Extended e2e test escrow.e2e-spec.ts (listing, role filter, balances, validation error on empty ids).
  5. README updated with: response shapes, 30s polling rule, empty state guidance, DoD mapping.

💰 Escrow Balance Logic

  • releasedAmount = sum(amount) of milestones with status approved.
  • remainingAmount = totalAmount - releasedAmount (floored at 0).
  • Status progression auto-updates when milestones are approved.

🧪 Tests Added

  • Buyer milestone approval retained.
  • Listing endpoints (all + role=buyer).
  • Balances map shape & keys.
  • Validation failure for empty ids array.

📸 Evidence (attach screenshots)

  1. GET /api/v1/escrows response.
  2. POST /api/v1/escrows/balances response.
  3. Jest e2e run output (all passing).

⏰ Time Spent Breakdown

Task Time
Endpoint design 15m
Service implementation 25m
DTOs + controller wiring 15m
Tests update 20m
README documentation 10m
Review & polish 10m
Total ~1h35m

🌌 Comments

  • Frontend should skip balances call if ids.length === 0 (backend validates non-empty).
  • Empty state handled via frontend messaging (README guidance).
  • Future: pagination, WebSocket push for milestone events, individual escrow detail expansion.

🧾 Suggested Squash Commit Message

feat(escrows): add indexer-style listing & balances polling endpoints (#10)

Adds GET /escrows, GET /escrows?role=buyer|seller and POST /escrows/balances.
Computes released/remaining amounts from approved milestones. Extends e2e tests
and updates README with polling (30s) & empty state guidance.

@coderabbitai
Copy link

coderabbitai bot commented Sep 27, 2025

Walkthrough

Adds Husky Git LFS hooks, introduces an Escrow domain (entities, DTOs, service, controller, module, migration), wires it in AppModule, and adds unit/e2e tests. Also updates some import paths. The Escrow API supports milestone approval, seller-driven status transitions, listing/filtering escrows, and fetching multiple escrow balances.

Changes

Cohort / File(s) Summary of Changes
Husky Git LFS hooks
.husky/post-checkout, .husky/post-commit, .husky/post-merge, .husky/pre-push
Added shell hooks that check for git-lfs and delegate to corresponding git lfs subcommands; exit with informative message if missing.
App module wiring + path tweaks
src/app.module.ts, src/modules/files/entities/file.entity.ts, src/src/modules/reviews/middlewares/review.middleware.ts
Registered EscrowsModule; added Escrow/Milestone entities; updated OffersModule import path; fixed import paths for User and BadRequestError.
Escrow entities and migration
src/modules/escrows/entities/escrow.entity.ts, src/modules/escrows/entities/milestone.entity.ts, src/modules/escrows/migrations/1752190000000-ExtendMilestoneStatusEnum.ts
Added Escrow and Milestone entities with enums, relations, constraints, and timestamps; migration extends milestone status enum in Postgres.
Escrow module and controller
src/modules/escrows/escrows.module.ts, src/modules/escrows/controllers/escrow.controller.ts
Created EscrowsModule; added controller with endpoints for approval, status change, listing, and balances.
Escrow DTOs
src/modules/escrows/dto/*.ts
Added DTOs: approve-milestone (optional type), update-milestone-status (enum-validated), get-escrows-query (role filter), multiple-balances (UUID array validation).
Escrow service + unit tests
src/modules/escrows/services/escrow.service.ts, src/modules/escrows/services/escrow.service.spec.ts
Implemented transactional escrow logic (computations, approvals, status transitions) with validations; added unit tests covering success and error paths.
E2E tests
test/escrow.e2e-spec.ts
Added end-to-end tests for milestone approval, role checks, idempotency, listings, and balances, including validation errors.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Buyer as Buyer (JWT)
  participant API as EscrowController
  participant Svc as EscrowService
  participant DB as DB (TypeORM)

  rect rgb(235, 248, 255)
  note right of Buyer: Approve milestone
  Buyer->>API: PATCH /escrows/:escrowId/milestones/:milestoneId/approve
  API->>Svc: approveMilestone(escrowId, milestoneId, userId)
  Svc->>DB: txn: load escrow, milestone
  Svc->>Svc: validate ownership & state
  Svc->>DB: update milestone.status=APPROVED, approvedAt
  Svc->>DB: maybe update escrow.status (IN_PROGRESS/COMPLETED)
  DB-->>Svc: commit
  Svc-->>API: Milestone
  API-->>Buyer: 200 OK (approved)
  end
Loading
sequenceDiagram
  autonumber
  actor Seller as Seller (JWT)
  participant API as EscrowController
  participant Svc as EscrowService
  participant DB as DB (TypeORM)

  rect rgb(245, 240, 255)
  note right of Seller: Change milestone status
  Seller->>API: PATCH /escrows/:eId/milestones/:mId/status {status}
  API->>Svc: changeMilestoneStatus(eId, mId, sellerId, nextStatus)
  Svc->>DB: txn: load escrow, milestone
  Svc->>Svc: validate seller, forward-only transition
  Svc->>DB: update milestone.status
  DB-->>Svc: commit
  Svc-->>API: Milestone
  API-->>Seller: 200 OK
  end
Loading
sequenceDiagram
  autonumber
  actor User as User (JWT)
  participant API as EscrowController
  participant Svc as EscrowService
  participant DB as DB

  rect rgb(238, 250, 240)
  note right of User: List & balances
  User->>API: GET /escrows[?role=buyer|seller]
  API->>Svc: getEscrowsBySigner/Role(userId)
  Svc->>DB: query escrows+milestones
  Svc->>Svc: compute released/remaining
  Svc-->>API: Escrows with amounts
  API-->>User: 200 OK

  User->>API: POST /escrows/balances {ids}
  API->>Svc: getMultipleEscrowBalances(ids)
  Svc->>DB: find escrows by ids
  Svc->>Svc: compute amounts map
  Svc-->>API: {id: {released, remaining, status}}...
  API-->>User: 200 OK
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • Escrow Indexer Queries #164 — Implements Escrow indexer queries with getMultipleEscrowBalances, getEscrowsBySigner, and getEscrowsByRole endpoints/services.

Possibly related PRs

Suggested reviewers

  • Villarley

Poem

A nibble of code, a hop through the fields,
Milestones march forward on transactional shields.
Buyers approve, sellers set the pace,
Balances tallied in each escrowed case.
Git hooks twitch whiskers—LFS on patrol.
Thump! Ship it—this rabbit’s on a roll. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title “escrow indexer queries” is fully related to the primary changes by highlighting the new escrow querying and balance polling endpoints and succinctly summarizes the main feature implemented.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (1)
src/app.module.ts (1)

52-90: Align indentation for new entities/modules

While functional, the added Escrow, Milestone, and EscrowsModule entries lost their indentation, which makes the array harder to scan. Please re-indent them to match surrounding entries for readability.

-        OfferAttachment,
-  Escrow,
-  Milestone,
+        OfferAttachment,
+        Escrow,
+        Milestone,
...
-    OffersModule,
-    SupabaseModule,
-  EscrowsModule,
+    OffersModule,
+    SupabaseModule,
+    EscrowsModule,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09b2bd9 and 084a2d9.

📒 Files selected for processing (19)
  • .husky/post-checkout (1 hunks)
  • .husky/post-commit (1 hunks)
  • .husky/post-merge (1 hunks)
  • .husky/pre-push (1 hunks)
  • src/app.module.ts (4 hunks)
  • src/modules/escrows/controllers/escrow.controller.ts (1 hunks)
  • src/modules/escrows/dto/approve-milestone.dto.ts (1 hunks)
  • src/modules/escrows/dto/get-escrows-query.dto.ts (1 hunks)
  • src/modules/escrows/dto/multiple-balances.dto.ts (1 hunks)
  • src/modules/escrows/dto/update-milestone-status.dto.ts (1 hunks)
  • src/modules/escrows/entities/escrow.entity.ts (1 hunks)
  • src/modules/escrows/entities/milestone.entity.ts (1 hunks)
  • src/modules/escrows/escrows.module.ts (1 hunks)
  • src/modules/escrows/migrations/1752190000000-ExtendMilestoneStatusEnum.ts (1 hunks)
  • src/modules/escrows/services/escrow.service.spec.ts (1 hunks)
  • src/modules/escrows/services/escrow.service.ts (1 hunks)
  • src/modules/files/entities/file.entity.ts (1 hunks)
  • src/src/modules/reviews/middlewares/review.middleware.ts (1 hunks)
  • test/escrow.e2e-spec.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/escrow.e2e-spec.ts (1)
src/src/modules/offers/entities/offer.entity.ts (1)
  • OfferStatus (18-18)
src/modules/escrows/controllers/escrow.controller.ts (4)
src/src/modules/wishlist/common/types/auth-request.type.ts (1)
  • AuthRequest (4-14)
src/modules/escrows/dto/update-milestone-status.dto.ts (1)
  • UpdateMilestoneStatusDto (5-8)
src/modules/escrows/dto/get-escrows-query.dto.ts (1)
  • GetEscrowsQueryDto (3-7)
src/modules/escrows/dto/multiple-balances.dto.ts (1)
  • MultipleEscrowBalancesDto (3-8)
🔇 Additional comments (5)
src/modules/escrows/dto/get-escrows-query.dto.ts (1)

3-7: LGTM on query validation

Optional role filtering is guarded with the right decorators and matches the controller expectations. Nicely done.

src/modules/escrows/dto/update-milestone-status.dto.ts (1)

5-8: Validation aligns with service contract

Enumerating against MilestoneStatus keeps the controller payload tight and mirrors the service safeguards. Looks solid.

test/escrow.e2e-spec.ts (1)

140-180: Comprehensive coverage appreciated

Great to see the end-to-end coverage spanning role filtering, balances, and validation error paths. This gives strong confidence in the new endpoints.

src/modules/escrows/services/escrow.service.spec.ts (1)

61-92: Unit tests capture key transition scenarios

The matrix of happy path, auth guard, backward transition, idempotency, and post-approval checks exercises the critical edges of changeMilestoneStatus. Thanks for covering these.

src/src/modules/reviews/middlewares/review.middleware.ts (1)

2-2: BadRequestError import path is correct
Import ‘../../../../utils/errors’ resolves to src/utils/errors.ts where BadRequestError is declared—no change needed.

Comment on lines +1 to +3
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-checkout' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-checkout "$@"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Load Husky shim before running the hook

Each Husky hook must source husky.sh; otherwise the standard skip logic (HUSKY=0, HUSKY_SKIP_HOOKS), ENV tweaks, and debug features never run. That means this script will execute even in CI environments that intentionally disable hooks, causing avoidable failures when git-lfs isn’t installed. Please restore the husky shim (same applies to the sibling hooks in this PR).

 #!/bin/sh
+. "$(dirname -- "$0")/_/husky.sh"
 command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-checkout' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
 git lfs post-checkout "$@"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-checkout' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-checkout "$@"
#!/bin/sh
. "$(dirname -- "$0")/_/husky.sh"
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-checkout' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-checkout "$@"
🤖 Prompt for AI Agents
In .husky/post-checkout lines 1-3, the hook runs git-lfs directly without
sourcing the husky shim so Husky skip logic and env handling are bypassed; fix
by sourcing the Husky shim immediately after the shebang (e.g. add a line to
source the husky.sh in the repo's .husky/_/husky.sh using dirname "$0" to locate
it) before any other commands, then leave the existing git-lfs detection and git
lfs post-checkout invocation unchanged so the hook respects
HUSKY/HUSKY_SKIP_HOOKS and CI behavior.

Comment on lines +1 to +3
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-commit' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-commit "$@"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Load Husky shim before running the hook

The Husky shim is absent here as well, so Husky’s native skip/disable features won’t work and the hook will always run. Add the shim first thing in the script to re-enable expected behavior.

 #!/bin/sh
+. "$(dirname -- "$0")/_/husky.sh"
 command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-commit' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
 git lfs post-commit "$@"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-commit' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-commit "$@"
#!/bin/sh
. "$(dirname -- "$0")/_/husky.sh"
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-commit' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-commit "$@"
🤖 Prompt for AI Agents
In .husky/post-commit around lines 1 to 3, the script never loads the Husky shim
so Husky skip/disable features won't work; fix by sourcing the Husky shim as the
first action in the hook (e.g. source or dot the .husky/shim file if it exists),
then keep the existing git-lfs presence check and git lfs post-commit call
unchanged; ensure the shim sourcing is conditional if the shim file may be
missing and that the script remains executable.

Comment on lines +1 to +3
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-merge' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-merge "$@"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Load Husky shim before running the hook

Same issue here: without sourcing husky.sh, Husky’s opt-out/skip mechanisms are bypassed and the hook still fires in contexts that expect it to be silent. Please add the shim before any logic so the environment is set up correctly.

 #!/bin/sh
+. "$(dirname -- "$0")/_/husky.sh"
 command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-merge' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
 git lfs post-merge "$@"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-merge' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs post-merge "$@"
#!/bin/sh
. "$(dirname -- "$0")/_/husky.sh"
command -v git-lfs >/dev/null 2>&1 || { \
printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'post-merge' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; \
exit 2; \
}
git lfs post-merge "$@"
🤖 Prompt for AI Agents
In .husky/post-merge lines 1-3, the hook runs Git LFS logic without sourcing the
Husky shim, so Husky's opt-out/skip mechanisms are bypassed; fix this by
sourcing the Husky shim at the very top of the file (before any logic) from the
repository's husky helper (usually .husky/_/husky.sh) so the environment and
skip flags are initialized, then retain the existing shebang and Git LFS checks
below.

Comment on lines +1 to +3
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs pre-push "$@"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Load Husky shim before running the hook

Please source husky.sh so that Husky’s own skip logic is honored; otherwise the hook can’t be disabled in CI or ad-hoc scenarios, leading to unexpected exits when git-lfs isn’t present.

 #!/bin/sh
+. "$(dirname -- "$0")/_/husky.sh"
 command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
 git lfs pre-push "$@"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs pre-push "$@"
#!/bin/sh
. "$(dirname -- "$0")/_/husky.sh"
command -v git-lfs >/dev/null 2>&1 || { printf >&2 "\n%s\n\n" "This repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks')."; exit 2; }
git lfs pre-push "$@"
🤖 Prompt for AI Agents
In .husky/pre-push around lines 1 to 3, the script runs git-lfs checks directly
and therefore bypasses Husky’s skip/disable logic; modify the script to source
the Husky shim (husky.sh) at the top before any checks so Husky’s SKIP_HOOKS and
CI handling are honored, then proceed with the existing git-lfs existence check
and git lfs pre-push invocation.

import { OrdersModule } from './modules/orders/orders.module';
import { BuyerRequestsModule } from './modules/buyer-requests/buyer-requests.module';
import { OffersModule } from './modules/offers/offers.module';
import { OffersModule } from './src/modules/offers/offers.module';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix the OffersModule import path

src/app.module.ts already lives under src/, so importing OffersModule via './src/modules/offers/offers.module' points to a non-existent src/src/... directory. Nest will fail to compile with Cannot find module './src/modules/offers/offers.module'. Please revert to the original relative path.

-import { OffersModule } from './src/modules/offers/offers.module';
+import { OffersModule } from './modules/offers/offers.module';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { OffersModule } from './src/modules/offers/offers.module';
import { OffersModule } from './modules/offers/offers.module';
🤖 Prompt for AI Agents
In src/app.module.ts around line 18, the import uses
'./src/modules/offers/offers.module' which resolves to src/src/... and fails;
change the import to the correct relative path (e.g.
'./modules/offers/offers.module') so it points to the actual OffersModule file
under src/modules.

}

@Entity('escrows')
@Check('"totalAmount" >= 0')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix the CHECK constraint column reference.

The CHECK clause targets "totalAmount", but the column is persisted as total_amount. Postgres will fail the migration (column "totalAmount" does not exist), so the table won’t be created. Update the constraint to reference the actual column name.

-@Check('"totalAmount" >= 0')
+@Check('"total_amount" >= 0')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Check('"totalAmount" >= 0')
@Check('"total_amount" >= 0')
🤖 Prompt for AI Agents
In src/modules/escrows/entities/escrow.entity.ts around line 13, the CHECK
constraint references "totalAmount" but the actual DB column is total_amount;
update the CHECK clause to reference total_amount (e.g., CHECK("total_amount" >=
0) or without quotes depending on naming) so Postgres can find the column and
the migration will succeed.

Comment on lines +17 to +21
await queryRunner.query(`ALTER TYPE "public"."escrow_milestones_status_enum" RENAME TO "escrow_milestones_status_enum_old"`);
await queryRunner.query(`CREATE TYPE "public"."escrow_milestones_status_enum" AS ENUM('pending','approved')`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" DROP DEFAULT`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" TYPE "public"."escrow_milestones_status_enum" USING "status"::text::"public"."escrow_milestones_status_enum"`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" SET DEFAULT 'pending'`);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Ensure down migration copes with the new statuses

As soon as any row is saved with ready, in_progress, or delivered, the down migration will crash when it tries to coerce those values into the reduced enum. Please normalise the data back to the legacy domain before altering the column type.

Apply this diff inside the down method:

   public async down(queryRunner: QueryRunner): Promise<void> {
     await queryRunner.query(`ALTER TYPE "public"."escrow_milestones_status_enum" RENAME TO "escrow_milestones_status_enum_old"`);
+    await queryRunner.query(`UPDATE "escrow_milestones" SET "status" = 'pending' WHERE "status"::text NOT IN ('pending','approved')`);
     await queryRunner.query(`CREATE TYPE "public"."escrow_milestones_status_enum" AS ENUM('pending','approved')`);
     await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" DROP DEFAULT`);
     await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" TYPE "public"."escrow_milestones_status_enum" USING "status"::text::"public"."escrow_milestones_status_enum"`);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await queryRunner.query(`ALTER TYPE "public"."escrow_milestones_status_enum" RENAME TO "escrow_milestones_status_enum_old"`);
await queryRunner.query(`CREATE TYPE "public"."escrow_milestones_status_enum" AS ENUM('pending','approved')`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" DROP DEFAULT`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" TYPE "public"."escrow_milestones_status_enum" USING "status"::text::"public"."escrow_milestones_status_enum"`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" SET DEFAULT 'pending'`);
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`ALTER TYPE "public"."escrow_milestones_status_enum" RENAME TO "escrow_milestones_status_enum_old"`);
await queryRunner.query(`UPDATE "escrow_milestones" SET "status" = 'pending' WHERE "status"::text NOT IN ('pending','approved')`);
await queryRunner.query(`CREATE TYPE "public"."escrow_milestones_status_enum" AS ENUM('pending','approved')`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" DROP DEFAULT`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" TYPE "public"."escrow_milestones_status_enum" USING "status"::text::"public"."escrow_milestones_status_enum"`);
await queryRunner.query(`ALTER TABLE "escrow_milestones" ALTER COLUMN "status" SET DEFAULT 'pending'`);
}

Comment on lines +40 to +47
async getMultipleEscrowBalances(ids: string[]): Promise<Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>> {
if (!ids.length) return {};
const escrows = await this.escrowRepo.find({ where: ids.map((id) => ({ id })), relations: ['milestones'] });
return escrows.reduce((acc, e) => {
const computed = this.computeAmounts(e);
acc[e.id] = { ...computed, status: e.status };
return acc;
}, {} as Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Enforce ownership when returning balances.

getMultipleEscrowBalances accepts arbitrary IDs and returns status/amounts without checking that the requester is the buyer or seller. Any authenticated user can enumerate escrow UUIDs and read other users’ financial data. Pass the caller’s userId into this method and restrict the query to escrows where they are the buyer or seller. Update the controller accordingly.

-  async getMultipleEscrowBalances(ids: string[]): Promise<Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>> {
+  async getMultipleEscrowBalances(
+    ids: string[],
+    userId: number,
+  ): Promise<Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>> {
     if (!ids.length) return {};
-    const escrows = await this.escrowRepo.find({ where: ids.map((id) => ({ id })), relations: ['milestones'] });
+    const escrows = await this.escrowRepo.find({
+      where: ids.flatMap((id) => [{ id, buyerId: userId }, { id, sellerId: userId }]),
+      relations: ['milestones'],
+    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async getMultipleEscrowBalances(ids: string[]): Promise<Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>> {
if (!ids.length) return {};
const escrows = await this.escrowRepo.find({ where: ids.map((id) => ({ id })), relations: ['milestones'] });
return escrows.reduce((acc, e) => {
const computed = this.computeAmounts(e);
acc[e.id] = { ...computed, status: e.status };
return acc;
}, {} as Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>);
async getMultipleEscrowBalances(
ids: string[],
userId: number,
): Promise<Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>> {
if (!ids.length) return {};
const escrows = await this.escrowRepo.find({
where: ids.flatMap((id) => [{ id, buyerId: userId }, { id, sellerId: userId }]),
relations: ['milestones'],
});
return escrows.reduce((acc, e) => {
const computed = this.computeAmounts(e);
acc[e.id] = { ...computed, status: e.status };
return acc;
}, {} as Record<string, { releasedAmount: number; remainingAmount: number; status: EscrowStatus }>);
}

@Joaco2603
Copy link
Collaborator

Joaco2603 commented Oct 1, 2025

@Lobito8 Fix conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Escrow Indexer Queries

2 participants