WEB-670: Working Capital loan delinquency grace days#3440
Merged
adamsaghy merged 1 commit intoopenMF:devfrom Apr 1, 2026
Merged
Conversation
alberto-art3ch
force-pushed
the
WEB-670/working-capital-loan-delinquency-grace-days
branch
from
bf01ee7 to
88a75a6
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Directives src/app/directives/positive-number.directive.ts |
Marked as standalone: true while preserving existing keydown handling and selector. |
Loan Resolvers src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts, src/app/loans/common-resolvers/loan-originators.resolver.ts, src/app/loans/common-resolvers/loans-account-and-template.resolver.ts |
Updated resolvers to extend LoanBaseResolver, call initialize(route), and conditionally return service data only when loanId is numeric and isLoanProduct is true; otherwise return EMPTY or skip execution. |
Loan Terms Components src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts, src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html |
Added delinquencyStartTypeOptions field; populated from template data and added form controls for delinquencyGraceDays and delinquencyStartType; updated template layout and added new delinquency input/select fields. |
Loan Create & Edit Components src/app/loans/create-loans-account/create-loans-account.component.ts, src/app/loans/edit-loans-account/edit-loans-account.component.ts |
Extended loansAccountProductTemplate.options to include delinquencyStartTypeOptions from template data. |
Loan View Components src/app/loans/loans-view/account-details/account-details.component.html, src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts, src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.html, src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html |
Added delinquency grace days and start type display rows in account details and preview; added maxDate property to approve-loan; updated expected disbursement date validation to use approvedOnDate as min and added maxDate constraint; removed debug console.log and made delinquency data retrieval conditional on isLoanProduct. |
Product Components src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html, src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts, src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html |
Added delinquency-related display rows in product summary; added delinquencyStartTypeOptions field and form controls (delinquencyGraceDays, delinquencyStartType) for working-capital configuration in product settings. |
Shared Input Component src/app/shared/input-amount/input-amount.component.ts, src/app/shared/input-amount/input-amount.component.html |
Implemented OnInit lifecycle hook to dynamically add amountValueValidator() to input control and refresh validation state; applied mifosxPositiveNumber directive to amount inputs in all branches; added mat-error block for highAmountValue validation errors. |
Translations src/assets/translations/cs-CS.json, src/assets/translations/de-DE.json, src/assets/translations/en-US.json, src/assets/translations/es-CL.json, src/assets/translations/es-MX.json, src/assets/translations/fr-FR.json, src/assets/translations/it-IT.json, src/assets/translations/ko-KO.json, src/assets/translations/lt-LT.json, src/assets/translations/lv-LV.json, src/assets/translations/ne-NE.json, src/assets/translations/pt-PT.json, src/assets/translations/sw-SW.json |
Added "Loan Creation" label under catalogs, "Invalid amount format" under commons, and "Delinquency Grace Days" and "Delinquency Start Type" under inputs across all locales; es-MX includes terminology shift from "ministración" to "dispersión" for disbursement-related translations. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- WEB-813: Working Capital loan account modification #3435: Makes overlapping changes to the same files including PositiveNumberDirective, multiple loan resolvers, and amount validator integration across shared and loan components.
- WEB-813: Working Capital loan account creation #3257: Modifies the same loan product and working-capital code paths with overlapping changes to loans-account-terms-step, loan-product-settings-step, and related resolvers.
- WEB-455: fix alignments #2874: Modifies the same loans-account-terms-step component (TypeScript form logic and HTML template) for loan account configuration.
Suggested reviewers
- adamsaghy
- IOhacker
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately captures the main change: adding delinquency grace days support for Working Capital loans, which is consistently reflected throughout the changeset across resolvers, components, templates, and translations. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
src/app/loans/common-resolvers/loan-originators.resolver.ts (1)
26-33: Make the non-loan branch explicit.Falling off the end of
resolve()relies on an implicitundefinedresult for working-capital routes. Returning an explicit fallback observable keeps the resolver contract clear and makes the branch behavior easier to reason about under strict typing.As per coding guidelines: `src/app/**/*.ts`: Use TypeScript for all application code with strict typing conventions.🧭 Explicit fallback
-import { Observable, throwError } from 'rxjs'; +import { Observable, of, throwError } from 'rxjs'; ... resolve(route: ActivatedRouteSnapshot): Observable<any> { const loanId = route.paramMap.get('loanId') ?? route.parent?.paramMap.get('loanId'); if (!loanId) { return throwError(() => new Error('Missing loanId route param')); } - if (this.loanProductService.isLoanProduct) { - return this.loansService.getLoanOriginators(loanId); - } + if (!this.loanProductService.isLoanProduct) { + return of(null); + } + return this.loansService.getLoanOriginators(loanId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-originators.resolver.ts` around lines 26 - 33, The resolve(route: ActivatedRouteSnapshot) function currently only returns an Observable when this.loanProductService.isLoanProduct is true and implicitly falls through otherwise; make the non-loan branch explicit by returning a fallback Observable (e.g., return EMPTY or return of(null/undefined) matching the resolver type) when loanProductService.isLoanProduct is false so resolve always returns an Observable<any>; update the branch in loan-originators.resolver.ts around the resolve method and add the necessary rxjs import (EMPTY or of) and maintain the existing error/loanId checks and loansService.getLoanOriginators(loanId) call.src/app/shared/validators/amount-value.validator.ts (1)
10-25: Hoist the constant regex and drop the stale walkthrough.The pattern is constant, so rebuilding it on every validation run is unnecessary, and the inline breakdown no longer describes the actual expression. Moving the regex to module scope with a single precision/scale comment will make this easier to maintain.
♻️ Small cleanup
+const AMOUNT_VALUE_REGEX = /^\d{1,13}(\.\d{1,6})?$/; + export function amountValueValidator(): ValidatorFn { return (control: AbstractControl): ValidationErrors | null => { - if (!control.value) return null; - const maxTotalDigits = 19; - const maxDecimals = 6; - - // Regex breakdown: - // ^ - Start of string - // (?=.{1,14}$) - Lookahead to ensure total length (including potential dot) - // [0-9]+ - One or more digits - // (\.[0-9]{1,6})? - Optional dot followed by 1 to 6 digits - // $ - End of string - const regex = new RegExp(`^\\d{1,${maxTotalDigits - maxDecimals}}(\\.\\d{1,${maxDecimals}})?$`); - - const valid = regex.test(control.value.toString()); + if (control.value === null || control.value === undefined || control.value === '') return null; + const valid = AMOUNT_VALUE_REGEX.test(String(control.value)); return valid ? null : { highAmountValue: true }; }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/validators/amount-value.validator.ts` around lines 10 - 25, The regex used inside amountValueValidator() should be hoisted to module scope to avoid rebuilding it on every call and to replace the stale inline walkthrough with a single precision/scale comment; create a top-level constant (e.g., AMOUNT_VALUE_REGEX) with a concise comment like "precision 19, scale 6" and use that constant inside amountValueValidator() instead of constructing new RegExp each invocation, removing the outdated step-by-step regex breakdown.src/assets/translations/ne-NE.json (1)
1765-1766: Use consistent Nepali terminology for “Delinquency”.Line 1766 uses transliteration (
डिलिंक्वेन्सी), while related labels in this file generally useअपराध. Consider aligning this for UX consistency.Suggested wording
- "Delinquency Start Type": "डिलिंक्वेन्सी सुरुवात प्रकार", + "Delinquency Start Type": "अपराध सुरु प्रकार",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/ne-NE.json` around lines 1765 - 1766, The translation for the key "Delinquency Start Type" uses the transliterated term "डिलिंक्वेन्सी"; update it to use the consistent Nepali term "अपराध" to match "Delinquency Grace Days" (key "Delinquency Grace Days") and other entries—e.g., change the value for "Delinquency Start Type" from "डिलिंक्वेन्सी सुरुवात प्रकार" to "अपराध सुरु हुने प्रकार" (or a similar phrasing using "अपराध") so terminology is consistent across the file.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html (1)
519-522: GuarddelinquencyStartTypeaccess to avoid template breakage.Line 521 dereferences
loanProduct.delinquencyStartType.valuedirectly. IfdelinquencyStartTypeis missing, this can fail at runtime and blank the section.Suggested fix
<div class="flex-fill layout-row"> <span class="flex-40">{{ 'labels.inputs.Delinquency Start Type' | translate }}:</span> - <span class="flex-60">{{ loanProduct.delinquencyStartType.value | translateKey: 'catalogs' }}</span> + <span class="flex-60"> + `@if` (loanProduct.delinquencyStartType?.value) { + {{ loanProduct.delinquencyStartType.value | translateKey: 'catalogs' }} + } `@else` { + {{ 'labels.inputs.Unassigned' | translate }} + } + </span> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html` around lines 519 - 522, The template is directly dereferencing loanProduct.delinquencyStartType.value which can throw if delinquencyStartType is absent; update the binding for loanProduct.delinquencyStartType to guard access (e.g. use the safe-navigation operator or an *ngIf guard) so the translateKey pipe only receives a defined value and the UI section doesn't break — locate the usage in loan-product-summary.component.html around the span showing {{ loanProduct.delinquencyStartType.value | translateKey: 'catalogs' }} and wrap or replace it with a guarded expression (loanProduct.delinquencyStartType?.value or conditionally render the span when loanProduct.delinquencyStartType exists).src/assets/translations/it-IT.json (1)
892-892: Use a more idiomatic singular label for the catalog entry.At Line 892,
Creazione di prestitiis understandable but a bit unnatural as a catalog value. ConsiderCreazione prestito(or domain-approved equivalent) for consistency with UI enum style.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/it-IT.json` at line 892, Update the translation value for the "Loan Creation" key to use the idiomatic singular form; locate the JSON entry with the key "Loan Creation" and replace the value "Creazione di prestiti" with "Creazione prestito" (or the approved domain-specific singular alternative) so it matches the UI enum style and other singular labels.src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts (1)
67-75: Consider managing the subscription lifecycle.The HTTP subscription is not explicitly cleaned up. While HTTP observables complete after emission, adding cleanup (e.g.,
takeUntilDestroyed()from@angular/core/rxjs-interop) is a defensive best practice, especially if the component could be destroyed mid-request.♻️ Suggested improvement using takeUntilDestroyed
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { DestroyRef, inject } from '@angular/core'; export class ApproveLoanComponent extends LoanAccountActionsBaseComponent implements OnInit { + private destroyRef = inject(DestroyRef); // ... ngOnInit() { this.setApproveLoanForm(); if (this.isLoanProduct) { - this.loanService.getLoanDelinquencyDataForTemplate(this.loanId).subscribe((delinquencyData: any) => { + this.loanService.getLoanDelinquencyDataForTemplate(this.loanId) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((delinquencyData: any) => { if (delinquencyData.availableDisbursementAmountWithOverApplied !== undefined) { this.loanData.availableDisbursementAmountWithOverApplied = delinquencyData.availableDisbursementAmountWithOverApplied; } }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts` around lines 67 - 75, The subscription in ApproveLoanComponent (approve-loan.component.ts) to loanService.getLoanDelinquencyDataForTemplate should use a teardown operator to avoid potential leaks; update the call in the isLoanProduct branch to pipe the observable through takeUntilDestroyed() (imported from `@angular/core/rxjs-interop`) before subscribe, so the subscription is automatically cleaned up if the component is destroyed, and keep the existing logic that assigns delinquencyData.availableDisbursementAmountWithOverApplied to this.loanData.availableDisbursementAmountWithOverApplied.src/app/loans/common-resolvers/loan-base.resolver.ts (1)
14-48: Duplicate product type state managers creating potential inconsistencies.
LoanBaseResolverreplicates theproductTypeBehaviorSubject and associated getters (isWorkingCapital,isLoanProduct,loanProductPath,loanAccountPath) that already exist inLoanProductService. Both classes have virtually identical implementations:
LoanDetailsResolverinitializesLoanProductServicefrom route queryParamsLoanDelinquencyDataResolverandLoansAccountAndTemplateResolverextendLoanBaseResolverand update only their local copy viathis.initialize(route)- Components depend on
loanProductService.isLoanProduct(40+ usages)- Resolvers depending on
LoanBaseResolverusethis.isLoanProduct(2 resolvers)This split creates two independent state sources with no synchronization. If resolvers extending
LoanBaseResolverexecute beforeLoanDetailsResolverinitializes the service, or if different route branches use different resolvers, the product type state could diverge between components (using service state) and resolvers (using local state).Consolidate by having all resolvers delegate product type queries to
LoanProductService, or document the intentional architectural separation and add explicit synchronization where both sources must remain consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-base.resolver.ts` around lines 14 - 48, LoanBaseResolver currently maintains its own productType BehaviorSubject and duplicate getters (productType, initialize, isWorkingCapital, isLoanProduct, loanProductPath, loanAccountPath) which creates a second, unsynchronized state source; remove the local BehaviorSubject and getters, inject LoanProductService into LoanBaseResolver, and delegate all queries to the service (e.g. replace uses of this.productType.value / this.isLoanProduct etc. with the corresponding LoanProductService property/methods), and in initialize(route) update the shared service state from route.queryParams['productType'] (call the service's setter/update method or next its BehaviorSubject) instead of calling this.productType.next; ensure LoanDetailsResolver remains the owner of initial service initialization so all resolvers read the same shared state.
🤖 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/directives/positive-number.directive.ts`:
- Around line 15-27: The ArrowDown check in onKeyDown currently blocks only when
currentValue <= 0, which allows decimal steps to cross zero; instead read the
input element's step value (const step = parseFloat(this.el.nativeElement.step)
|| 1), compute the next value (currentValue - step) and call
event.preventDefault() when that next value would be < 0; keep the existing
logic for ArrowUp unchanged and use the same inputElement/this.el.nativeElement
and event.key symbols to locate the fix.
In `@src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts`:
- Around line 35-43: The resolver method resolve calls initialize(route) then
fetches loanId and currently coerces it with +loanId which treats null as 0;
first ensure you check loanId !== null before performing numeric coercion (use
the existing loanId variable from route.paramMap or route.parent.paramMap) so
you never call loansService.getDelinquencyData with null. Second, guarantee an
Observable is returned on every path: if isLoanProduct is false or loanId is
invalid, return a safe fallback Observable (e.g., of(null)) instead of falling
through to undefined; update the resolve method to return
loansService.getDelinquencyData(loanId) only after the loanId !== null &&
!isNaN(+loanId) && this.isLoanProduct checks, otherwise return of(null).
In `@src/app/loans/common-resolvers/loans-account-and-template.resolver.ts`:
- Around line 35-43: The resolve method in
loans-account-and-template.resolver.ts can implicitly return undefined when
loanId is invalid; update the resolve(route: ActivatedRouteSnapshot)
implementation to explicitly return an Observable in that branch (e.g., return
EMPTY or of(null) or throw an error) instead of falling through: locate the
loanId check and after the if (!isNaN(+loanId)) block add a clear return (for
example return EMPTY) and add the necessary import from 'rxjs' (EMPTY or of) so
callers of getLoansAccountAndTemplateResource/getWorkingCapitalLoanDetails
always receive an Observable.
In `@src/app/shared/input-amount/input-amount.component.html`:
- Around line 48-52: The highAmountValue error is misleading and only shown in
the required branch; update the template so the same error message appears for
both required and non-required inputs and replace the hardcoded text with an
i18n key describing a format/precision problem. Specifically, in the
inputFormControl.hasError('highAmountValue') block (and the corresponding
!isRequired branch) show a mat-error that uses the translate pipe with a
semantic key such as 'errors.invalidAmountFormat' (or the existing project key
for precision/format errors) instead of the current "Must be less or equal to"
text so the message correctly reflects digit/decimal limits and follows
`@ngx-translate/core` usage.
In `@src/app/shared/input-amount/input-amount.component.ts`:
- Around line 41-43: In ngOnInit(), after calling
this.inputFormControl.addValidators(amountValueValidator()), call
this.inputFormControl.updateValueAndValidity() so the newly added validator
(amountValueValidator) runs against any pre-populated value and updates the
control's validity/state immediately; update the method ngOnInit to use
inputFormControl.addValidators(...) followed by
inputFormControl.updateValueAndValidity() to ensure correct initial validation.
In `@src/assets/translations/de-DE.json`:
- Around line 1768-1769: Replace the incorrect criminal-justice wording for the
keys "Delinquency Grace Days" and "Delinquency Start Type": change the value for
"Delinquency Grace Days" from "Gnadentage für Jugendstraftäter" to a financial
term like "Kulanztage bei Zahlungsverzug" and change the value for "Delinquency
Start Type" from "Delinquenz-Starttyp" to "Art des Verzugsbeginns" so both
labels use proper loan/delinquency vocabulary.
In `@src/assets/translations/es-CL.json`:
- Line 1769: The translation value for the key "Delinquency Start Type" uses
inconsistent wording ("Inicio de delincuencia"); update the JSON entry for
"Delinquency Start Type" to use the locale's existing finance term and include
"Type" as in "Tipo de inicio de morosidad" so it reads consistently with other
keys using morosidad.
In `@src/assets/translations/es-MX.json`:
- Line 4176: Update the Spanish tooltip value for the key "Leave this checkbox
unchecked" to use the consistent terminology "dispersión" instead of
"ministraciones"; specifically replace "ministraciones múltiples" with
"dispersión múltiple" and ensure the sentence reads consistently (e.g., "Deje
esta casilla sin marcar si el Crédito es de dispersión única. Marque esta
casilla de verificación si el Crédito es un Crédito de dispersión múltiple.
Consulte los campos adicionales para obtener información adicional requerida
para este tipo de Crédito.").
- Line 895: Update the Spanish locale entries to use the established domain
terms "Crédito" and "morosidad" instead of "préstamo" and "delincuencia": change
the value for the "Loan Creation" key from "Creación del préstamo" to "Creación
del crédito" (and apply the same vocabulary change for the related delinquency
keys such as "Delinquency Start Type" and any other keys introduced at the other
occurrence around line 1771 so they use "morosidad" rather than "delincuencia").
Ensure the same capitalization style is preserved across all affected keys.
In `@src/assets/translations/it-IT.json`:
- Around line 1767-1768: The Italian translations for the keys "Delinquency
Grace Days" and "Delinquency Start Type" use criminal-context terms
("delinquenti"/"delinquenza"); update their values to lending-domain wording
using "morosità" (e.g., change "Giorni di grazia per i delinquenti" to "Giorni
di grazia della morosità" and "Tipo di inizio della delinquenza" to "Tipo di
inizio della morosità") so the UX uses correct financial terminology; locate the
keys "Delinquency Grace Days" and "Delinquency Start Type" in the translations
and replace the Italian strings accordingly.
In `@src/assets/translations/ko-KO.json`:
- Around line 1769-1770: The Korean translations for the keys "Delinquency Grace
Days" and "Delinquency Start Type" use 비행 (misconduct) instead of the financial
term 연체; update the values for those keys to use 연체 (e.g., "Delinquency Grace
Days": "연체 유예 기간" and "Delinquency Start Type": "연체 시작 유형") so they reflect
correct loan/financial terminology and preserve original key names ("Delinquency
Grace Days", "Delinquency Start Type").
---
Nitpick comments:
In `@src/app/loans/common-resolvers/loan-base.resolver.ts`:
- Around line 14-48: LoanBaseResolver currently maintains its own productType
BehaviorSubject and duplicate getters (productType, initialize,
isWorkingCapital, isLoanProduct, loanProductPath, loanAccountPath) which creates
a second, unsynchronized state source; remove the local BehaviorSubject and
getters, inject LoanProductService into LoanBaseResolver, and delegate all
queries to the service (e.g. replace uses of this.productType.value /
this.isLoanProduct etc. with the corresponding LoanProductService
property/methods), and in initialize(route) update the shared service state from
route.queryParams['productType'] (call the service's setter/update method or
next its BehaviorSubject) instead of calling this.productType.next; ensure
LoanDetailsResolver remains the owner of initial service initialization so all
resolvers read the same shared state.
In `@src/app/loans/common-resolvers/loan-originators.resolver.ts`:
- Around line 26-33: The resolve(route: ActivatedRouteSnapshot) function
currently only returns an Observable when this.loanProductService.isLoanProduct
is true and implicitly falls through otherwise; make the non-loan branch
explicit by returning a fallback Observable (e.g., return EMPTY or return
of(null/undefined) matching the resolver type) when
loanProductService.isLoanProduct is false so resolve always returns an
Observable<any>; update the branch in loan-originators.resolver.ts around the
resolve method and add the necessary rxjs import (EMPTY or of) and maintain the
existing error/loanId checks and loansService.getLoanOriginators(loanId) call.
In
`@src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts`:
- Around line 67-75: The subscription in ApproveLoanComponent
(approve-loan.component.ts) to loanService.getLoanDelinquencyDataForTemplate
should use a teardown operator to avoid potential leaks; update the call in the
isLoanProduct branch to pipe the observable through takeUntilDestroyed()
(imported from `@angular/core/rxjs-interop`) before subscribe, so the subscription
is automatically cleaned up if the component is destroyed, and keep the existing
logic that assigns delinquencyData.availableDisbursementAmountWithOverApplied to
this.loanData.availableDisbursementAmountWithOverApplied.
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html`:
- Around line 519-522: The template is directly dereferencing
loanProduct.delinquencyStartType.value which can throw if delinquencyStartType
is absent; update the binding for loanProduct.delinquencyStartType to guard
access (e.g. use the safe-navigation operator or an *ngIf guard) so the
translateKey pipe only receives a defined value and the UI section doesn't break
— locate the usage in loan-product-summary.component.html around the span
showing {{ loanProduct.delinquencyStartType.value | translateKey: 'catalogs' }}
and wrap or replace it with a guarded expression
(loanProduct.delinquencyStartType?.value or conditionally render the span when
loanProduct.delinquencyStartType exists).
In `@src/app/shared/validators/amount-value.validator.ts`:
- Around line 10-25: The regex used inside amountValueValidator() should be
hoisted to module scope to avoid rebuilding it on every call and to replace the
stale inline walkthrough with a single precision/scale comment; create a
top-level constant (e.g., AMOUNT_VALUE_REGEX) with a concise comment like
"precision 19, scale 6" and use that constant inside amountValueValidator()
instead of constructing new RegExp each invocation, removing the outdated
step-by-step regex breakdown.
In `@src/assets/translations/it-IT.json`:
- Line 892: Update the translation value for the "Loan Creation" key to use the
idiomatic singular form; locate the JSON entry with the key "Loan Creation" and
replace the value "Creazione di prestiti" with "Creazione prestito" (or the
approved domain-specific singular alternative) so it matches the UI enum style
and other singular labels.
In `@src/assets/translations/ne-NE.json`:
- Around line 1765-1766: The translation for the key "Delinquency Start Type"
uses the transliterated term "डिलिंक्वेन्सी"; update it to use the consistent
Nepali term "अपराध" to match "Delinquency Grace Days" (key "Delinquency Grace
Days") and other entries—e.g., change the value for "Delinquency Start Type"
from "डिलिंक्वेन्सी सुरुवात प्रकार" to "अपराध सुरु हुने प्रकार" (or a similar
phrasing using "अपराध") so terminology is consistent across the file.
🪄 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: 206898af-5cd2-473f-a7e6-a51e8a64c1fa
📒 Files selected for processing (33)
src/app/directives/directives.module.tssrc/app/directives/positive-number.directive.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/common-resolvers/loan-originators.resolver.tssrc/app/loans/common-resolvers/loans-account-and-template.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/loans/loans.service.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/shared/input-amount/input-amount.component.htmlsrc/app/shared/input-amount/input-amount.component.tssrc/app/shared/validators/amount-value.validator.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
Comment on lines
+15
to
+27
| @HostListener('keydown', ['$event']) | ||
| onKeyDown(event: KeyboardEvent): void { | ||
| const inputElement = this.el.nativeElement; | ||
|
|
||
| // Check if the key is ArrowUp or ArrowDown | ||
| if (event.key === 'ArrowUp' || event.key === 'ArrowDown') { | ||
| const currentValue = parseFloat(inputElement.value) || 0; | ||
|
|
||
| if (event.key === 'ArrowDown' && currentValue <= 0) { | ||
| // Prevent default if trying to decrement below or at 0 | ||
| event.preventDefault(); | ||
| } | ||
| } |
There was a problem hiding this comment.
This still lets ArrowDown cross zero for decimal values.
The check only blocks when the field is already <= 0. With the default step=1, pressing ArrowDown on 0.5 still yields -0.5, so the shared amount input can still jump into an invalid negative state before the validator catches it. Compare against the next stepped value instead of the current value.
🔧 Narrow fix for the ArrowDown case
- if (event.key === 'ArrowUp' || event.key === 'ArrowDown') {
- const currentValue = parseFloat(inputElement.value) || 0;
-
- if (event.key === 'ArrowDown' && currentValue <= 0) {
+ if (event.key === 'ArrowDown') {
+ const currentValue = Number(inputElement.value || 0);
+ const step = Number(inputElement.step || 1);
+ const nextValue = currentValue - step;
+
+ if (!Number.isNaN(nextValue) && nextValue < 0) {
event.preventDefault();
}
}📝 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.
Suggested change
| @HostListener('keydown', ['$event']) | |
| onKeyDown(event: KeyboardEvent): void { | |
| const inputElement = this.el.nativeElement; | |
| // Check if the key is ArrowUp or ArrowDown | |
| if (event.key === 'ArrowUp' || event.key === 'ArrowDown') { | |
| const currentValue = parseFloat(inputElement.value) || 0; | |
| if (event.key === 'ArrowDown' && currentValue <= 0) { | |
| // Prevent default if trying to decrement below or at 0 | |
| event.preventDefault(); | |
| } | |
| } | |
| `@HostListener`('keydown', ['$event']) | |
| onKeyDown(event: KeyboardEvent): void { | |
| const inputElement = this.el.nativeElement; | |
| // Check if the key is ArrowDown | |
| if (event.key === 'ArrowDown') { | |
| const currentValue = Number(inputElement.value || 0); | |
| const step = Number(inputElement.step || 1); | |
| const nextValue = currentValue - step; | |
| if (!Number.isNaN(nextValue) && nextValue < 0) { | |
| event.preventDefault(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/directives/positive-number.directive.ts` around lines 15 - 27, The
ArrowDown check in onKeyDown currently blocks only when currentValue <= 0, which
allows decimal steps to cross zero; instead read the input element's step value
(const step = parseFloat(this.el.nativeElement.step) || 1), compute the next
value (currentValue - step) and call event.preventDefault() when that next value
would be < 0; keep the existing logic for ArrowUp unchanged and use the same
inputElement/this.el.nativeElement and event.key symbols to locate the fix.
Comment on lines
35
to
43
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| this.initialize(route); | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| return this.loansService.getDelinquencyData(loanId); | ||
| if (!isNaN(+loanId)) { | ||
| if (this.isLoanProduct) { | ||
| return this.loansService.getDelinquencyData(loanId); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Resolver wiring =="
rg -n -C4 --glob '*routing*.ts' 'LoanDelinquencyDataResolver|resolve\s*:|loanId|productType' src/app
echo
echo "== Resolver consumers =="
rg -n -C4 'delinquencyData|loanDelinquency|snapshot\.data|route\.data' src/app/loans
echo
echo "== Base resolver product-type logic =="
sed -n '1,120p' src/app/loans/common-resolvers/loan-base.resolver.ts
echo
echo "== TypeScript strictness flags =="
find . -maxdepth 3 -name 'tsconfig*.json' -print -exec rg -n '"(strict|noImplicitReturns|strictNullChecks)"' {} \;Repository: openMF/web-app
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== LoanDelinquencyDataResolver implementation ==="
find src/app/loans -name '*delinquency*resolver*' -type f
echo
echo "=== LoanBaseResolver implementation (first 100 lines) ==="
find src/app/loans -name '*base*resolver*' -type fRepository: openMF/web-app
Length of output: 485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check tsconfig noImplicitReturns and strict mode ==="
rg '"noImplicitReturns"|"strict"' tsconfig.json tsconfig.app.json --color=never
echo
echo "=== Loans routing module (look for delinquencyData) ==="
fd -e routing.module.ts . src/app/loans | head -5Repository: openMF/web-app
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== LoanDelinquencyDataResolver source ==="
cat src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts 2>/dev/null || echo "File not found with standard path"
echo
echo "=== LoanBaseResolver source (first 120 lines) ==="
cat src/app/loans/common-resolvers/loan-base.resolver.ts 2>/dev/null | head -120 || echo "File not found"Repository: openMF/web-app
Length of output: 2839
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find loans routing module ==="
find src/app/loans -name '*routing*' -type f
echo
echo "=== Search for LoanDelinquencyDataResolver usage in routing ==="
rg -n 'LoanDelinquencyDataResolver|delinquencyData' src/app/loans --glob '*routing*'Repository: openMF/web-app
Length of output: 513
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find tsconfig files ==="
find . -maxdepth 3 -name 'tsconfig*.json' -type f | head -10
echo
echo "=== Check noImplicitReturns in all tsconfig files ==="
find . -maxdepth 3 -name 'tsconfig*.json' -exec grep -l 'noImplicitReturns' {} \;Repository: openMF/web-app
Length of output: 328
🏁 Script executed:
#!/bin/bash
set -euso pipefail
echo "=== Check main tsconfig strict mode ==="
cat tsconfig.json | head -50
echo
echo "=== Check src/tsconfig.app.json compilerOptions ==="
cat src/tsconfig.app.json | grep -A 20 '"compilerOptions"' || cat src/tsconfig.app.json | head -40Repository: openMF/web-app
Length of output: 1387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Review loans routing around line 170 (delinquencyData resolver) ==="
sed -n '160,180p' src/app/loans/loans-routing.module.ts
echo
echo "=== Check for working-capital routes using LoanDelinquencyDataResolver ==="
rg -B5 -A5 'working.capital|delinquencyData' src/app/loans/loans-routing.module.ts | head -60Repository: openMF/web-app
Length of output: 916
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Loans routing full context around delinquencyData (lines 150-200) ==="
sed -n '150,200p' src/app/loans/loans-routing.module.ts
echo
echo "=== Check parent route context (lines 100-150) ==="
sed -n '100,150p' src/app/loans/loans-routing.module.tsRepository: openMF/web-app
Length of output: 3719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for working-capital route configuration in loans routing ==="
rg -n 'working.capital' src/app/loans/loans-routing.module.ts
echo
echo "=== Check if delinquencytags route is reachable for working-capital ==="
rg -B20 'delinquencytags' src/app/loans/loans-routing.module.ts | head -50Repository: openMF/web-app
Length of output: 129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full parent route definition (lines 80-120) ==="
sed -n '80,120p' src/app/loans/loans-routing.module.ts
echo
echo "=== Look for the root-level loan routes (lines 1-80) ==="
sed -n '1,80p' src/app/loans/loans-routing.module.tsRepository: openMF/web-app
Length of output: 8530
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search delinquencytags with full upstream context ==="
rg -B30 'delinquencytags' src/app/loans/loans-routing.module.ts | head -80Repository: openMF/web-app
Length of output: 1170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify working-capital product flow: search for working-capital routes ==="
rg -n "working.capital|productType.*working" src/app/loans --glob '*.ts' | head -20
echo
echo "=== Verify LoansViewComponent uses product type ==="
rg -n "productType|isLoanProduct|isWorkingCapital" src/app/loans/loans-view/loans-view.component.ts | head -15Repository: openMF/web-app
Length of output: 1504
Return an Observable for all code paths in the resolver.
The resolver has two unsafe branches:
-
Numeric coercion on
loanId:+null === 0, so!isNaN(+null)passes the guard even whenloanIdisnull, causinggetDelinquencyData(null)to be called unsafely. Guard withloanId !== nullbefore numeric conversion. -
Missing return for working-capital routes: When
productType=working-capitalvia queryParams,isLoanProductisfalse, the innerifblock skips, and the method returnsundefinedinstead of an Observable. This breaks the resolver contract. Return an explicit fallback likeof(null)when the fetch is intentionally skipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts` around
lines 35 - 43, The resolver method resolve calls initialize(route) then fetches
loanId and currently coerces it with +loanId which treats null as 0; first
ensure you check loanId !== null before performing numeric coercion (use the
existing loanId variable from route.paramMap or route.parent.paramMap) so you
never call loansService.getDelinquencyData with null. Second, guarantee an
Observable is returned on every path: if isLoanProduct is false or loanId is
invalid, return a safe fallback Observable (e.g., of(null)) instead of falling
through to undefined; update the resolve method to return
loansService.getDelinquencyData(loanId) only after the loanId !== null &&
!isNaN(+loanId) && this.isLoanProduct checks, otherwise return of(null).
Comment on lines
+1767
to
+1768
| "Delinquency Grace Days": "Giorni di grazia per i delinquenti", | ||
| "Delinquency Start Type": "Tipo di inizio della delinquenza", |
There was a problem hiding this comment.
Replace criminal-context terms with lending-domain terminology.
At Line 1767 and Line 1768, delinquenti/delinquenza is misleading in Italian financial UX. Use morosità terminology instead (e.g., Giorni di grazia della morosità, Tipo di inizio della morosità).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/it-IT.json` around lines 1767 - 1768, The Italian
translations for the keys "Delinquency Grace Days" and "Delinquency Start Type"
use criminal-context terms ("delinquenti"/"delinquenza"); update their values to
lending-domain wording using "morosità" (e.g., change "Giorni di grazia per i
delinquenti" to "Giorni di grazia della morosità" and "Tipo di inizio della
delinquenza" to "Tipo di inizio della morosità") so the UX uses correct
financial terminology; locate the keys "Delinquency Grace Days" and "Delinquency
Start Type" in the translations and replace the Italian strings accordingly.
Comment on lines
+1769
to
+1770
| "Delinquency Grace Days": "비행 청소년 사면 기간", | ||
| "Delinquency Start Type": "비행 시작 유형", |
There was a problem hiding this comment.
Use financial-domain Korean for “delinquency” labels.
These translations currently use “비행” (misconduct), which is incorrect in loan context. This should use “연체” terminology to avoid user confusion.
💡 Suggested fix
- "Delinquency Grace Days": "비행 청소년 사면 기간",
- "Delinquency Start Type": "비행 시작 유형",
+ "Delinquency Grace Days": "연체 유예 일수",
+ "Delinquency Start Type": "연체 시작 유형",📝 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.
Suggested change
| "Delinquency Grace Days": "비행 청소년 사면 기간", | |
| "Delinquency Start Type": "비행 시작 유형", | |
| "Delinquency Grace Days": "연체 유예 일수", | |
| "Delinquency Start Type": "연체 시작 유형", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/ko-KO.json` around lines 1769 - 1770, The Korean
translations for the keys "Delinquency Grace Days" and "Delinquency Start Type"
use 비행 (misconduct) instead of the financial term 연체; update the values for
those keys to use 연체 (e.g., "Delinquency Grace Days": "연체 유예 기간" and
"Delinquency Start Type": "연체 시작 유형") so they reflect correct loan/financial
terminology and preserve original key names ("Delinquency Grace Days",
"Delinquency Start Type").
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/assets/translations/es-MX.json (1)
4177-4177:⚠️ Potential issue | 🟡 MinorFinish the terminology switch in this tooltip.
This string still mixes
dispersiónwithministraciones múltiples, so the updated loan vocabulary remains inconsistent in the same sentence.✏️ Suggested copy
- "Leave this checkbox unchecked": "Deje esta casilla sin marcar si el Crédito es de dispersión única. Marque esta casilla de verificación si el Crédito es un Crédito de ministraciones múltiples. Consulte los campos adicionales para obtener información adicional requerida para este tipo de Crédito.", + "Leave this checkbox unchecked": "Deje esta casilla sin marcar si el Crédito es de dispersión única. Marque esta casilla si el Crédito es de dispersión múltiple. Consulte los campos adicionales para obtener la información requerida para este tipo de Crédito.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 4177, The tooltip value for the translation key "Leave this checkbox unchecked" mixes two loan terms ("dispersión" and "ministraciones múltiples"); update the string so both occurrences use the new consistent vocabulary (e.g., change "dispersión" to "ministración" or "ministración única" to match "ministraciones múltiples"), keeping the rest of the sentence intact and preserving punctuation and context.
🤖 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/loans-account-and-template.resolver.ts`:
- Line 24: The resolver stores productType on the instance in LoanBaseResolver
via initialize(route), making endpoint selection stateful; change initialize so
it does not persist productType on the resolver instance but instead derives
productType from the incoming route each time (or explicitly reset/clear any
stored productType at start of initialize), and update any endpoint selection
logic in LoansAccountAndTemplateResolver to use the local productType from the
current route rather than an instance field; also tighten the loanId check by
replacing !isNaN(+loanId) with an explicit null/undefined guard plus numeric
test (e.g. loanId != null && !isNaN(Number(loanId))) so null is not treated as
0.
In `@src/assets/translations/es-MX.json`:
- Line 3816: Update the Spanish translation for the permission key
DISBURSALLASTUNDO so it matches the existing menu copy; replace the value
"Reversar Última dispersión" with "Reversar la última dispersión" in the
src/assets/translations/es-MX.json entry for DISBURSALLASTUNDO to ensure
consistent wording with the string already used elsewhere.
- Line 1171: The translation for the JSON key "Loan Disbursal" currently uses
the plural value "Dispersión de créditos"; update this value to the singular
form "Dispersión del Crédito" to match the other occurrences (e.g., lines using
"Dispersión del Crédito") so copy is consistent across the app.
- Line 2214: Replace the Spanish translation value for the key "Minimum days
between disbursal and first repayment date" so it uses the established term
"Fecha de reembolso" instead of "fecha de amortización"; update the value to
something like "Días mínimos entre la dispersión y la primera Fecha de
reembolso" (matching casing/format used elsewhere, e.g., the existing "Fecha de
reembolso" entry) to keep terminology consistent.
- Line 703: Update the Spanish translations that use the wrong casing/article
for "dispersión": change the "Disbursement" value from "dispersión" to "la
dispersión" and replace any occurrences of the phrase "al momento del
dispersión" with "al momento de la dispersión"; locate and edit the JSON entries
for the "Disbursement" key and any other keys where that phrase appears (the
duplicate at the other occurrence noted) to apply these exact wording fixes.
---
Duplicate comments:
In `@src/assets/translations/es-MX.json`:
- Line 4177: The tooltip value for the translation key "Leave this checkbox
unchecked" mixes two loan terms ("dispersión" and "ministraciones múltiples");
update the string so both occurrences use the new consistent vocabulary (e.g.,
change "dispersión" to "ministración" or "ministración única" to match
"ministraciones múltiples"), keeping the rest of the sentence intact and
preserving punctuation and context.
🪄 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: a73aef24-4e56-49e2-ae49-017ed0e6cf29
📒 Files selected for processing (16)
src/app/loans/common-resolvers/loans-account-and-template.resolver.tssrc/app/shared/input-amount/input-amount.component.htmlsrc/app/shared/input-amount/input-amount.component.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 due to trivial changes (7)
- src/app/shared/input-amount/input-amount.component.html
- src/assets/translations/es-CL.json
- src/assets/translations/ko-KO.json
- src/assets/translations/pt-PT.json
- src/assets/translations/lt-LT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/de-DE.json
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app/shared/input-amount/input-amount.component.ts
- src/assets/translations/it-IT.json
- src/assets/translations/ne-NE.json
- src/assets/translations/en-US.json
- src/assets/translations/lv-LV.json
- src/assets/translations/sw-SW.json
| */ | ||
| @Injectable() | ||
| export class LoansAccountAndTemplateResolver { | ||
| export class LoansAccountAndTemplateResolver extends LoanBaseResolver { |
There was a problem hiding this comment.
This branch can select the wrong API after a previous navigation.
LoanBaseResolver keeps productType on the resolver instance, so the new initialize(route) call makes endpoint selection stateful. When a later navigation omits productType, a reused resolver can keep 'working-capital' and route a plain loan edit through the wrong service; !isNaN(+loanId) also still treats null as valid because +null === 0.
Suggested change
resolve(route: ActivatedRouteSnapshot): Observable<any> {
- this.initialize(route);
- const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId');
- if (!isNaN(+loanId)) {
- return this.isLoanProduct
+ const productType = route.queryParamMap.get('productType');
+ const isLoanProduct = productType !== 'working-capital';
+ const loanId = route.paramMap.get('loanId') ?? route.parent?.paramMap.get('loanId');
+ if (loanId !== null && loanId.trim() !== '' && !Number.isNaN(Number(loanId))) {
+ return isLoanProduct
? this.loansService.getLoansAccountAndTemplateResource(loanId)
: this.loansService.getWorkingCapitalLoanDetails(loanId);
}
return EMPTY;
}Also applies to: 36-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/common-resolvers/loans-account-and-template.resolver.ts` at
line 24, The resolver stores productType on the instance in LoanBaseResolver via
initialize(route), making endpoint selection stateful; change initialize so it
does not persist productType on the resolver instance but instead derives
productType from the incoming route each time (or explicitly reset/clear any
stored productType at start of initialize), and update any endpoint selection
logic in LoansAccountAndTemplateResolver to use the local productType from the
current route rather than an instance field; also tighten the loanId check by
replacing !isNaN(+loanId) with an explicit null/undefined guard plus numeric
test (e.g. loanId != null && !isNaN(Number(loanId))) so null is not treated as
0.
| "Minimum Required Balance": "Balance mínimo requerido", | ||
| "Minimum Value": "Valor mínimo", | ||
| "Minimum days between disbursal and first repayment date": "Días mínimos entre la ministración y la primera fecha de amortización", | ||
| "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de amortización", |
There was a problem hiding this comment.
Don’t translate repayment date as fecha de amortización.
This constraint is about the first repayment date, and the locale already uses Fecha de reembolso for that concept on Line 2454. Fecha de amortización shifts the meaning.
✏️ Suggested copy
- "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de amortización",
+ "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de reembolso",📝 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.
Suggested change
| "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de amortización", | |
| "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de reembolso", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/es-MX.json` at line 2214, Replace the Spanish
translation value for the key "Minimum days between disbursal and first
repayment date" so it uses the established term "Fecha de reembolso" instead of
"fecha de amortización"; update the value to something like "Días mínimos entre
la dispersión y la primera Fecha de reembolso" (matching casing/format used
elsewhere, e.g., the existing "Fecha de reembolso" entry) to keep terminology
consistent.
2beef89 to
6db0569
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts (1)
35-43:⚠️ Potential issue | 🔴 CriticalResolver must return an
Observableon all code paths.Two issues remain unaddressed:
Null coercion:
+null === 0passes!isNaN(+loanId), allowinggetDelinquencyData(null)to be called whenloanIdisnull.Missing return: When
loanIdis invalid orisLoanProductisfalse, the method returnsundefined. This breaks the resolver contract and will crash consumers that expectdata.loanDelinquencyData.delinquent(seeloan-delinquency-tags-tab.component.tslines 119-120).🐛 Proposed fix
+import { Observable, of } from 'rxjs'; ... - resolve(route: ActivatedRouteSnapshot): Observable<any> { + resolve(route: ActivatedRouteSnapshot): Observable<any> { this.initialize(route); const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); - if (!isNaN(+loanId)) { - if (this.isLoanProduct) { - return this.loansService.getDelinquencyData(loanId); - } + if (loanId !== null && !isNaN(+loanId) && this.isLoanProduct) { + return this.loansService.getDelinquencyData(loanId); } + return of(null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts` around lines 35 - 43, The resolver resolve method must always return an Observable and must not treat null as a valid numeric id; update resolve (in loan-delinquency-data.resolver.ts) to (1) explicitly check loanId !== null and validate it's a numeric id (e.g., Number.isFinite(Number(loanId)) or /^\d+$/.test(loanId)) before calling this.loansService.getDelinquencyData(loanId), and (2) return a fallback Observable on all other paths (e.g., import { of } from 'rxjs' and return of(null) or EMPTY) so that when isLoanProduct is false or loanId is invalid the method still returns an Observable rather than undefined.
🤖 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/loan-originators.resolver.ts`:
- Around line 31-33: The resolve() method currently only returns when
this.loanProductService.isLoanProduct is true, causing undefined when false;
update resolve() to always return an Observable by returning the same-shaped
fallback observable when loanProductService.isLoanProduct is false (use rxjs
'of' to emit an object matching the API shape expected by the component, e.g. an
object with loanOriginatorsData and originators array), ensuring callers of
loansService.getLoanOriginators and consumers of
data.loanOriginatorsData.originators can always safely read the fields.
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html`:
- Around line 523-526: The template directly accesses
loanProduct.delinquencyStartType.value which can be missing; update the
loan-product-summary.component.html to guard that access (e.g., use the safe
navigation operator loanProduct.delinquencyStartType?.value or wrap the span in
an *ngIf="loanProduct.delinquencyStartType") so you only call the translateKey
pipe when delinquencyStartType exists, and optionally provide a fallback
string/empty output when it's null; adjust the span displaying the value (the
one using translateKey: 'catalogs') accordingly.
In `@src/app/shared/validators/amount-value.validator.ts`:
- Around line 16-21: Update the comment explaining the regex in
amount-value.validator.ts to match the actual pattern used: replace the
incorrect description of a lookahead `(?=.{1,14}$)` with a concise explanation
of `^\d{1,13}(\.\d{1,6})?$` — i.e., the integer part is 1–13 digits and an
optional fractional part is a dot followed by 1–6 digits; reference the regex
symbol (^\d{1,13}(\.\d{1,6})?$) in the comment so the documentation aligns with
the implementation (see AMOUNT_REGEX / the regex literal in this file).
---
Duplicate comments:
In `@src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts`:
- Around line 35-43: The resolver resolve method must always return an
Observable and must not treat null as a valid numeric id; update resolve (in
loan-delinquency-data.resolver.ts) to (1) explicitly check loanId !== null and
validate it's a numeric id (e.g., Number.isFinite(Number(loanId)) or
/^\d+$/.test(loanId)) before calling
this.loansService.getDelinquencyData(loanId), and (2) return a fallback
Observable on all other paths (e.g., import { of } from 'rxjs' and return
of(null) or EMPTY) so that when isLoanProduct is false or loanId is invalid the
method still returns an Observable rather than undefined.
🪄 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: a2206232-96cc-4fa1-abe0-688f81420282
📒 Files selected for processing (33)
src/app/directives/directives.module.tssrc/app/directives/positive-number.directive.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/common-resolvers/loan-originators.resolver.tssrc/app/loans/common-resolvers/loans-account-and-template.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/loans/loans.service.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/shared/input-amount/input-amount.component.htmlsrc/app/shared/input-amount/input-amount.component.tssrc/app/shared/validators/amount-value.validator.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 due to trivial changes (11)
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.html
- src/app/loans/create-loans-account/create-loans-account.component.ts
- src/app/loans/loans-view/account-details/account-details.component.html
- src/app/shared/input-amount/input-amount.component.html
- src/assets/translations/ko-KO.json
- src/assets/translations/es-CL.json
- src/assets/translations/fr-FR.json
- src/assets/translations/en-US.json
- src/app/loans/common-resolvers/loans-account-and-template.resolver.ts
- src/assets/translations/cs-CS.json
- src/assets/translations/lt-LT.json
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/directives/directives.module.ts
- src/app/directives/positive-number.directive.ts
- src/app/shared/input-amount/input-amount.component.ts
- src/app/loans/loans.service.ts
- src/app/loans/common-resolvers/loan-details.resolver.ts
- src/app/loans/common-resolvers/loan-base.resolver.ts
- src/assets/translations/pt-PT.json
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts
- src/assets/translations/it-IT.json
- src/assets/translations/ne-NE.json
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
- src/assets/translations/sw-SW.json
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/assets/translations/de-DE.json
- src/assets/translations/lv-LV.json
- src/assets/translations/es-MX.json
Comment on lines
+31
to
+33
| if (this.loanProductService.isLoanProduct) { | ||
| return this.loansService.getLoanOriginators(loanId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '24,36p' src/app/loans/common-resolvers/loan-originators.resolver.ts
sed -n '80,86p' src/app/loans/loans-view/loan-originators-tab/loan-originators-tab.component.ts
sed -n '280,289p' src/app/loans/loans-routing.module.tsRepository: openMF/web-app
Length of output: 1054
🏁 Script executed:
rg -A 10 'getLoanOriginators' src/app/loans --type tsRepository: openMF/web-app
Length of output: 1548
🏁 Script executed:
rg -B 5 -A 10 'loanOriginatorsData' src/app/loans --type tsRepository: openMF/web-app
Length of output: 4964
🏁 Script executed:
grep -r 'isLoanProduct' src/app/loans --include='*.ts' | head -20Repository: openMF/web-app
Length of output: 2458
Return an observable on the non-loan-product path to prevent undefined resolver data.
The resolve() method falls through without returning an observable when this.loanProductService.isLoanProduct is false. This violates the Observable<any> return contract and causes the component to fail when it attempts to dereference data.loanOriginatorsData.originators on an undefined value.
Return a fallback observable that matches the expected API response structure:
Suggested fix
-import { Observable, throwError } from 'rxjs';
+import { Observable, of, throwError } from 'rxjs';
...
if (this.loanProductService.isLoanProduct) {
return this.loansService.getLoanOriginators(loanId);
}
+ return of({ originators: [] });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/common-resolvers/loan-originators.resolver.ts` around lines 31
- 33, The resolve() method currently only returns when
this.loanProductService.isLoanProduct is true, causing undefined when false;
update resolve() to always return an Observable by returning the same-shaped
fallback observable when loanProductService.isLoanProduct is false (use rxjs
'of' to emit an object matching the API shape expected by the component, e.g. an
object with loanOriginatorsData and originators array), ensuring callers of
loansService.getLoanOriginators and consumers of
data.loanOriginatorsData.originators can always safely read the fields.
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
Outdated
Show resolved
Hide resolved
Comment on lines
+16
to
+21
| // Regex breakdown: | ||
| // ^ - Start of string | ||
| // (?=.{1,14}$) - Lookahead to ensure total length (including potential dot) | ||
| // [0-9]+ - One or more digits | ||
| // (\.[0-9]{1,6})? - Optional dot followed by 1 to 6 digits | ||
| // $ - End of string |
There was a problem hiding this comment.
Correct the regex description comment.
The comment on line 18 describes a lookahead assertion (?=.{1,14}$) that does not exist in the actual regex pattern on line 22. The actual pattern is ^\d{1,13}(\.\d{1,6})?$, which directly constrains the integer part to 1-13 digits and the optional decimal part to 1-6 digits—no lookahead is used.
📝 Proposed fix to correct the documentation
// Regex breakdown:
// ^ - Start of string
- // (?=.{1,14}$) - Lookahead to ensure total length (including potential dot)
- // [0-9]+ - One or more digits
- // (\.[0-9]{1,6})? - Optional dot followed by 1 to 6 digits
+ // \d{1,13} - One to thirteen digits (integer part)
+ // (\.\d{1,6})? - Optional: dot followed by 1 to 6 digits (decimal part)
// $ - End of string🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 21-21: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^\\d{1,${maxTotalDigits - maxDecimals}}(\\.\\d{1,${maxDecimals}})?$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/shared/validators/amount-value.validator.ts` around lines 16 - 21,
Update the comment explaining the regex in amount-value.validator.ts to match
the actual pattern used: replace the incorrect description of a lookahead
`(?=.{1,14}$)` with a concise explanation of `^\d{1,13}(\.\d{1,6})?$` — i.e.,
the integer part is 1–13 digits and an optional fractional part is a dot
followed by 1–6 digits; reference the regex symbol (^\d{1,13}(\.\d{1,6})?$) in
the comment so the documentation aligns with the implementation (see
AMOUNT_REGEX / the regex literal in this file).
6db0569 to
8012ea9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/app/loans/common-resolvers/loan-originators.resolver.ts (1)
13-13:⚠️ Potential issue | 🔴 CriticalAlways return an observable on the working-capital path.
If
isLoanProductis false, Line 34 exitsresolve()without returning anything. That breaks the resolver contract and leavesloanOriginatorsDataundefined for the new non-loan flow. Return an empty observable payload with the same shape asgetLoanOriginators()instead of falling through.Suggested fix
-import { Observable, throwError } from 'rxjs'; +import { Observable, of, throwError } from 'rxjs'; ... if (this.loanProductService.isLoanProduct) { return this.loansService.getLoanOriginators(loanId); } + return of({ originators: [] }); }As per coding guidelines,
src/app/**/*.{service.ts,component.ts,resolver.ts}should use RxJS Observables, andsrc/app/**should keep strict type safety and clean observable patterns.Also applies to: 31-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-originators.resolver.ts` at line 13, The resolve() method currently exits without returning when isLoanProduct is false, breaking the resolver contract; modify resolve() (in loan-originators.resolver) to always return an Observable by returning an empty observable payload shaped like getLoanOriginators() when isLoanProduct is false (e.g., Observable.of or of({...}) with the same properties/type as getLoanOriginators() result), ensuring loanOriginatorsData is defined and types remain strict; keep the existing branch for the loan path that calls getLoanOriginators() unchanged but replace the fall-through return with a typed empty observable.src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts (1)
35-43:⚠️ Potential issue | 🟠 MajorHandle the invalid/non-loan branches explicitly.
Line 37 still lets
nullthrough because+null === 0, so this can callgetDelinquencyData(null). On the new working-capital path,this.isLoanProductis false and the resolver also falls through without an explicit observable.🩹 Proposed fix
resolve(route: ActivatedRouteSnapshot): Observable<any> { this.initialize(route); - const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); - if (!isNaN(+loanId)) { - if (this.isLoanProduct) { - return this.loansService.getDelinquencyData(loanId); - } - } + const loanId = route.paramMap.get('loanId') ?? route.parent?.paramMap.get('loanId') ?? null; + if (loanId !== null && !isNaN(+loanId) && this.isLoanProduct) { + return this.loansService.getDelinquencyData(loanId); + } + return of(null); }Also update the RxJS import:
import { Observable, of } from 'rxjs';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts` around lines 35 - 43, The resolver's resolve method can call getDelinquencyData with null (because +null === 0) or fall through without returning an Observable when this.isLoanProduct is false; update resolve in loan-delinquency-data.resolver.ts to explicitly validate loanId with a null check (e.g., loanId != null && !isNaN(Number(loanId))) and return the delinquency data only when valid and isLoanProduct is true, otherwise return an explicit Observable like of(null) (so callers always get an Observable); also add the RxJS import for of by importing Observable and of from 'rxjs'.
🧹 Nitpick comments (1)
src/app/shared/validators/amount-value.validator.ts (1)
13-22: Consider using a static regex literal.Since
maxTotalDigitsandmaxDecimalsare hardcoded constants that never change, constructing the regex dynamically adds unnecessary complexity and triggers the static analysis ReDoS warning (which is a false positive here since no user input is involved). A static regex would be cleaner and self-documenting.♻️ Suggested refactor
export function amountValueValidator(): ValidatorFn { return (control: AbstractControl): ValidationErrors | null => { if (!control.value) return null; - const maxTotalDigits = 19; - const maxDecimals = 6; // Regex breakdown: // ^ - Start of string - // (?=.{1,14}$) - Lookahead to ensure total length (including potential dot) - // [0-9]+ - One or more digits - // (\.[0-9]{1,6})? - Optional dot followed by 1 to 6 digits + // \d{1,13} - 1 to 13 integer digits + // (\.\d{1,6})? - Optional: dot followed by 1 to 6 decimal digits // $ - End of string - const regex = new RegExp(`^\\d{1,${maxTotalDigits - maxDecimals}}(\\.\\d{1,${maxDecimals}})?$`); + const regex = /^\d{1,13}(\.\d{1,6})?$/; const valid = regex.test(control.value.toString()); return valid ? null : { highAmountValue: true }; }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/validators/amount-value.validator.ts` around lines 13 - 22, The dynamic RegExp construction using maxTotalDigits and maxDecimals triggers a static-analysis ReDoS warning and is unnecessary because those constants are fixed; replace the RegExp created with new RegExp(...) by a static regex literal assigned to the same regex variable so the pattern remains /^\d{1,<calculated>}(\.\d{1,6})?$/ equivalent to the original, keep or remove the now-unused maxTotalDigits/maxDecimals constants as desired, and run tests to ensure AmountValueValidator (the module using regex) still validates the same ranges.
🤖 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/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`:
- Around line 70-73: The input for days (formControlName "delinquencyGraceDays")
allows decimals; constrain it to integers by adding a step="1" attribute to the
<input> in loans-account-terms-step.component.html and by enforcing integer
validation on the FormControl in LoansAccountTermsStepComponent (add
Validators.pattern(/^\d+$/) and keep Validators.min(0) or equivalent). Also
ensure any submit/path that reads the control parses/uses integer values (e.g.,
parseInt) so decimals cannot be persisted.
---
Duplicate comments:
In `@src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts`:
- Around line 35-43: The resolver's resolve method can call getDelinquencyData
with null (because +null === 0) or fall through without returning an Observable
when this.isLoanProduct is false; update resolve in
loan-delinquency-data.resolver.ts to explicitly validate loanId with a null
check (e.g., loanId != null && !isNaN(Number(loanId))) and return the
delinquency data only when valid and isLoanProduct is true, otherwise return an
explicit Observable like of(null) (so callers always get an Observable); also
add the RxJS import for of by importing Observable and of from 'rxjs'.
In `@src/app/loans/common-resolvers/loan-originators.resolver.ts`:
- Line 13: The resolve() method currently exits without returning when
isLoanProduct is false, breaking the resolver contract; modify resolve() (in
loan-originators.resolver) to always return an Observable by returning an empty
observable payload shaped like getLoanOriginators() when isLoanProduct is false
(e.g., Observable.of or of({...}) with the same properties/type as
getLoanOriginators() result), ensuring loanOriginatorsData is defined and types
remain strict; keep the existing branch for the loan path that calls
getLoanOriginators() unchanged but replace the fall-through return with a typed
empty observable.
---
Nitpick comments:
In `@src/app/shared/validators/amount-value.validator.ts`:
- Around line 13-22: The dynamic RegExp construction using maxTotalDigits and
maxDecimals triggers a static-analysis ReDoS warning and is unnecessary because
those constants are fixed; replace the RegExp created with new RegExp(...) by a
static regex literal assigned to the same regex variable so the pattern remains
/^\d{1,<calculated>}(\.\d{1,6})?$/ equivalent to the original, keep or remove
the now-unused maxTotalDigits/maxDecimals constants as desired, and run tests to
ensure AmountValueValidator (the module using regex) still validates the same
ranges.
🪄 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: cf8635d5-8363-45e8-a4b8-401a7f5b4d92
📒 Files selected for processing (33)
src/app/directives/directives.module.tssrc/app/directives/positive-number.directive.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/common-resolvers/loan-originators.resolver.tssrc/app/loans/common-resolvers/loans-account-and-template.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/loans/loans.service.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/shared/input-amount/input-amount.component.htmlsrc/app/shared/input-amount/input-amount.component.tssrc/app/shared/validators/amount-value.validator.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 due to trivial changes (15)
- src/app/shared/input-amount/input-amount.component.html
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.html
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
- src/app/loans/create-loans-account/create-loans-account.component.ts
- src/app/loans/loans-view/account-details/account-details.component.html
- src/assets/translations/ne-NE.json
- src/assets/translations/es-CL.json
- src/assets/translations/lv-LV.json
- src/assets/translations/ko-KO.json
- src/assets/translations/pt-PT.json
- src/assets/translations/cs-CS.json
- src/assets/translations/de-DE.json
- src/assets/translations/it-IT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/lt-LT.json
🚧 Files skipped from review as they are similar to previous changes (12)
- src/app/loans/common-resolvers/loan-details.resolver.ts
- src/app/directives/positive-number.directive.ts
- src/app/loans/common-resolvers/loans-account-and-template.resolver.ts
- src/app/loans/loans.service.ts
- src/app/shared/input-amount/input-amount.component.ts
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
- src/app/loans/common-resolvers/loan-base.resolver.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/assets/translations/sw-SW.json
- src/assets/translations/es-MX.json
Comment on lines
+70
to
+73
| <mat-form-field class="flex-48"> | ||
| <mat-label>{{ 'labels.inputs.Delinquency Grace Days' | translate }}</mat-label> | ||
| <input type="number" min="0" matInput formControlName="delinquencyGraceDays" /> | ||
| </mat-form-field> |
There was a problem hiding this comment.
Constrain grace days to whole numbers.
delinquencyGraceDays represents days, but this input currently permits decimals. Please restrict it to integer values.
🔧 Suggested fix
- <input type="number" min="0" matInput formControlName="delinquencyGraceDays" />
+ <input type="number" min="0" step="1" matInput formControlName="delinquencyGraceDays" />📝 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.
Suggested change
| <mat-form-field class="flex-48"> | |
| <mat-label>{{ 'labels.inputs.Delinquency Grace Days' | translate }}</mat-label> | |
| <input type="number" min="0" matInput formControlName="delinquencyGraceDays" /> | |
| </mat-form-field> | |
| <mat-form-field class="flex-48"> | |
| <mat-label>{{ 'labels.inputs.Delinquency Grace Days' | translate }}</mat-label> | |
| <input type="number" min="0" step="1" matInput formControlName="delinquencyGraceDays" /> | |
| </mat-form-field> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`
around lines 70 - 73, The input for days (formControlName
"delinquencyGraceDays") allows decimals; constrain it to integers by adding a
step="1" attribute to the <input> in loans-account-terms-step.component.html and
by enforcing integer validation on the FormControl in
LoansAccountTermsStepComponent (add Validators.pattern(/^\d+$/) and keep
Validators.min(0) or equivalent). Also ensure any submit/path that reads the
control parses/uses integer values (e.g., parseInt) so decimals cannot be
persisted.
8012ea9 to
59372f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html (1)
79-82:⚠️ Potential issue | 🟡 MinorConstrain delinquency grace days to whole numbers.
Line 81 allows decimals for a “days” field. Please enforce integer input in the control.
🔧 Suggested fix
- <input type="number" min="0" matInput formControlName="delinquencyGraceDays" /> + <input type="number" min="0" step="1" matInput formControlName="delinquencyGraceDays" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 79 - 82, The delinquencyGraceDays input must accept only whole numbers: update the template input (the element with formControlName="delinquencyGraceDays") to include step="1" and a digit-only pattern (e.g., input attribute pattern="\d*" and/or inputmode="numeric") to prevent decimals in the UI, and update the LoansAccountTermsStepComponent FormControl for 'delinquencyGraceDays' to add a validator like Validators.pattern(/^\d+$/) (and optionally clamp/Math.floor values in the control's valueChanges subscription) so the model never stores decimals.src/assets/translations/es-MX.json (5)
2215-2215:⚠️ Potential issue | 🟡 MinorUse
reembolsoinstead ofamortizaciónfor first repayment date.Line 2215 shifts meaning by translating repayment date as
fecha de amortización.✏️ Suggested patch
- "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de amortización", + "Minimum days between disbursal and first repayment date": "Días mínimos entre la dispersión y la primera fecha de reembolso",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 2215, The Spanish translation for the key "Minimum days between disbursal and first repayment date" uses "fecha de amortización" which is incorrect; update the value to use "fecha de reembolso" (i.e., replace "Días mínimos entre la dispersión y la primera fecha de amortización" with "Días mínimos entre la dispersión y la primera fecha de reembolso") so the term matches domain terminology.
1172-1172:⚠️ Potential issue | 🟡 MinorKeep
Loan Disbursalconsistent with the singular form used elsewhere.Line 1172 uses
Dispersión de créditos, while other updated occurrences useDispersión del Crédito.✏️ Suggested patch
- "Loan Disbursal": "Dispersión de créditos", + "Loan Disbursal": "Dispersión del Crédito",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 1172, Update the translation for the key "Loan Disbursal" to use the singular form used elsewhere by replacing "Dispersión de créditos" with "Dispersión del Crédito" so the key "Loan Disbursal" matches the other occurrences' phrasing.
3817-3817:⚠️ Potential issue | 🟡 MinorNormalize the
DISBURSALLASTUNDOpermission label copy.Line 3817 is still missing the article and has awkward capitalization.
✏️ Suggested patch
- "DISBURSALLASTUNDO": "Reversar Última dispersión", + "DISBURSALLASTUNDO": "Reversar la última dispersión",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 3817, The translation entry DISBURSALLASTUNDO should be normalized to include the definite article and proper lowercase/accents; update the JSON value for the key DISBURSALLASTUNDO to a natural Spanish label such as "Deshacer la última dispersión" (replace the current "Reversar Última dispersión") and keep JSON string formatting consistent with surrounding entries.
4178-4178:⚠️ Potential issue | 🟡 MinorFinish replacing
ministracioneswithdispersiónin this tooltip.Line 4178 is inconsistent with the rest of the updated terminology.
✏️ Suggested patch
- "Leave this checkbox unchecked": "Deje esta casilla sin marcar si el Crédito es de dispersión única. Marque esta casilla de verificación si el Crédito es un Crédito de ministraciones múltiples. Consulte los campos adicionales para obtener información adicional requerida para este tipo de Crédito.", + "Leave this checkbox unchecked": "Deje esta casilla sin marcar si el Crédito es de dispersión única. Marque esta casilla si el Crédito es de dispersión múltiple. Consulte los campos adicionales para obtener la información adicional requerida para este tipo de Crédito.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 4178, Update the Spanish translation value for the key "Leave this checkbox unchecked": replace the word "ministraciones" with "dispersión" so the tooltip consistently uses "dispersión" (e.g., change "...Crédito de ministraciones múltiples..." to "...Crédito de dispersión múltiples...") and ensure the rest of the sentence remains grammatically correct.
703-703:⚠️ Potential issue | 🟡 MinorFix the disbursement wording/casing in these catalog strings.
Line 703 uses lowercase (
dispersión) while nearby catalog values are title-cased, and Line 814 still has the incorrect phrasedel dispersión(should bede la dispersión).✏️ Suggested patch
- "Disbursement": "dispersión", + "Disbursement": "Dispersión", ... - "Repayment (at time of disbursement)": "Reembolso (al momento del dispersión)", + "Repayment (at time of disbursement)": "Reembolso (al momento de la dispersión)",Also applies to: 814-814
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 703, Update the Spanish catalog entries so the "Disbursement" value uses title case and correct gendered phrasing: change the value for the "Disbursement" key from "dispersión" to "Dispersión" and fix the other occurrence that currently reads "del dispersión" to the grammatically correct "de la dispersión"; ensure both entries match the surrounding title-casing convention in the JSON (look for the "Disbursement" key and the offending phrase "del dispersión" to locate the lines).
🤖 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/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html`:
- Around line 110-115: The preview currently uses a truthy check that hides a
valid 0 value for loansAccount.delinquencyGraceDays; update the ngIf to
explicitly check for null/undefined instead (e.g. use
loansAccount?.delinquencyGraceDays != null or loansAccount.delinquencyGraceDays
!== null && loansAccount.delinquencyGraceDays !== undefined) in
loans-account-preview-step.component.html so that 0 renders while null/undefined
still hide the element.
---
Duplicate comments:
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`:
- Around line 79-82: The delinquencyGraceDays input must accept only whole
numbers: update the template input (the element with
formControlName="delinquencyGraceDays") to include step="1" and a digit-only
pattern (e.g., input attribute pattern="\d*" and/or inputmode="numeric") to
prevent decimals in the UI, and update the LoansAccountTermsStepComponent
FormControl for 'delinquencyGraceDays' to add a validator like
Validators.pattern(/^\d+$/) (and optionally clamp/Math.floor values in the
control's valueChanges subscription) so the model never stores decimals.
In `@src/assets/translations/es-MX.json`:
- Line 2215: The Spanish translation for the key "Minimum days between disbursal
and first repayment date" uses "fecha de amortización" which is incorrect;
update the value to use "fecha de reembolso" (i.e., replace "Días mínimos entre
la dispersión y la primera fecha de amortización" with "Días mínimos entre la
dispersión y la primera fecha de reembolso") so the term matches domain
terminology.
- Line 1172: Update the translation for the key "Loan Disbursal" to use the
singular form used elsewhere by replacing "Dispersión de créditos" with
"Dispersión del Crédito" so the key "Loan Disbursal" matches the other
occurrences' phrasing.
- Line 3817: The translation entry DISBURSALLASTUNDO should be normalized to
include the definite article and proper lowercase/accents; update the JSON value
for the key DISBURSALLASTUNDO to a natural Spanish label such as "Deshacer la
última dispersión" (replace the current "Reversar Última dispersión") and keep
JSON string formatting consistent with surrounding entries.
- Line 4178: Update the Spanish translation value for the key "Leave this
checkbox unchecked": replace the word "ministraciones" with "dispersión" so the
tooltip consistently uses "dispersión" (e.g., change "...Crédito de
ministraciones múltiples..." to "...Crédito de dispersión múltiples...") and
ensure the rest of the sentence remains grammatically correct.
- Line 703: Update the Spanish catalog entries so the "Disbursement" value uses
title case and correct gendered phrasing: change the value for the
"Disbursement" key from "dispersión" to "Dispersión" and fix the other
occurrence that currently reads "del dispersión" to the grammatically correct
"de la dispersión"; ensure both entries match the surrounding title-casing
convention in the JSON (look for the "Disbursement" key and the offending phrase
"del dispersión" to locate the lines).
🪄 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: 8f089dc4-f689-44eb-9636-b045b4303b2a
📒 Files selected for processing (30)
src/app/directives/positive-number.directive.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/common-resolvers/loan-originators.resolver.tssrc/app/loans/common-resolvers/loans-account-and-template.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/edit-loans-account/edit-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/shared/input-amount/input-amount.component.htmlsrc/app/shared/input-amount/input-amount.component.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 due to trivial changes (11)
- src/app/shared/input-amount/input-amount.component.html
- src/app/directives/positive-number.directive.ts
- src/app/loans/create-loans-account/create-loans-account.component.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/loans/loans-view/account-details/account-details.component.html
- src/assets/translations/pt-PT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/cs-CS.json
- src/assets/translations/en-US.json
- src/assets/translations/ne-NE.json
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.html
🚧 Files skipped from review as they are similar to previous changes (15)
- src/app/loans/common-resolvers/loan-originators.resolver.ts
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
- src/assets/translations/es-CL.json
- src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
- src/assets/translations/ko-KO.json
- src/assets/translations/de-DE.json
- src/app/loans/common-resolvers/loans-account-and-template.resolver.ts
- src/app/shared/input-amount/input-amount.component.ts
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/assets/translations/lv-LV.json
- src/assets/translations/sw-SW.json
- src/assets/translations/lt-LT.json
- src/assets/translations/it-IT.json
Comment on lines
+110
to
+115
| @if (loansAccount.delinquencyGraceDays) { | ||
| <div class="flex-fill"> | ||
| <span class="flex-30">{{ 'labels.inputs.Delinquency Grace Days' | translate }}:</span> | ||
| <span class="flex-70"> {{ loansAccount.delinquencyGraceDays | formatNumber }}</span> | ||
| </div> | ||
| } |
There was a problem hiding this comment.
Render 0 grace days in preview.
Line 110 uses a truthy check, so a valid value of 0 won’t render. Use a null/undefined guard instead.
🔧 Suggested fix
- `@if` (loansAccount.delinquencyGraceDays) {
+ `@if` (loansAccount.delinquencyGraceDays !== null && loansAccount.delinquencyGraceDays !== undefined) {
<div class="flex-fill">
<span class="flex-30">{{ 'labels.inputs.Delinquency Grace Days' | translate }}:</span>
<span class="flex-70"> {{ loansAccount.delinquencyGraceDays | formatNumber }}</span>
</div>
}📝 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.
Suggested change
| @if (loansAccount.delinquencyGraceDays) { | |
| <div class="flex-fill"> | |
| <span class="flex-30">{{ 'labels.inputs.Delinquency Grace Days' | translate }}:</span> | |
| <span class="flex-70"> {{ loansAccount.delinquencyGraceDays | formatNumber }}</span> | |
| </div> | |
| } | |
| `@if` (loansAccount.delinquencyGraceDays !== null && loansAccount.delinquencyGraceDays !== undefined) { | |
| <div class="flex-fill"> | |
| <span class="flex-30">{{ 'labels.inputs.Delinquency Grace Days' | translate }}:</span> | |
| <span class="flex-70"> {{ loansAccount.delinquencyGraceDays | formatNumber }}</span> | |
| </div> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html`
around lines 110 - 115, The preview currently uses a truthy check that hides a
valid 0 value for loansAccount.delinquencyGraceDays; update the ngIf to
explicitly check for null/undefined instead (e.g. use
loansAccount?.delinquencyGraceDays != null or loansAccount.delinquencyGraceDays
!== null && loansAccount.delinquencyGraceDays !== undefined) in
loans-account-preview-step.component.html so that 0 renders while null/undefined
still hide the element.
59372f2 to
e01d3c4
Compare
alberto-art3ch
force-pushed
the
WEB-670/working-capital-loan-delinquency-grace-days
branch
from
e01d3c4 to
0970516
Compare
This was referenced Apr 2, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
For supporting business requirements Fineract needs to support “start date” (of delinquency range) and “grace days”.
Grace days will be defining how many extra days to be added to the “start date”.
Related issues and discussion
WEB-670
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
New Features
Improvements
Localization