feat(frontend): add defaulted loan recovery UI#609
feat(frontend): add defaulted loan recovery UI#609Feyisara2108 wants to merge 3 commits intoLabsCrypt:mainfrom
Conversation
ogazboiz
left a comment
There was a problem hiding this comment.
Frontend Prettier check is failing. Run this from the frontend/ directory and commit:
cd frontend
npx prettier --write .
git add -A
git commit -m 'style: apply prettier formatting'
git pushAlso rebase on main afterward to pick up a recent test fix in eventIndexer.test.ts.
|
The codebase issues on main have been resolved and all CI checks are passing now. Please rebase your branch to pull in the latest changes before continuing. Thanks for your patience. |
ogazboiz
left a comment
There was a problem hiding this comment.
Join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0
Review Summary
Good progress on the defaulted-loan recovery UI and the backend test alignment for bulk score updates. A few things still need to be addressed before this can merge.
Blockers
1. Frontend CI is still failing (Prettier / Lint step)
The previous review flagged this too. The formatting commit (style: format defaulted loan pages) did not fully fix the lint check. Please run npx prettier --write . from the frontend/ directory, verify it passes locally with npx prettier --check ., then push.
2. Merge conflicts -- needs rebase on main
The merge state is CONFLICTING / DIRTY. The eventIndexer.test.ts changes in this PR overlap with recent main-branch work. Please rebase on main, resolve conflicts in the test file, and force-push the branch.
Code Quality Issues
3. Duplicated SUPPORT_URL constant
SUPPORT_URL is defined identically in two files:
frontend/src/app/[locale]/loans/page.tsxfrontend/src/app/[locale]/loans/[loanId]/page.tsx
Extract it into a shared constants file (e.g. frontend/src/app/constants.ts) and import it from both pages. This avoids the risk of the URLs drifting apart if one is updated but not the other.
4. Duplicated penaltyFees calculation
The formula Math.max(totalOwed - (principal + accruedInterest), 0) appears in both loans/page.tsx (as the getPenaltyFees helper) and loans/[loanId]/page.tsx (inline). Pull this into a shared utility so there is a single source of truth.
5. Duplicated "Contact Support" button markup
The loan detail page ([loanId]/page.tsx) renders a near-identical "Contact Support" <a> button in two separate places (the defaulted banner and the recovery-options sidebar panel). Consider extracting a small ContactSupportButton component or at least a shared class string to avoid maintaining the same markup twice.
6. seizureStatus variable is only used inside the defaulted branch
In loans/page.tsx, seizureStatus is computed on every loan card render even for non-defaulted loans. Move it inside the isDefaulted conditional block so it is only calculated when actually needed.
7. Unused import: ShieldCheck
After your changes, ShieldCheck is imported in loans/page.tsx but never used. The new code uses ShieldAlert instead. Remove the stale import.
Minor / Nits
- The removed JSX comments (
{/* Breadcrumb */},{/* Header */}, etc.) are fine to clean up, but keep it consistent. A few section comments remain in other parts of the file. Either remove all of them or keep them all. - The
activeTabstate type got reformatted across two lines. Not a problem, but double check that Prettier actually wants that. If it does, great.
Branch Name
feat/588-defaulted-loan-ui -- descriptive and linked to the issue. No issues there.
What Looks Good
- The amber color scheme for defaulted loans provides clear visual differentiation.
- Collateral seizure status now distinguishes between "seized" and "in review," which is a meaningful UX improvement.
- Backend test updates correctly align with the new
updateUserScoresBulkservice interface. - The
BorrowerLoanstatus type was updated to include"defaulted", keeping the type system honest.
TL;DR: Fix the frontend lint failure, rebase on main to resolve conflicts, and consolidate the duplicated constants / helpers / button markup. After that this should be in good shape.
ogazboiz
left a comment
There was a problem hiding this comment.
Good UI work for the defaulted loan states. The amber color scheme and collateral messaging improvements are solid.
Needs fixing:
-
Merge conflicts. Please rebase onto latest main.
-
Frontend lint failing. Prettier check still not passing despite the formatting commit.
-
Duplicated code. SUPPORT_URL constant, penalty fees calculation, and "Contact Support" button markup are all duplicated between loans/page.tsx and loans/[loanId]/page.tsx. Extract those into shared constants/utilities/components.
-
Unused import. ShieldCheck is imported in loans/page.tsx but replaced by ShieldAlert. Remove the old import.
-
Unconditional computation. seizureStatus is calculated for every loan even though it's only used for defaulted ones. Move it inside the conditional.
|
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 |
#588
closed