feat(api): add token filter query param to transfers endpoint#53
Conversation
- Read ?token= from GET /transfers/address/:address - Validate: must start with C and be 56 characters (Stellar SAC contract address) - Invalid value returns 400 with a descriptive error message - Valid value forwarded to queryAllTransfers as field - Absent param preserves existing behaviour (no filtering) - Add four tests: valid filter, wrong prefix, wrong length, absent param Closes Miracle656#35
|
@ezedike-evan Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
left a comment
There was a problem hiding this comment.
Good structure and solid tests — two things to fix before this can merge:
1. token is not wired into the DB query (critical)
queryAllTransfers in src/db.ts has AllTransfersQueryParams which doesn't include token. TypeScript will error on the call in api.ts, and even if it didn't, the filter would be silently ignored — no actual filtering would happen in the DB.
Fix in src/db.ts:
// In AllTransfersQueryParams:
token?: string;
// In the where clause:
...(token ? { contractId: token } : {}),(Token IS a contract ID — it's just a more semantic query param name, so mapping it to contractId in the where clause is correct.)
2. Remove leftover draft comment
Line ~305 in src/api.ts:
// ── PASTE THIS to replace the GET /transfers/address/:address handler in src/api.ts ──This is a work note that snuck into the final code — please remove it.
Everything else (validation, test coverage, response format) looks great. Once these two are fixed this is ready to merge.
…-by-token # Conflicts: # src/api.ts # src/db.ts
Miracle656
left a comment
There was a problem hiding this comment.
Merging. The ?token= filter is safe — validated as a 56-char C… contract address and applied via Prisma where: { contractId }, so it's parameterized/injection-proof. Build + the DB-restart chaos test pass.
The failing npm test step is not from this PR: it's Cannot access 'opaRequestSpy' before initialization, a vitest mock-hoisting bug in the OPA authorization test (from #94) — a file this PR doesn't touch. (Separately, your updated lockfile here actually fixes main's broken npm ci, which was missing fast-check/pure-rand after #88.) Both are tracked for the post-Wave CI cleanup.
Nit (non-blocking): in queryAllTransfers, baseWhere spreads both contractId and token onto the same contractId key, so if a client passes both params, token silently wins. Fine since they target the same column, but worth a guard or a documented precedence later.
Closes #35