WEB-505 feat:implement waive penalties for loan repayment and resch…#2941
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loan Reschedule Component src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html, .../loan-reschedule.component.scss, .../loan-reschedule.component.ts |
Adds waive-penalties checkbox and conditional penalty list UI; new styles for penalty items; TypeScript integrates PenaltyManagementService, adds waivePenalties form control, penalties/selection state, load/setup methods, selection helpers, and includes selected penalty IDs in submission payload. |
Make-Repayment Component src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html, .../make-repayment.component.scss, .../make-repayment.component.ts |
Replaces payment-details toggle with mat-slide-toggle; adds waive-penalties slide-toggle and conditional penalty list UI; new styles for penalty items; TypeScript loads penalties, manages selection state, recalculates transaction amount based on waived penalties, and attaches selected penalty IDs on submit. |
Loan / Penalty Services src/app/loans/loans.service.ts, src/app/loans/services/penalty-management.service.ts |
Adds getLoanCharges(loanId) to LoansService. New PenaltyManagementService (providedIn: 'root') exposes load/filter/sort for penalty charges, selection helpers (toggle all, toggle single, isSelected), and display-key normalization. |
Translations src/assets/translations/*.json (cs-CS, de-DE, en-US, es-CL, es-MX, fr-FR, it-IT, ko-KO, lt-LT, lv-LV, ne-NE, pt-PT, sw-SW) |
Adds localization keys such as "Overdue", "Waive Penalties", "Overdue Fees", and "No penalties found" across locale files; minor wording adjustments in some files. |
Sequence Diagram(s)
sequenceDiagram
actor User
participant Component as LoanReschedule / MakeRepayment
participant PenaltySvc as PenaltyManagementService
participant LoansSvc as LoansService
participant Backend
User->>Component: Component initializes
Component->>PenaltySvc: loadPenalties(loanId)
PenaltySvc->>LoansSvc: getLoanCharges(loanId)
LoansSvc->>Backend: GET /loans/{loanId}/charges
Backend-->>LoansSvc: charges payload
LoansSvc-->>PenaltySvc: charges
rect rgb(220,235,255)
Note over PenaltySvc: filter penalty charges,\nparse due dates, sort
end
PenaltySvc-->>Component: penalties[]
Component->>Component: store penalties, init UI state
User->>Component: toggle "Waive Penalties"
Component->>Component: show penalty list
User->>Component: select/deselect penalties
Component->>Component: togglePenaltySelection()\nupdate selectedPenalties
Component->>Component: recalculateTransactionAmount()
User->>Component: submit form
Component->>Backend: POST submission (includes selected penalty chargeIds)
Backend-->>Component: success/failure
Component->>User: show result
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
- gkbishnoi07
- IOhacker
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly identifies the main change: implementing waive penalties feature for loan repayment and reschedule flows, which aligns with the substantial code additions across multiple components. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Warning
Review ran into problems
🔥 Problems
Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.
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.
shubhamkumar9199
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scss (1)
17-31: Consider responsive design for narrow viewports.The grid layout with 5 columns and the third column using
minmax(140px, 100%)may cause layout issues on narrow screens or mobile devices. Consider:
- Adding responsive breakpoints using
@mediaqueries to reduce columns on smaller screens- Testing the layout at <640px viewport widths
- Using
overflow-x: autoif horizontal scrolling is acceptable, or restructuring to a vertical layout on mobileExample approach:
🔎 Suggested responsive enhancement
.penalty-item-container { width: 100%; max-width: 100%; display: grid; grid-template-columns: auto minmax(80px, 110px) minmax(140px, 100%) minmax(130px, 170px) minmax(90px, 120px); align-items: center; gap: 8px; margin-top: 10px; padding: 12px 16px; background-color: #f5f5f5; border: 1px solid #e0e0e0; border-radius: 4px; box-sizing: border-box; overflow: hidden; + + @media (max-width: 768px) { + grid-template-columns: 1fr; + gap: 4px; + } }src/assets/translations/en-US.json (1)
3539-3539: Clarify unrelated translation change.The addition of "UploadDocumentHint" on line 3539 appears unrelated to the penalty waiving feature described in the PR. While the AI summary mentions this key was "temporarily removed and then re-introduced," it's unclear why this document upload translation is included in a PR focused on loan penalty management.
Is this intentional cleanup or an accidental inclusion from another branch?
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss (1)
5-45: Penalty grid styling works, but consider responsiveness and reuseThe layout and dark-theme override look fine, but:
- The sum of
min-width/minmaxvalues across 5 columns can exceed narrow viewports; withoverflow: hiddenthis may clip content rather than reflowing. Usingfrunits (for the text columns) instead ofminmax(..., 100%)would be more robust on small screens.- The same
.penalty-*rules appear to be needed in multiple loan-action components; if this pattern grows, consider a shared SCSS partial or a common penalty-item component to avoid style duplication.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html (1)
120-154: Guard against undefined penalties array and consider de-duplicating the penalty blockThe waive‑penalties flow here looks coherent and matches the intended UX. Two small points:
@if (penalties.length > 0)assumespenaltiesis always a defined array; if it can beundefinedbefore loading, initialize it to[]in the component/service to avoid template errors.- The penalties list (select‑all, per‑item checkboxes, layout) seems effectively identical to the make‑repayment view. If more flows start using this, consider a shared penalty‑selection component/fragment to centralize behavior and layout.
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html (1)
92-134: Penalty-waiver UI wiring looks good; double-check state initialization and reuse opportunitiesThe revised toggles and penalty-waiver block are consistent with the reschedule flow and should behave as expected, provided that:
waivePenaltiesandshowPaymentDetailsbooleans are kept in sync with any related form state in the component (e.g.,addPaymentDetails()andtoggleWaivePenalties()correctly toggle these flags and adjust validators/amounts as needed).penaltiesis always initialized to an empty array before first render so thatpenalties.lengthis safe while data loads.Given the near-identical penalty list between make‑repayment and reschedule, you may later want to extract a shared penalty‑selection component or directive to avoid duplication and keep behavior (e.g., select‑all logic) in one place.
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
161-169: Consider guarding or cleaning up the waivePenalties valueChanges subscription
setupWaivePenaltiesListener()subscribes tothis.waivePenalties.valueChangeswithout any teardown. In practice this component is short‑lived, but for cleaner observable patterns you could adopt adestroy$+takeUntilpattern and implementOnDestroy, or usetake(1)if you only ever need a single reaction.src/app/loans/services/penalty-management.service.ts (2)
25-60: Penalty loading/filtering is robust; consider documenting the magicchargeTimeType.id === 9The way
loadPenalties()normalizes different response shapes and then relies onfilterPenaltyCharges()for penalty selection and outstanding logic is sensible and defensive. The outstanding calculation (amountOutstandingwith a safe fallback toamount) ensures only charges with a positive balance are exposed to the UI.The only brittle bit is the hard‑coded
charge.chargeTimeType.id === 9as the “overdue” type. If this ID is a Fineract enum constant, consider extracting it into a named constant (e.g.OVERDUE_CHARGE_TIME_TYPE_ID) and/or documenting the coupling so future backend changes don’t silently break the filter.Also applies to: 72-103
111-189: Selection helpers and display-key normalization are clean; types could be tightenedThe selection helpers (
toggleSelectAllPenalties,togglePenaltySelection,isPenaltySelected) are straightforward and make it easy for components to stay dumb.getPenaltyDisplayKey()centralizes the mapping of backend penalty names/aliases (e.g.MORA,OVERDUE,OVERDUE FEES) tolabels.inputs.*translation keys, which is a good place to keep that knowledge.Two small improvements you might consider:
- Introduce a
PenaltyChargeinterface so these methods don’t need to rely onanyand implicit.id/.nameshapes.- Ensure all locales define
labels.inputs.Overdueandlabels.inputs.Overdue Fees; otherwise the fallback string will show up untranslated.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scsssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans.service.tssrc/app/loans/services/penalty-management.service.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
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scsssrc/app/loans/loans.service.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/services/penalty-management.service.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T14:24:31.429Z
Learnt from: alberto-art3ch
Repo: openMF/web-app PR: 2876
File: src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts:91-94
Timestamp: 2025-12-09T14:24:31.429Z
Learning: In the LoanReagingComponent (src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts), the transactionAmount field uses both Validators.min and Validators.max set to the same value (loanTransactionData.amount). This is intentional - it requires users to enter the exact amount as a confirmation/validation step before submitting the re-age operation.
Applied to files:
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
🔇 Additional comments (18)
src/assets/translations/it-IT.json (3)
1110-1110: LGTM! Accurate translation for "Overdue"The Italian translation "Scaduto" correctly conveys the meaning of "Overdue" in a financial context.
2362-2362: LGTM! Consistent translation for "Waive Penalties"The translation "Rinuncia alle penalità" is grammatically correct and maintains consistency with other "Waive" translations in the file (e.g., "Rinuncia all'addebito" for "Waive Charge", "Rinuncia agli interessi" for "Waive Interest").
3174-3174: LGTM! Proper Italian translation for "No penalties found"The translation "Nessuna penalità trovata" is grammatically correct and uses consistent terminology with the other penalty-related translations in this file.
src/assets/translations/de-DE.json (1)
1110-1110: German translations for the penalty waiver feature look good.The new translations for the waive penalties feature are accurate and consistent:
- "Overdue" → "Überfällig"
- "Waive Penalties" → "Auf Strafen verzichten"
- "No penalties found" → "Keine Strafen gefunden"
Also applies to: 2362-2362, 3173-3173
src/assets/translations/pt-PT.json (3)
1110-1110: LGTM! Translation is accurate and consistent.The translation of "Overdue" as "Vencido" is grammatically correct and consistent with existing patterns in the file, such as "Overdue Fees" → "Taxas vencidas" (line 628) and "Overdue Charges" → "Cobranças vencidas" (line 1111).
2362-2362: LGTM! Translation follows established patterns.The translation "Isenção de penalidades" is consistent with the pattern established for "Waive Charge" → "Isenção de cobrança" (line 538) and uses the appropriate plural form "penalidades" matching other penalty-related translations in the file.
3174-3174: LGTM! Translation is grammatically correct and consistent.The translation "Nenhuma penalidade encontrada" correctly follows the pattern of similar "No ... found" messages in the file (e.g., line 3173: "No data found" → "Nenhum dado encontrado"). The feminine article "Nenhuma" properly agrees with the feminine noun "penalidade", and the singular form is grammatically appropriate for this construction in Portuguese.
src/assets/translations/ne-NE.json (1)
1108-1108: LGTM! Nepali translations added for penalty waiver feature.The three new translation keys appropriately support the penalty waiver functionality:
- "Overdue" (अतिदेय) for displaying overdue status
- "Waive Penalties" (दण्ड छुट) for the waiver action
- "No penalties found" (कुनै दण्ड फेला परेन) for empty state messaging
The translations are correctly placed within the JSON structure and align with the PR objectives.
Also applies to: 2360-2360, 3171-3171
src/assets/translations/en-US.json (1)
1115-1115: LGTM - New heading translation for penalty feature.The addition of the "Overdue" heading translation correctly supports the new penalty waiving UI.
src/assets/translations/sw-SW.json (3)
1109-1109: LGTM! Translation addition for overdue penalties.This translation entry for "Overdue" has been correctly placed in the
headingsection and will support the penalty waiver feature by allowing users to identify overdue penalties.
2360-2360: LGTM! Translation for waive penalties action.The "Waive Penalties" translation is properly placed in the
inputssection and aligns with the PR objective to enable penalty waiving during loan repayment and reschedule flows. The JSON syntax is correct.
3171-3171: LGTM! Empty state translation for penalty list.This translation for "No penalties found" is appropriately placed in the
textsection and provides a user-friendly empty state message when no penalties are available to waive.src/assets/translations/ko-KO.json (1)
1110-1110: LGTM! Translation is consistent with existing usage.The translation of "Overdue" to "연체" is accurate and consistent with existing translations in the file (e.g., lines 2108, 2113, 2114).
src/assets/translations/es-CL.json (1)
1110-1110: New penalty-related translations look correct and consistentThe added strings for “Overdue”, “Waive Penalties”, and “No penalties found” are grammatically sound and match existing translation patterns (e.g., “Renunciar a Comisión”, “No se encontraron …”). No changes needed.
Also applies to: 2363-2363, 3175-3175
src/app/loans/loans.service.ts (1)
531-537: getLoanCharges is consistent with existing service patternsThe new method cleanly wraps
/loans/{loanId}/charges, matches surrounding REST conventions, and fits the existing typing style in this service. No issues from a correctness or integration standpoint.src/assets/translations/fr-FR.json (1)
1110-1110: Penalty‑related French translations look consistent and correctThe new strings are correctly namespaced and the French phrasing is clear and aligned with existing terminology (“Renoncer aux frais”, etc.). No issues from an i18n or wiring perspective.
Also applies to: 2362-2362, 3173-3173
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
16-17: Penalty waiver wiring in reschedule flow looks solidImports, injection of
PenaltyManagementService, state (waivePenalties,penalties,selectedPenalties,selectAllPenalties), and the way you passchargeIdsinto the submit payload are consistent with the shared service and repayment flow. The selection helpers delegate cleanly to the service, which keeps the component lean.Also applies to: 24-28, 37-38, 53-60, 73-79, 127-130, 145-208
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (1)
11-15: Penalty waiver integration in repayment flow is coherent and reuses the shared service wellImports, injected
PenaltyManagementService, additional state (waivePenalties,penalties,selectedPenalties,selectAllPenalties), and the wayloadPenalties(), selection toggles, andgetPenaltyDisplayKey()delegate to the shared service all look consistent with the reschedule flow. WiringchargeIdsinto the submit payload only when waivers are active and selections exist is a reasonable contract.Also applies to: 26-32, 42-58, 87-95, 185-200, 206-255, 319-322
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
Show resolved
Hide resolved
shubhamkumar9199
force-pushed
the
feature/WEB-505-waive-penalties
branch
from
82ab240 to
ce3bd05
Compare
shubhamkumar9199
force-pushed
the
feature/WEB-505-waive-penalties
branch
from
ce3bd05 to
24e64a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/assets/translations/lt-LT.json (1)
2587-2590: Align penalty-related Lithuanian translations with existing catalog entriesThe diacritic fix for
"Waive Penalties"looks good, but there are still small consistency issues:
"Overdue Fees": "Delspinigiai"here differs from the catalogs entry"Overdue Fees": "Pradelsti mokesčiai". Consider reusing the catalogs phrasing for consistency."No penalties found": "Delspinigiai nerasti"is understandable but slightly awkward; a form like"Delspinigių nerasta"better matches existing style and grammar.Adjusting these keeps penalty terminology uniform across the UI.
Also applies to: 3175-3175
🧹 Nitpick comments (5)
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scss (1)
17-57: Consider making penalty grid more responsive on narrow screensThe fixed
min-widthvalues across five grid columns plus gaps can exceed viewport width on small devices, andoverflow: hiddenwill then truncate the amount/date columns. Consider using fractional units (fr) or reducing min‑widths at a breakpoint so penalties remain fully visible on mobile.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.spec.ts (1)
1-450: Solid, focused test coverage for waive‑penalties behaviorThe new suite thoroughly exercises loading, filtering, selecting, and submitting penalties, along with edge cases and navigation, and uses clear, isolated mocks for
LoansService,SettingsService, dates, and routing. This should give good confidence around the new waiver flow.If
LoanRescheduleComponentlater starts using additionalLoansServicemethods, remember to extend the mock accordingly to avoid undefined‑method errors in tests.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
53-60: Consider strengthening type safety for penalty propertiesThe penalty-related properties use weak typing that could be improved:
penalties: any[]→ Consider defining aPenaltyinterfacewaivePenalties = new UntypedFormControl(false)→ ConsiderFormControl<boolean>This would improve type safety and developer experience.
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (2)
51-58: Consider strengthening type safety for penalty propertiesSimilar to the reschedule component,
penalties: any[]could benefit from a proper interface type for better type safety and maintainability.
252-292: Verify reset-to-original behavior when disabling penalty waiverThe recalculation logic has improved from earlier versions, but there's a subtle UX concern:
When
waivePenaltiesis toggled OFF (line 260-269), the amount resets tobaseAmount(which equalscurrentAmountfrom line 258). This means:
- User sees original amount: $100
- User enables waiver, selects $30 in penalties → amount becomes $70
- User disables waiver → amount stays at $70 (not $100)
The comment at line 261 says "Reset to original amount", but the code keeps the calculated value rather than restoring
dataObject.amount.🔎 Possible adjustment to reset to original due amount
if (!this.waivePenalties || this.selectedPenalties.length === 0) { // Reset to original amount if no penalties selected this.repaymentLoanForm.patchValue( { - transactionAmount: baseAmount + transactionAmount: this.dataObject.amount }, { emitEvent: false } ); return; }However, verify this doesn't interfere with cases where the user manually edits the amount for legitimate reasons (e.g., partial payment scenarios).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.spec.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scsssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans.service.tssrc/app/loans/services/penalty-management.service.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (14)
- src/assets/translations/de-DE.json
- src/assets/translations/it-IT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/ko-KO.json
- src/assets/translations/ne-NE.json
- src/assets/translations/sw-SW.json
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss
- src/assets/translations/lv-LV.json
- src/assets/translations/es-MX.json
- src/assets/translations/pt-PT.json
- src/assets/translations/es-CL.json
- src/app/loans/services/penalty-management.service.ts
- src/assets/translations/cs-CS.json
- src/assets/translations/en-US.json
🧰 Additional context used
📓 Path-based instructions (2)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.spec.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans.service.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
**/*.spec.ts
⚙️ CodeRabbit configuration file
**/*.spec.ts: For tests: encourage clear Arrange-Act-Assert structure,
stable selectors, and minimal brittle timing dependencies.
Files:
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T14:24:31.429Z
Learnt from: alberto-art3ch
Repo: openMF/web-app PR: 2876
File: src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts:91-94
Timestamp: 2025-12-09T14:24:31.429Z
Learning: In the LoanReagingComponent (src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts), the transactionAmount field uses both Validators.min and Validators.max set to the same value (loanTransactionData.amount). This is intentional - it requires users to enter the exact amount as a confirmation/validation step before submitting the re-age operation.
Applied to files:
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
🔇 Additional comments (18)
src/app/loans/loans.service.ts (1)
531-537:getLoanChargesimplementation is consistent with existing service patternsThe new method cleanly exposes
/loans/{loanId}/chargesand aligns with nearby charge APIs and typing conventions.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html (1)
120-154: No changes needed. The penalty amount binding already passes the entire fallback expression throughformatNumber.In Angular templates, logical operators (
||) have higher precedence than the pipe (|), so the expression{{ penalty.amountOutstanding || penalty.amount || 0 | formatNumber }}is naturally evaluated as(penalty.amountOutstanding || penalty.amount || 0) | formatNumber. The entire OR chain is piped through the formatter, not just the literal0. The parentheses are not required.Likely an incorrect or invalid review comment.
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html (1)
92-134: The expression already produces the correct result. Angular's operator precedence rules dictate that the pipe|has lower precedence than the logical OR||operator, sopenalty.amountOutstanding || penalty.amount || 0 | formatNumberis automatically parsed as(penalty.amountOutstanding || penalty.amount || 0) | formatNumber. No code change is required.Likely an incorrect or invalid review comment.
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (6)
16-17: LGTM: Clean integration of penalty dependenciesThe penalty management dependencies (PenaltyManagementService, FormatNumberPipe, MatCheckbox) are properly imported and registered for the standalone component.
Also applies to: 26-27
37-37: LGTM: Proper service injectionThe PenaltyManagementService is correctly injected using the modern
inject()function.
77-78: LGTM: Proper initialization sequenceThe penalty initialization correctly loads data and sets up listeners during component initialization.
145-155: LGTM: Robust penalty loading with error handlingThe penalty loading correctly delegates to the service and includes graceful error handling with fallback to an empty array.
175-208: LGTM: Clean delegation to penalty management serviceThe penalty selection methods properly delegate business logic to the service while maintaining local component state. The pattern is consistent and maintainable.
127-130: LGTM: Proper penalty integration in submissionThe submission logic correctly includes waived penalty IDs only when both the waive toggle is enabled and penalties are selected.
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (9)
11-11: LGTM: Proper dependency importsThe penalty management dependencies are correctly imported and registered.
Also applies to: 14-14, 30-30
42-42: LGTM: Proper service injectionThe PenaltyManagementService is correctly injected using the modern
inject()function.
94-94: LGTM: Proper penalty loading in initializationThe penalty loading is correctly integrated into the component initialization flow.
113-118: LGTM: Consistent validator initializationThe transaction amount validators are properly initialized in both code paths with the correct initial state (not allowing zero).
179-189: LGTM: Robust penalty loading with error handlingThe penalty loading correctly delegates to the service with graceful error handling.
195-206: LGTM: Proper state management in penalty toggleThe toggle correctly manages state transitions, resetting selections when disabled and triggering recalculation in both directions.
212-247: LGTM: Clean penalty selection with proper recalculationThe penalty selection methods properly delegate to the service and trigger transaction amount recalculation when selections change.
297-307: LGTM: Proper dynamic validator managementThe validator update logic correctly handles the zero-amount case when penalties fully cover the due amount, addressing the validation conflict mentioned in previous reviews.
329-332: LGTM: Proper penalty integration in submissionThe submission logic correctly includes waived penalty IDs when the feature is enabled and selections exist.
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
161-169: Clean up waivePenalties.valueChanges subscription to avoid lingering listeners
setupWaivePenaltiesListener()subscribes tothis.waivePenalties.valueChangeswithout any teardown. While the control is owned by the component, adding an explicit unsubscribe (e.g.,takeUntil(this.destroy$)plusngOnDestroywith aSubject<void>) is the safer pattern and avoids potential leaks if the component is frequently created/destroyed or the control is ever reused.The earlier review already flagged this pattern; updating it here would resolve that concern across the new penalty flow as well.
You can confirm the recommended pattern in Angular/RxJS docs for cleaning up
valueChangessubscriptions in components that don’t live for the entire app lifetime.
🧹 Nitpick comments (4)
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html (1)
92-102: Clarify pipe precedence in penalty amount expressionThe penalties block is structurally sound (tracked @for, proper toggles/checkboxes). For readability, consider making the pipe precedence explicit:
<span class="penalty-amount r-amount"> {{ (penalty.amountOutstanding ?? penalty.amount ?? 0) | formatNumber }} </span>This makes it obvious that the nullish/OR chain is evaluated before
formatNumber, and avoids any ambiguity for future readers.Also applies to: 104-135
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (2)
51-59: PenaltyManagementService integration and state shape look coherentThe new penalty-related state (
waivePenalties,penalties,selectedPenalties,selectAllPenalties) together withloadPenalties,toggleSelectAllPenalties,togglePenaltySelection,isPenaltySelected, andgetPenaltyDisplayKeycleanly delegate selection logic toPenaltyManagementServiceand keep the component thin. This is a good separation of concerns; only suggestion (non‑blocking) would be to introduce a typedPenaltyinterface and usePenalty[]instead ofany[]to tighten type-safety as this feature evolves.Also applies to: 94-95, 174-248
249-307: Confirm intended behavior when turning off “Waive Penalties” after auto-reduction
recalculateTransactionAmount()now:
- Derives
baseAmountfrom the currenttransactionAmountvalue (falling back todataObject.amountif NaN).- Subtracts the sum of selected penalties, clamps at 0, and relaxes
Validators.minto allow zero only when fully waived.- When
!waivePenalties || selectedPenalties.length === 0, patchestransactionAmountback tobaseAmount(i.e., the current value), not the original due amount.This fixes the earlier zero/validator conflict and preserves user edits, but it also means:
- Scenario: user enables waivers, penalties reduce the amount from 100 → 90, then turns waivers off.
transactionAmountstays at 90 rather than reverting to the original 100.If the intent is “waiver off = go back to the canonical due amount”, you might instead want to reset to
this.dataObject.amountin that branch, and keep the “start from current value” behavior only while waivers are active. If preserving the reduced amount even after turning waivers off is deliberate (i.e., treating it as an explicit partial payment), then this logic is fine as‑is—just worth confirming.Based on learnings, this keeps stricter validation while making the waiver UX more flexible, but the toggle-off semantics are a product decision worth double-checking.
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
16-18: Penalty waiver wiring for reschedule matches repayment flowThe added
waivePenaltiescontrol pluspenalties,selectedPenalties, andselectAllPenaltiesstate, together withloadPenalties,toggleSelectAllPenalties,togglePenaltySelection,isPenaltySelected, andgetPenaltyDisplayKey, mirror the MakeRepayment implementation and reusePenaltyManagementServicenicely. SubmittingchargeIdswhenwaivePenalties.valueis true and selections exist is a clean way to surface waived penalties to the backend.Also applies to: 37-60, 73-79, 127-131, 140-208
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scsssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans.service.tssrc/app/loans/services/penalty-management.service.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scss
- src/assets/translations/ne-NE.json
- src/app/loans/loans.service.ts
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html
- src/assets/translations/ko-KO.json
- src/assets/translations/cs-CS.json
- src/assets/translations/pt-PT.json
- src/assets/translations/en-US.json
- src/assets/translations/fr-FR.json
- src/assets/translations/es-CL.json
- src/assets/translations/es-MX.json
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss
- src/assets/translations/sw-SW.json
- src/assets/translations/lv-LV.json
- src/app/loans/services/penalty-management.service.ts
- src/assets/translations/it-IT.json
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html
🧠 Learnings (1)
📚 Learning: 2025-12-09T14:24:31.429Z
Learnt from: alberto-art3ch
Repo: openMF/web-app PR: 2876
File: src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts:91-94
Timestamp: 2025-12-09T14:24:31.429Z
Learning: In the LoanReagingComponent (src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts), the transactionAmount field uses both Validators.min and Validators.max set to the same value (loanTransactionData.amount). This is intentional - it requires users to enter the exact amount as a confirmation/validation step before submitting the re-age operation.
Applied to files:
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html
🔇 Additional comments (3)
src/assets/translations/de-DE.json (1)
1110-1110: LGTM! German translations are accurate and consistent.All German translations for the penalty waiver functionality are correct and grammatically sound:
- "Waive Penalties" → "Auf Strafen verzichten" is the proper German translation
- "Overdue" → "Überfällig" is consistent throughout the file
- "Overdue Fees" → "Überfällige Gebühren" correctly addresses the previous review feedback
- "No penalties found" → "Keine Strafen gefunden" is appropriate for the new feature
The translations align well with the PR objectives and maintain consistency with existing translations in the file.
Also applies to: 2362-2362, 2588-2590, 2806-2806, 3173-3173
src/assets/translations/lt-LT.json (1)
1110-1110: Penalty-related translations wired correctly to new UI keys
heading.Overdue,inputs.Waive Penalties/Overdue/Overdue Fees, andtext.No penalties foundare now present under the expectedlabels.*sections and match the template keys used in the repayment/reschedule penalty UI. From a wiring and consistency standpoint this looks good.Also applies to: 2587-2590, 2807-2807, 3175-3175
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (1)
329-332: Including selected penalty IDs in repayment payload is consistent with reschedule flowConditionally adding
chargeIdswhenwaivePenaltiesis on andselectedPenaltiesis non‑empty mirrors the loan reschedule implementation and keeps the backend contract for waived penalties consistent between flows. Assuming the API expectschargeIdsas a flat list of charge identifiers, this wiring looks correct.Please just confirm that the backend endpoint for
submitLoanActionButtonindeed consumeschargeIds: number[]for waiving penalties in this context.
…edule Fix waive-penalties amount calc and align translations across locales fix:test issue Fix waive penalties recalculation, clean up subscription leaks, and align penalty translations
shubhamkumar9199
force-pushed
the
feature/WEB-505-waive-penalties
branch
from
24e64a1 to
ed43a62
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/assets/translations/es-MX.json (1)
2592-2592: Translation inconsistency: Align "Overdue Fees" with existing translation.Line 2592 translates "Overdue Fees" as "Cargos vencidos", but line 628 already translates the same key as "Cuotas vencidas". This inconsistency should be resolved to maintain translation uniformity across the application.
🔎 Proposed fix for consistency
- "Overdue Fees": "Cargos vencidos" + "Overdue Fees": "Cuotas vencidas"Note: This aligns with the existing translation at line 628 and the previous review comment suggestion.
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html (1)
120-154: Penalty waiver UI wiring looks correct; consider labeling the per‑penalty checkboxesFunctionally this block is sound:
waivePenaltiesFormControl gates the entire penalties section.penalties.length > 0and@for (penalty of penalties; track penalty.id)satisfy the “trackBy” guideline.- Selection state flows through
isPenaltySelected,togglePenaltySelection, andtoggleSelectAllPenaltiesas expected.- Amount expression
penalty.amountOutstanding || penalty.amount || 0 | formatNumberis fine given Angular’s pipe precedence.Two small follow‑ups:
The individual
<mat-checkbox>elements have no inner text, and the label content is rendered in separate<span>s. This hurts accessibility and click‑target size. Consider moving the label spans inside the checkbox so the entire row is clickable and screen readers get a proper label.This block relies on
labels.text.No penalties found, which is not currently defined inen-US.json; you’ll see the raw key in the UI until that’s added (already noted in the translations file).src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html (1)
92-135: Penalty and details toggles are wired correctly; apply same checkbox labeling improvementThe new slide toggles and penalties section behave coherently:
mat-slide-toggleforshowPaymentDetailsjust flips the existing boolean and dynamic controls; that’s fine.mat-slide-toggleforwaivePenaltiesmatches the TStoggleWaivePenalties()boolean flag.- Penalty list structure (select‑all, per‑penalty checkboxes, due date, formatted amount) matches the component’s penalty state and
PenaltyManagementServiceAPI.Same suggestion as in the reschedule template:
- The per‑penalty
<mat-checkbox>instances have no inner label; the descriptive text is outside, so labels aren’t associated and text clicks won’t toggle the box. Consider wrapping the label spans inside the checkbox for better accessibility and UX.Also note this block uses
labels.text.No penalties found, which is currently missing inen-US.json(flagged in the translations review).
🧹 Nitpick comments (4)
src/app/loans/loans.service.ts (1)
531-537: getLoanCharges endpoint is consistent; consider stronger typing laterThe new
getLoanCharges(loanId: string)method correctly follows existing service patterns and the/loans/{loanId}/chargesURL matches the other charge-related endpoints. Once the penalty/charge model is stabilized, you may want to replaceObservable<any>with a concrete interface/type to tighten type safety, but it’s fine to keep as-is for now given the surrounding code.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
1-40: Penalty integration and subscription lifecycle look solid
- Using
DestroyRefwithtakeUntilDestroyed(this.destroyRef)onwaivePenalties.valueChangesis a clean way to avoid leaks from the long‑lived FormControl subscription.- Penalty state (
penalties,selectedPenalties,selectAllPenalties) is kept local and updated exclusively viaPenaltyManagementServicehelpers, which keeps the component logic thin.submit()only addschargeIdswhenwaivePenaltiesis true and the selection is non‑empty, so existing behavior is preserved when penalties aren’t being waived.loadPenalties()’s single HTTP subscription is fine without explicit teardown, and the error branch resetspenaltiesto an empty list for a graceful “no penalties” UI.If you touch this again, minor niceties would be:
- Narrow
loanIdfromanytostringto match the service API.- Consider surfacing a user‑visible error (snackbar, banner) when
loadPenaltiesfails instead of only logging to the console.Also applies to: 51-63, 75-81, 106-140, 142-210
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (1)
11-18: Penalty waiver logic and dynamic validators are well‑structuredKey points in this component look good:
- Penalty handling is centralized via
PenaltyManagementService(loadPenalties, toggle helpers,isPenaltySelected,getPenaltyDisplayKey), keeping the component lean.recalculateTransactionAmount()now derivesbaseAmountfrom the current form value (with a safe fallback todataObject.amount), so user‑edited amounts are respected while still adjusting for selected waivers.- The waived amount is computed by summing each selected penalty’s outstanding/amount, and the new amount is clamped with
Math.max(0, baseAmount - totalWaived), which avoids negative values.updateTransactionAmountValidators(allowZero)cleanly toggles between min0.001and0so “full waiver” cases with zero payable amount remain valid, while still enforcing a strict positive amount when appropriate. The conditionalValidators.max(this.dataObject.amount)for capitalized income flows preserves the existing confirmation semantics.submit()only attacheschargeIdswhenwaivePenaltiesis true and there is at least one selected penalty, so previous behavior remains unchanged otherwise.If you iterate on this later, you could consider:
- Promoting
waivePenaltiesto aFormControl(similar to the reschedule component) for a fully form‑driven API, though the current boolean flag is perfectly serviceable.- Typing
penaltiesanddataObjectwith interfaces to improve IntelliSense and reduceanyusage.Also applies to: 35-66, 86-122, 174-207, 212-247, 249-307, 309-333
src/app/loans/services/penalty-management.service.ts (1)
1-190: Shared penalty management service is a good abstractionThis service cleanly encapsulates all penalty‑related behavior:
loadPenalties()tolerates several possible response envelopes ([],pageItems,content,charges,data) before filtering withfilterPenaltyCharges()and normalizingdueDateand sort order.filterPenaltyCharges()’s criteria (penalty flag or overdue charge time type, exclude waived/paid, require positive outstanding) match how penalties are conceptually used in the UI.- Selection helpers (
toggleSelectAllPenalties,togglePenaltySelection,isPenaltySelected) give components a simple, deterministic contract for managing selected IDs and “select all” state.getPenaltyDisplayKey()provides a single normalization point for backend names (e.g.MORA,OVERDUE,OVERDUE FEES) intolabels.inputs.*keys, with a sensible pass‑through for existinglabels.*keys and raw names.Nice use of a dedicated service to avoid duplicating this logic across components. If you refine this later, tightening the method signatures with explicit interfaces (for charge/penalty shapes) would improve type safety, but the current implementation is readable and maintainable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scsssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans.service.tssrc/app/loans/services/penalty-management.service.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (11)
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss
- src/assets/translations/it-IT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/ko-KO.json
- src/assets/translations/lv-LV.json
- src/assets/translations/de-DE.json
- src/assets/translations/sw-SW.json
- src/assets/translations/cs-CS.json
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.scss
- src/assets/translations/lt-LT.json
- src/assets/translations/es-CL.json
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/loans/loans.service.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/services/penalty-management.service.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T14:24:31.429Z
Learnt from: alberto-art3ch
Repo: openMF/web-app PR: 2876
File: src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts:91-94
Timestamp: 2025-12-09T14:24:31.429Z
Learning: In the LoanReagingComponent (src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.ts), the transactionAmount field uses both Validators.min and Validators.max set to the same value (loanTransactionData.amount). This is intentional - it requires users to enter the exact amount as a confirmation/validation step before submitting the re-age operation.
Applied to files:
src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
🔇 Additional comments (11)
src/assets/translations/es-MX.json (5)
1109-1109: LGTM: Correct translation for "Overdue".The translation "Vencido" is accurate and appropriate for the Spanish (Mexico) locale.
2363-2363: LGTM: Appropriate translation for penalty waiver functionality.The translation "Renunciar a Penalizaciones" correctly conveys the meaning of waiving penalties in the loan repayment and reschedule context.
2590-2590: LGTM: Clear financial terminology translation.The translation accurately conveys the technical concept of available disbursement amount with over-applied funds.
2591-2591: LGTM: Consistent translation maintained.The translation "Vencido" is consistent with the same key at line 1109.
3177-3177: LGTM: Grammatically correct empty state message.The translation "No se encontraron penalizaciones" is grammatically correct and appropriate for indicating when no penalties are available to waive.
src/assets/translations/ne-NE.json (1)
1108-1108: Translations for penalties-related labels look correct and consistentThe Nepali strings for “Overdue”, “Waive Penalties”, and “No penalties found” are grammatically sound, context-appropriate, and align with nearby terminology (e.g., “Waive Charge”, “Overdue Charges”). No structural JSON issues spotted.
Also applies to: 2360-2360, 3171-3171
src/assets/translations/pt-PT.json (3)
1110-1110: LGTM! Correct translation for "Overdue".The translation "Vencido" is the appropriate and standard Portuguese term for "Overdue" in a financial/loan context, meaning "past due" or "late".
2362-2362: LGTM! Translation follows established patterns.The translation "Isenção de penalidades" correctly translates "Waive Penalties" and follows the consistent pattern used in the file for similar terms (e.g., "Waive Charge" → "Isenção de cobrança" at line 650). This is appropriate for the penalty waiver feature being added to loan repayment flows.
3174-3174: LGTM! Correct message translation.The translation "Nenhuma penalidade encontrada" is grammatically correct with proper gender/number agreement and appropriately conveys the message "No penalties found" for the UI. This aligns with the PR objectives to display penalty information in the waiver flows.
src/assets/translations/en-US.json (2)
1115-1115: New “Overdue” heading label looks consistentThe added
labels.heading.Overdueentry aligns with existing heading keys and penalty‑related UI; no issues from a translations perspective.
3539-3539: UploadDocumentHint key addition is fine
"UploadDocumentHint": "Upload a PDF or image to generate a preview."is self‑explanatory and consistent with existing helper texts; no changes needed.
| "Available Disbursement Amount (with Over Applied)": "Available Disbursement Amount (with Over Applied)", | ||
| "Waive Penalties": "Waive Penalties", | ||
| "Overdue": "Overdue", | ||
| "Overdue Fees": "Overdue Fees" |
There was a problem hiding this comment.
Add translation for labels.text.No penalties found to avoid raw key in UI
Both make‑repayment and reschedule templates use {{ 'labels.text.No penalties found' | translate }}, but this file doesn’t define "No penalties found" under labels.text. That will likely render the raw key instead of a user‑friendly message.
Please add a text entry, e.g.:
"No transaction was found": "No transaction was found.",
+ "No penalties found": "No penalties found",
"Notes": "Notes",Also applies to: 3529-3540
🤖 Prompt for AI Agents
In src/assets/translations/en-US.json around lines 2595-2598 and also around
3529-3540, the translation key "labels.text.No penalties found" is missing
causing the raw key to render; add a JSON entry under the labels.text section
such as "No penalties found": "No penalties found" (or localized phrasing you
prefer) making sure to place the key in the correct nested labels.text object,
maintain proper comma separators for surrounding entries, avoid duplicating
keys, and validate the JSON after insertion.
There was a problem hiding this comment.
@shubhamkumar9199 could you check this comment?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
…edule
This PR adds support for waiving penalties during loan repayment and loan reschedule flows. Previously, users could not selectively waive penalties
WEB-505
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
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.