fix: resolve swagger import, remittance status update, and TS duplica…#674
fix: resolve swagger import, remittance status update, and TS duplica…#674Skinny001 wants to merge 9 commits intoLabsCrypt:mainfrom
Conversation
ogazboiz
left a comment
There was a problem hiding this comment.
The dispute/appeal feature is a solid addition, but there are several issues that need to be sorted out before this can merge:
CI is failing (backend):
-
loanDispute.test.tsusesjest.mock()which doesn't work with ESM. You needjest.unstable_mockModule()and dynamicawait import()for the module under test. Check howeventIndexer.test.tsdoes it for the correct pattern. -
The existing
loanEndpoints.test.tstest forGET /api/loans/:loanIdnow returns 500 instead of 200 becausegetLoanDetailswas updated to also queryloan_disputes, but the test mock chain doesn't have a response for that extra query.
Other issues:
-
Duplicate route -
/:loanId/mark-defaultedis registered twice in identicalif (NODE_ENV)blocks. Looks like a copy-paste accident. -
Stray
backend/backend/directory - There's abackend/backend/package.jsonandbackend/backend/package-lock.jsonthat got committed. Looks like an accidentalnpm initin the wrong directory. Please remove these. -
Swagger gutted -
swagger.tswas replaced with justexport const swaggerSpec = {};. Any code importing fromswagger.js(notswagger.esm.js) will get an empty object. The swagger docs forPOST /admin/check-defaultswere also deleted. -
Migration number -
1783000000013_add-loan-disputes.jsuses the current highest number. Double check no other in-flight PR is using the same number. -
Jest downgraded from 30 to 29 - This is a pretty big change that affects the whole test suite. If it's needed for ESM compatibility, call it out explicitly so other contributors are aware.
-
Test-only endpoints (
createTestLoan,markLoanDefaulted,registerTestUser) are exposed whenNODE_ENV === "development". Consider restricting these totestonly so they can't accidentally be hit in a dev environment.
Please also rebase onto the latest main to pick up recent CI and codebase fixes.
|
Friendly reminder to rebase your branch onto the latest main and address the review feedback when you get a chance. The codebase had some fixes recently so a rebase is needed before we can move forward. Let me know if you need help. |
|
heads up, a few important changes just landed on main that affect your PR:
please rebase on latest main: git fetch upstream
git rebase upstream/main
git push --force-with-lease |
bf4423a to
1af467f
Compare
This pull request introduces a loan dispute/appeal mechanism to the backend, adds new admin endpoints for managing loan disputes, and refactors Swagger/OpenAPI documentation loading to better support ESM and test environments. It also includes improvements to test configuration and adds comprehensive tests for the new dispute flow.
Loan Dispute/Audit Mechanism:
loan_disputestable, enabling the tracking of disputed loans and their resolution status.loanDispute.test.ts.Swagger/OpenAPI Documentation Refactor:
swagger.esm.tsand updating the app to load Swagger docs from a dedicated entrypoint (app.with-swagger.ts). [1] [2] [3] [4]app.tsto avoid loading in test environments.Testing and Environment Setup:
jest.config.tsfor better ESM support, test matching, and environment setup, and ensures correct file extensions and mocks..env.testfile with required test environment variables for consistent test execution.jestand@types/jestinpackage.jsonfor compatibility with ESM and ts-jest. [1] [2]expressas a dependency in the backend package.Codebase Refactoring:
asyncHandlerutility from middleware to a sharedutilsdirectory and updates all imports accordingly. [1] [2] [3] [4]Other:
close #552