fix(donation-api): add controller contract tests and stabilize donati…#69
fix(donation-api): add controller contract tests and stabilize donati…#69Joyyyb wants to merge 1 commit into
Conversation
|
@Joyyyb 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! 🚀 |
📝 WalkthroughWalkthroughAdds direct ChangesDonation Error Mapping and Integration Tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/appMiddlewares/policy.middleware.ts (1)
9-9: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winUse the actual response object in this branch.
Line 9 still names the parameter
_res, but Lines 24-29 callsendError(res, ...). That leavesresundefined here, so this middleware will not type-check/compile.Suggested fix
- (req: IRequest, _res: Response, next: NextFunction) => { + (req: IRequest, res: Response, next: NextFunction) => {Also applies to: 24-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/appMiddlewares/policy.middleware.ts` at line 9, The middleware in policy.middleware.ts is still naming the response parameter `_res`, but the body uses `res` when calling sendError, so the branch won’t type-check. Update the middleware signature to use the actual response object name consistently, and make sure the same identifier is passed through the error handling path in this function so sendError can reference it correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/donation.controller.test.ts`:
- Around line 42-88: The repository mock in donation.controller.test.ts is
ignoring the collected query filters, so the tests can’t detect incorrect
campaign/user/auth wiring. Update the createQueryBuilder mock’s getMany(),
getCount(), and getRawOne() behavior to apply the stored _wheres clauses before
returning results, using the qb state built by where() and andWhere(). This
should make the mocked repository honor route-specific filters in the
controller/service tests.
- Around line 96-103: The test helper makeApp is mutating the shared
AppDataSource singleton and leaving mocked isInitialized/getRepository state
behind, which can leak into other tests. Update the donation.controller.test
setup to save the original AppDataSource state before mocking, and restore it
after each test in the relevant describe block(s), including the helper used
around makeApp and the other affected setup at the referenced later location.
Ensure any overrides to AppDataSource as any are reset in afterEach so tests
remain isolated and order-independent.
---
Outside diff comments:
In `@src/appMiddlewares/policy.middleware.ts`:
- Line 9: The middleware in policy.middleware.ts is still naming the response
parameter `_res`, but the body uses `res` when calling sendError, so the branch
won’t type-check. Update the middleware signature to use the actual response
object name consistently, and make sure the same identifier is passed through
the error handling path in this function so sendError can reference it
correctly.
🪄 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: 3519ffb2-9de1-427f-9b45-16654767b44e
📒 Files selected for processing (3)
src/__tests__/donation.controller.test.tssrc/appMiddlewares/policy.middleware.tssrc/components/v1/Donation/donation.controller.ts
|
Hi @Joyyyb Thank you for your awesome contribution, however after analyzing your implementation, there are some minor fixes to be done. Kindly fix them to merge your PR asap. Also do not forget to use fundable.finance to offramp. |
…on error responses This change adds route/controller coverage for donation creation, listing, detail retrieval, campaign/user/my donation endpoints, admin auth checks, policy validation failures, and not-found behavior. It also ensures donation controllers return shared API payloads consistently and map database-not-ready failures to the stable DB_NOT_READY error code instead of collapsing them into a generic internal error. The implementation updates the donation controller to recognize database initialization failures explicitly, sends validation issues from policyMiddleware through the shared sendError helper, and introduces API-level tests that exercise success and expected failure responses without changing donation schema fields or endpoint surface area. Closes #7ea6c6de-c6ed-4cea-9e3b-fe610b2ec172
Joyyyb
force-pushed
the
fix/donation-controller-api-contract
branch
from
a628f8b to
ea61d4e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/donation.controller.test.ts (1)
351-380: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated app-bootstrap logic for the DB_NOT_READY test.
This test re-implements
makeApp's body (express init, route mounting,AppDataSourcestubbing) instead of parameterizing the existing helper with anisInitializedflag.♻️ Suggested refactor
-const makeApp = (repo: any) => { +const makeApp = (repo: any, isInitialized = true) => { const app = express(); app.use(express.json()); - (AppDataSource as any).isInitialized = true; + (AppDataSource as any).isInitialized = isInitialized; (AppDataSource as any).getRepository = () => repo; app.use('/donations', donationRoutes); return app; };test('POST /donations returns DB_NOT_READY when the data source is unavailable', async () => { const repo = makeRepo(); - const app = express(); - app.use(express.json()); - (AppDataSource as any).isInitialized = false; - (AppDataSource as any).getRepository = () => repo; - app.use('/donations', donationRoutes); + const app = makeApp(repo, false);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/donation.controller.test.ts` around lines 351 - 380, The DB_NOT_READY test is duplicating the app setup logic instead of reusing the shared helper. Update the test to use the existing makeApp helper in donation.controller.test.ts and add a way to pass the AppDataSource initialization state (for example, an isInitialized flag) so the test can set the source as unavailable without rebuilding the express app and route wiring inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/donation.controller.test.ts`:
- Around line 128-129: The test helper that creates auth tokens is signing with
a hardcoded secret, which can diverge from the app’s JWT verification config.
Update the bearerToken helper in donation.controller.test.ts to read the same
JWT secret used by the middleware via appConfigs.authConfig.jwtSecret so the
generated tokens match the app configuration. Use the existing helper name
bearerToken and the auth config symbol appConfigs.authConfig.jwtSecret to locate
the change.
---
Nitpick comments:
In `@src/__tests__/donation.controller.test.ts`:
- Around line 351-380: The DB_NOT_READY test is duplicating the app setup logic
instead of reusing the shared helper. Update the test to use the existing
makeApp helper in donation.controller.test.ts and add a way to pass the
AppDataSource initialization state (for example, an isInitialized flag) so the
test can set the source as unavailable without rebuilding the express app and
route wiring inline.
🪄 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: 3a866cbf-9633-41d8-b585-314469c724f9
📒 Files selected for processing (3)
src/__tests__/donation.controller.test.tssrc/appMiddlewares/policy.middleware.tssrc/components/v1/Donation/donation.controller.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/appMiddlewares/policy.middleware.ts
- src/components/v1/Donation/donation.controller.ts
Conflict has been resolved. Thanks for calling my attention to it, and I will surely check out Fundable.finance. |
2 participants
…on error responses
This change adds route/controller coverage for donation creation, listing, detail retrieval, campaign/user/my donation endpoints, admin auth checks, policy validation failures, and not-found behavior. It also ensures donation controllers return shared API payloads consistently and map database-not-ready failures to the stable DB_NOT_READY error code instead of collapsing them into a generic internal error.
The implementation updates the donation controller to recognize database initialization failures explicitly, sends validation issues from policyMiddleware through the shared sendError helper, and introduces API-level tests that exercise success and expected failure responses without changing donation schema fields or endpoint surface area.
Closes #7ea6c6de-c6ed-4cea-9e3b-fe610b2ec172
Summary
Describe what changed and why.
Area
src/)indexer/common/)indexer/streams/)indexer/distributions/)Scope
Verification
bun run type-checkbun run testbun run lintbun run indexer:type-checkif indexer files changedbun run indexer:testif indexer files changedbun run indexer:lintif indexer files changedIndexer Safety
Notes
Closes #62
Summary by CodeRabbit
VALIDATION_ERRORresponse, including structured issue details.DB_NOT_READY(500) response.