Web 657: Working Capital loan account details enhanced#3554
Web 657: Working Capital loan account details enhanced#3554alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Data Models src/app/loans/models/working-capital/working-capital-loan-account.model.ts |
Three interfaces added: ProjectedAmortizationSchedule (schedule metadata and payments array), Payment (per-period payment details including forecast and NPV fields), and WorkingCapitalBalances (principal, income, and overpayment summary). |
Service Layer src/app/loans/loans.service.ts |
New method getWorkingCapitalLoanAmortizationSchedule(loanId) performs HTTP GET to /working-capital-loans/{loanId}/amortization-schedule. |
Core Resolver src/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts |
New LoanAmortizationScheduleResolver extends LoanBaseResolver, injects LoansService, and resolves amortization schedule data when loanId is numeric and isWorkingCapital is true; returns null otherwise. |
Routing & Wiring src/app/loans/loans-routing.module.ts |
Adds two new child routes under :loanId: balances (using LoanBalancesTabComponent) and amortization-schedule (using LoanAmortizationScheduleTabComponent with resolver). Wires resolver into LoansRoutingModule providers. |
UI Components src/app/loans/loans-view/working-capital/loan-balances-tab/*, src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/* |
Two new standalone components: LoanBalancesTabComponent renders currency-formatted balance fields (principal, income, overpayment); LoanAmortizationScheduleTabComponent renders a Material table of payment schedules with export-to-PDF functionality using jsPDF and jspdf-autotable. |
Integration & View Updates src/app/loans/loans-view/account-details/account-details.component.html, src/app/loans/loans-view/loans-view.component.html, src/app/loans/loans-view/loans-view.component.ts |
Account details template adds Currency and Amortization Type rows, expands Discount display (proposed/approved/actual). Loans view template gates Repayment Schedule tab behind isLoanProduct, adds working-capital delinquency block separate from standard delinquency, and conditionally renders Balances/Amortization Schedule tabs. Component methods (iconLoanStatusColor, loanStatusTooltip) now differentiate delinquency logic by product type using collectionData.delinquentDays for working-capital vs. inArrears for standard loans. |
Sequence Diagram
sequenceDiagram
participant Route as ActivatedRoute
participant Resolver as LoanAmortizationScheduleResolver
participant Service as LoansService
participant API as HTTP Backend
participant Component as LoanAmortizationScheduleTabComponent
participant Template as View/Template
Route->>Resolver: resolve(route)
Resolver->>Resolver: initialize(route)
Resolver->>Resolver: extract loanId from route params
Resolver->>Resolver: check if isWorkingCapital && numeric loanId
Resolver->>Service: getWorkingCapitalLoanAmortizationSchedule(loanId)
Service->>API: GET /working-capital-loans/{loanId}/amortization-schedule
API-->>Service: ProjectedAmortizationSchedule
Service-->>Resolver: Observable<ProjectedAmortizationSchedule>
Resolver-->>Route: Observable<ProjectedAmortizationSchedule>
Route->>Component: ActivatedRoute.data resolved
Component->>Component: amortizationSchedule = route.data.amortizationSchedule
Component->>Component: derive currencyCode from parent route data
Component-->>Template: render with amortizationSchedule & currencyCode
Template->>Template: display payments table & PDF export button
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
- openMF/web-app#3435: Introduces or refines the LoanBaseResolver that LoanAmortizationScheduleResolver extends.
- openMF/web-app#3257: Concurrent working-capital loan feature development; adds complementary working-capital endpoints, models, and UI infrastructure.
Suggested reviewers
- adamsaghy
- IOhacker
- shubhamkumar9199
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately summarizes the main change: enhancing Working Capital loan account details with new tabs and display features. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Tip
💬 Introducing Slack Agent: The best way for teams to turn conversations into code.
Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
- Generate code and open pull requests
- Plan features and break down work
- Investigate incidents and troubleshoot customer tickets together
- Automate recurring tasks and respond to alerts with triggers
- Summarize progress and report instantly
Built for teams:
- Shared memory across your entire org—no repeating context
- Per-thread sandboxes to safely plan and execute work
- Governance built-in—scoped access, auditability, and budget controls
One agent for your entire SDLC. Right inside Slack.
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 help to get the list of available commands and usage tips.
alberto-art3ch
force-pushed
the
WEB-657/working-capital-loan-account-details-enhanced
branch
from
77d572a to
79c5bce
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (4)
src/app/loans/loans.service.ts (1)
152-154: ⚡ Quick winAdd an explicit return type using the newly defined interface.
Since
ProjectedAmortizationScheduleis introduced in this same PR, there's no incremental cost to type this method correctly from the start.✏️ Proposed fix
+import { ProjectedAmortizationSchedule } from './models/working-capital/working-capital-loan-account.model'; - getWorkingCapitalLoanAmortizationSchedule(loanId: string) { - return this.http.get(`/working-capital-loans/${loanId}/amortization-schedule`); + getWorkingCapitalLoanAmortizationSchedule(loanId: string): Observable<ProjectedAmortizationSchedule> { + return this.http.get<ProjectedAmortizationSchedule>(`/working-capital-loans/${loanId}/amortization-schedule`); }Based on learnings: "avoid using
Observable<any>as a project-wide pattern for API responses… introduce specific interfaces/types for the response shapes and use proper typing instead ofany."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans.service.ts` around lines 152 - 154, The method getWorkingCapitalLoanAmortizationSchedule currently returns an untyped HTTP observable; update it to use the new ProjectedAmortizationSchedule interface by declaring an explicit return type (Observable<ProjectedAmortizationSchedule>) and call this.http.get with a generic type parameter (this.http.get<ProjectedAmortizationSchedule>(...)) so the response is properly typed; ensure Observable is imported from 'rxjs' and ProjectedAmortizationSchedule is imported from its module.src/app/loans/loans-routing.module.ts (1)
142-154: LGTM — new routes and resolver wiring look correct.
LoanBalancesTabComponentcorrectly relies on the parent resolver'sloanDetailsData.LoanAmortizationScheduleResolveris correctly added toproviders. The UI-level tab visibility is gated byloanProductService.isWorkingCapital, though note that the routes have nocanActivateguard, so a working-capital user who manually constructs a URL with a regularloanIdwould trigger the resolver against the wrong API endpoint. Consider adding a route guard if direct URL access is a concern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-routing.module.ts` around lines 142 - 154, Add a route guard to prevent working-capital users from directly hitting non-WC endpoints: implement or reuse a guard (e.g., LoanProductGuard or WorkingCapitalGuard) that checks loanProductService.isWorkingCapital for the current loanId and returns false for disallowed combinations, then attach it to the affected route definitions (the route objects for LoanBalancesTabComponent and LoanAmortizationScheduleTabComponent) via canActivate; ensure the guard short-circuits before the resolver (so LoanAmortizationScheduleResolver won’t run for disallowed users) and provide a redirect or error navigation when the guard blocks access.src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss (1)
30-38: Update.mat-header-cellselectors to.mat-mdc-header-cellfor Angular Material v20 (MDC).The project uses Angular Material v20 (confirmed in package.json:
@angular/material@^20.2.14), which removes non-MDC components. The legacy.mat-header-cellselector is no longer rendered on the DOM; use.mat-mdc-header-cellinstead for styling table header cells.✏️ Proposed fix
-.mat-header-cell { +.mat-mdc-header-cell { color: rgb(0 0 0 / 54%); font-size: 12px; font-weight: 500; } -:host-context(.dark-theme) .mat-header-cell { +:host-context(.dark-theme) .mat-mdc-header-cell { color: rgb(255 255 255 / 70%); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss` around lines 30 - 38, Update the CSS selectors for table header cells from the legacy .mat-header-cell to the MDC variant .mat-mdc-header-cell in this component's stylesheet; specifically replace both occurrences in the rules for .mat-header-cell and :host-context(.dark-theme) .mat-header-cell so the styles apply with Angular Material v20's MDC table header class (.mat-mdc-header-cell).src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts (1)
100-103: 💤 Low valueCommented-out call to
buildDisplayedColumns()leaves dynamic column logic unused.The
buildDisplayedColumns()method is defined (lines 116-125) but the call is commented out. This means optional columns likeforecastPaymentAmount,netAmortizationAmount, andincomeModificationdefined in the template will never render, even when data exists.Either enable the call or remove the dead code and template column definitions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts` around lines 100 - 103, The call to buildDisplayedColumns() is commented out after assigning this.amortizationSchedule in the subscribe callback, so the dynamic column logic in buildDisplayedColumns (which controls optional columns like forecastPaymentAmount, netAmortizationAmount, incomeModification) never runs; either re-enable the call by invoking this.buildDisplayedColumns() immediately after this.amortizationSchedule = data.amortizationSchedule in the subscribe callback of the observable, or if you intend not to support dynamic columns remove the buildDisplayedColumns() method and the corresponding optional column definitions from the template to avoid dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts`:
- Around line 29-37: The resolve method can throw if route.parent is null and
can return undefined for invalid loanId or non-working capital loans; update
resolve(ActivatedRouteSnapshot) to safely read loanId by checking route.paramMap
first then only accessing route.parent?.paramMap, validate and coerce loanId to
a number, and always return an Observable (e.g., of(null) or of([]) or a typed
empty response) when this.isWorkingCapital is false or loanId is invalid; ensure
the branch that calls
loansService.getWorkingCapitalLoanAmortizationSchedule(loanId) is only reached
for a valid numeric loanId and that resolve never returns undefined so
subscribers always receive a defined Observable.
In `@src/app/loans/loans-view/account-details/account-details.component.html`:
- Around line 25-28: The template accesses a deep property
loanDetails.product.amortizationType.value which can throw if product or
amortizationType is missing; update the binding to use optional chaining (e.g.,
loanDetails.product?.amortizationType?.value) and provide a safe fallback (empty
string or a translated "N/A") so the translateKey pipe and UI won't crash when
product or amortizationType is undefined; update the span that currently binds
{{ loanDetails.product.amortizationType.value | translateKey: 'catalogs' }}
accordingly.
- Around line 19-22: The Currency row currently accesses
loanDetails.currency.code directly and can throw if currency is missing; update
the template to guard this access by using optional chaining
(loanDetails.currency?.code) or an *ngIf that checks loanDetails.currency before
rendering; modify the span showing the value (the one using
loanDetails.currency.code) to use loanDetails.currency?.code so it safely
renders when currency is undefined.
- Around line 19-28: The template uses missing translation keys
'labels.inputs.Currency' and 'labels.inputs.Amortization Type' (referenced where
loanDetails.currency.code and loanDetails.product.amortizationType.value are
displayed and inside the loanProductService.isWorkingCapital conditional); run
the extraction tool (npm run translations:extract), merge/commit the generated
keys into all locale files under src/assets/translations/, add appropriate
default values for each locale, and verify the updated JSONs include both keys
so the translate pipe and translateKey pipe resolve at runtime.
In `@src/app/loans/loans-view/general-tab/general-tab.component.html`:
- Around line 294-311: The template adds three new i18n keys
(labels.inputs.Proposed Discount, labels.inputs.Approved Discount,
labels.inputs.Discount) in general-tab.component.html but the translation
catalogue wasn’t updated; run the extractor (npm run translations:extract),
verify the new keys are added to the source locale files, fill or mark
translations as needed, and commit the updated translation JSON/po files
alongside this change so locales don’t show raw keys.
- Line 297: Replace the use of the formatNumber pipe for discount fields with
the currency pipe to match other monetary fields: change occurrences like {{
loanDetails.discountProposed | formatNumber }} to use currency with the view's
currencyCode and narrow symbol formatting (currency: currencyCode :
'symbol-narrow' : '1.2-2'); apply the same replacement for the other
discount-related bindings mentioned (the occurrences around lines 303 and 309)
so all monetary fields (including discountProposed and the other discount
fields) render with the same currency symbol and precision.
In `@src/app/loans/loans-view/loans-view.component.html`:
- Around line 116-138: The template accesses
loanDetailsData?.delinquencyBucket.name without ensuring delinquencyBucket
exists, which can throw if it's null/undefined; update the
loans-view.component.html conditional/rendering so the delinquency bucket is
checked (either add && loanDetailsData?.delinquencyBucket to the outer `@if`
condition or use a safe navigation operator like
loanDetailsData?.delinquencyBucket?.name in the binding) and ensure the icon
binding (loanDelinquencyClassificationStyle) is only shown when
delinquencyBucket is present.
In `@src/app/loans/loans-view/loans-view.component.ts`:
- Around line 527-541: The tooltip logic in loanStatusTooltip() is inconsistent
with iconLoanStatusColor(): update loanStatusTooltip() to mirror the same
branching used in iconLoanStatusColor() by checking
loanProductService.isWorkingCapital and treating collectionData?.delinquentDays
> 0 as 'loanStatusType.activeOverdue' (in addition to the existing checks for
chargedOff, isContractTermination(this.loanSubStatus), and
loanDetailsData.inArrears) so that working-capital loans with delinquentDays > 0
return 'loanStatusType.activeOverdue' in the tooltip; reference the
loanStatusTooltip() and iconLoanStatusColor() functions and the fields
loanProductService.isWorkingCapital,
loanDetailsData.collectionData?.delinquentDays, loanDetailsData.inArrears, and
this.loanSubStatus when making the change.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html`:
- Line 36: Replace the hardcoded "Total" footer label in the
loan-amortization-schedule-tab template (the <td mat-footer-cell class="center"
*matFooterCellDef> element) with a translate-pipe expression (e.g. {{
'loanAmortization.total' | translate }}) and add the corresponding i18n key to
the translation JSONs; ensure the chosen key name (like loanAmortization.total
or LOANS.TOTAL) is used consistently in the template and translation files so
the footer renders the localized string.
- Around line 31-37: The template defines a paymentsLeft column but the
component's baseColumns array (in LoanAmortizationScheduleTabComponent) does not
include "paymentsLeft", so the column won't render; update the baseColumns array
to include the string "paymentsLeft" in the correct position (alongside the
other column keys used by the table) so the mat-table will display the
paymentsLeft column.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts`:
- Around line 88-93: The optionalColumns array contains a duplicate entry for
expectedAmortizationAmount which is already present in baseColumns; remove the {
field: 'expectedAmortizationAmount', column: 'expectedAmortizationAmount' } item
from optionalColumns so buildDisplayedColumns()/displayedColumns won't include
the column twice, and run unit/UI checks around buildDisplayedColumns in
loan-amortization-schedule-tab.component to verify no other duplicates remain.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.html`:
- Around line 9-53: The template accesses loanBalances properties directly which
will throw when loanBalances is null; wrap the whole balances block with an
Angular null-guard (for example add *ngIf="loanBalances" on the outer <div
class="responsive-column"> or surrounding container) or change each access to
use the safe navigation operator (loanBalances?.principalOutstanding, etc.) and
adjust the overpayment check to *ngIf="loanBalances?.overpaymentAmount > 0" so
all property reads (principalOutstanding, totalPaidPrincipal, totalPayment,
overpaymentAmount, realizedIncome, unrealizedIncome) are protected.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts`:
- Line 38: Remove the debug console.log call that prints sensitive
WorkingCapitalBalances data; locate the console.log(this.loanBalances) call
inside the LoanBalancesTabComponent (where loanBalances is used/initialized,
e.g., ngOnInit or the data-fetch callback) and delete that statement so no
production logging of principal/income/other balance fields remains.
---
Nitpick comments:
In `@src/app/loans/loans-routing.module.ts`:
- Around line 142-154: Add a route guard to prevent working-capital users from
directly hitting non-WC endpoints: implement or reuse a guard (e.g.,
LoanProductGuard or WorkingCapitalGuard) that checks
loanProductService.isWorkingCapital for the current loanId and returns false for
disallowed combinations, then attach it to the affected route definitions (the
route objects for LoanBalancesTabComponent and
LoanAmortizationScheduleTabComponent) via canActivate; ensure the guard
short-circuits before the resolver (so LoanAmortizationScheduleResolver won’t
run for disallowed users) and provide a redirect or error navigation when the
guard blocks access.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss`:
- Around line 30-38: Update the CSS selectors for table header cells from the
legacy .mat-header-cell to the MDC variant .mat-mdc-header-cell in this
component's stylesheet; specifically replace both occurrences in the rules for
.mat-header-cell and :host-context(.dark-theme) .mat-header-cell so the styles
apply with Angular Material v20's MDC table header class (.mat-mdc-header-cell).
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts`:
- Around line 100-103: The call to buildDisplayedColumns() is commented out
after assigning this.amortizationSchedule in the subscribe callback, so the
dynamic column logic in buildDisplayedColumns (which controls optional columns
like forecastPaymentAmount, netAmortizationAmount, incomeModification) never
runs; either re-enable the call by invoking this.buildDisplayedColumns()
immediately after this.amortizationSchedule = data.amortizationSchedule in the
subscribe callback of the observable, or if you intend not to support dynamic
columns remove the buildDisplayedColumns() method and the corresponding optional
column definitions from the template to avoid dead code.
In `@src/app/loans/loans.service.ts`:
- Around line 152-154: The method getWorkingCapitalLoanAmortizationSchedule
currently returns an untyped HTTP observable; update it to use the new
ProjectedAmortizationSchedule interface by declaring an explicit return type
(Observable<ProjectedAmortizationSchedule>) and call this.http.get with a
generic type parameter (this.http.get<ProjectedAmortizationSchedule>(...)) so
the response is properly typed; ensure Observable is imported from 'rxjs' and
ProjectedAmortizationSchedule is imported from its module.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f88bad77-6f32-4961-9a9b-14265429d157
📒 Files selected for processing (27)
src/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.htmlsrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scsssrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.tssrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.htmlsrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.scsssrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.tssrc/app/loans/loans.service.tssrc/app/loans/models/working-capital/working-capital-loan-account.model.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
| <div class="flex-fill layout-row"> | ||
| <span class="flex-50">{{ 'labels.inputs.Currency' | translate }}</span> | ||
| <span class="flex-50">{{ loanDetails.currency.code }}</span> | ||
| </div> | ||
|
|
||
| @if (loanProductService.isWorkingCapital) { | ||
| <div class="flex-fill layout-row"> | ||
| <span class="flex-50"> {{ 'labels.inputs.Amortization Type' | translate }} </span> | ||
| <span class="flex-50"> {{ loanDetails.product.amortizationType.value | translateKey: 'catalogs' }} </span> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search translation asset files for the two new keys.
fd -e json . src/assets/i18n --exec grep -l "labels.inputs" {} \; | head -5 | xargs -I{} grep -n '"Currency"\|"Amortization Type"' {}Repository: openMF/web-app
Length of output: 161
🏁 Script executed:
#!/bin/bash
# Find translation-related files and directories
echo "=== Looking for translation files ==="
find . -type f -name "*translation*" -o -name "*i18n*" -o -name "*.i18n.*" 2>/dev/null | head -20
echo -e "\n=== Checking for language/locale directories ==="
find . -type d -name "i18n" -o -name "locale*" -o -name "translations" -o -name "lang*" 2>/dev/null | head -10
echo -e "\n=== Checking for JSON files in common asset paths ==="
fd -t f -e json . assets 2>/dev/null | head -20Repository: openMF/web-app
Length of output: 596
🏁 Script executed:
#!/bin/bash
# Explore the translations directory structure
echo "=== Translation directory contents ==="
ls -la src/assets/translations/
echo -e "\n=== Looking for the specific i18n keys ==="
rg "labels\.inputs\.Currency|labels\.inputs\.Amortization Type" src/assets/translations/ -l
echo -e "\n=== Search for labels.inputs. pattern ==="
rg "labels\.inputs\." src/assets/translations/ --no-filename | head -30Repository: openMF/web-app
Length of output: 1108
🏁 Script executed:
#!/bin/bash
# Check if the keys exist in any form or if they're simply missing
echo "=== Searching for 'Currency' key in translation files ==="
rg "Currency" src/assets/translations/ --max-count 5
echo -e "\n=== Searching for 'Amortization' in translation files ==="
rg "Amortization" src/assets/translations/ --max-count 5
echo -e "\n=== Checking the component file to verify the keys being used ==="
cat -n src/app/loans/loans-view/account-details/account-details.component.html | sed -n '19,28p'
echo -e "\n=== Checking if there's a translation extraction configuration ==="
find . -name "*.json" -o -name "*.config.*" | xargs grep -l "translations:extract" 2>/dev/null | head -5Repository: openMF/web-app
Length of output: 12611
🏁 Script executed:
#!/bin/bash
# Check the translation extraction script in package.json
grep -A 5 "translations:extract" package.json
echo -e "\n=== Verify that labels.inputs keys are missing ==="
rg "labels\.inputs\." src/assets/translations/ | wc -l
echo -e "\n=== Show existing structure of labels.inputs in code ==="
rg "labels\.inputs\." src/ --max-count 10Repository: openMF/web-app
Length of output: 50371
🏁 Script executed:
#!/bin/bash
# Double-check the exact keys in the component and search for them in translation files with proper escaping
echo "=== Verify exact keys in component ==="
grep -n "labels\.inputs\." src/app/loans/loans-view/account-details/account-details.component.html
echo -e "\n=== Final verification: Search for exact keys in ALL translation files ==="
rg "labels\.inputs\.Currency|labels\.inputs\.Amortization" src/assets/translations/ | wc -lRepository: openMF/web-app
Length of output: 5733
The translation keys labels.inputs.Currency and labels.inputs.Amortization Type are missing from all translation files.
These keys are used in the component (lines 20 and 26) but do not exist in any of the 13 supported locale files in src/assets/translations/. Run npm run translations:extract to generate the missing keys in all locale JSON files, per the coding guidelines for i18n strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/account-details/account-details.component.html`
around lines 19 - 28, The template uses missing translation keys
'labels.inputs.Currency' and 'labels.inputs.Amortization Type' (referenced where
loanDetails.currency.code and loanDetails.product.amortizationType.value are
displayed and inside the loanProductService.isWorkingCapital conditional); run
the extraction tool (npm run translations:extract), merge/commit the generated
keys into all locale files under src/assets/translations/, add appropriate
default values for each locale, and verify the updated JSONs include both keys
so the translate pipe and translateKey pipe resolve at runtime.
| <div class="flex-fill layout-row"> | ||
| <span class="flex-50"> {{ 'labels.inputs.Amortization Type' | translate }} </span> | ||
| <span class="flex-50"> {{ loanDetails.product.amortizationType.value | translateKey: 'catalogs' }} </span> | ||
| </div> |
There was a problem hiding this comment.
Add optional chaining on the product.amortizationType chain to prevent a runtime crash.
loanDetails.product.amortizationType.value introduces a 3-level deep access through a product sub-object — a new pattern not present elsewhere in this template. If either product or amortizationType is absent in the working capital API response, this throws without any UI feedback.
🛡️ Proposed fix
- <span class="flex-50"> {{ loanDetails.product.amortizationType.value | translateKey: 'catalogs' }} </span>
+ <span class="flex-50"> {{ loanDetails.product?.amortizationType?.value | translateKey: 'catalogs' }} </span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="flex-fill layout-row"> | |
| <span class="flex-50"> {{ 'labels.inputs.Amortization Type' | translate }} </span> | |
| <span class="flex-50"> {{ loanDetails.product.amortizationType.value | translateKey: 'catalogs' }} </span> | |
| </div> | |
| <div class="flex-fill layout-row"> | |
| <span class="flex-50"> {{ 'labels.inputs.Amortization Type' | translate }} </span> | |
| <span class="flex-50"> {{ loanDetails.product?.amortizationType?.value | translateKey: 'catalogs' }} </span> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/account-details/account-details.component.html`
around lines 25 - 28, The template accesses a deep property
loanDetails.product.amortizationType.value which can throw if product or
amortizationType is missing; update the binding to use optional chaining (e.g.,
loanDetails.product?.amortizationType?.value) and provide a safe fallback (empty
string or a translated "N/A") so the translateKey pipe and UI won't crash when
product or amortizationType is undefined; update the span that currently binds
{{ loanDetails.product.amortizationType.value | translateKey: 'catalogs' }}
accordingly.
| @if (loanDetails.discountProposed > 0) { | ||
| <div class="flex-contents"> | ||
| <span class="flex-50">{{ 'labels.inputs.Proposed Discount' | translate }}:</span> | ||
| <span class="flex-50">{{ loanDetails.discountProposed | formatNumber }}</span> | ||
| </div> | ||
| } | ||
| @if (loanDetails.discountApproved > 0) { | ||
| <div class="flex-contents"> | ||
| <span class="flex-50">{{ 'labels.inputs.Approved Discount' | translate }}:</span> | ||
| <span class="flex-50">{{ loanDetails.discountApproved | formatNumber }}</span> | ||
| </div> | ||
| } | ||
| @if (loanDetails.discount > 0) { | ||
| <div class="flex-contents"> | ||
| <span class="flex-50">{{ 'labels.inputs.Discount' | translate }}:</span> | ||
| <span class="flex-50">{{ loanDetails.discount | formatNumber }}</span> | ||
| </div> | ||
| } |
There was a problem hiding this comment.
Run npm run translations:extract for the three new i18n keys.
Three new translation keys are introduced (labels.inputs.Proposed Discount, labels.inputs.Approved Discount, labels.inputs.Discount) but are not yet registered in the translation source files. Without running the extraction step, these keys may render as raw strings in locales that don't define them and the source translation catalogue becomes stale.
As per coding guidelines: "Run npm run translations:extract to extract i18n variables whenever new strings are added to code."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/general-tab/general-tab.component.html` around lines
294 - 311, The template adds three new i18n keys (labels.inputs.Proposed
Discount, labels.inputs.Approved Discount, labels.inputs.Discount) in
general-tab.component.html but the translation catalogue wasn’t updated; run the
extractor (npm run translations:extract), verify the new keys are added to the
source locale files, fill or mark translations as needed, and commit the updated
translation JSON/po files alongside this change so locales don’t show raw keys.
| <ng-container matColumnDef="paymentsLeft"> | ||
| <th mat-header-cell class="center mat-header-cell" *matHeaderCellDef> | ||
| {{ 'labels.inputs.Payments Left' | translate }} | ||
| </th> | ||
| <td mat-cell class="center" *matCellDef="let item">{{ item.paymentsLeft }}</td> | ||
| <td mat-footer-cell class="center" *matFooterCellDef><b>Total</b></td> | ||
| </ng-container> |
There was a problem hiding this comment.
Column paymentsLeft is defined in template but missing from displayedColumns.
The paymentsLeft column definition exists in the template, but in the component's baseColumns array (line 77-86 of the .ts file), paymentsLeft is not included. This column will not be rendered in the table.
Add paymentsLeft to the baseColumns array in the component:
private readonly baseColumns: string[] = [
'number',
'paymentDate',
+ 'paymentsLeft',
'expectedPaymentAmount',
'discountFactor',
'npvValue',
'deferredBalance',
'expectedAmortizationAmount',
'balance'
];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html`
around lines 31 - 37, The template defines a paymentsLeft column but the
component's baseColumns array (in LoanAmortizationScheduleTabComponent) does not
include "paymentsLeft", so the column won't render; update the baseColumns array
to include the string "paymentsLeft" in the correct position (alongside the
other column keys used by the table) so the mat-table will display the
paymentsLeft column.
79c5bce to
f69238e
Compare
alberto-art3ch
force-pushed
the
WEB-657/working-capital-loan-account-details-enhanced
branch
from
f69238e to
58b7b83
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/loans/loans-view/loans-view.component.ts (1)
523-562:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
chargedOffhandling is asymmetric between icon color and tooltip.In
iconLoanStatusColor()(lines 527–541),chargedOffis only honored under theisLoanProductbranch. InloanStatusTooltip()(lines 549–551),chargedOffis checked unconditionally, before the working-capital branch. Net effect: a working-capital loan withchargedOff === truewould render the regular status color fromstatus?.codebut show a"Chargeoff"tooltip — a confusing mismatch.If chargeoff is not applicable to working capital, gate the tooltip the same way; otherwise also include the chargeoff color path under the working-capital branch.
🛡️ Suggested fix to align both methods
loanStatusTooltip() { if (!this.loanDetailsData) { return ''; } - if (this.loanDetailsData.chargedOff) { - return 'Chargeoff'; - } - if (this.loanProductService.isWorkingCapital) { + if (this.loanProductService.isWorkingCapital) { if (this.loanDetailsData.collectionData?.delinquentDays > 0) { return 'activeOverdue'; } } else { + if (this.loanDetailsData.chargedOff) { + return 'Chargeoff'; + } if (this.loanDetailsData.inArrears) { return 'activeOverdue'; } } return this.loanDetailsData.status?.code; }🤖 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/app/loans/loans-view/loans-view.component.ts` around lines 523 - 562, The chargedOff case is handled under the isLoanProduct branch in iconLoanStatusColor but is checked unconditionally in loanStatusTooltip, causing a mismatch; update loanStatusTooltip to mirror iconLoanStatusColor by only returning 'Chargeoff' when this.loanProductService.isLoanProduct && this.loanDetailsData.chargedOff (or conversely, if chargeoff should apply to working-capital loans, add the same chargedOff branch into iconLoanStatusColor under isWorkingCapital) so both iconLoanStatusColor and loanStatusTooltip use the same conditional logic on this.loanProductService and this.loanDetailsData.chargedOff.
♻️ Duplicate comments (1)
src/app/loans/loans-view/account-details/account-details.component.html (1)
27-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOptional chaining stops one level too short.
loanDetails.product.amortizationType?.valueonly guardsamortizationTypebut still throws ifproductitself isnull/undefined. Apply the safe-navigation operator at theproductlevel too.🛡️ Suggested fix
- <span class="flex-50"> {{ loanDetails.product.amortizationType?.value | translateKey: 'catalogs' }} </span> + <span class="flex-50"> {{ loanDetails.product?.amortizationType?.value | translateKey: 'catalogs' }} </span>🤖 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/app/loans/loans-view/account-details/account-details.component.html` at line 27, The template uses the expression starting with loanDetails.product.amortizationType?.value which only guards amortizationType and will throw if product is null/undefined; update the span in account-details.component.html to apply the safe-navigation operator on product as well so the binding first checks loanDetails.product exists before accessing amortizationType and its value, preserving the translateKey pipe and overall formatting.
🧹 Nitpick comments (9)
src/app/loans/loans-routing.module.ts (1)
142-154: ⚖️ Poor tradeoffConsider guarding working-capital-only routes.
The new
balancesandamortization-schedulepaths are reachable for any loanId regardless of product type — the only protection is the@if (loanProductService.isWorkingCapital)gating in the parent template, which doesn't prevent direct URL navigation. Loading the balances tab for a regular loan will quietly readloanDetailsData.balance, which isundefined, and render an empty card. Adding aCanActivateguard (or redirecting togeneralwhen the resolvedloanDetailsDatais not a working-capital product) makes the contract explicit and avoids confusing empty UIs.🤖 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/app/loans/loans-routing.module.ts` around lines 142 - 154, Add a CanActivate guard to the 'balances' and 'amortization-schedule' routes that verifies the loan is a working-capital product before activating; implement the guard to use the existing loanDetailsData (or call loanProductService.isWorkingCapital) and if not working-capital either redirect to the 'general' route or return false. Update the route definitions that reference LoanBalancesTabComponent and LoanAmortizationScheduleTabComponent to include canActivate: [WorkingCapitalGuard] (or similar) and ensure the guard reads the resolved amortizationSchedule/loanDetailsData where applicable so direct URL navigation cannot show empty UIs.src/app/loans/models/working-capital/working-capital-loan-account.model.ts (1)
22-22: 💤 Low valueConsider renaming
Paymentto a more specific name.
Paymentis exported with a very generic name that is likely to clash with other concepts (transactions, savings payments, etc.) and obscures its working-capital scope at call sites. Renaming toWorkingCapitalPayment(orAmortizationPayment) keeps the module surface unambiguous and avoids future import conflicts.🤖 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/app/loans/models/working-capital/working-capital-loan-account.model.ts` at line 22, Rename the exported interface Payment in working-capital-loan-account.model.ts to a more specific name (e.g., WorkingCapitalPayment or AmortizationPayment) and update all local references and the export to use that new identifier; then find and update any external imports/usages across the codebase to import the new interface name instead of Payment to avoid naming collisions (reference: the interface named Payment in working-capital-loan-account.model.ts).src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.scss (1)
9-15: 💤 Low valueConsider scoping the bare
tableandspanselectors.These rules style every
<table>and<span>rendered by this component, including nested ones in future additions. Even with Angular's emulated encapsulation isolating the styles to the component, the broad selectors can produce surprising visual side effects when the template grows. Consider scoping them, e.g. under.containeror via a dedicated class such as.balances-row.🤖 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/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.scss` around lines 9 - 15, The global table and span selectors in loan-balances-tab.component.scss are too broad; update them to be scoped under a component-specific wrapper or class (e.g., .balances-container or .balances-row) so only the loan-balances-tab template elements are affected—replace the bare selectors "table" and "span" with scoped selectors like ".balances-container table" and ".balances-container .balances-row span" and ensure the template/root element in loan-balances-tab.component.html uses that wrapper class.src/app/loans/loans-view/loans-view.component.html (1)
327-352: ⚖️ Poor tradeoffAvoid duplicating per-tab
[queryParams]/routerLinkActivemarkup.The two new working-capital tabs (and the repayment-schedule tab above) repeat the exact same
[queryParams]="{ productType: loanProductService.productType.value }"plusrouterLinkActiveboilerplate already used by every sibling tab. This file already has 20+ near-identical<a mat-tab-link>blocks; consider driving the tab list from a small array of{ path, labelKey, isVisible }objects rendered in a single@forto cut the duplication and reduce the chance of regressions when adding tabs (e.g., a newrouterLinkActivereference name being copy-pasted, like#generalreused on lines 270 and 307 — already an existing template variable shadow).🤖 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/app/loans/loans-view/loans-view.component.html` around lines 327 - 352, Create a small tabs array in the component (e.g. a property named tabs: Array<{path:string,labelKey:string,visible:()=>boolean}>) that lists each tab's route path and translation key and a visible predicate (use loanProductService.isWorkingCapital for the two new tabs), then replace the repeated <a mat-tab-link> blocks with a single *ngFor="let tab of tabs" that renders each anchor using [routerLink]="[tab.path]" [queryParams]="{ productType: loanProductService.productType.value }" routerLinkActive `#rla`="routerLinkActive" [active]="rla.isActive" and {{ tab.labelKey | translate }}; ensure the tabs array only includes items where tab.visible() is true and remove the duplicated per-tab boilerplate blocks (references: tabs property, loanProductService.productType.value, loanProductService.isWorkingCapital).src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts (1)
33-39: ⚡ Quick winUse
takeUntilDestroyed()to clean up the subscription and prevent memory leaks.The subscription to
route.parent.datain the constructor lacks a cleanup mechanism. With Angular 20, use thetakeUntilDestroyed()operator (imported from@angular/core/rxjs-interop) which runs within the injection context and automatically unsubscribes when the component is destroyed.♻️ Suggested refactor
import { CurrencyPipe } from '@angular/common'; import { Component, inject } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { FormatNumberPipe } from '@pipes/format-number.pipe'; import { WorkingCapitalBalances } from 'app/loans/models/working-capital/working-capital-loan-account.model'; import { LoanProductBaseComponent } from 'app/products/loan-products/common/loan-product-base.component'; import { STANDALONE_SHARED_IMPORTS } from 'app/standalone-shared.module'; ... constructor() { super(); - this.route.parent.data.subscribe((data: { loanDetailsData: any }) => { + this.route.parent.data.pipe(takeUntilDestroyed()).subscribe((data: { loanDetailsData: any }) => { this.currencyCode = data.loanDetailsData.currency.code; this.loanBalances = data.loanDetailsData.balance; }); }🤖 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/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts` around lines 33 - 39, The constructor currently subscribes to route.parent.data without cleanup; update the subscription in the constructor to pipe through takeUntilDestroyed() (imported from `@angular/core/rxjs-interop`) before subscribing so the observable automatically unsubscribes on component destroy; keep assignment of this.currencyCode and this.loanBalances inside the subscription callback (referencing the existing constructor, route.parent.data, currencyCode, and loanBalances).src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html (1)
17-21: ⚡ Quick winRemove the manually added
mat-header-cellclass from all<th>elements.
mat-header-cellis redundant on<th mat-header-cell ...>— Angular Material's directive already sets this host class. This manual addition exists only to support the legacy SCSS selector (see related comment on the.scssfile). Once the SCSS is updated to target.mat-mdc-header-cell, these manual class additions should be removed.♻️ Proposed fix (representative example; apply to all header cells)
- <th mat-header-cell class="center mat-header-cell" *matHeaderCellDef>#</th> + <th mat-header-cell class="center" *matHeaderCellDef>#</th>Also applies to: 23-29, 31-39
🤖 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/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html` around lines 17 - 21, Several <th> elements (e.g., the <th mat-header-cell ...> in the "number" column) include a redundant manual "mat-header-cell" class; remove the manually added mat-header-cell class from all <th> header elements (for example those in the number column and the other header cells referenced in the diff) so the Angular Material directive alone provides the host class, and ensure the SCSS is updated to target the new .mat-mdc-header-cell selector instead of relying on the manual class.src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss (2)
30-38: ⚡ Quick winReplace legacy
.mat-header-cellselector with the MDC class.mat-mdc-header-cell.In Angular Material v20 (MDC-based), the host class rendered by
<th mat-header-cell>ismat-mdc-header-cell, notmat-header-cell. The styles here only work because the template manually addsclass="... mat-header-cell"as a workaround—both the manual class and the selector should be updated to the MDC equivalent. Additionally, the explicit12pxvalue should use an SCSS variable fromsrc/theme/mifosx-theme.scssrather than a hardcoded pixel value.♻️ Proposed fix
-.mat-header-cell { +.mat-mdc-header-cell { color: rgb(0 0 0 / 54%); - font-size: 12px; + font-size: $font-size-caption; /* or appropriate SCSS variable */ font-weight: 500; } -:host-context(.dark-theme) .mat-header-cell { +:host-context(.dark-theme) .mat-mdc-header-cell { color: rgb(255 255 255 / 70%); }Based on learnings: "In Angular Material v20+ (MDC-based), prefer styling MDC DOM classes (e.g.,
.mat-mdc-list-item) in SCSS instead of legacy selectors (e.g.,.mat-list-item)."As per coding guidelines: "Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values."🤖 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/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss` around lines 30 - 38, The styles use the legacy selector `.mat-header-cell` and a hardcoded `font-size: 12px`; change the selector to the MDC class `.mat-mdc-header-cell` in this SCSS (and update any template usage that manually adds `class="... mat-header-cell"` to `mat-mdc-header-cell`) and update the dark-theme rule `:host-context(.dark-theme) .mat-header-cell` accordingly; replace the hardcoded `12px` with the appropriate SCSS variable from your theme (import/use the variable from src/theme/mifosx-theme.scss) so the font-size follows the shared theme token.
21-24: 💤 Low valueMerge the two
.container/div.containerrule blocks into one.The container is styled across two separate rule blocks with no functional reason to split them. Merging removes duplication.
♻️ Proposed fix
-.container { - padding-bottom: 2%; - width: 98%; -} ... -div.container { +.container { overflow: auto; max-width: 100%; + padding-bottom: 2%; + width: 98%; }Also applies to: 40-43
🤖 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/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss` around lines 21 - 24, Merge the duplicate CSS rule blocks for .container (and div.container) in loan-amortization-schedule-tab.component.scss into a single rule: locate both occurrences of the .container selector (the blocks at lines referenced in the review) and combine their properties (e.g., padding-bottom: 2%; width: 98%) into one .container { ... } block, then remove the redundant/empty duplicate to avoid duplication and ensure all container styles are sourced from that single rule.src/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts (1)
25-28: 💤 Low valueStale JSDoc: update the comment to describe this resolver accurately.
♻️ Proposed fix
- /** - * Returns the Loans with Association data. - * `@returns` {Observable<any>} - */ + /** + * Returns the working capital loan amortization schedule. + * `@returns` {Observable<ProjectedAmortizationSchedule> | null} + */🤖 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/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts` around lines 25 - 28, The JSDoc above the resolver is stale; update the comment to accurately describe the resolver's behavior by replacing the generic "Returns the Loans with Association data" and wrong return type with a concise description of what LoanAmortizationScheduleResolver (or the resolver function/method defined in loan-amortization-schedule.resolver.ts) actually resolves and the correct return type (e.g., "Returns the amortization schedule for a loan as Observable<LoanAmortizationSchedule>" or similar), and adjust the `@returns` tag to match the real return type and shape used by the resolver.
🤖 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/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts`:
- Around line 29-37: The resolve method currently allows loanId === null to pass
the numeric coercion guard; change the guard to explicitly check that loanId is
a non-empty string (e.g. loanId !== null && loanId.trim() !== '') before calling
this.loansService.getWorkingCapitalLoanAmortizationSchedule(loanId), keeping the
existing initialize(route) and isWorkingCapital check; also tighten the resolver
return type from Observable<any> | null to
Observable<ProjectedAmortizationSchedule> | null and update imports/types to use
the ProjectedAmortizationSchedule interface.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html`:
- Around line 65-71: The template defines optional columns
(forecastPaymentAmount, netAmortizationAmount, incomeModification) that never
render because buildDisplayedColumns() is commented out and displayedColumns
stays as baseColumns; either remove these dead <ng-container> blocks from
loan-amortization-schedule-tab.component.html or restore and call
buildDisplayedColumns() in loan-amortization-schedule-tab.component.ts so
displayedColumns includes those optional keys; search for
buildDisplayedColumns(), displayedColumns, and baseColumns to decide whether to
re-enable dynamic column building or delete the unused column templates.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts`:
- Line 101: The call to buildDisplayedColumns() is commented out so
displayedColumns never includes the optional columns (forecastPaymentAmount,
netAmortizationAmount, incomeModification) and the ng-container blocks become
dead code; either restore the call to buildDisplayedColumns() (e.g., invoke
buildDisplayedColumns() from the component lifecycle where displayedColumns is
initialized so that displayedColumns merges baseColumns with the optional
columns when API data is present) or else remove the buildDisplayedColumns()
method and the template ng-container blocks for those optional columns and any
related references to displayedColumns/baseColumns to eliminate dead code.
- Line 40: Upgrade the jspdf-autotable dependency to ^5.0.7 (or later) in
package.json to include jsPDF 4.x in its peer deps, and change the default
import statement currently using "import autoTable from 'jspdf-autotable';" to
the v5 recommended named import (use the named import symbol autoTable from
'jspdf-autotable') so the plugin is loaded correctly with jsPDF 4.2.1; update
any related imports/usages in loan-amortization-schedule-tab.component.ts that
reference autoTable to match the named-import pattern.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.html`:
- Around line 44-52: The Realized and Unrealized Income rows use the
formatNumber pipe while other monetary fields use the currency pipe with
currencyCode, causing inconsistent UI; if these income fields are monetary
values on WorkingCapitalBalances, change the bindings in
loan-balances-tab.component.html to use the currency pipe with the same
currencyCode as the other payment rows (replace usages of "{{
loanBalances.realizedIncome | formatNumber }}" and "{{
loanBalances.unrealizedIncome | formatNumber }}" to use the currency pipe),
ensuring the display matches the Principal/Payment fields and preserves the
existing currencyCode variable.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts`:
- Line 36: The direct access to data.loanDetailsData.currency.code can throw if
currency is missing; update the assignment that sets currencyCode in the
LoanBalancesTabComponent to use safe navigation and a null/default fallback
(e.g., read currency via optional chaining and assign null when absent) so that
currencyCode becomes null instead of throwing when data.loanDetailsData.currency
is undefined; locate the assignment to this.currencyCode in the
LoanBalancesTabComponent and replace it with a safe-access pattern (use
currency?.code with a null/default fallback).
In `@src/app/loans/models/working-capital/working-capital-loan-account.model.ts`:
- Line 15: Update the type annotations for the date fields in the
WorkingCapitalLoanAccount model: change expectedDisbursementDate and paymentDate
from Date to string | Date (or string if you prefer strictly matching raw JSON)
so the TypeScript types match HttpClient JSON responses and the existing
dateUtils.parseDate/formatDate usage; locate these fields in
working-capital-loan-account.model.ts and adjust their declarations accordingly.
---
Outside diff comments:
In `@src/app/loans/loans-view/loans-view.component.ts`:
- Around line 523-562: The chargedOff case is handled under the isLoanProduct
branch in iconLoanStatusColor but is checked unconditionally in
loanStatusTooltip, causing a mismatch; update loanStatusTooltip to mirror
iconLoanStatusColor by only returning 'Chargeoff' when
this.loanProductService.isLoanProduct && this.loanDetailsData.chargedOff (or
conversely, if chargeoff should apply to working-capital loans, add the same
chargedOff branch into iconLoanStatusColor under isWorkingCapital) so both
iconLoanStatusColor and loanStatusTooltip use the same conditional logic on
this.loanProductService and this.loanDetailsData.chargedOff.
---
Duplicate comments:
In `@src/app/loans/loans-view/account-details/account-details.component.html`:
- Line 27: The template uses the expression starting with
loanDetails.product.amortizationType?.value which only guards amortizationType
and will throw if product is null/undefined; update the span in
account-details.component.html to apply the safe-navigation operator on product
as well so the binding first checks loanDetails.product exists before accessing
amortizationType and its value, preserving the translateKey pipe and overall
formatting.
---
Nitpick comments:
In
`@src/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts`:
- Around line 25-28: The JSDoc above the resolver is stale; update the comment
to accurately describe the resolver's behavior by replacing the generic "Returns
the Loans with Association data" and wrong return type with a concise
description of what LoanAmortizationScheduleResolver (or the resolver
function/method defined in loan-amortization-schedule.resolver.ts) actually
resolves and the correct return type (e.g., "Returns the amortization schedule
for a loan as Observable<LoanAmortizationSchedule>" or similar), and adjust the
`@returns` tag to match the real return type and shape used by the resolver.
In `@src/app/loans/loans-routing.module.ts`:
- Around line 142-154: Add a CanActivate guard to the 'balances' and
'amortization-schedule' routes that verifies the loan is a working-capital
product before activating; implement the guard to use the existing
loanDetailsData (or call loanProductService.isWorkingCapital) and if not
working-capital either redirect to the 'general' route or return false. Update
the route definitions that reference LoanBalancesTabComponent and
LoanAmortizationScheduleTabComponent to include canActivate:
[WorkingCapitalGuard] (or similar) and ensure the guard reads the resolved
amortizationSchedule/loanDetailsData where applicable so direct URL navigation
cannot show empty UIs.
In `@src/app/loans/loans-view/loans-view.component.html`:
- Around line 327-352: Create a small tabs array in the component (e.g. a
property named tabs: Array<{path:string,labelKey:string,visible:()=>boolean}>)
that lists each tab's route path and translation key and a visible predicate
(use loanProductService.isWorkingCapital for the two new tabs), then replace the
repeated <a mat-tab-link> blocks with a single *ngFor="let tab of tabs" that
renders each anchor using [routerLink]="[tab.path]" [queryParams]="{
productType: loanProductService.productType.value }" routerLinkActive
`#rla`="routerLinkActive" [active]="rla.isActive" and {{ tab.labelKey | translate
}}; ensure the tabs array only includes items where tab.visible() is true and
remove the duplicated per-tab boilerplate blocks (references: tabs property,
loanProductService.productType.value, loanProductService.isWorkingCapital).
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html`:
- Around line 17-21: Several <th> elements (e.g., the <th mat-header-cell ...>
in the "number" column) include a redundant manual "mat-header-cell" class;
remove the manually added mat-header-cell class from all <th> header elements
(for example those in the number column and the other header cells referenced in
the diff) so the Angular Material directive alone provides the host class, and
ensure the SCSS is updated to target the new .mat-mdc-header-cell selector
instead of relying on the manual class.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scss`:
- Around line 30-38: The styles use the legacy selector `.mat-header-cell` and a
hardcoded `font-size: 12px`; change the selector to the MDC class
`.mat-mdc-header-cell` in this SCSS (and update any template usage that manually
adds `class="... mat-header-cell"` to `mat-mdc-header-cell`) and update the
dark-theme rule `:host-context(.dark-theme) .mat-header-cell` accordingly;
replace the hardcoded `12px` with the appropriate SCSS variable from your theme
(import/use the variable from src/theme/mifosx-theme.scss) so the font-size
follows the shared theme token.
- Around line 21-24: Merge the duplicate CSS rule blocks for .container (and
div.container) in loan-amortization-schedule-tab.component.scss into a single
rule: locate both occurrences of the .container selector (the blocks at lines
referenced in the review) and combine their properties (e.g., padding-bottom:
2%; width: 98%) into one .container { ... } block, then remove the
redundant/empty duplicate to avoid duplication and ensure all container styles
are sourced from that single rule.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.scss`:
- Around line 9-15: The global table and span selectors in
loan-balances-tab.component.scss are too broad; update them to be scoped under a
component-specific wrapper or class (e.g., .balances-container or .balances-row)
so only the loan-balances-tab template elements are affected—replace the bare
selectors "table" and "span" with scoped selectors like ".balances-container
table" and ".balances-container .balances-row span" and ensure the template/root
element in loan-balances-tab.component.html uses that wrapper class.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts`:
- Around line 33-39: The constructor currently subscribes to route.parent.data
without cleanup; update the subscription in the constructor to pipe through
takeUntilDestroyed() (imported from `@angular/core/rxjs-interop`) before
subscribing so the observable automatically unsubscribes on component destroy;
keep assignment of this.currencyCode and this.loanBalances inside the
subscription callback (referencing the existing constructor, route.parent.data,
currencyCode, and loanBalances).
In `@src/app/loans/models/working-capital/working-capital-loan-account.model.ts`:
- Line 22: Rename the exported interface Payment in
working-capital-loan-account.model.ts to a more specific name (e.g.,
WorkingCapitalPayment or AmortizationPayment) and update all local references
and the export to use that new identifier; then find and update any external
imports/usages across the codebase to import the new interface name instead of
Payment to avoid naming collisions (reference: the interface named Payment in
working-capital-loan-account.model.ts).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea42cadb-98cd-44de-afee-2478f7612a31
📒 Files selected for processing (26)
src/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.htmlsrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.scsssrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.tssrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.htmlsrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.scsssrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.tssrc/app/loans/loans.service.tssrc/app/loans/models/working-capital/working-capital-loan-account.model.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/loans/loans.service.ts
| resolve(route: ActivatedRouteSnapshot): Observable<any> | null { | ||
| this.initialize(route); | ||
| const loanId = route.paramMap.get('loanId') || route.parent?.paramMap.get('loanId'); | ||
| if (!isNaN(+loanId)) { | ||
| if (this.isWorkingCapital) { | ||
| return this.loansService.getWorkingCapitalLoanAmortizationSchedule(loanId); | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
loanId can be null but still passes the !isNaN(+loanId) guard.
When neither route.paramMap nor route.parent?.paramMap contains loanId, the expression evaluates to null. Since +null === 0 and !isNaN(0) is true, the guard is entered and getWorkingCapitalLoanAmortizationSchedule(null) would be called.
Replace the numeric coercion check with an explicit string guard:
🐛 Proposed fix
- const loanId = route.paramMap.get('loanId') || route.parent?.paramMap.get('loanId');
- if (!isNaN(+loanId)) {
+ const loanId = route.paramMap.get('loanId') ?? route.parent?.paramMap.get('loanId');
+ if (loanId != null && !isNaN(+loanId)) {Also, per project conventions, prefer Observable<ProjectedAmortizationSchedule> over Observable<any> for the return type.
Based on learnings: "avoid using Observable<any> as a project-wide pattern for API responses…introduce specific interfaces/types for the response shapes."
🤖 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/app/loans/common-resolvers/working-capital/loan-amortization-schedule.resolver.ts`
around lines 29 - 37, The resolve method currently allows loanId === null to
pass the numeric coercion guard; change the guard to explicitly check that
loanId is a non-empty string (e.g. loanId !== null && loanId.trim() !== '')
before calling
this.loansService.getWorkingCapitalLoanAmortizationSchedule(loanId), keeping the
existing initialize(route) and isWorkingCapital check; also tighten the resolver
return type from Observable<any> | null to
Observable<ProjectedAmortizationSchedule> | null and update imports/types to use
the ProjectedAmortizationSchedule interface.
| <ng-container matColumnDef="forecastPaymentAmount"> | ||
| <th mat-header-cell class="r-amount mat-header-cell" *matHeaderCellDef> | ||
| {{ 'labels.inputs.Forecast Payment Amount' | translate }} | ||
| </th> | ||
| <td mat-cell class="r-amount" *matCellDef="let item">{{ item.forecastPaymentAmount | formatNumber }}</td> | ||
| <td mat-footer-cell *matFooterCellDef></td> | ||
| </ng-container> |
There was a problem hiding this comment.
Optional columns (forecastPaymentAmount, netAmortizationAmount, incomeModification) are defined in the template but will never render.
buildDisplayedColumns() is commented out in the component (line 101 of the .ts file), so displayedColumns is always the static baseColumns array. These three ng-container column definitions are unreachable dead code until that call is restored.
If these columns are deferred intentionally for this PR, consider either removing the dead template code or restoring the buildDisplayedColumns() call and uncommenting it.
Also applies to: 81-95
🤖 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/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html`
around lines 65 - 71, The template defines optional columns
(forecastPaymentAmount, netAmortizationAmount, incomeModification) that never
render because buildDisplayedColumns() is commented out and displayedColumns
stays as baseColumns; either remove these dead <ng-container> blocks from
loan-amortization-schedule-tab.component.html or restore and call
buildDisplayedColumns() in loan-amortization-schedule-tab.component.ts so
displayedColumns includes those optional keys; search for
buildDisplayedColumns(), displayedColumns, and baseColumns to decide whether to
re-enable dynamic column building or delete the unused column templates.
| import { SettingsService } from 'app/settings/settings.service'; | ||
| import { STANDALONE_SHARED_IMPORTS } from 'app/standalone-shared.module'; | ||
| import { jsPDF, jsPDFOptions } from 'jspdf'; | ||
| import autoTable from 'jspdf-autotable'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify declared peer dependency of jspdf-autotable@5.0.2
curl -s https://registry.npmjs.org/jspdf-autotable/5.0.2 | python3 -c "
import json, sys
data = json.load(sys.stdin)
print('Peer deps:', json.dumps(data.get('peerDependencies', {}), indent=2))
"Repository: openMF/web-app
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Check jspdf-autotable@5.0.7 peer dependencies
curl -s https://registry.npmjs.org/jspdf-autotable/5.0.7 | python3 -c "
import json, sys
data = json.load(sys.stdin)
print('jspdf-autotable@5.0.7 peer deps:', json.dumps(data.get('peerDependencies', {}), indent=2))
"
# Also check other recent versions
for version in 5.0.6 5.0.5; do
echo "---"
curl -s https://registry.npmjs.org/jspdf-autotable/$version | python3 -c "
import json, sys
data = json.load(sys.stdin)
print('jspdf-autotable@$version peer deps:', json.dumps(data.get('peerDependencies', {}), indent=2))
" 2>/dev/null || echo "Could not fetch v$version"
doneRepository: openMF/web-app
Length of output: 177
🏁 Script executed:
# Check the actual jsPDF version in the project
find . -name "package.json" -type f | head -5 | xargs grep -l "jsPDF" | head -1 | xargs cat | grep -A 2 -B 2 '"jspdf"'Repository: openMF/web-app
Length of output: 40
🏁 Script executed:
# Check if there are other imports of jspdf-autotable in the codebase
rg "from\s+['\"]jspdf-autotable" --type ts --type tsxRepository: openMF/web-app
Length of output: 86
🏁 Script executed:
# Find and read the root package.json
git ls-files package.json | head -1 | xargs cat | grep -A 1 '"jspdf"'Repository: openMF/web-app
Length of output: 114
🏁 Script executed:
# Search for jspdf-autotable imports (fixing the file type issue)
rg "from\s+['\"]jspdf-autotable" --type tsRepository: openMF/web-app
Length of output: 340
🏁 Script executed:
# Also check current import usage patterns in the file we're reviewing
head -50 src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts | grep -A 2 -B 2 "jspdf"Repository: openMF/web-app
Length of output: 297
🌐 Web query:
jspdf-autotable v5 import pattern named export documentation
💡 Result:
For jsPDF-AutoTable v5, the recommended ES module named imports are: 1) Import autoTable (recommended) import { jsPDF } from 'jspdf' import { autoTable } from 'jspdf-autotable' const doc = new jsPDF autoTable(doc, { head: [...], body: [...] }) This is explicitly recommended in the v5 release notes because the plugin is no longer auto-applied to the jsPDF instance in non-browser environments; you should import autoTable and pass it the jsPDF document instead. [1] 2) Import applyPlugin only if you want doc.autoTable(...) style import { jsPDF } from 'jspdf' import { applyPlugin } from 'jspdf-autotable' applyPlugin(jsPDF) const doc = new jsPDF doc.autoTable({ head: [...], body: [...] }) This “applyPlugin(jsPDF)” approach is also shown in the v5 release notes as the way to keep using doc.autoTable(...) in environments where the plugin is not auto-applied. [1] Reference (how usage is documented): The npm package README shows the same named export pattern (autoTable(doc, …)) and also documents applyPlugin(jsPDF) for enabling doc.autoTable(...) on instances. [2] So in practice (v5): - Prefer: import { autoTable } from 'jspdf-autotable' and call autoTable(doc, options). [1][2] - Optional legacy-style: import { applyPlugin }... and call applyPlugin(jsPDF) to enable doc.autoTable. [1][2]
Citations:
- 1: https://github.com/simonbengtsson/jsPDF-AutoTable/releases/tag/v5.0.0
- 2: https://www.npmjs.com/package/jspdf-autotable
Upgrade jspdf-autotable to ^5.0.7 to resolve peer dependency mismatch with jsPDF 4.2.1, and use the named import pattern as recommended by jsPDF-AutoTable v5.
The project uses jsPDF 4.2.1 with jspdf-autotable 5.0.2, but v5.0.2 declares peer dependencies on jsPDF ^2 || ^3 only, which excludes version 4.x. This causes a peer dependency violation that will trigger warnings during npm install. Upgrading jspdf-autotable to ^5.0.7 (or later) allows jsPDF 4.x as declared in its peer dependencies: ^2 || ^3 || ^4.
Additionally, the default import pattern on this line does not follow the v5 official recommendation. The jsPDF-AutoTable v5 release notes explicitly recommend using named imports. Update the import to:
-import autoTable from 'jspdf-autotable';
+import { autoTable } from 'jspdf-autotable';This change is necessary because in v5, the plugin is no longer auto-applied to the jsPDF instance in non-browser environments, and the named import pattern ensures proper behavior across all environments.
🤖 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/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts`
at line 40, Upgrade the jspdf-autotable dependency to ^5.0.7 (or later) in
package.json to include jsPDF 4.x in its peer deps, and change the default
import statement currently using "import autoTable from 'jspdf-autotable';" to
the v5 recommended named import (use the named import symbol autoTable from
'jspdf-autotable') so the plugin is loaded correctly with jsPDF 4.2.1; update
any related imports/usages in loan-amortization-schedule-tab.component.ts that
reference autoTable to match the named-import pattern.
| .pipe(takeUntilDestroyed(this.destroyRef)) | ||
| .subscribe((data: { amortizationSchedule: ProjectedAmortizationSchedule }) => { | ||
| this.amortizationSchedule = data.amortizationSchedule; | ||
| // this.buildDisplayedColumns(); |
There was a problem hiding this comment.
buildDisplayedColumns() is commented out, leaving optional columns as dead code.
With the call at line 101 commented out, displayedColumns is always the static baseColumns. The optional columns (forecastPaymentAmount, netAmortizationAmount, incomeModification) will never appear in the table regardless of whether the API returns data for them. The buildDisplayedColumns() helper and the corresponding ng-container blocks in the template are functionally inert.
If these columns are intentionally deferred, remove the dead template code and the buildDisplayedColumns() method for this PR. If they should be active, restore the call:
♻️ Proposed fix (if columns should be active)
.subscribe((data: { amortizationSchedule: ProjectedAmortizationSchedule }) => {
this.amortizationSchedule = data.amortizationSchedule;
- // this.buildDisplayedColumns();
+ this.buildDisplayedColumns();
});Also applies to: 115-124
🤖 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/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.ts`
at line 101, The call to buildDisplayedColumns() is commented out so
displayedColumns never includes the optional columns (forecastPaymentAmount,
netAmortizationAmount, incomeModification) and the ng-container blocks become
dead code; either restore the call to buildDisplayedColumns() (e.g., invoke
buildDisplayedColumns() from the component lifecycle where displayedColumns is
initialized so that displayedColumns merges baseColumns with the optional
columns when API data is present) or else remove the buildDisplayedColumns()
method and the template ng-container blocks for those optional columns and any
related references to displayedColumns/baseColumns to eliminate dead code.
| <div class="flex-fill layout-row"> | ||
| <span class="flex-30"> {{ 'labels.inputs.Realized Income' | translate }} </span> | ||
| <span class="flex-30 r-amount">{{ loanBalances.realizedIncome | formatNumber }}</span> | ||
| </div> | ||
|
|
||
| <div class="flex-fill layout-row"> | ||
| <span class="flex-30"> {{ 'labels.inputs.Unrealized Income' | translate }} </span> | ||
| <span class="flex-30 r-amount">{{ loanBalances.unrealizedIncome | formatNumber }}</span> | ||
| </div> |
There was a problem hiding this comment.
Inconsistent number formatting for income vs. payment fields.
The payment-related rows (Principal Outstanding, Total Paid Principal, Total Payment, Overpayment Amount) format values with the currency pipe + currencyCode, while Realized Income and Unrealized Income use the plain formatNumber pipe. If the income fields are also monetary amounts (which seems likely from the model WorkingCapitalBalances), this asymmetry will surface as missing currency symbols in the UI. Confirm this is intentional, otherwise apply the currency pipe here as well.
🛡️ Suggested fix if currency display is desired
- <span class="flex-30 r-amount">{{ loanBalances.realizedIncome | formatNumber }}</span>
+ <span class="flex-30 r-amount">{{
+ loanBalances.realizedIncome | currency: currencyCode : 'symbol-narrow' : '1.2-2'
+ }}</span>
...
- <span class="flex-30 r-amount">{{ loanBalances.unrealizedIncome | formatNumber }}</span>
+ <span class="flex-30 r-amount">{{
+ loanBalances.unrealizedIncome | currency: currencyCode : 'symbol-narrow' : '1.2-2'
+ }}</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="flex-fill layout-row"> | |
| <span class="flex-30"> {{ 'labels.inputs.Realized Income' | translate }} </span> | |
| <span class="flex-30 r-amount">{{ loanBalances.realizedIncome | formatNumber }}</span> | |
| </div> | |
| <div class="flex-fill layout-row"> | |
| <span class="flex-30"> {{ 'labels.inputs.Unrealized Income' | translate }} </span> | |
| <span class="flex-30 r-amount">{{ loanBalances.unrealizedIncome | formatNumber }}</span> | |
| </div> | |
| <div class="flex-fill layout-row"> | |
| <span class="flex-30"> {{ 'labels.inputs.Realized Income' | translate }} </span> | |
| <span class="flex-30 r-amount">{{ | |
| loanBalances.realizedIncome | currency: currencyCode : 'symbol-narrow' : '1.2-2' | |
| }}</span> | |
| </div> | |
| <div class="flex-fill layout-row"> | |
| <span class="flex-30"> {{ 'labels.inputs.Unrealized Income' | translate }} </span> | |
| <span class="flex-30 r-amount">{{ | |
| loanBalances.unrealizedIncome | currency: currencyCode : 'symbol-narrow' : '1.2-2' | |
| }}</span> | |
| </div> |
🤖 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/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.html`
around lines 44 - 52, The Realized and Unrealized Income rows use the
formatNumber pipe while other monetary fields use the currency pipe with
currencyCode, causing inconsistent UI; if these income fields are monetary
values on WorkingCapitalBalances, change the bindings in
loan-balances-tab.component.html to use the currency pipe with the same
currencyCode as the other payment rows (replace usages of "{{
loanBalances.realizedIncome | formatNumber }}" and "{{
loanBalances.unrealizedIncome | formatNumber }}" to use the currency pipe),
ensuring the display matches the Principal/Payment fields and preserves the
existing currencyCode variable.
| constructor() { | ||
| super(); | ||
| this.route.parent.data.subscribe((data: { loanDetailsData: any }) => { | ||
| this.currencyCode = data.loanDetailsData.currency.code; |
There was a problem hiding this comment.
Guard currency.code access.
data.loanDetailsData.currency.code will throw if currency is absent (e.g., a loan without currency configured yet). The template already tolerates a null currencyCode, so a safe-navigation read here is enough.
🛡️ Suggested fix
- this.currencyCode = data.loanDetailsData.currency.code;
+ this.currencyCode = data.loanDetailsData.currency?.code ?? null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.currencyCode = data.loanDetailsData.currency.code; | |
| this.currencyCode = data.loanDetailsData.currency?.code ?? null; |
🤖 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/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts`
at line 36, The direct access to data.loanDetailsData.currency.code can throw if
currency is missing; update the assignment that sets currencyCode in the
LoanBalancesTabComponent to use safe navigation and a null/default fallback
(e.g., read currency via optional chaining and assign null when absent) so that
currencyCode becomes null instead of throwing when data.loanDetailsData.currency
is undefined; locate the assignment to this.currencyCode in the
LoanBalancesTabComponent and replace it with a safe-access pattern (use
currency?.code with a null/default fallback).
| totalPaymentValue: number; | ||
| periodPaymentRate: number; | ||
| npvDayCount: number; | ||
| expectedDisbursementDate: Date; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how date fields are typed/used elsewhere in loan models
rg -n -C2 '(expectedDisbursementDate|paymentDate|disbursementDate)\s*[:?]' src/app/loansRepository: openMF/web-app
Length of output: 10405
Change Date type annotations to string or string | Date for date fields in API responses.
The expectedDisbursementDate (line 15) and paymentDate (line 24) fields are typed as Date, but Angular's HttpClient returns JSON with date fields as strings. This creates a type mismatch: code appears safe to call methods like .getTime() while actually holding a string. The codebase already applies defensive dateUtils.parseDate() and dateUtils.formatDate() patterns to work around this, and loans-account-terms-step.component.ts already uses the correct string | Date pattern elsewhere. Use string (or string | Date if conversion is applied) to match the actual runtime shape.
🤖 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/app/loans/models/working-capital/working-capital-loan-account.model.ts`
at line 15, Update the type annotations for the date fields in the
WorkingCapitalLoanAccount model: change expectedDisbursementDate and paymentDate
from Date to string | Date (or string if you prefer strictly matching raw JSON)
so the TypeScript types match HttpClient JSON responses and the existing
dateUtils.parseDate/formatDate usage; locate these fields in
working-capital-loan-account.model.ts and adjust their declarations accordingly.
1 participant
Description
Now the we have more Working Capital details we need to update the loan account details views
WEB-657
Screenshots
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
Release Notes