Skip to content

feat(frontend): add defaulted loan recovery UI#609

Open
Feyisara2108 wants to merge 3 commits intoLabsCrypt:mainfrom
Feyisara2108:feat/588-defaulted-loan-ui
Open

feat(frontend): add defaulted loan recovery UI#609
Feyisara2108 wants to merge 3 commits intoLabsCrypt:mainfrom
Feyisara2108:feat/588-defaulted-loan-ui

Conversation

@Feyisara2108
Copy link
Copy Markdown

#588
closed

Copy link
Copy Markdown
Contributor

@ogazboiz ogazboiz left a comment

Choose a reason for hiding this comment

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

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 push

Also rebase on main afterward to pick up a recent test fix in eventIndexer.test.ts.

@ogazboiz
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

@ogazboiz ogazboiz left a comment

Choose a reason for hiding this comment

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

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.tsx
  • frontend/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 activeTab state 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 updateUserScoresBulk service interface.
  • The BorrowerLoan status 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.

Copy link
Copy Markdown
Contributor

@ogazboiz ogazboiz left a comment

Choose a reason for hiding this comment

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

Good UI work for the defaulted loan states. The amber color scheme and collateral messaging improvements are solid.

Needs fixing:

  1. Merge conflicts. Please rebase onto latest main.

  2. Frontend lint failing. Prettier check still not passing despite the formatting commit.

  3. 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.

  4. Unused import. ShieldCheck is imported in loans/page.tsx but replaced by ShieldAlert. Remove the old import.

  5. Unconditional computation. seizureStatus is calculated for every loan even though it's only used for defaulted ones. Move it inside the conditional.

@ogazboiz
Copy link
Copy Markdown
Contributor

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.

@ogazboiz
Copy link
Copy Markdown
Contributor

heads up, a few important changes just landed on main that affect your PR:

  1. axios pinned to 1.13.5 - there's an active supply chain attack on axios 1.14.1 (pulls in confirmed malware). we added overrides in all package.json files to block it.

  2. CI now runs a supply chain audit before backend/frontend jobs. if your lockfile has a compromised package, CI will fail with a clear error.

  3. backend test fixes - loanEndpoints tests now use valid Stellar addresses and base64 strings. if your PR was failing backend CI but you didn't touch backend code, this should fix it after rebase.

please rebase on latest main:

git fetch upstream
git rebase upstream/main
git push --force-with-lease

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.

2 participants