WEB-657: Working Capital Loan support charges#3628
Open
alberto-art3ch wants to merge 1 commit into
Open
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Loan Account Path Type & Contract src/app/loans/loans.service.ts, src/app/products/loan-products/services/loan-product.service.ts, src/app/loans/common-resolvers/loan-base.resolver.ts, src/app/loans/models/loan-charge.model.ts, src/app/loans/services/penalty-management.service.ts |
New LoanAccountPath union type routes service calls to either 'loans' or 'working-capital-loans' endpoints; method signatures for getLoanChargeTemplateResource, createLoanCharge, executeLoansAccountChargesCommand, and waivePenalties updated to accept and pass the path; public return types in services and resolvers now use typed path contract. |
Working-Capital Charges Resolution & Routing src/app/loans/common-resolvers/loan-charges.resolver.ts, src/app/loans/loans-routing.module.ts, src/app/loans/common-resolvers/loan-action-button.resolver.ts |
New LoanChargesResolver conditionally fetches working-capital loan charges when isWorkingCapital is true; resolver wired into routing module and integrated into the charges tab route via loanChargeData resolver block; "Add Loan Charge" action resolver updated to pass loanAccountPath to template resource fetch. |
Charges Tab Complete UI Refactor src/app/loans/loans-view/charges-tab/charges-tab.component.ts, src/app/loans/loans-view/charges-tab/charges-tab.component.html, src/app/loans/loans-view/charges-tab/charges-tab.component.scss |
Component refactored with dynamic column building, MatSelectionModel integration for multi-select rows with master toggle, bulk-waive workflow via forkJoin, and computed getters for totals/progress/state. Template redesigned with financial summary strip (Total Due, Paid, Waived, Outstanding), bulk action bar, grouped headers, checkboxes, conditional menu-based row actions, and progress-bar rendering per charge. Stylesheet completely rewritten around :host design tokens, dark-theme overrides, Material table theming, and animated bulk action bar. |
Add Loan Charge Form Modernization src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.ts, src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.html |
Form migrated to typed FormGroup and typed form controls; DestroyRef/takeUntilDestroyed added for lifecycle-aware subscription cleanup; isSubmitting signal prevents duplicate submissions; submit logic navigates to charges view on success and resets submission state on error; button disabled state includes submission status check. |
Loan Charge Action Callers src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts, src/app/loans/loans-view/view-charge/view-charge.component.ts, src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts, src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts |
All charge command dispatchers updated to pass loanAccountPath as first argument to executeLoansAccountChargesCommand and waivePenalties calls; control flow and navigation behavior unchanged. |
Supporting Styling & Data Model src/app/loans/models/loan-charge.model.ts, src/assets/styles/_status.scss, src/app/loans/loans-view/loans-view.component.html |
LoanCharge interface extended with optional actionFlag property; global CSS row-state tokens (paid/partial/alert/waived) introduced with light/dark overrides for left-border indicators; charges tab visibility updated to show for working capital OR when charges exist. |
Multi-Language Translations src/assets/translations/*.json (12 language files) |
Added translation keys across Czech, German, English, Spanish (CL, MX), French, Italian, Korean, Lithuanian, Latvian, Nepali, Portuguese, and Kiswahili for new UI actions ("Pay Charge", "Waive Selected"), field labels ("Financial Summary", "Selected", "Total Paid"), and confirmation dialogs with count interpolation. |
Sequence Diagram
sequenceDiagram
participant Route as Route Resolver
participant LCR as LoanChargesResolver
participant Service as LoansService
participant Component as ChargesTabComponent
participant Dialog as Confirmation Dialog
participant API as API Endpoint
Route->>LCR: Activate charges route
LCR->>LCR: Check isWorkingCapital flag
alt is working capital
LCR->>Service: getWorkingCapitalLoanCharges(loanId)
Service->>API: GET /working-capital-loans/{id}/charges
API-->>Service: Charge list
Service-->>LCR: Observable<LoanCharge[]>
else standard loan
LCR-->>Route: of([])
end
LCR-->>Route: loanChargeData resolved
Route->>Component: Initialize with charges
Component->>Component: Parse dates, compute actionFlag
Component->>Component: buildColumns() based on count
Component->>Component: Initialize selection & sorting
Note over Component: User interacts with table
Component->>Dialog: Open bulk waive confirmation
Dialog-->>Component: User confirms
Component->>Service: forkJoin([waivePenalties calls])
Service->>API: POST /{loanAccountPath}/{id}/charges/{id}
API-->>Service: Waive responses
Service-->>Component: Complete
Component->>Component: Clear selection, reload charges
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
- openMF/web-app#3027: Both PRs touch the "Add Loan Charge" action flow—main PR updates the resolver to pass
loanAccountPathfor template fetching, while the retrieved PR adds charge button support for the Overpaid charge status. - openMF/web-app#2957: Both PRs refactor penalty waiving by updating
PenaltyManagementService.waivePenaltiesand its callers in loan-reschedule and make-repayment to passloanAccountPathinto the command invocation. - openMF/web-app#3419: Both PRs modify
LoanActionButtonResolver.resolve()to route actions differently for working-capital vs standard loans using the loan account path for template/resource selection.
Suggested reviewers
- adamsaghy
- IOhacker
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The PR title clearly and specifically identifies the main feature: adding support for charges on Working Capital Loan accounts. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ 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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/loans/loans-view/view-charge/view-charge.component.ts (1)
137-139:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winBroken tagged-template:
dialogContextwill throw at runtime.
translateService.instant(...)returns a string, and the trailing template literal turns this into a tagged-template call — JS attempts to invoke the returned string as a function, throwing aTypeErrorwheneverwaiveCharge()opens the dialog. This appears to be a missing+for concatenation.🐛 Proposed fix
- dialogContext: this.translateService.instant( - 'labels.dialogContext.Are you sure you want to waive charge with id:' - )` ${this.chargeData.id}`, + dialogContext: + this.translateService.instant('labels.dialogContext.Are you sure you want to waive charge with id:') + + ` ${this.chargeData.id}`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/view-charge/view-charge.component.ts` around lines 137 - 139, The dialogContext currently uses a trailing template literal after translateService.instant(...) which causes a tagged-template runtime error; update the construction in the waiveCharge() dialog options so dialogContext is a proper concatenated string (e.g., combine translateService.instant(...) and this.chargeData.id with + or use a single template literal) — locate the dialogContext assignment near translateService.instant(...) and replace the incorrect tagged-template usage with string concatenation or a single template literal that includes this.chargeData.id.src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.ts (1)
116-135:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify
loanAccountPathinitialization for loan account action screens
LoanProductService.loanAccountPathis a getter that always returns'loans'or'working-capital-loans'(it can’t beundefined).- For
:loanId/actions/:action(and other charge actions nested under:loanId),LoansViewComponent+LoanDetailsResolverinitializeLoanProductService.productTypefromroute.queryParams.productTypebefore the child resolver/component runs, soloanAccountPathis available forLoanActionButtonResolverand later component submits.- If
productTypequery param is missing/mismatched, initialization falls back to the default loan product (/loans), so the main risk is hitting the wrong endpoint—not an undefined path; ensure working-capital navigation always carriesproductType=working-capital(or derive product type from resolved loan details rather than query params).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.ts` around lines 116 - 135, The submit uses LoanProductService.loanAccountPath but productType can be mismatched if navigation to action screens lacks the query param; either ensure navigations to :loanId/actions/* always include query param productType=working-capital when appropriate, or change initialization so LoanProductService.productType is derived from resolved loan details (LoanDetailsResolver) instead of relying solely on route.queryParams; update LoansViewComponent/LoanDetailsResolver to set LoanProductService.productType from the resolved loan DTO and confirm LoanActionButtonResolver and add-loan-charge.component submit read LoanProductService.loanAccountPath only after that initialization.src/app/loans/loans-view/charges-tab/charges-tab.component.ts (2)
236-247:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove the new dialog copy fully behind translation keys.
Payment Date,Confirm,Pay Charge ...,Amount,Edit Charge ..., andcharge id:...are user-facing strings added in TS, and the waive message appends the id outside the translated string. That will leak English text and prevents translators from reordering the placeholder.As per coding guidelines "Use proper i18n variables from
@ngx-translate/corefor all user-facing strings instead of hardcoded text".Also applies to: 275-278, 299-310, 326-326
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts` around lines 236 - 247, The dialog uses hardcoded UI strings; replace them with translation keys via `@ngx-translate/core`: change the DatepickerBase label 'Payment Date' to a translation key (e.g., use translate.instant or pass the key to the form metadata), replace layout.addButtonText 'Confirm' and the dialog title template `Pay Charge ${chargeId}` with translatable keys that include placeholders for chargeId, and update the waive/edit/amount strings (and the waiver message concatenation) to use translation keys with placeholders so the charge id is injected via the translator rather than appended; update all occurrences referenced (DatepickerBase label, data.title, layout.addButtonText and the waive/edit/amount strings around lines ~275-310 and ~326) to use the translate service or keys accordingly.
250-267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard dialog results before reading them.
afterClosed()can emitundefinedon cancel/backdrop close. The currentresponse.data/response.confirm/response.deletereads will throw on those paths.Suggested fix
- payChargeDialogRef.afterClosed().subscribe((response: any) => { - if (response.data) { + payChargeDialogRef.afterClosed().subscribe((response: any) => { + if (response?.data) { const locale = this.settingsService.language.code; const dateFormat = this.settingsService.dateFormat; const dataObject = { transactionDate: this.dateUtils.formatDate(response.data.value.transactionDate, dateFormat), dateFormat, @@ - waiveChargeDialogRef.afterClosed().subscribe((response: any) => { - if (response.confirm) { + waiveChargeDialogRef.afterClosed().subscribe((response: any) => { + if (response?.confirm) { @@ - editChargeDialogRef.afterClosed().subscribe((response: any) => { - if (response.data) { + editChargeDialogRef.afterClosed().subscribe((response: any) => { + if (response?.data) { @@ - deleteChargeDialogRef.afterClosed().subscribe((response: any) => { - if (response.delete) { + deleteChargeDialogRef.afterClosed().subscribe((response: any) => { + if (response?.delete) {Also applies to: 282-293, 313-319, 328-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts` around lines 250 - 267, The subscriber to payChargeDialogRef.afterClosed() (and the similar subscribers at the other noted ranges) is reading response.data / response.confirm / response.delete without guarding for an undefined response when the dialog is cancelled; update the callback for payChargeDialogRef.afterClosed() so it first checks that response is truthy (or uses optional chaining) and that the expected property (e.g., response.data) exists before accessing it and calling executeLoansAccountChargesCommand/this.reload(), and apply the same defensive check pattern to the other dialog subscribers referenced (the blocks that read response.confirm and response.delete).
🧹 Nitpick comments (5)
src/assets/translations/ko-KO.json (1)
620-620: 💤 Low valueConsider terminology consistency for "charge".
The new translation "Pay Charge": "수수료 납부" uses "수수료" for "charge," while the existing "Waive Charge" (line 739) uses "요금 면제" with "요금" for "charge." Both terms are correct in Korean, but using consistent terminology throughout the UI would improve user experience.
Consider aligning with the existing term "요금" if consistency is preferred.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/translations/ko-KO.json` at line 620, The translation for the key "Pay Charge" ("수수료 납부") is inconsistent with the existing "Waive Charge" ("요금 면제"); update the "Pay Charge" entry to use the same terminology as "Waive Charge" (e.g., change "수수료 납부" to "요금 납부") so both keys ("Pay Charge" and "Waive Charge") use "요금" for "charge" for consistent UI wording.src/app/loans/loans.service.ts (1)
14-17: 💤 Low valueMove the type export below the import block.
Placing a
typedeclaration between twoimportstatements interrupts the import group and reads awkwardly. Relocate it after imports (e.g., below line 19).♻️ Proposed reorganization
import { Observable } from 'rxjs'; - -export type LoanAccountPath = 'loans' | 'working-capital-loans'; import { Dates } from 'app/core/utils/dates'; import { SettingsService } from 'app/settings/settings.service'; import { DisbursementData } from './models/loan-account.model'; + +export type LoanAccountPath = 'loans' | 'working-capital-loans';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans.service.ts` around lines 14 - 17, Move the exported type declaration LoanAccountPath out of the import block and place it after the import statements so imports remain grouped; specifically, remove the line "export type LoanAccountPath = 'loans' | 'working-capital-loans';" from between the import lines and re-add it immediately after the last import (e.g., below Dates import) in src/app/loans/loans.service.ts.src/app/loans/loans-view/charges-tab/charges-tab.component.scss (2)
10-56: ⚖️ Poor tradeoffReuse theme variables instead of a local color/dark-mode palette.
This block redefines a full blue/surface/text/state palette plus
:host-context(.dark-theme)overrides locally. The theme palette and dark-mode handling already live insrc/main.scss/src/theme/mifosx-theme.scss; duplicating them here means future theme changes won't propagate and risks visual drift between this tab and the rest of the app. Map these tokens onto the shared SCSS variables/theme values where they exist.As per coding guidelines: "Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/charges-tab/charges-tab.component.scss` around lines 10 - 56, The component currently defines a local color palette in :host and :host-context(.dark-theme) (tokens like --ch-blue-700, --ch-surface, --ch-text-1, --ch-paid, etc.); replace these local token definitions by mapping them to the shared SCSS theme variables defined in src/main.scss / src/theme/mifosx-theme.scss (i.e. remove the hard-coded hex values in :host and :host-context(.dark-theme) and reassign each --ch-* token to the corresponding global SCSS variable or mixin used by the app theme), ensuring dark-mode uses the global theme overrides rather than duplicating colors so the ChargesTab styles inherit application-wide theming.
87-117: ⚖️ Poor tradeoffOff-grid pixel values break the 8px spacing system.
Values such as
font-size: 10px,font-size: 13.5px,gap: 5px, andgap: 6pxdon't align with the 8px grid system the project follows for spacing and sizing. Prefer grid-aligned values (or shared spacing/typography variables) here and across the other off-grid declarations in this file.As per coding guidelines: "Stick to the 8px grid system for visual design and spacing".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/charges-tab/charges-tab.component.scss` around lines 87 - 117, The CSS uses off-grid pixel values in .summary-label and .summary-value (e.g. font-size: 10px, gap: 5px, font-size: 13.5px, gap: 6px) which breaks the 8px spacing system; replace those literal values with 8px-grid-aligned sizes or existing design tokens (e.g. spacing/typography variables) so .summary-label uses a gap that is a multiple of 8 (or var(--spacing-1/etc.)) and .summary-value uses font-size values aligned to the scale (or shared font-size variables) while preserving the .c-paid and .c-outstanding color classes—update any other off-grid declarations in this SCSS file accordingly.src/assets/styles/_status.scss (1)
72-84: 💤 Low valueSource row-state colors from the
colourspartial.This file already
@use 'colours'and consumes$status-*tokens, yet these new row-state custom properties hardcode hex values (and a separate dark set) that overlap with existing status semantics (paid/overdue/etc.). Define them from the shared$coloursvariables so the palette stays single-sourced and theme-consistent.As per coding guidelines: "Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/styles/_status.scss` around lines 72 - 84, Replace the hardcoded hex values for the CSS custom properties (--row-state-paid, --row-state-partial, --row-state-alert, --row-state-waived) inside :root and .dark-theme with the corresponding SCSS colour variables from the colours partial (the existing $status-* tokens you already `@use`), e.g. set each --row-state-* to the appropriate $status-... variable (and dark theme counterparts) so the palette is single-sourced and theme-consistent; update the :root and .dark-theme blocks where those custom properties are defined to reference the $status variables instead of literal hex codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.html`:
- Around line 108-124: Add translated accessible labels and replace native
button with Material icon button for the selection and action controls: add
aria-label attributes using the translate pipe (e.g.
[attr.aria-label]="'LOANS.CHARGES.SELECT_ALL' | translate") to the select-all
checkbox (associated with masterToggle()), and to each row checkbox (used with
selection.toggle(charge) and guarded by isPaid(charge)/isWaived(charge)); also
convert the plain action trigger to a <button mat-icon-button> and give it a
translated aria-label (e.g. 'LOANS.CHARGES.ROW_ACTIONS' | translate). Apply the
same pattern for the other controls referenced around lines 272–274 so all
icon-only controls use mat-icon-button and translated aria-labels via
`@ngx-translate/core`.
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts`:
- Around line 359-360: The currencyCode getter currently falls back to a
hardcoded 'EUR' when chargesData is empty; update the get currencyCode() method
to first fall back to the loan's currency code (e.g. use
this.loan?.currency?.code) before using a final hardcoded default, so change the
fallback order from this.chargesData?.[0]?.currency?.code ?? 'EUR' to prefer
this.loan?.currency?.code and only then a safe default.
- Around line 317-330: The edit/delete calls use
loansService.editLoansAccountCharge and loansService.deleteLoansAccountCharge
which currently build URLs with a hardcoded "/loans/..." path; update them to
support a variable loan account base path (eg. "/working-capital-loans") by
changing the service helpers to accept an additional loanAccountPath parameter
(or make it optional and derive from loanDetails) and construct the endpoint as
`/${loanAccountPath}/${accountId}/charges/${chargeId}`; then update
charges-tab.component.ts where editLoansAccountCharge(...) and
deleteLoansAccountCharge(...) are called (references: editLoansAccountCharge,
deleteLoansAccountCharge, this.loanDetails.id, deleteChargeDialogRef) to pass
the appropriate loanAccountPath (or ensure loanAccountPath is available on the
component) so working-capital accounts route to
/working-capital-loans/.../charges/....
- Around line 208-223: The bulk-waive flow currently uses forkJoin over
executeLoansAccountChargesCommand calls which fails entirely if any single
request errors; change to swallow per-request errors so the aggregate always
completes: map each observable returned by
this.loansService.executeLoansAccountChargesCommand(...) to handle errors (e.g.,
pipe with catchError and return a sentinel result object), then forkJoin those
mapped observables and in the forkJoin subscription always call
this.selection.clear() and this.reload() and surface any per-request failures
(e.g., via a toast) so the UI is not left stale; refer to
confirmRef.afterClosed(), the array built from this.selection.selected,
executeLoansAccountChargesCommand, this.selection.clear and this.reload to
locate where to apply the change.
In `@src/assets/translations/cs-CS.json`:
- Line 3261: The Czech translation for the key "Are you sure you want to waive
charges" uses a fixed plural form ("{{ count }} poplatků") which yields
incorrect grammar for singular; update the value for that key to a count-safe
phrasing (either a neutral form that doesn't inject the count, e.g. a question
like "Opravdu chcete poplatky odpustit?" or implement proper ICU pluralization
for the variable {{ count }} using Czech plural rules) so singular/plural cases
render correctly.
In `@src/assets/translations/de-DE.json`:
- Line 742: Replace the unidiomatic German value for the "Waive Selected"
translation (currently "Ausgewählte verzichten") with the correct phrasing using
"erlassen" (for example "Ausgewählte erlassen"); update both occurrences of the
"Waive Selected" key so the UI reads idiomatically when waiving charges.
In `@src/assets/translations/es-CL.json`:
- Line 620: The Spanish label "Pay Charge" uses inconsistent terminology and
incorrect grammar; update the translation for the "Pay Charge" key from "Pagar
Cargo" to "Pagar Comisión" (use singular when the button pays a single charge)
and audit other related keys (the other affected key referenced in the comment)
to use a consistent "Comisión" / "Comisiones" phrasing (e.g., change list or
plural labels to "Comisiones") so all charge-related strings use the same term
and correct singular/plural grammar.
- Line 3262: The translation string for the key "Are you sure you want to waive
charges" is missing the closing question mark and uses inconsistent
interpolation spacing; update the value to include the closing question mark and
normalize the placeholder to {{count}} (e.g., "¿Está seguro de que desea
renunciar a {{count}} comisiones?") so it matches the app's interpolation style
and punctuation.
In `@src/assets/translations/es-MX.json`:
- Line 740: The Spanish translation for the key "Waive Selected" is
grammatically incorrect; update the value for the "Waive Selected" key in
src/assets/translations/es-MX.json to a natural imperative such as "Renunciar a
los seleccionados" (or the project-preferred equivalent) so the key remains
unchanged and only the value string is corrected.
- Line 3265: The Spanish translation for the key "Are you sure you want to waive
charges" is missing the closing question mark; update the value so the sentence
is wrapped with both opening and closing question marks (e.g., "¿Está seguro de
que desea renunciar a {{ count }} cargos?") to properly close the confirmation
prompt in the UI.
In `@src/assets/translations/fr-FR.json`:
- Line 620: Update the inconsistent French phrases in the translations: replace
the value for the "Pay Charge" key ("Payer la charge") with "Payer les frais",
and replace the string "Renoncer aux sélectionnés" (the UI label for waiving
selected items) with "Renoncer aux frais sélectionnés" so they match the
established "frais" vocabulary across the locale file.
In `@src/assets/translations/it-IT.json`:
- Line 740: Update the Italian translations for the selection-related labels to
more natural UI phrasing: change the value for the translation key "Waive
Selected" from "Rinuncia selezionati" to a clearer phrase such as "Rinuncia ai
selezionati" (or "Condona selezionati" if product terminology prefers
"condonare"), and change the value for the translation key "Selected" (currently
"Selezionato/i") to "Selezionati" to match plural UI usage; locate and edit
these keys ("Waive Selected" and "Selected") in
src/assets/translations/it-IT.json and update their string values accordingly.
- Line 3260: The Italian translation for the key "Are you sure you want to waive
charges" always uses the plural form ("{{ count }} commissioni") which is
incorrect when count === 1; update the translation to handle singular/plural by
either adding separate keys (e.g. "Are you sure you want to waive charges.one"
-> "Sei sicuro di voler rinunciare a {{ count }} commissione" and "Are you sure
you want to waive charges.other" -> "Sei sicuro di voler rinunciare a {{ count
}} commissioni") or convert the value to an ICU pluralized string, and ensure
the calling code uses the i18n pluralization API with the count parameter for
the "Are you sure you want to waive charges" key so the correct form is
rendered.
In `@src/assets/translations/lv-LV.json`:
- Line 620: The translation for the key "Pay Charge" is inconsistent with the
other occurrences; replace the value "Maksāt maksu" with the existing consistent
CTA form "Maksājiet maksu" so the "Pay Charge" entry matches the other
translations in the locale (look for the "Pay Charge" key in the JSON and update
its value).
In `@src/assets/translations/pt-PT.json`:
- Line 620: The pt-PT locale uses mixed terms like "encargo", "cobrança",
"renunciar" and "isenção" for the same loan charge/waive actions; update the
keys (e.g., "Pay Charge" and the other affected keys around the same areas
referenced in the review) to use the single, existing term family used elsewhere
in this file (pick the dominant term used across the file—e.g., "cobrança" or
whatever is most common—and replace other variants like
"encargo"/"renunciar"/"isenção" to match). Ensure all occurrences at the three
noted locations are aligned to that single term so UI wording is consistent.
---
Outside diff comments:
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts`:
- Around line 236-247: The dialog uses hardcoded UI strings; replace them with
translation keys via `@ngx-translate/core`: change the DatepickerBase label
'Payment Date' to a translation key (e.g., use translate.instant or pass the key
to the form metadata), replace layout.addButtonText 'Confirm' and the dialog
title template `Pay Charge ${chargeId}` with translatable keys that include
placeholders for chargeId, and update the waive/edit/amount strings (and the
waiver message concatenation) to use translation keys with placeholders so the
charge id is injected via the translator rather than appended; update all
occurrences referenced (DatepickerBase label, data.title, layout.addButtonText
and the waive/edit/amount strings around lines ~275-310 and ~326) to use the
translate service or keys accordingly.
- Around line 250-267: The subscriber to payChargeDialogRef.afterClosed() (and
the similar subscribers at the other noted ranges) is reading response.data /
response.confirm / response.delete without guarding for an undefined response
when the dialog is cancelled; update the callback for
payChargeDialogRef.afterClosed() so it first checks that response is truthy (or
uses optional chaining) and that the expected property (e.g., response.data)
exists before accessing it and calling
executeLoansAccountChargesCommand/this.reload(), and apply the same defensive
check pattern to the other dialog subscribers referenced (the blocks that read
response.confirm and response.delete).
In
`@src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.ts`:
- Around line 116-135: The submit uses LoanProductService.loanAccountPath but
productType can be mismatched if navigation to action screens lacks the query
param; either ensure navigations to :loanId/actions/* always include query param
productType=working-capital when appropriate, or change initialization so
LoanProductService.productType is derived from resolved loan details
(LoanDetailsResolver) instead of relying solely on route.queryParams; update
LoansViewComponent/LoanDetailsResolver to set LoanProductService.productType
from the resolved loan DTO and confirm LoanActionButtonResolver and
add-loan-charge.component submit read LoanProductService.loanAccountPath only
after that initialization.
In `@src/app/loans/loans-view/view-charge/view-charge.component.ts`:
- Around line 137-139: The dialogContext currently uses a trailing template
literal after translateService.instant(...) which causes a tagged-template
runtime error; update the construction in the waiveCharge() dialog options so
dialogContext is a proper concatenated string (e.g., combine
translateService.instant(...) and this.chargeData.id with + or use a single
template literal) — locate the dialogContext assignment near
translateService.instant(...) and replace the incorrect tagged-template usage
with string concatenation or a single template literal that includes
this.chargeData.id.
---
Nitpick comments:
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.scss`:
- Around line 10-56: The component currently defines a local color palette in
:host and :host-context(.dark-theme) (tokens like --ch-blue-700, --ch-surface,
--ch-text-1, --ch-paid, etc.); replace these local token definitions by mapping
them to the shared SCSS theme variables defined in src/main.scss /
src/theme/mifosx-theme.scss (i.e. remove the hard-coded hex values in :host and
:host-context(.dark-theme) and reassign each --ch-* token to the corresponding
global SCSS variable or mixin used by the app theme), ensuring dark-mode uses
the global theme overrides rather than duplicating colors so the ChargesTab
styles inherit application-wide theming.
- Around line 87-117: The CSS uses off-grid pixel values in .summary-label and
.summary-value (e.g. font-size: 10px, gap: 5px, font-size: 13.5px, gap: 6px)
which breaks the 8px spacing system; replace those literal values with
8px-grid-aligned sizes or existing design tokens (e.g. spacing/typography
variables) so .summary-label uses a gap that is a multiple of 8 (or
var(--spacing-1/etc.)) and .summary-value uses font-size values aligned to the
scale (or shared font-size variables) while preserving the .c-paid and
.c-outstanding color classes—update any other off-grid declarations in this SCSS
file accordingly.
In `@src/app/loans/loans.service.ts`:
- Around line 14-17: Move the exported type declaration LoanAccountPath out of
the import block and place it after the import statements so imports remain
grouped; specifically, remove the line "export type LoanAccountPath = 'loans' |
'working-capital-loans';" from between the import lines and re-add it
immediately after the last import (e.g., below Dates import) in
src/app/loans/loans.service.ts.
In `@src/assets/styles/_status.scss`:
- Around line 72-84: Replace the hardcoded hex values for the CSS custom
properties (--row-state-paid, --row-state-partial, --row-state-alert,
--row-state-waived) inside :root and .dark-theme with the corresponding SCSS
colour variables from the colours partial (the existing $status-* tokens you
already `@use`), e.g. set each --row-state-* to the appropriate $status-...
variable (and dark theme counterparts) so the palette is single-sourced and
theme-consistent; update the :root and .dark-theme blocks where those custom
properties are defined to reference the $status variables instead of literal hex
codes.
In `@src/assets/translations/ko-KO.json`:
- Line 620: The translation for the key "Pay Charge" ("수수료 납부") is inconsistent
with the existing "Waive Charge" ("요금 면제"); update the "Pay Charge" entry to use
the same terminology as "Waive Charge" (e.g., change "수수료 납부" to "요금 납부") so
both keys ("Pay Charge" and "Waive Charge") use "요금" for "charge" for consistent
UI wording.
🪄 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: 04849bcb-7667-4c8e-a6ff-6fa54f1abf8b
📒 Files selected for processing (32)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-charges.resolver.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/charges-tab/charges-tab.component.htmlsrc/app/loans/loans-view/charges-tab/charges-tab.component.scsssrc/app/loans/loans-view/charges-tab/charges-tab.component.tssrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.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.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/view-charge/view-charge.component.tssrc/app/loans/loans.service.tssrc/app/loans/models/loan-charge.model.tssrc/app/loans/services/penalty-management.service.tssrc/app/products/loan-products/services/loan-product.service.tssrc/assets/styles/_status.scsssrc/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
+108
to
+124
| <mat-checkbox | ||
| (click)="$event.stopPropagation()" | ||
| (change)="masterToggle()" | ||
| [checked]="isAllSelected()" | ||
| [indeterminate]="selection.hasValue() && !isAllSelected()" | ||
| > | ||
| </mat-checkbox> | ||
| </th> | ||
| <td mat-cell *matCellDef="let charge" class="col-c col-sel" (click)="$event.stopPropagation()"> | ||
| @if (!isPaid(charge) && !isWaived(charge)) { | ||
| <mat-checkbox | ||
| [disabled]="charge.actionFlag" | ||
| [checked]="selection.isSelected(charge)" | ||
| (click)="$event.stopPropagation()" | ||
| (change)="$event ? selection.toggle(charge) : null" | ||
| > | ||
| <i class="fa fa-trash"></i> | ||
| </button> | ||
| </mat-checkbox> |
There was a problem hiding this comment.
Add translated accessible labels to the new selection and action controls.
The select-all checkbox, row checkboxes, and the icon-only actions trigger are currently unlabeled, so screen readers will announce anonymous controls in the bulk-waive workflow. Please add translated aria-labels here; the menu trigger should also be a Material icon button rather than a plain button.
As per coding guidelines "All visual components must strictly use Angular Material elements (e.g., <mat-card>, <mat-select>) instead of native HTML where possible" and "Use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text".
Also applies to: 272-274
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.html` around lines
108 - 124, Add translated accessible labels and replace native button with
Material icon button for the selection and action controls: add aria-label
attributes using the translate pipe (e.g.
[attr.aria-label]="'LOANS.CHARGES.SELECT_ALL' | translate") to the select-all
checkbox (associated with masterToggle()), and to each row checkbox (used with
selection.toggle(charge) and guarded by isPaid(charge)/isWaived(charge)); also
convert the plain action trigger to a <button mat-icon-button> and give it a
translated aria-label (e.g. 'LOANS.CHARGES.ROW_ACTIONS' | translate). Apply the
same pattern for the other controls referenced around lines 272–274 so all
icon-only controls use mat-icon-button and translated aria-labels via
`@ngx-translate/core`.
Comment on lines
+208
to
+223
| confirmRef.afterClosed().subscribe((response: any) => { | ||
| if (response?.confirm) { | ||
| const requests = this.selection.selected.map((charge) => | ||
| this.loansService.executeLoansAccountChargesCommand( | ||
| this.loanProductService.loanAccountPath, | ||
| this.loanDetails.id, | ||
| 'waive', | ||
| {}, | ||
| charge.id | ||
| ) | ||
| ); | ||
| forkJoin(requests).subscribe(() => { | ||
| this.selection.clear(); | ||
| this.reload(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Handle partial failures in the bulk waive flow.
If one waive request fails after others succeed, forkJoin errors and skips the reload path, so the table stays stale and the user can retry against charges that were already updated.
Suggested direction
- forkJoin(requests).subscribe(() => {
- this.selection.clear();
- this.reload();
- });
+ forkJoin(
+ requests.map((request) =>
+ request.pipe(
+ catchError((error) => of({ error }))
+ )
+ )
+ ).subscribe(() => {
+ this.selection.clear();
+ this.reload();
+ });📝 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
| confirmRef.afterClosed().subscribe((response: any) => { | |
| if (response?.confirm) { | |
| const requests = this.selection.selected.map((charge) => | |
| this.loansService.executeLoansAccountChargesCommand( | |
| this.loanProductService.loanAccountPath, | |
| this.loanDetails.id, | |
| 'waive', | |
| {}, | |
| charge.id | |
| ) | |
| ); | |
| forkJoin(requests).subscribe(() => { | |
| this.selection.clear(); | |
| this.reload(); | |
| }); | |
| } | |
| confirmRef.afterClosed().subscribe((response: any) => { | |
| if (response?.confirm) { | |
| const requests = this.selection.selected.map((charge) => | |
| this.loansService.executeLoansAccountChargesCommand( | |
| this.loanProductService.loanAccountPath, | |
| this.loanDetails.id, | |
| 'waive', | |
| {}, | |
| charge.id | |
| ) | |
| ); | |
| forkJoin( | |
| requests.map((request) => | |
| request.pipe( | |
| catchError((error) => of({ error })) | |
| ) | |
| ) | |
| ).subscribe(() => { | |
| this.selection.clear(); | |
| this.reload(); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts` around lines
208 - 223, The bulk-waive flow currently uses forkJoin over
executeLoansAccountChargesCommand calls which fails entirely if any single
request errors; change to swallow per-request errors so the aggregate always
completes: map each observable returned by
this.loansService.executeLoansAccountChargesCommand(...) to handle errors (e.g.,
pipe with catchError and return a sentinel result object), then forkJoin those
mapped observables and in the forkJoin subscription always call
this.selection.clear() and this.reload() and surface any per-request failures
(e.g., via a toast) so the UI is not left stale; refer to
confirmRef.afterClosed(), the array built from this.selection.selected,
executeLoansAccountChargesCommand, this.selection.clear and this.reload to
locate where to apply the change.
Comment on lines
+317
to
+330
| this.loansService | ||
| .editLoansAccountCharge(this.loanDetails.id, { ...response.data.value, dateFormat, locale }, charge.id) | ||
| .subscribe(() => this.reload()); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Deletes the charge | ||
| * @param {any} chargeId Charge Id | ||
| */ | ||
| deleteCharge(chargeId: any) { | ||
| const deleteChargeDialogRef = this.dialog.open(DeleteDialogComponent, { | ||
| data: { deleteContext: `charge id:${chargeId}` } | ||
| }); | ||
| deleteChargeDialogRef.afterClosed().subscribe((response: any) => { | ||
| if (response.delete) { | ||
| this.loansService.deleteLoansAccountCharge(this.loanDetails.id, chargeId).subscribe(() => { | ||
| this.reload(); | ||
| }); | ||
| this.loansService.deleteLoansAccountCharge(this.loanDetails.id, chargeId).subscribe(() => this.reload()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting charge mutation helpers in src/app/loans/loans.service.ts"
rg -n -C6 'editLoansAccountCharge|deleteLoansAccountCharge|executeLoansAccountChargesCommand' src/app/loans/loans.service.tsRepository: openMF/web-app
Length of output: 1352
Fix working-capital routing for charge edit/delete
editLoansAccountCharge() and deleteLoansAccountCharge() in src/app/loans/loans.service.ts are hardcoded to PUT/DELETE /loans/${accountId}/charges/${chargeId} and charges-tab.component.ts calls them without passing any loanAccountPath. Update the helpers (and/or callers) to use /${loanAccountPath}/${accountId}/charges/${chargeId} so WC accounts route to /working-capital-loans/.../charges/....
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts` around lines
317 - 330, The edit/delete calls use loansService.editLoansAccountCharge and
loansService.deleteLoansAccountCharge which currently build URLs with a
hardcoded "/loans/..." path; update them to support a variable loan account base
path (eg. "/working-capital-loans") by changing the service helpers to accept an
additional loanAccountPath parameter (or make it optional and derive from
loanDetails) and construct the endpoint as
`/${loanAccountPath}/${accountId}/charges/${chargeId}`; then update
charges-tab.component.ts where editLoansAccountCharge(...) and
deleteLoansAccountCharge(...) are called (references: editLoansAccountCharge,
deleteLoansAccountCharge, this.loanDetails.id, deleteChargeDialogRef) to pass
the appropriate loanAccountPath (or ensure loanAccountPath is available on the
component) so working-capital accounts route to
/working-capital-loans/.../charges/....
Comment on lines
+359
to
+360
| get currencyCode(): string { | ||
| return this.chargesData?.[0]?.currency?.code ?? 'EUR'; |
There was a problem hiding this comment.
Don't default the summary currency to EUR.
When an account has no charges yet, this will render €0.00 even for non-EUR loans. Fall back to the loan currency before using a hardcoded default.
Suggested fix
get currencyCode(): string {
- return this.chargesData?.[0]?.currency?.code ?? 'EUR';
+ return this.chargesData?.[0]?.currency?.code ?? this.loanDetails?.currency?.code ?? 'EUR';
}📝 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
| get currencyCode(): string { | |
| return this.chargesData?.[0]?.currency?.code ?? 'EUR'; | |
| get currencyCode(): string { | |
| return this.chargesData?.[0]?.currency?.code ?? this.loanDetails?.currency?.code ?? 'EUR'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/loans/loans-view/charges-tab/charges-tab.component.ts` around lines
359 - 360, The currencyCode getter currently falls back to a hardcoded 'EUR'
when chargesData is empty; update the get currencyCode() method to first fall
back to the loan's currency code (e.g. use this.loan?.currency?.code) before
using a final hardcoded default, so change the fallback order from
this.chargesData?.[0]?.currency?.code ?? 'EUR' to prefer
this.loan?.currency?.code and only then a safe default.
| "Enable withhold tax for this account ?": "Povolit zadrženou daň pro tento účet?", | ||
| "Disable withhold tax for this account ?": "Zakázat zadržení daně pro tento účet?", | ||
| "Are you sure you want to waive charge with id:": "Určitě se chcete vzdát poplatku s ID:", | ||
| "Are you sure you want to waive charges": "Opravdu chcete odpustit {{ count }} poplatků", |
There was a problem hiding this comment.
Use count-safe Czech phrasing for waive confirmation.
Current text can read awkwardly for singular counts (e.g., 1 poplatků). Prefer a neutral/count-safe wording or proper pluralization.
✏️ Suggested wording update
- "Are you sure you want to waive charges": "Opravdu chcete odpustit {{ count }} poplatků",
+ "Are you sure you want to waive charges": "Opravdu chcete odpustit vybrané poplatky ({{count}})?",📝 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
| "Are you sure you want to waive charges": "Opravdu chcete odpustit {{ count }} poplatků", | |
| "Are you sure you want to waive charges": "Opravdu chcete odpustit vybrané poplatky ({{count}})?", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/cs-CS.json` at line 3261, The Czech translation for
the key "Are you sure you want to waive charges" uses a fixed plural form ("{{
count }} poplatků") which yields incorrect grammar for singular; update the
value for that key to a count-safe phrasing (either a neutral form that doesn't
inject the count, e.g. a question like "Opravdu chcete poplatky odpustit?" or
implement proper ICU pluralization for the variable {{ count }} using Czech
plural rules) so singular/plural cases render correctly.
| "Export XLS": "Exporter au format XLS", | ||
| "Export to PDF": "Exporter en PDF", | ||
| "Export KYC": "Exporter KYC", | ||
| "Pay Charge": "Payer la charge", |
There was a problem hiding this comment.
Use consistent French terminology for “charge” actions.
Line 620 ("Payer la charge") and Line 740 ("Renoncer aux sélectionnés") are inconsistent with the established “frais” vocabulary used across this locale file and read awkwardly in UI. Prefer phrasing aligned with existing strings (e.g., “Payer les frais”, “Renoncer aux frais sélectionnés”).
Also applies to: 740-740
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/fr-FR.json` at line 620, Update the inconsistent
French phrases in the translations: replace the value for the "Pay Charge" key
("Payer la charge") with "Payer les frais", and replace the string "Renoncer aux
sélectionnés" (the UI label for waiving selected items) with "Renoncer aux frais
sélectionnés" so they match the established "frais" vocabulary across the locale
file.
| "View Transactions": "Visualizza transazioni", | ||
| "Waive": "Rinuncia", | ||
| "Waive Charge": "Rinuncia all'addebito", | ||
| "Waive Selected": "Rinuncia selezionati", |
There was a problem hiding this comment.
Improve Italian wording for selection-related labels.
Line 740 ("Rinuncia selezionati") and Line 2813 ("Selezionato/i") are grammatically awkward in UI context. Prefer natural labels like:
"Waive Selected": "Rinuncia ai selezionati"(or"Condona selezionati"depending on product terminology)"Selected": "Selezionati"
Also applies to: 2813-2813
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/it-IT.json` at line 740, Update the Italian
translations for the selection-related labels to more natural UI phrasing:
change the value for the translation key "Waive Selected" from "Rinuncia
selezionati" to a clearer phrase such as "Rinuncia ai selezionati" (or "Condona
selezionati" if product terminology prefers "condonare"), and change the value
for the translation key "Selected" (currently "Selezionato/i") to "Selezionati"
to match plural UI usage; locate and edit these keys ("Waive Selected" and
"Selected") in src/assets/translations/it-IT.json and update their string values
accordingly.
| "Enable withhold tax for this account ?": "Vuoi abilitare la ritenuta d'acconto per questo conto ?", | ||
| "Disable withhold tax for this account ?": "Vuoi disabilitare la ritenuta d'acconto per questo conto ?", | ||
| "Are you sure you want to waive charge with id:": "Sei sicuro di voler rinunciare alla tariffa con ID:", | ||
| "Are you sure you want to waive charges": "Sei sicuro di voler rinunciare a {{ count }} commissioni", |
There was a problem hiding this comment.
Handle singular/plural in waive confirmation message.
Line 3260 always renders plural ("{{ count }} commissioni"). If count can be 1, the message becomes grammatically wrong. Consider using separate singular/plural keys or ICU pluralization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/it-IT.json` at line 3260, The Italian translation for
the key "Are you sure you want to waive charges" always uses the plural form
("{{ count }} commissioni") which is incorrect when count === 1; update the
translation to handle singular/plural by either adding separate keys (e.g. "Are
you sure you want to waive charges.one" -> "Sei sicuro di voler rinunciare a {{
count }} commissione" and "Are you sure you want to waive charges.other" -> "Sei
sicuro di voler rinunciare a {{ count }} commissioni") or convert the value to
an ICU pluralized string, and ensure the calling code uses the i18n
pluralization API with the count parameter for the "Are you sure you want to
waive charges" key so the correct form is rendered.
| "Export XLS": "Eksportēt XLS", | ||
| "Export to PDF": "Eksportēt kā PDF", | ||
| "Export KYC": "Eksportēt KYC", | ||
| "Pay Charge": "Maksāt maksu", |
There was a problem hiding this comment.
Align Pay Charge translation with existing locale usage.
This key uses "Maksāt maksu", while the same phrase is already translated elsewhere in this file as "Maksājiet maksu". Please keep one consistent CTA form across contexts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/lv-LV.json` at line 620, The translation for the key
"Pay Charge" is inconsistent with the other occurrences; replace the value
"Maksāt maksu" with the existing consistent CTA form "Maksājiet maksu" so the
"Pay Charge" entry matches the other translations in the locale (look for the
"Pay Charge" key in the JSON and update its value).
| "Export XLS": "Exportar XLS", | ||
| "Export to PDF": "Exportar para PDF", | ||
| "Export KYC": "Exportar KYC", | ||
| "Pay Charge": "Pagar Encargo", |
There was a problem hiding this comment.
Unify loan-charge terminology in pt-PT strings.
These new strings mix terms (encargo, cobrança, renunciar, isenção) for the same charge/waive actions, which makes the UI inconsistent. Please normalize wording to the same term family already used in this locale file.
Suggested wording alignment
- "Pay Charge": "Pagar Encargo",
+ "Pay Charge": "Pagar cobrança",
- "Waive Selected": "Renunciar selecionados",
+ "Waive Selected": "Isentar selecionadas",
- "Are you sure you want to waive charges": "Tem a certeza de que pretende renunciar a {{ count }} encargos",
+ "Are you sure you want to waive charges": "Tem a certeza de que pretende isentar {{ count }} cobranças?"Also applies to: 740-740, 3260-3260
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/pt-PT.json` at line 620, The pt-PT locale uses mixed
terms like "encargo", "cobrança", "renunciar" and "isenção" for the same loan
charge/waive actions; update the keys (e.g., "Pay Charge" and the other affected
keys around the same areas referenced in the review) to use the single, existing
term family used elsewhere in this file (pick the dominant term used across the
file—e.g., "cobrança" or whatever is most common—and replace other variants like
"encargo"/"renunciar"/"isenção" to match). Ensure all occurrences at the three
noted locations are aligned to that single term so UI wording is consistent.
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
1 participant
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
Working Capital Loan account must to support Charges, Specific Due Date charges for now.
Related issues and discussion
WEB-657
Screenshots
Working Capital case
https://github.com/user-attachments/assets/d990f51e-7801-48c1-be36-bf6b8cf52829
Loan Charges with the Loan Product
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
Documentation