WEB-711: Working Capital product#3159
Merged
alberto-art3ch merged 1 commit intoopenMF:devfrom Feb 27, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Core product infra src/app/products/loan-products/common/loan-product-base.component.ts, src/app/products/loan-products/services/loan-product.service.ts, src/app/products/loan-products/models/loan-product.model.ts |
Add abstract base component, new LoanProductService managing productType (isLoanProduct/isWorkingCapital/labels/paths), and product-type constants/types/options. |
API & payloads src/app/products/products.service.ts, src/app/products/loan-products/loan-products.ts |
Parameterize productType across product APIs and payload builder; endpoints and default-field logic gated by productType (loan vs working-capital). |
Resolvers src/app/products/loan-products/loan-product.resolver.ts, src/app/products/loan-products/loan-products-template.resolver.ts, src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts |
Resolvers now use productType (route or LoanProductService.loanProductPath) when fetching templates/products. |
Create / Edit flows src/app/products/loan-products/create-loan-product/*, src/app/products/loan-products/edit-loan-product/* |
Components extend base, call super(), initialize productType, adjust ViewChild static flags, add ngAfterViewInit/cdr, branch payload/submit for loan vs working-capital, and prepend dynamic product-type labels in step headings. |
Stepper & step components src/app/products/loan-products/loan-product-stepper/... (details, currency, settings, terms, charges, payment-strategy, summary, etc.) |
Many step components now extend base; templates and form controls are conditionally created/rendered via loanProductService.isLoanProduct / isWorkingCapital; terms/settings split into type-specific branches and validators adjusted. |
List, View, and UI changes src/app/products/loan-products/loan-products.component.ts, .../loan-products.component.html, .../loan-products.component.scss, src/app/products/loan-products/view-loan-product/* |
Add product-type selector and Create dropdown (loan / working-capital), modify create/edit/view links to include productType queryParam, components extend base; layout/icon updates. |
Loans account stepper src/app/loans/loans-account-stepper/loans-account-terms-step/* |
Component now extends base; template and TS gated by loanProductService.isLoanProduct (e.g., delinquency/installment-level controls only shown for loan products). |
Toolbar, currency form, icons src/app/core/shell/toolbar/toolbar.component.html, src/app/organization/currencies/manage-currencies/manage-currencies.component.html, src/app/shared/icons.module.ts |
Removed Tasks link from toolbar, changed add-currency to explicit button click (removed ngSubmit), and registered faCaretDown icon. |
Validation adjustments src/app/products/manage-delinquency-buckets/delinquency-range/* |
Replace some max validators with min validators and make maximumAgeDays optional in template. |
| View-only / allocation guards assorted view-* files and advance-payment-allocation components |
Add guards so advanced/future allocation blocks and certain panels render only for loan products; update permissions/Export/Edit button behaviors and queryParam passing. |
Translations src/assets/translations/*.json (multiple locales) |
Add Working Capital product strings and new UI labels (Create Working Capital Product, Working Capital Product, Period Payment Rate, amortization options Effective Interest Rate/Flat and other keys) across locales. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant UI as Create/Edit UI
participant LPS as LoanProductService (productType)
participant ProductsSvc as ProductsService
participant API as BackendAPI
User->>UI: Open Create (select productType)
UI->>LPS: initialize(productType)
UI->>UI: Render steps guarded by LPS.isLoanProduct / isWorkingCapital
User->>UI: Submit form
UI->>LPS: check isLoanProduct()
alt Loan product
UI->>ProductsSvc: createLoanProduct('', payload)
else Working Capital
UI->>ProductsSvc: createLoanProduct('workingcapital', payload)
end
ProductsSvc->>API: POST /{productType}loanproducts
API-->>ProductsSvc: 201 Created
ProductsSvc-->>UI: success
UI-->>User: navigate to list/view
Estimated code review effort
🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
- WEB-673 Create a tasks sections just like it was available in the legacy mifos community app #3106: touches the same toolbar Tasks link (this PR removes it while
#3106modifies it) — direct overlap on toolbar template. - WEB-632-loan-product-default-empty-multiples-fields #3084: modifies loan-product currency/multiples UI and conditional controls — overlaps with currency-step simplification and control gating here.
- WEB-630 Days till field allows negative values in create delinquency range form #3051: changes delinquency-range validators/attributes — overlaps with the age-field validation adjustments in this PR.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The pull request title 'WEB-711: Working Capital product' directly aligns with the primary objective, introducing a new Working Capital product configuration feature. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Tip
Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
alberto-art3ch
force-pushed
the
WEB-711/wc-product-configuration
branch
from
7834709 to
706e702
Compare
alberto-art3ch
force-pushed
the
WEB-711/wc-product-configuration
branch
from
706e702 to
1a35e9e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/organization/currencies/manage-currencies/manage-currencies.component.html (1)
12-41:⚠️ Potential issue | 🟠 MajorPrevent native form submit and preserve Enter key behavior.
Removing
(ngSubmit)while keepingtype="submit"means clicking the button can trigger a native form submit (page reload/navigation) afteraddCurrency()runs, and pressing Enter won’t add a currency. RestorengSubmitor switch the button totype="button"and explicitly handle Enter if desired.🔧 Suggested fix (restore ngSubmit)
- <form `#formRef`="ngForm" [formGroup]="currencyForm" class="layout-row gap-20px"> + <form + `#formRef`="ngForm" + [formGroup]="currencyForm" + (ngSubmit)="addCurrency()" + class="layout-row gap-20px" + > ... - <button - type="submit" + <button + type="submit" mat-raised-button ... - (click)="addCurrency()"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/organization/currencies/manage-currencies/manage-currencies.component.html` around lines 12 - 41, The form currently has type="submit" on the add button but no (ngSubmit) handler, which allows a native form submit and breaks Enter-key handling; restore the Angular submit binding by adding (ngSubmit)="addCurrency()" to the <form `#formRef`="ngForm" [formGroup]="currencyForm"> declaration (so addCurrency() is invoked for both button clicks and Enter) and remove any direct (click) reliance if you prefer form submit semantics, ensuring currencyForm and addCurrency() remain the canonical handlers.src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.html (1)
27-27:⚠️ Potential issue | 🟡 MinorHTML
min="0"conflicts with the TypeScriptValidators.min(1)forminimumAgeDays.The native HTML attribute allows
0but the reactive form validator (Validators.min(1)in the TS) rejects it. This mismatch can confuse users: the browser's native validation hint will accept0, while the Angular form will show an error. Align the HTML attribute with the validator.Proposed fix
- <input matInput type="number" required formControlName="minimumAgeDays" min="0" /> + <input matInput type="number" required formControlName="minimumAgeDays" min="1" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.html` at line 27, The HTML input's min attribute conflicts with the reactive form validator: change the input with formControlName "minimumAgeDays" to use min="1" so the native browser constraint matches the TypeScript validator Validators.min(1); update any related UI hints/placeholder text or aria-describedby that reference the minimum if present to reflect the new minimum of 1 to keep UX consistent with the FormControl validation.src/app/products/loan-products/loan-products.component.html (1)
102-103:⚠️ Potential issue | 🟠 MajorPass
productTypeas a query parameter in row navigation.When a user clicks a row, the navigation uses
[routerLink]="[row.id]"without aproductTypequery parameter. TheLoanProductResolverexecutes immediately and depends onloanProductService.loanProductPath, which is a computed getter that returns different API paths (''for loan products,'workingcapital'for working capital) based on the service'sproductTypevalue.Since the service is never initialized during navigation,
productTyperemains an empty string. This causesisLoanProductto returnfalse(empty string ≠LOAN_PRODUCT_TYPE.LOAN), andloanProductPathincorrectly returns'workingcapital'. The resolver then requests/workingcapitalloanproducts/{id}instead of/loanproducts/{id}, breaking the view for standard loan products.Update the row link to pass the product type:
[routerLink]="[row.id]" [queryParams]="{ productType: loanProductSelector.value }"(or similar), or initialize the service in the resolver or view component before the product is fetched.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.html` around lines 102 - 103, The row navigation is missing the productType query param, causing LoanProductResolver (and loanProductService.loanProductPath) to compute the wrong API path; update the mat-row/routerLink usage (the <tr mat-row ... [routerLink]="[row.id]" ...> element) to include the productType as a query parameter (e.g., add [queryParams]="{ productType: loanProductSelector.value }" or equivalent bound value) so the resolver receives productType on navigation, or alternatively ensure LoanProductResolver initializes loanProductService.productType before calling loanProductPath; reference mat-row/routerLink, LoanProductResolver, and loanProductService.loanProductPath when making the change.
🟡 Minor comments (13)
src/assets/translations/cs-CS.json-2719-2722 (1)
2719-2722:⚠️ Potential issue | 🟡 MinorFix mistranslation of “Flat” amortization type.
“Byt” means “apartment,” which is incorrect for a flat‑interest method and inconsistent with the existing “Flat” label in catalogs. Consider “Paušální” or “Plochá sazba” for clarity and consistency.
✏️ Suggested fix
- "Flat": "Byt" + "Flat": "Paušální"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` around lines 2719 - 2722, The translation for the key "Flat" is incorrect ("Byt" means apartment); update the value for the "Flat" entry in the JSON (near the "Equal installments", "Equal principal payments", "Effective Interest Rate" keys) to a correct amortization term such as "Paušální" or "Plochá sazba" (pick one for consistency with catalog labels) so the "Flat" amortization type is translated accurately.src/assets/translations/de-DE.json-2721-2723 (1)
2721-2723:⚠️ Potential issue | 🟡 MinorFix mistranslation of “Flat” in amortization types.
“Wohnung” means apartment and is incorrect here. Use a finance-appropriate term (e.g., “Pauschal” or keep “Flat”) to avoid user confusion.
✅ Suggested fix
- "Flat": "Wohnung" + "Flat": "Pauschal"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/de-DE.json` around lines 2721 - 2723, The translation for the amortization type key "Flat" is incorrect ("Wohnung" means apartment); update the value for the "Flat" key in the JSON (the translation entry with "Flat": ...) to a finance-appropriate term such as "Pauschal" or simply "Flat" to reflect the amortization meaning instead of an apartment. Ensure you only change the value string for the "Flat" key and keep surrounding JSON formatting intact.src/assets/translations/lv-LV.json-2721-2723 (1)
2721-2723:⚠️ Potential issue | 🟡 MinorFix incorrect translation for “Flat” (currently “Dzīvoklis”).
“Dzīvoklis” means “apartment,” which is incorrect for interest/amortization context and inconsistent with other “Flat” translations in this file.✏️ Suggested fix
- "Flat": "Dzīvoklis" + "Flat": "Plakans"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lv-LV.json` around lines 2721 - 2723, The translation for the key "Flat" is incorrect ("Dzīvoklis"); replace the value for the "Flat" key with the correct financial term (e.g., "Fiksēts") so it reflects a flat/fixed rate in interest/amortization context and matches other "Flat" translations in the file; update the entry "Flat": "Fiksēts" in src/assets/translations/lv-LV.json.src/assets/translations/fr-FR.json-428-428 (1)
428-428:⚠️ Potential issue | 🟡 MinorNormalize capitalization to match existing French style.
Current strings are in title case, while nearby entries use sentence case (e.g., “Créer un produit de dépôt fixe”). Suggest aligning for consistency.
💡 Suggested wording
- "Create Working Capital Product": "Créer un Produit de Fonds de Roulement", + "Create Working Capital Product": "Créer un produit de fonds de roulement", ... - "Working Capital Product": "Produit de Fonds de Roulement", + "Working Capital Product": "Produit de fonds de roulement",Also applies to: 1155-1155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/fr-FR.json` at line 428, The French translation for the key "Create Working Capital Product" uses title case; change its value to sentence case to match existing style (e.g., update "Créer un Produit de Fonds de Roulement" to "Créer un produit de fonds de roulement"); apply the same normalization to the other occurrence mentioned (the key at the other location referenced in the review) so all instances use sentence case consistently.src/assets/translations/es-CL.json-2722-2724 (1)
2722-2724:⚠️ Potential issue | 🟡 MinorFix incorrect translation for “Flat”.
“Departamento” doesn’t match the meaning. Consider “Monto fijo” (used elsewhere) or “Plano”.
✅ Suggested fix (aligning with existing “Flat” translation)
- "Flat": "Departamento" + "Flat": "Monto fijo"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` around lines 2722 - 2724, The translation for the key "Flat" is incorrect ("Departamento"); update the value for the "Flat" key in src/assets/translations/es-CL.json to the correct phrase used elsewhere (e.g., "Monto fijo") so it matches existing translations and meaning; ensure you only change the value for the "Flat" key and keep the surrounding JSON structure intact.src/assets/translations/pt-PT.json-428-429 (1)
428-429:⚠️ Potential issue | 🟡 MinorUse PT‑PT terminology for “Working Capital”.
In European Portuguese, “working capital” is typically “fundo de maneio”, while “capital de giro” is BR‑PT. To keep pt‑PT consistent, consider updating both labels.
✏️ Suggested translation tweak
- "Create Working Capital Product": "Criar Produto de Capital de Giro", + "Create Working Capital Product": "Criar Produto de Fundo de Maneio", ... - "Working Capital Product": "Produto de Capital de Giro", + "Working Capital Product": "Produto de Fundo de Maneio",Also applies to: 1151-1152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/pt-PT.json` around lines 428 - 429, Replace BR‑Portuguese "capital de giro" with PT‑PT "fundo de maneio" in this translations file: update the value for the key "Create Working Capital Product" (currently "Criar Produto de Capital de Giro") to use "fundo de maneio" and also update the other occurrences referenced in the comment (the entries around the keys at lines ~1151-1152) so all translations consistently use "fundo de maneio" instead of "capital de giro".src/assets/translations/it-IT.json-428-428 (1)
428-428:⚠️ Potential issue | 🟡 MinorAlign working‑capital labels with existing Italian phrasing.
These two strings diverge from the existing “Prodotto di …” sentence‑case pattern (e.g., “Prodotto di prestito”). Consider aligning for consistency.
✏️ Suggested wording update
- "Create Working Capital Product": "Crea Prodotto Capitale Circolante", + "Create Working Capital Product": "Crea prodotto di capitale circolante", ... - "Working Capital Product": "Prodotto Capitale Circolante", + "Working Capital Product": "Prodotto di capitale circolante",Also applies to: 1151-1151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/it-IT.json` at line 428, Update the Italian translation keys that currently read "Create Working Capital Product" to match the project's sentence-case "Prodotto di …" pattern; locate the JSON entries for the key "Create Working Capital Product" (occurrences include the shown entry and the other occurrence around the file) and change the value to "Prodotto di capitale circolante" so it aligns with other strings like "Prodotto di prestito".src/assets/translations/lt-LT.json-2718-2720 (1)
2718-2720:⚠️ Potential issue | 🟡 Minor“Flat” is mistranslated in this financial context.
“Butas” means “apartment” in Lithuanian; for amortization type this should be closer to “Fixed/Flat rate.” Consider updating the label (and optionally aligning other “Flat” entries in this locale for consistency).
📝 Suggested fix
- "Flat": "Butas" + "Flat": "Fiksuota palūkanų norma"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lt-LT.json` around lines 2718 - 2720, The translation for the key "Flat" is incorrect ("Butas" means apartment); update the value for the "Flat" key in the lt-LT JSON to a finance-appropriate term such as "Fiksuota" or "Fiksuotos palūkanos" (choose the concise form used elsewhere in the file for consistency), and scan other occurrences of the "Flat" label in this locale to apply the same corrected translation for uniformity.src/app/products/loan-products/loan-products.resolver.ts-29-31 (1)
29-31:⚠️ Potential issue | 🟡 MinorResolver should read
route.queryParams['productType']to support working capital products, matching the pattern used in LoanProductsTemplateResolver.The resolver currently hardcodes an empty string, which only pre-loads standard loan products. The component can switch to working capital products via a selector and dynamically refetch, so the feature works at runtime. However, this creates an inconsistency:
LoanProductsTemplateResolvercorrectly readsroute.queryParams['productType'] || ''(line 29 of loan-products-template.resolver.ts), butLoanProductsResolverignores query parameters entirely. The resolver should follow the same pattern to support pre-loading the appropriate product type based on route parameters, improving consistency and future extensibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.resolver.ts` around lines 29 - 31, The resolve() method in LoanProductsResolver currently calls productsService.getLoanProducts('') and ignores route query params; update it to read the product type from the ActivatedRouteSnapshot (use route.queryParams['productType'] || '') and pass that value into this.productsService.getLoanProducts(...) so it matches the pattern used by LoanProductsTemplateResolver and preloads working-capital products when present.src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.ts-53-56 (1)
53-56:⚠️ Potential issue | 🟡 MinorRemove unused import:
LoanProductServiceis accessed through the base component.The
LoanProductServiceimport at line 22 is redundant. The service is injected and accessed viathis.loanProductService(inherited fromLoanProductBaseComponent), not directly via the import. Remove the import statement to clean up unused dependencies.The timing concern in the original comment is not an issue:
loanProductService.initialize(productType)is called in the parent component's constructor (line 114) before the child component is instantiated, ensuring the service state is set beforecreateLoanProductDetailsForm()accesses it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.ts` around lines 53 - 56, Remove the unused import of LoanProductService from loan-product-details-step.component.ts: the component inherits and uses the service via this.loanProductService from LoanProductBaseComponent, so delete the standalone LoanProductService import (and any unused import entry) to clean up the file; verify there are no other direct references to the LoanProductService symbol in the file after removal.src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html-40-80 (1)
40-80:⚠️ Potential issue | 🟡 MinorGuard error rendering with
touchedorform.submittedto match mat-error's default behavior.Switching to
@if (control.hasError(...))bypasses Angular Material's defaultErrorStateMatcher, which shows errors only whencontrol.invalid && (control.touched || form.submitted). Update guards to use the correct condition:🧩 Corrected guard pattern
- `@if` (loanProductCurrencyForm.get('digitsAfterDecimal')?.hasError('required')) { + `@if` ( + loanProductCurrencyForm.get('digitsAfterDecimal')?.hasError('required') && + (loanProductCurrencyForm.get('digitsAfterDecimal')?.touched || + loanProductCurrencyForm.submitted) + ) {Apply the same pattern (
touched || form.submitted) to all other error checks:digitsAfterDecimal?.hasError('min'),inMultiplesOf?.hasError('min'), andinstallmentAmountInMultiplesOf?.hasError('min').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html` around lines 40 - 80, The template currently guards mat-error rendering with only hasError checks which bypass Angular Material's ErrorStateMatcher; update each conditional to mirror mat-error behavior by checking the control's invalid state combined with touched or the parent form's submitted flag (e.g., use loanProductCurrencyForm.get('digitsAfterDecimal') and test .invalid && (loanProductCurrencyForm.get('digitsAfterDecimal')?.touched || loanProductCurrencyForm.submitted)). Apply the same pattern for digitsAfterDecimal (min and required), inMultiplesOf (min), and installmentAmountInMultiplesOf (min), keeping the existing formControlName bindings and respecting loanProductService.isLoanProduct block for the installment field.src/assets/translations/ne-NE.json-2719-2721 (1)
2719-2721:⚠️ Potential issue | 🟡 MinorKeep amortization option translations consistent.
FlatandEqual principal paymentsappear elsewhere in this locale file as “फ्ल्याट” and “समान मूल भुक्तानी”; consider matching those terms here to avoid mixed terminology.📝 Consistency tweak (optional)
- "Equal principal payments": "समान मूल भुक्तानीहरू", - "Flat": "समतल" + "Equal principal payments": "समान मूल भुक्तानी", + "Flat": "फ्ल्याट"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/ne-NE.json` around lines 2719 - 2721, The translations for amortization options are inconsistent: change the value for the "Flat" key from "समतल" to the same term used elsewhere ("फ्ल्याट") so it matches existing occurrences, and verify "Equal principal payments" remains "समान मूल भुक्तानी" to preserve consistency; update the JSON entries for the keys "Flat" and "Equal principal payments" accordingly.src/assets/translations/es-MX.json-2726-2729 (1)
2726-2729:⚠️ Potential issue | 🟡 MinorFix mistranslation of “Flat” amortization type.
Line 2729 uses “Departamento”, which reads as “apartment” and doesn’t match the financial meaning. Consider “Plana” (or “Tasa plana”) to align terminology.💡 Suggested translation
- "Flat": "Departamento" + "Flat": "Plana"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` around lines 2726 - 2729, The translation for the amortization type key "Flat" is incorrect ("Departamento"); update the JSON value for the "Flat" key to a finance-accurate term such as "Plana" or "Tasa plana" so it reflects flat/amortized interest instead of "apartment"; locate the "Flat" entry in the translations block alongside "Equal installments", "Equal principal payments", and "Effective Interest Rate" and replace its value accordingly.
🧹 Nitpick comments (10)
src/assets/translations/es-CL.json (1)
428-428: Match capitalization with other “Create … Product” labels.Most button labels use sentence case (“Crear producto de …”). Consider lowering “Producto” here for UI consistency.
✅ Suggested adjustment
- "Create Working Capital Product": "Crear Producto de Capital de Trabajo", + "Create Working Capital Product": "Crear producto de Capital de Trabajo",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` at line 428, The Spanish translation key "Create Working Capital Product" currently maps to "Crear Producto de Capital de Trabajo" and should use sentence case to match other "Create … Product" labels; update the value to "Crear producto de capital de trabajo" for consistency (locate the string by the key "Create Working Capital Product" in the es-CL.json file and replace the mapped text).src/app/shared/icons.module.ts (1)
154-283: Pre-existing duplicate icon registrations inlibrary.addIcons().Three icons are registered twice (not introduced by this PR, but visible in the final state):
Icon First occurrence Second occurrence faBanLine 159 Line 258 faBarsLine 160 Line 277 faUndoLine 244 Line 278
FaIconLibrary.addIconsis idempotent so there's no functional harm, but removing the duplicates reduces noise. Consider cleaning these up while touching this file.🧹 Remove duplicate registrations
- faBan, // line 159 - keep this one faBars, ... - faBan, // line 258 - remove ... - faBars, // line 277 - remove - faUndo, // line 278 - remove🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/icons.module.ts` around lines 154 - 283, The icons list passed to library.addIcons contains duplicate entries for faBan, faBars, and faUndo; remove the second occurrences so each of those symbols appears only once in the array passed to library.addIcons (refer to the faBan, faBars, and faUndo identifiers within the library.addIcons call) to clean up the registration list.src/app/products/loan-products/models/loan-product.model.ts (1)
32-41:PRODUCT_TYPESlabels are hardcoded English strings — consider using translation keys.The rest of the application uses
translatepipes and translation keys for user-facing text. These labels will bypass i18n if rendered directly.Suggested approach
export const PRODUCT_TYPES = [ { type: LOAN_PRODUCT_TYPE.LOAN, - label: 'Loan' + label: 'labels.catalogs.Loan' }, { type: LOAN_PRODUCT_TYPE.WORKING_CAPITAL, - label: 'Working Capital' + label: 'labels.catalogs.Working Capital' } ] as const;Then use
| translatein the template when renderinglabel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/models/loan-product.model.ts` around lines 32 - 41, PRODUCT_TYPES currently uses hardcoded English labels which bypass i18n; replace the label values with translation keys (e.g., 'loanProduct.type.loan', 'loanProduct.type.workingCapital') in the PRODUCT_TYPES constant and keep the type references to LOAN_PRODUCT_TYPE, then ensure templates that render these labels use the translate pipe (e.g., label | translate) so the UI picks up translations; update any consumers of PRODUCT_TYPES to expect keys instead of literal text.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (2)
62-99: Two adjacent@ifblocks check the same condition — consolidate them.Lines 62 and 85 both guard on
loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied. These can be merged into a single@ifblock containing bothmat-form-fieldelements.Proposed consolidation
- <div class="flex-98 layout-row-wrap responsive-column gap-2percent"> - `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) { - <mat-form-field class="flex-fill flex-32"> - ...Over Amount Calculation Type... - </mat-form-field> - } - - `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) { - <mat-form-field class="flex-fill flex-32"> - ...Over Amount... - </mat-form-field> - } - </div> + <div class="flex-98 layout-row-wrap responsive-column gap-2percent"> + `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) { + <mat-form-field class="flex-fill flex-32"> + ...Over Amount Calculation Type... + </mat-form-field> + <mat-form-field class="flex-fill flex-32"> + ...Over Amount... + </mat-form-field> + } + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 62 - 99, The two adjacent template guards both check loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied; consolidate them by replacing the two separate `@if` blocks with a single `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) that wraps both mat-form-field elements (the mat-select for overAppliedCalculationType and the number input for overAppliedNumber), removing the duplicate conditional and ensuring the combined block has a single opening and closing `@if` so validation mat-error blocks remain intact for loanProductTermsForm.get('overAppliedCalculationType') and loanProductTermsForm.get('overAppliedNumber').
654-677: Three separate@if (allowFixedLength())blocks can be consolidated.Lines 655, 665, and 671 all check the same
allowFixedLength()condition independently. Merging them into a single@ifblock reduces template noise and repeated evaluations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 654 - 677, The three repeated guards all calling allowFixedLength() should be merged into one `@if` (allowFixedLength()) block inside the existing `@if` (loanProductService.isLoanProduct) so the <h4>, <mat-form-field> (formControlName="fixedLength") and the <span> that renders loanProductTermsForm.value.repaymentFrequencyType via repaymentFrequencyTypeData are all rendered together; update the template to wrap those three elements in a single conditional to avoid repeated evaluations while keeping the existing translate/find/translateKey pipes and formControlName intact.src/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.ts (1)
55-61: Inconsistent regex pattern forminimumAgeDaysbetween create-range and edit-range.In
create-range.component.ts,minimumAgeDaysuses pattern'^(0|[1-9][0-9]*)$'(which matches0), while here in edit-range it uses'^(0*[1-9][0-9]*?)$'(which does not match0). Both rely onValidators.min(1)to enforce ≥ 1, so the behavioral outcome is the same, but the inconsistency makes the intent unclear and could confuse future maintainers. Consider unifying the patterns across both components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.ts` around lines 55 - 61, The regex for the FormControl minimumAgeDays is inconsistent between the create and edit flows: update the pattern used in edit-range (the control named minimumAgeDays in EditRangeComponent) to match the pattern used in CreateRangeComponent (i.e. use the same regex as in create-range for minimumAgeDays) so both components behave and read the same; keep the Validators.min(1) validation as-is to enforce >=1.src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.ts (1)
22-22: Unused import:LoanProductService.
LoanProductServiceis imported directly (line 22) but the component accesses it through the inheritedloanProductServicefromLoanProductBaseComponent. This direct import appears unused.Proposed fix
-import { LoanProductService } from '../../services/loan-product.service';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.ts` at line 22, Remove the unused direct import of LoanProductService from LoanProductDetailsStepComponent: the component should use the inherited loanProductService provided by LoanProductBaseComponent instead. Open the LoanProductDetailsStepComponent and delete the import line for LoanProductService and any constructor parameter or private property that injects it directly (if present), leaving only the inherited loanProductService reference from LoanProductBaseComponent in methods and template. Ensure no other references to the deleted import remain and run a quick typecheck to confirm compilation.src/app/products/loan-products/services/loan-product.service.ts (1)
36-44:loanProductTypeLabelandloanProductPathimplicitly treat non-loan as Working Capital.These getters use a binary
isLoanProductcheck, so any value that isn't'loan'(including'', typos, or future product types) is treated as Working Capital. A more defensive approach would explicitly check for both known types.Suggested defensive approach
get loanProductTypeLabel(): string { - return this.isLoanProduct - ? this.translateService.instant('labels.heading.Loan Product') - : this.translateService.instant('labels.heading.Working Capital Product'); + if (this.isLoanProduct) { + return this.translateService.instant('labels.heading.Loan Product'); + } + if (this.isWorkingCapital) { + return this.translateService.instant('labels.heading.Working Capital Product'); + } + return this.translateService.instant('labels.heading.Loan Product'); // safe fallback } get loanProductPath(): string { - return this.isLoanProduct ? '' : 'workingcapital'; + if (this.isWorkingCapital) { + return 'workingcapital'; + } + return ''; // default to loan product path }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/services/loan-product.service.ts` around lines 36 - 44, The getters loanProductTypeLabel and loanProductPath currently treat any non-'loan' value as Working Capital via isLoanProduct; update them to explicitly check for known product types (e.g., productType === 'loan' and productType === 'workingCapital') and handle unknown values with a safe default or fallback (e.g., return an 'Unknown' label or empty path and optionally log/warn). Modify loanProductTypeLabel and loanProductPath to use explicit comparisons against the concrete product type field (instead of relying on isLoanProduct) and ensure the fallback branch does not incorrectly classify typos or empty strings as Working Capital.src/app/products/products.service.ts (1)
32-51: Consider typed responses now that signatures changed.
These methods still returnObservable<any>and acceptanypayloads. With the newproductTypeparameter, this is a good point to introduce response interfaces and aLoanProductTypeunion for stricter typing (even as a follow-up task).Based on learnings: “In TypeScript files (e.g., src/app/.../*.ts), avoid using Observable as a project-wide pattern for API responses. Treat typing the full API response layer as a cross-cutting refactor and track it as a separate enhancement backlog item rather than addressing it in individual bug fixes.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 32 - 51, The methods getLoanProducts, createLoanProduct, getLoanProductsTemplate, getLoanProduct and updateLoanProduct currently use Observable<any> and any payloads; define a LoanProduct interface (and any DTOs such as CreateLoanProductDto/UpdateLoanProductDto) and a LoanProductType union (e.g., LoanProductType = 'personal'|'business'|...) in a new products.models.ts, then update those method signatures to use typed payloads and responses (e.g., Observable<LoanProduct[]> for getLoanProducts, Observable<LoanProduct> for getLoanProduct/create/update, and typed template response for getLoanProductsTemplate) and replace any occurrences of parameter type any with the new DTO types so the service is strictly typed.src/app/products/loan-products/loan-products.component.ts (1)
97-97: Tighten typing for product options and fetch results.Line 97 and Line 275 still use
any. Consider aLoanProductinterface and typed options sodataSourceand the fetch callback are strongly typed.♻️ Suggested refactor
- loanProductOptions: any = PRODUCT_TYPES; + loanProductOptions: readonly typeof PRODUCT_TYPES = PRODUCT_TYPES; ... - next: (data: any) => { + next: (data: LoanProduct[]) => { this.loanProductsData = data; this.dataSource.data = this.loanProductsData; },As per coding guidelines: "For Angular code: verify component separation, trackBy on ngFor, strict type safety, and clean observable patterns."
Based on learnings: "In TypeScript files (e.g., src/app/.../.ts), avoid using Observable as a project-wide pattern for API responses. Treat typing the full API response layer as a cross-cutting refactor and track it as a separate enhancement backlog item rather than addressing it in individual bug fixes. When you encounter API responses, introduce specific interfaces/types for the response shapes and use proper typing instead of any, updating services and resolvers accordingly."Also applies to: 275-277
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` at line 97, Replace the loose any usage by introducing a LoanProduct interface (fields matching PRODUCT_TYPES entries and the API fetch shape) and use it to type loanProductOptions and related vars: change loanProductOptions: any = PRODUCT_TYPES to loanProductOptions: LoanProduct[] = PRODUCT_TYPES as LoanProduct[]; update dataSource to a strongly typed container (e.g., MatTableDataSource<LoanProduct> or LoanProduct[]) and change the fetch callback signature/return type to Observable<LoanProduct[]> or Promise<LoanProduct[]> so the component and any handlers operate on LoanProduct instead of any; adjust any downstream uses (sorting, filtering, templates) to the new type but leave full API-layer response refactors to the separate backlog item.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
src/app/core/shell/toolbar/toolbar.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/organization/currencies/manage-currencies/manage-currencies.component.htmlsrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.htmlsrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.tssrc/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-product.resolver.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.component.scsssrc/app/products/loan-products/loan-products.component.tssrc/app/products/loan-products/loan-products.resolver.tssrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/services/loan-product.service.tssrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.htmlsrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.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 with no reviewable changes (1)
- src/app/core/shell/toolbar/toolbar.component.html
...p/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
Show resolved
Hide resolved
src/app/products/loan-products/create-loan-product/create-loan-product.component.html
Show resolved
Hide resolved
src/app/products/loan-products/create-loan-product/create-loan-product.component.ts
Show resolved
Hide resolved
src/app/products/loan-products/edit-loan-product/edit-loan-product.component.html
Show resolved
Hide resolved
src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts
Show resolved
Hide resolved
...n-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
Show resolved
Hide resolved
...n-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
Show resolved
Hide resolved
src/app/products/loan-products/services/loan-product.service.ts
Outdated
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-711/wc-product-configuration
branch
from
1a35e9e to
2640904
Compare
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/products/products.service.ts (1)
386-392:⚠️ Potential issue | 🟠 MajorUse parameterized endpoint for
getProductMixes()to match other loan product methods.The
getProductMixes()method hardcodes/loanproducts(line 388-392), while other loan product methods use/${productType}loanproductspattern (e.g., lines 33-50). For consistency withgetLoanProducts(),createLoanProduct(), and similar methods,getProductMixes()should accept and use aproductTypeparameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 386 - 392, getProductMixes currently calls the hardcoded '/loanproducts' endpoint; change the method signature getProductMixes() to accept a productType parameter (e.g., productType: string) and use it to build the endpoint as `${productType}loanproducts`, while preserving the HttpParams logic (set 'associations' to 'productMixes') and returning this.http.get with the new URL; update any callers to pass the appropriate productType to getProductMixes.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts (1)
872-898:⚠️ Potential issue | 🟠 Major
loanScheduleProcessingType.codethrows when the field is absent (create path).For a new loan product,
loanProductsTemplate.loanScheduleProcessingTypemay benull/undefinedbefore the user selects a schedule type. Accessing.codedirectly throws aTypeError. The same risk applies todaysInYearCustomStrategyOptions[0].idwhen the options array is empty.🛠️ Proposed fix
this.loanProductSettingsForm.addControl( 'loanScheduleProcessingType', new UntypedFormControl( - this.loanProductsTemplate.loanScheduleProcessingType.code || + this.loanProductsTemplate.loanScheduleProcessingType?.code || LoanProducts.LOAN_SCHEDULE_PROCESSING_TYPE_HORIZONTAL, [Validators.required] ) ); ... if (this.useDaysInYearCustomStrategy) { const daysInYearCustomStrategy: string = this.loanProductsTemplate.daysInYearCustomStrategy?.id ? this.loanProductsTemplate.daysInYearCustomStrategy.id - : this.daysInYearCustomStrategyOptions[0].id; + : this.daysInYearCustomStrategyOptions[0]?.id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 872 - 898, In validateAdvancedPaymentStrategyControls, avoid directly accessing loanProductsTemplate.loanScheduleProcessingType.code and daysInYearCustomStrategyOptions[0].id which can be undefined on the create path; change the logic to read a safe default (use optional chaining or conditional checks) for loanScheduleProcessingType (fall back to LoanProducts.LOAN_SCHEDULE_PROCESSING_TYPE_HORIZONTAL if loanProductsTemplate.loanScheduleProcessingType is missing) and for daysInYearCustomStrategy only use daysInYearCustomStrategyOptions[0].id after confirming the array has at least one element (otherwise use a sensible default or null), then add/remove the 'loanScheduleProcessingType' and 'daysInYearCustomStrategy' controls on loanProductSettingsForm as before.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html (1)
321-325:⚠️ Potential issue | 🟡 MinorClear button is always visible — condition is always truthy.
loanProductSettingsForm.controls.delinquencyBucketIdevaluates to theFormControlobject itself, which is always truthy regardless of whether a value has been selected. The button should only appear when a delinquency bucket is actually chosen.🛠️ Proposed fix
- `@if` (loanProductSettingsForm.controls.delinquencyBucketId) { + `@if` (loanProductSettingsForm.controls.delinquencyBucketId.value) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html` around lines 321 - 325, The template currently shows the clear button based on the existence of the FormControl object (loanProductSettingsForm.controls.delinquencyBucketId) which is always truthy; change the condition to check the control's selected value instead (e.g., loanProductSettingsForm.controls.delinquencyBucketId.value or loanProductSettingsForm.get('delinquencyBucketId')?.value) so the button only renders when a delinquency bucket has been chosen, and keep the existing (click)="clearProperty($event, 'delinquencyBucketId')" wiring.
♻️ Duplicate comments (10)
src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts (1)
32-35:⚠️ Potential issue | 🟠 MajorResolver depends on pre-initialized
LoanProductServicestate — fragile on direct navigation/refresh.The resolver reads
loanProductService.loanProductPathwithout extractingproductTypefromroute.queryParams, unlikeLoanProductsTemplateResolverwhich does. On page refresh or direct URL navigation for a working-capital product, theBehaviorSubjectretains its default ('loan'), causing the wrong API endpoint to be called.Consider reading
productTypefrom query params and callingthis.loanProductService.initialize(productType)before accessingloanProductPath, matching the pattern inLoanProductsTemplateResolver.Proposed fix
resolve(route: ActivatedRouteSnapshot): Observable<any> { const loanProductId = route.parent.paramMap.get('productId'); + const productType = route.queryParams['productType'] || ''; + this.loanProductService.initialize(productType); return this.productsService.getLoanProduct(this.loanProductService.loanProductPath, loanProductId, true); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts` around lines 32 - 35, The resolver's resolve method reads loanProductService.loanProductPath before ensuring the service is initialized, causing wrong endpoint on direct navigation; update resolve to read productType from route.queryParams (as LoanProductsTemplateResolver does), call this.loanProductService.initialize(productType) before accessing this.loanProductService.loanProductPath, and then call this.productsService.getLoanProduct(this.loanProductService.loanProductPath, loanProductId, true); if initialize is async, wait for/chain its completion (Promise/Observable) so loanProductPath is correct before calling getLoanProduct.src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.html (1)
88-97: Usetrack fund.idinstead oftrack fundfor the@forloop.Tracking by object reference forces unnecessary DOM re-creation when the array is reassigned. Use the stable unique identifier
fund.id.As per coding guidelines: verify
trackByon*ngFor— this applies equally to the new@forcontrol flow syntax.Proposed fix
- `@for` (fund of fundData; track fund) { + `@for` (fund of fundData; track fund.id) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.html` around lines 88 - 97, The `@for` loop is tracking by object reference ("track fund") which causes unnecessary DOM rebuilds; change the track expression to the stable identifier by updating the loop to use "track fund.id" (i.e., the `@for` declaration iterating over fundData with loop variable fund) and confirm fund.id is present on the objects used by the mat-select/formControlName="fundId" binding.src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts (1)
44-55:⚠️ Potential issue | 🟠 MajorGuard against missing currency options before patching the form.
IfcurrencyOptionsis empty/undefined,currencybecomesundefinedandpatchValuewill throw when accessingcurrency.code.🛡️ Suggested safeguard
- this.currencyData = this.loanProductsTemplate.currencyOptions; - const currency = this.loanProductsTemplate.currency ? this.loanProductsTemplate.currency : this.currencyData[0]; + this.currencyData = this.loanProductsTemplate.currencyOptions ?? []; + const currency = this.loanProductsTemplate.currency ?? this.currencyData[0]; + if (!currency) { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts` around lines 44 - 55, The code assumes this.loanProductsTemplate.currencyOptions has at least one entry and uses currency.code without guarding; update the initialization in the loan-product-currency-step.component so you first verify this.loanProductsTemplate.currencyOptions is an array with length > 0 (populate this.currencyData accordingly), then compute currency as either this.loanProductsTemplate.currency or a safe fallback object (e.g., with empty string/0 fields) when currencyOptions is empty/undefined, and finally call this.loanProductCurrencyForm.patchValue using that safe currency (or set default/blank values for currencyCode, digitsAfterDecimal, inMultiplesOf) to avoid accessing properties on undefined.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
324-390:⚠️ Potential issue | 🟠 MajorAvoid re‑creating the form after
ngOnChangespatches.
ngOnChangesnow patches values and adds/removes controls. RecallingcreateloansAccountTermsForm()at the start ofngOnInitcan wipe those changes when inputs are already present, leading to missing controls/disabled states.🐛 Suggested fix
ngOnInit() { - this.createloansAccountTermsForm(); this.maxDate = this.settingsService.maxFutureDate;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 324 - 390, ngOnInit currently always calls createloansAccountTermsForm(), which can overwrite controls and patches applied earlier in ngOnChanges; update ngOnInit to only create the form if it doesn't already exist (check loansAccountTermsForm truthiness) or move createloansAccountTermsForm() into a guarded branch so ngOnChanges-patched controls/disabled states are preserved; refer to createloansAccountTermsForm, ngOnInit, ngOnChanges and loansAccountTermsForm to locate where to add the guard and ensure subsequent code (patchValue, addControl/removeControl) operates on the existing form instance.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (1)
114-149:⚠️ Potential issue | 🟠 MajorAlign Period Payment Rate min constraints and error text.
Min/default use0.01while max enforcesmin=1and the error fallback shows1. This blocks valid values (e.g.,0.5) and displays misleading errors. Align everything to0.01.🐛 Proposed fix
- <input type="number" matInput formControlName="minPeriodPaymentRate" [min]="0.01" step="0.01" /> + <input type="number" matInput formControlName="minPeriodPaymentRate" [min]="0.01" step="0.01" /> ... - <strong>{{ loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong> ... - <input type="number" matInput formControlName="periodPaymentRate" required [min]="0.01" step="0.01" /> + <input type="number" matInput formControlName="periodPaymentRate" required [min]="0.01" step="0.01" /> ... - <strong>{{ loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong> ... - <input type="number" matInput formControlName="maxPeriodPaymentRate" [min]="1" step="0.01" /> + <input type="number" matInput formControlName="maxPeriodPaymentRate" [min]="0.01" step="0.01" /> ... - <strong>{{ loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 114 - 149, The maxPeriodPaymentRate field enforces min=1 and shows a fallback of 1 while the other rate fields use 0.01; update the template and form validators so all period payment rate minima are 0.01: change the input attribute [min] on the element with formControlName="maxPeriodPaymentRate" to 0.01 and update the error fallback expression (loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 0.01) to match, and also ensure the FormControl definition for "maxPeriodPaymentRate" in the loanProductTermsForm (component class) uses Validators.min(0.01) so template and model validations are aligned.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts (2)
747-751:⚠️ Potential issue | 🟠 Major
some()used for side effects — still unresolved.The
some()callback never returns a value; it is being used purely for iteration with a push side-effect. Biome also flags this aslint/suspicious/useIterableCallbackReturn.🛠️ Proposed fix
- this.transactionProcessingStrategyDataBase.some((cn: CodeName) => { + this.transactionProcessingStrategyDataBase.forEach((cn: CodeName) => { if (LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)) { this.transactionProcessingStrategyData.push(cn); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 747 - 751, The code is using Array.prototype.some for iteration with side effects: transactionProcessingStrategyDataBase.some(...) pushes into transactionProcessingStrategyData but the callback never returns a boolean; replace the some usage with an appropriate iterator such as forEach or build a filtered array via filter and assign or concat. Specifically, change the block that calls transactionProcessingStrategyDataBase.some(...) and instead either iterate with transactionProcessingStrategyDataBase.forEach((cn: CodeName) => { if (LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)) this.transactionProcessingStrategyData.push(cn); }) or use transactionProcessingStrategyData = transactionProcessingStrategyDataBase.filter((cn: CodeName) => LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)); to fix the lint warning and preserve behavior.
126-166:⚠️ Potential issue | 🟠 MajorForm patched with
nulltransactionProcessingStrategyCode— still unresolved.
transactionProcessingStrategyCodeisnullat line 99 and is patched into the form at line 132. The actual value is assigned at lines 164–165, afterpatchValuehas already run. The form control remainsnulluntil the user or a subsequent change triggers an update.🛠️ Proposed fix
+ const transactionProcessingStrategyCode = + this.loanProductsTemplate.transactionProcessingStrategyCode || + this.transactionProcessingStrategyData[0].code; this.loanProductSettingsForm.patchValue({ amortizationType: this.loanProductsTemplate.amortizationType.id, ... transactionProcessingStrategyCode: transactionProcessingStrategyCode, ... }); - - transactionProcessingStrategyCode = - this.loanProductsTemplate.transactionProcessingStrategyCode || this.transactionProcessingStrategyData[0].code;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 126 - 166, The patch is setting transactionProcessingStrategyCode into the form before it’s assigned, causing a null value to be written; move the fallback assignment of transactionProcessingStrategyCode (currently done with this.transactionProcessingStrategyData[0].code) to occur before calling loanProductSettingsForm.patchValue (or use the fallback inline when building the object), so ensure transactionProcessingStrategyCode is resolved from this.loanProductsTemplate.transactionProcessingStrategyCode || this.transactionProcessingStrategyData[0].code prior to invoking loanProductSettingsForm.patchValue.src/app/products/loan-products/create-loan-product/create-loan-product.component.ts (1)
267-269:loanIncomeCapitalizationFormnull guard — LGTM.The null check added at Line 267 before accessing
.validaddresses the prior NPE concern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/create-loan-product/create-loan-product.component.ts` around lines 267 - 269, You added a null-check for loanIncomeCapitalizationForm before accessing .valid which fixes the NPE; keep that guard and make the conditional consistent by using the same null-safety approach for loanIncomeCapitalizationForm and loanProductAccountingForm (i.e., ensure both are checked for null/undefined before reading .valid) so the combined condition only evaluates .valid when the form exists; update the conditional around loanIncomeCapitalizationForm and loanProductAccountingForm in create-loan-product.component to use consistent null-safety and remove any remaining direct .valid access without a guard.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (2)
363-372: Booleantruefix — LGTM.The control is now correctly initialized with
true(boolean) instead of'true'(string).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 363 - 372, The change ensures the form control 'disallowExpectedDisbursements' is added with a boolean true instead of the string 'true'; verify in the valueChanges handler on this.loanProductTermsForm.get('allowApprovedDisbursedAmountsOverApplied') that this.loanProductTermsForm.addControl('disallowExpectedDisbursements', new UntypedFormControl(true)) uses the boolean literal true (not a string) and ensure any other initializations or tests referencing 'disallowExpectedDisbursements' expect a boolean; also confirm the guard this.loanProductService.isLoanProduct remains in place so the control is only added for loan products.
420-425:interestRateFrequencyTypeData[0]?.idfix — LGTM.The control initialization now falls back to the first array element with optional chaining, addressing the previous
undefinedvalue issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 420 - 425, The form control 'interestRateFrequencyType' in loan-product-terms-step.component.ts was previously getting undefined; fix it by initializing the UntypedFormControl with a safe fallback using this.loanProductsTemplate.interestRateFrequencyType?.id ?? +this.interestRateFrequencyTypeData[0]?.id so it uses the template value when present or the first entry from interestRateFrequencyTypeData (coerced to number) otherwise, and keep Validators.required on the control to preserve validation.
🧹 Nitpick comments (11)
src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html (1)
18-22: Use a stable identifier for the@fortrack expression.
track currencytracks by object reference, which forces DOM re-creation when the array reference changes (e.g., from a fresh API response). Usecurrency.codeinstead.As per coding guidelines: verify
trackByon*ngFor— this applies equally to the new@forcontrol flow syntax.Proposed fix
- `@for` (currency of currencyData; track currency) { + `@for` (currency of currencyData; track currency.code) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html` around lines 18 - 22, The `@for` loop is using an unstable tracker "track currency" which tracks by object reference and causes DOM recreation when the array is replaced; update the control flow to track by a stable identifier by replacing "track currency" with "track currency.code" so the loop over currencyData uses currency.code as the trackBy key (in the template containing the `@for` and <mat-option> elements).src/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.ts (1)
100-103: Subscriptions are not cleaned up — memory leak risk.Both
.valueChangessubscriptions (lines 100, 102) are never unsubscribed. Consider usingDestroyRef+takeUntilDestroyed()or storing subscriptions and cleaning them up inngOnDestroyto avoid leaks when the component is destroyed and re-created (e.g., navigating between stepper steps).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.ts` around lines 100 - 103, The two subscriptions on currencyCode.valueChanges and multiDisburseLoan.valueChanges (conditional on loanProductService.isLoanProduct) are never unsubscribed which can leak memory; update loan-product-charges-step.component to unsubscribe when the component is destroyed by using Angular's DestroyRef + takeUntilDestroyed() on both observables (or store the Subscription objects and call unsubscribe in ngOnDestroy), e.g., apply takeUntilDestroyed(this.destroyRef) to the currencyCode.valueChanges and to multiDisburseLoan.valueChanges so chargesDataSource is still reset but no subscriptions remain after component teardown.src/app/products/loan-products/loan-products.resolver.ts (1)
29-31: Hardcoded empty string works, but consider parameterizing for consistency.This resolver always initializes with standard loan products, while the component provides a selector to dynamically switch to working capital products at runtime. This design works, but it's inconsistent with
LoanProductsTemplateResolver, which acceptsproductTypefrom route queryParams. For improved flexibility and consistency, consider accepting a route parameter (or queryParam) similar to the template resolver, allowing the resolver to support both product types rather than leaving dynamic fetching to the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.resolver.ts` around lines 29 - 31, The resolver's resolve() currently calls productsService.getLoanProducts('') with a hardcoded empty string; update resolve() in LoanProductsResolver to accept the product type from the ActivatedRoute (either route params or queryParams) like LoanProductsTemplateResolver does, extract the productType (e.g. 'standard' or 'workingCapital') and pass it into productsService.getLoanProducts(productType) so the resolver can return the appropriate product set rather than always using the empty-string default.src/app/products/loan-products/loan-products.component.ts (2)
238-244: Consider centralizing the UI → API product-type mapping.
The''vs'workingcapital'mapping is duplicated inimportLoanProduct()andfetchProducts(). Extracting it to a small helper (or LoanProductService) reduces drift if the backend path changes.Also applies to: 266-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` around lines 238 - 244, The mapping of UI product type to the API path ('' vs 'workingcapital') is duplicated in importLoanProduct() and fetchProducts(); introduce a single helper to centralize this logic (e.g., a private method getApiProductType(selected: LOAN_PRODUCT_TYPE) in the component or a new public method mapUiToApiType(selected: LOAN_PRODUCT_TYPE) on ProductsService) and replace the inline ternaries using loanProductSelector.value with calls to that helper when calling productsService.createLoanProduct(...) and productsService.getLoanProducts(...), ensuring both importLoanProduct() and fetchProducts() use the same mapping point.
45-98: Tighten typing for the product selector/options.
Using a typedFormControl<LOAN_PRODUCT_TYPE>and avoidinganyhere will keep the new selector aligned with the strict type-safety goal.♻️ Suggested typing
-import { UntypedFormControl } from '@angular/forms'; +import { FormControl } from '@angular/forms'; ... - loanProductSelector = new UntypedFormControl(); + loanProductSelector = new FormControl<LOAN_PRODUCT_TYPE>(LOAN_PRODUCT_TYPE.LOAN); ... - loanProductOptions: any = PRODUCT_TYPES; + readonly loanProductOptions = PRODUCT_TYPES;As per coding guidelines:
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` around lines 45 - 98, The loan product selector and options are using broad types—replace loanProductSelector: UntypedFormControl and loanProductOptions: any with strict types: declare loanProductSelector as FormControl<LOAN_PRODUCT_TYPE | null> (or FormControl<LOAN_PRODUCT_TYPE> if non-nullable) and type loanProductOptions to match PRODUCT_TYPES' shape (e.g., ReadonlyArray<LOAN_PRODUCT_TYPE> or Record<LOAN_PRODUCT_TYPE, string> depending on how PRODUCT_TYPES is defined); update imports to use FormControl from `@angular/forms` and adjust any code that reads/writes loanProductSelector.value to account for the new type, and change loanProductsData and dataSource to specific types instead of any (e.g., Array<LoanProduct> and MatTableDataSource<LoanProduct>) to keep overall strict typing consistent.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (2)
56-101: Use a stabletrackkey for over‑applied calculation options.
track overAppliedCalculationTypetracks by object identity and can cause churn when the array instance changes. Prefer a stable key such asoverAppliedCalculationType.id.🔧 Suggested tweak
- `@for` (overAppliedCalculationType of overAppliedCalculationTypeData; track overAppliedCalculationType) { + `@for` (overAppliedCalculationType of overAppliedCalculationTypeData; track overAppliedCalculationType.id) {As per coding guidelines:
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 56 - 101, The ng-template loop for overAppliedCalculationType uses object identity tracking ("track overAppliedCalculationType"), which causes unnecessary DOM churn; update the template's for loop over overAppliedCalculationTypeData to track by a stable key (use overAppliedCalculationType.id) instead, i.e., change the track expression to use the id property on overAppliedCalculationType and ensure the overAppliedCalculationTypeData items expose a unique id field referenced by the template (overAppliedCalculationType and overAppliedCalculationTypeData).
153-620: Use stabletrackkeys for the option loops in this section.
The@forblocks for repayment start date types, interest rate frequency types, and floating rates track by object reference. Switching to a stable key (e.g.,.id) improves DOM reuse and avoids unnecessary re-renders.🔧 Suggested tweak
- `@for` (repaymentStartDateType of repaymentStartDateTypeOptions; track repaymentStartDateType) { + `@for` (repaymentStartDateType of repaymentStartDateTypeOptions; track repaymentStartDateType.id) { ... - `@for` (interestRateFrequencyType of interestRateFrequencyTypeData; track interestRateFrequencyType) { + `@for` (interestRateFrequencyType of interestRateFrequencyTypeData; track interestRateFrequencyType.id) { ... - `@for` (floatingRate of floatingRateData; track floatingRate) { + `@for` (floatingRate of floatingRateData; track floatingRate.id) {As per coding guidelines:
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 153 - 620, The template uses object-reference tracking in the `@for` option loops causing unstable DOM updates; change the loop tracking to stable id keys (e.g., replace track repaymentStartDateType with track repaymentStartDateType.id for the repaymentStartDateTypeOptions loop, use track interestRateFrequencyType.id for interestRateFrequencyTypeData, and track floatingRate.id for floatingRateData) or implement and reference dedicated trackBy functions (e.g., trackByRepaymentStartDateId, trackByInterestRateFreqId, trackByFloatingRateId) in the component to return item.id so Angular can reuse DOM nodes reliably.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
187-322: Please resolve the TODO onisFloatingInterestRate.
Leaving the TODO unresolved makes it unclear whether the defaulting logic is correct. Clarify the intended behavior and remove the TODO once confirmed.If helpful, I can help validate the expected state transitions for
isLoanProductLinkedToFloatingRate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 187 - 322, The TODO around isFloatingInterestRate is unresolved and ambiguous; confirm the intended default behavior and then replace the ternary with explicit logic: use loansAccountTermsData.isLoanProductLinkedToFloatingRate to decide the default value for the form field (isFloatingInterestRate) and update loansAccountTermsForm accordingly—for example, if a linked-to-floating-rate product should not allow toggling, set isFloatingInterestRate to null and disable the control (loansAccountTermsForm.controls.isFloatingInterestRate.disable()), otherwise set a concrete default (true or false) and ensure the control is enabled; remove the TODO after making this explicit change in ngOnChanges.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (1)
189-191: Subscription toadvancedTransactionProcessingStrategyis never unsubscribed.This observable subscription leaks when the component is destroyed. Use
takeUntilDestroyed()(Angular 16+) for automatic cleanup.♻️ Proposed fix
+ import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; this.processingStrategyService.advancedTransactionProcessingStrategy + .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe((value: boolean) => { this.isAdvancedTransactionProcessingStrategy = value; });Inject
DestroyRefviainject(DestroyRef)(from@angular/core) or use thetakeUntilDestroyed()overload that captures it from the injection context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 189 - 191, The subscription to processingStrategyService.advancedTransactionProcessingStrategy in the component leaks because it is never unsubscribed; replace the plain subscribe call with a takeUntilDestroyed() pipe to auto-cleanup (Angular 16+), e.g. pipe( takeUntilDestroyed() ). If your project requires explicit DestroyRef injection, inject DestroyRef via inject(DestroyRef) from `@angular/core` and pass it to takeUntilDestroyed(thisDestroyRef) or use the injection-context overload; update the subscription that sets isAdvancedTransactionProcessingStrategy accordingly to use this piped observable.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html (1)
19-24:@fortracks items by object reference throughout the template.Every
@forloop usestrack <variable>(e.g.,track amortizationType), which tracks by object identity. If the underlying data arrays are recreated (e.g., on component input change), Angular will tear down and recreate all DOM nodes. Prefer tracking by a stable unique key.Example:
- `@for` (amortizationType of amortizationTypeData; track amortizationType) { + `@for` (amortizationType of amortizationTypeData; track amortizationType.id) {Apply the same change to all other
@forloops over option arrays in this template. As per coding guidelines,trackBy(or its@forequivalenttrack) should use a stable identity key.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html` around lines 19 - 24, The template's `@for` loops currently use object identity tracking (e.g., "track amortizationType") which causes DOM recreation when arrays are reallocated; update the track expressions to use a stable unique key such as the item's id (for example change "track amortizationType" to "track amortizationType.id" in the loop over amortizationTypeData) and make the same change for every other `@for` loop rendering option arrays in this template (use each item's stable unique property like id or value in the track clause to ensure stable DOM nodes).src/app/products/loan-products/create-loan-product/create-loan-product.component.ts (1)
290-298: No null guard onstatic: falseViewChild steps in theloanProductgetter.
loanProductChargesStepandloanProductAccountingStepare both declared with{ static: false }, meaning they resolve after view initialization and may beundefinedif their host template element is conditionally rendered. Spreadingundefined.loanProductChargesin the getter throws aTypeError.Although the edit-loan-product component follows the same pattern (consistent within the codebase), the create path introduces new conditional rendering for WC vs loan that increases the risk surface.
♻️ Proposed fix
const loanProduct = { ...this.loanProductDetailsStep.loanProductDetails, ...this.loanProductCurrencyStep.loanProductCurrency, ...this.loanProductTermsStep.loanProductTerms, ...this.loanProductSettingsStep.loanProductSettings, - ...this.loanProductChargesStep.loanProductCharges, - ...this.loanProductAccountingStep.loanProductAccounting + ...(this.loanProductChargesStep?.loanProductCharges ?? {}), + ...(this.loanProductAccountingStep?.loanProductAccounting ?? {}) };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/create-loan-product/create-loan-product.component.ts` around lines 290 - 298, The loanProduct getter spreads properties from ViewChild steps that are declared with { static: false } (notably loanProductChargesStep and loanProductAccountingStep) and can be undefined at runtime; modify the getter (loanProduct) to guard those ViewChilds by using safe fallbacks when spreading (e.g., spread this.loanProductChargesStep?.loanProductCharges or an empty object, and likewise for this.loanProductAccountingStep) so spreading undefined cannot throw a TypeError; update any similar steps in the same getter (loanProductDetailsStep, loanProductCurrencyStep, loanProductTermsStep, loanProductSettingsStep) if they may also be absent.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
src/app/core/shell/toolbar/toolbar.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/organization/currencies/manage-currencies/manage-currencies.component.htmlsrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.htmlsrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.tssrc/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-product.resolver.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.component.scsssrc/app/products/loan-products/loan-products.component.tssrc/app/products/loan-products/loan-products.resolver.tssrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/services/loan-product.service.tssrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.htmlsrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.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 with no reviewable changes (1)
- src/app/core/shell/toolbar/toolbar.component.html
🚧 Files skipped from review as they are similar to previous changes (12)
- src/app/products/loan-products/loan-product.resolver.ts
- src/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.ts
- src/assets/translations/de-DE.json
- src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.html
- src/app/products/loan-products/models/loan-product.model.ts
- src/app/products/loan-products/loan-products.ts
- src/assets/translations/pt-PT.json
- src/app/products/loan-products/loan-products.component.scss
- src/assets/translations/cs-CS.json
- src/assets/translations/ne-NE.json
- src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts
- src/app/shared/icons.module.ts
src/app/organization/currencies/manage-currencies/manage-currencies.component.html
Show resolved
Hide resolved
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
Show resolved
Hide resolved
src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts
Show resolved
Hide resolved
...oducts/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.ts
Outdated
Show resolved
Hide resolved
...ts/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-711/wc-product-configuration
branch
from
2640904 to
b61977c
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/products/loan-products/loan-products.component.ts (1)
87-98:⚠️ Potential issue | 🟡 MinorStrengthen selector/options typing—trackBy already implemented.
loanProductSelectoris currentlyUntypedFormControlandloanProductOptionsis typed asany, which weakens compile-time safety. The template already uses the@forcontrol flow withtrack loanProduct.type, so the trackBy concern is addressed. However, consider typingFormControlwithLoanProductTypeand markingloanProductOptionsasreadonlyfor consistency.♻️ Suggested typing
-import { UntypedFormControl } from '@angular/forms'; +import { FormControl } from '@angular/forms'; -import { LOAN_PRODUCT_TYPE, PRODUCT_TYPES } from './models/loan-product.model'; +import { LOAN_PRODUCT_TYPE, PRODUCT_TYPES, LoanProductType } from './models/loan-product.model'; -loanProductSelector = new UntypedFormControl(); +loanProductSelector = new FormControl<LoanProductType>(LOAN_PRODUCT_TYPE.LOAN); -loanProductOptions: any = PRODUCT_TYPES; +readonly loanProductOptions = PRODUCT_TYPES;The template already implements proper tracking in the product selector's
@forloop withtrack loanProduct.type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` around lines 87 - 98, Replace the untyped control and any-typed options with specific types: change loanProductSelector (currently UntypedFormControl) to a typed FormControl<LoanProductType | null> (import FormControl from `@angular/forms`) and type loanProductOptions as a readonly array (e.g., readonly loanProductOptions: ReadonlyArray<LoanProductType> = PRODUCT_TYPES) or the exact type of PRODUCT_TYPES; mark it readonly for immutability. Keep the existing PRODUCT_TYPES constant as the value and update any usages to match the new types (references: loanProductSelector, loanProductOptions, PRODUCT_TYPES, and ensure LoanProductType is imported/defined).
♻️ Duplicate comments (13)
src/assets/translations/es-CL.json (3)
2721-2724:⚠️ Potential issue | 🟡 MinorUse “capital” to match existing phrasing for equal principal payments.
There’s already “Pagos iguales de capital” elsewhere in the file; keep the term consistent.✏️ Suggested adjustment
- "Equal principal payments": "Pagos iguales de principal", + "Equal principal payments": "Pagos iguales de capital",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` around lines 2721 - 2724, Replace the Spanish translation value for the key "Equal principal payments" so it matches the existing phrasing used elsewhere; change its value from "Pagos iguales de principal" to "Pagos iguales de capital" to ensure consistency across translations.
427-428:⚠️ Potential issue | 🟡 MinorLowercase “producto” for label consistency.
“Create Loan Product” uses “Crear producto …”; keep the same style here.✏️ Suggested adjustment
- "Create Working Capital Product": "Crear Producto de Capital de Trabajo", + "Create Working Capital Product": "Crear producto de Capital de Trabajo",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` around lines 427 - 428, Change the translation value for the key "Create Working Capital Product" to use a lowercase "producto" to match the existing style used for "Create Loan Product"; locate the entry for the "Create Working Capital Product" key in src/assets/translations/es-CL.json and replace "Crear Producto de Capital de Trabajo" with "Crear producto de Capital de Trabajo".
2790-2795:⚠️ Potential issue | 🟡 MinorAlign delinquency/period-frequency terminology with existing labels.
“Delinquency Buckets” uses “Grupos de morosidad” and earlier labels use “del período”; mirroring that keeps phrasing consistent.✏️ Suggested adjustment
- "Delinquency Bucket Classification": "Clasificación del segmento de morosidad", + "Delinquency Bucket Classification": "Clasificación de grupos de morosidad", - "Period Payment Frequency": "Frecuencia de pago por período", - "Period Payment Frequency Type": "Tipo de frecuencia de pago por período" + "Period Payment Frequency": "Frecuencia de pago del período", + "Period Payment Frequency Type": "Tipo de frecuencia de pago del período"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` around lines 2790 - 2795, Update the Spanish translations to match existing terminology by changing the values for the keys "Delinquency Bucket Classification", "Period Payment Frequency", and "Period Payment Frequency Type" so they use the same phrasing with "del período" and the established term for delinquency groups (e.g., use "Clasificación de Grupos de morosidad" or "Clasificación de Grupos de morosidad del período" and change "Period Payment Frequency" -> "Frecuencia de pago del período" and "Period Payment Frequency Type" -> "Tipo de frecuencia de pago del período"), ensuring consistency with the existing "Grupos de morosidad" and "del período" labels.src/assets/translations/lt-LT.json (1)
2789-2794:⚠️ Potential issue | 🟡 MinorNormalize the period-frequency phrasing/casing.
“Laikotarpis Mokėjimo dažnumas” should align with the lowercase/genitive phrasing used elsewhere.
🔧 Suggested update
- "Period Payment Frequency": "Laikotarpis Mokėjimo dažnumas", + "Period Payment Frequency": "Laikotarpio mokėjimo dažnumas",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lt-LT.json` around lines 2789 - 2794, The Lithuanian value "Laikotarpis Mokėjimo dažnumas" should be normalized to the lowercase/genitive phrasing used elsewhere; update that translation value to "Laikotarpio mokėjimo dažnumas" by replacing the existing string for the key "Period Payment Frequency" (look for the exact value "Laikotarpis Mokėjimo dažnumas" and change it to "Laikotarpio mokėjimo dažnumas") so it matches the genitive/casing pattern used by "Period Payment Frequency Type".src/app/products/loan-products/services/loan-product.service.ts (1)
24-29:⚠️ Potential issue | 🟠 MajorTighten
initialize()toLoanProductTypeand avoid raw string checks.Line 24 accepts
string, so invalid values can slip in and leave the service in an unintended state. PreferLoanProductTypeand rely on the enum values (or map/validate before calling).🔧 Suggested fix
- initialize(productType: string): void { - if (productType === 'loan') { - this.productType.next(LOAN_PRODUCT_TYPE.LOAN); - } else if (productType === 'working-capital') { - this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL); - } - } + initialize(productType: LoanProductType): void { + this.productType.next(productType); + }As per coding guidelines,
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/services/loan-product.service.ts` around lines 24 - 29, The initialize method currently accepts a raw string and checks for 'loan'/'working-capital', which allows invalid inputs; change the initialize signature to accept the LoanProductType type (the enum used is LOAN_PRODUCT_TYPE) or a validated/mappped value, then remove the raw string comparisons and directly call this.productType.next(...) with the corresponding LOAN_PRODUCT_TYPE enum value; update any callers to pass a LoanProductType (or perform mapping/validation before calling initialize) so the service cannot enter an unintended state.src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts (1)
44-55:⚠️ Potential issue | 🟠 MajorGuard against empty currency options before patching the form.
Line 45 falls back to
currencyData[0]; ifcurrencyOptionsis empty/undefined,currencybecomesundefinedandpatchValuewill throw oncurrency.code.🛡️ Suggested safeguard
- this.currencyData = this.loanProductsTemplate.currencyOptions; - const currency = this.loanProductsTemplate.currency ? this.loanProductsTemplate.currency : this.currencyData[0]; + this.currencyData = this.loanProductsTemplate.currencyOptions ?? []; + const currency = this.loanProductsTemplate.currency ?? this.currencyData[0]; + if (!currency) { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts` around lines 44 - 55, The code assumes this.loanProductsTemplate.currencyOptions and its first element exist before calling this.loanProductCurrencyForm.patchValue, which will throw if currencyOptions is empty or undefined; update the initialization around currencyData and currency (referencing currencyData, loanProductsTemplate.currencyOptions, and currency) to first guard that this.loanProductsTemplate?.currencyOptions is a non-empty array (or that this.loanProductsTemplate.currency exists), and if not either return/skip patching or set a safe default currency object (with code, decimalPlaces and inMultiplesOf defaults) before calling this.loanProductCurrencyForm.patchValue so accessing currency.code, currency.decimalPlaces and currency.inMultiplesOf is always safe.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts (1)
490-502:⚠️ Potential issue | 🟠 MajorGuard against undefined
chargeOffBehaviourbefore accessing.value.
If the field is missing, this throws at runtime.🛡️ Suggested guard
- if (!this.loanProduct.chargeOffBehaviour.value) { + if (!this.loanProduct.chargeOffBehaviour || !this.loanProduct.chargeOffBehaviour.value) { this.loanProduct.chargeOffBehaviour = this.stringEnumOptionDataLookUp( this.loanProduct.chargeOffBehaviour, this.loanProductsTemplate.chargeOffBehaviourOptions ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 490 - 502, The code accesses this.loanProduct.chargeOffBehaviour.value without ensuring chargeOffBehaviour is defined; update the guard in loan-product-summary.component (inside the isAdvancedPaymentAllocation block) to check for this.loanProduct.chargeOffBehaviour truthiness (e.g., if (!this.loanProduct.chargeOffBehaviour || !this.loanProduct.chargeOffBehaviour.value) ) before calling stringEnumOptionDataLookUp so stringEnumOptionDataLookUp receives a safe value and no runtime error is thrown.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts (2)
747-751:⚠️ Potential issue | 🟠 MajorReplace
some()withforEach()to match intent and linting.The callback passed to
some()never returns a boolean (Line 747), which trips linting and is semantically incorrect for side effects.because this matches prior feedback in this file.
🛠️ Suggested fix
- this.transactionProcessingStrategyDataBase.some((cn: CodeName) => { + this.transactionProcessingStrategyDataBase.forEach((cn: CodeName) => { if (LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)) { this.transactionProcessingStrategyData.push(cn); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 747 - 751, The code uses transactionProcessingStrategyDataBase.some(...) with a callback that performs side effects without returning a boolean; replace that some call with transactionProcessingStrategyDataBase.forEach(...) so each CodeName (cn) is checked via LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code) and matching cn values are pushed onto transactionProcessingStrategyData, removing the incorrect use of some and satisfying the linter.
126-165:⚠️ Potential issue | 🟠 MajorPatch
transactionProcessingStrategyCodeafter computing it.Line 132 patches
transactionProcessingStrategyCodewhile it is stillnull, so the form keeps a null value until user interaction. Compute the value first, then patch it.because this matches prior feedback in this file.
🛠️ Suggested fix
- this.loanProductSettingsForm.patchValue({ - ... - transactionProcessingStrategyCode: transactionProcessingStrategyCode, - ... - }); - - transactionProcessingStrategyCode = - this.loanProductsTemplate.transactionProcessingStrategyCode || this.transactionProcessingStrategyData[0].code; + transactionProcessingStrategyCode = + this.loanProductsTemplate.transactionProcessingStrategyCode || this.transactionProcessingStrategyData[0].code; + this.loanProductSettingsForm.patchValue({ + ... + transactionProcessingStrategyCode: transactionProcessingStrategyCode, + ... + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 126 - 165, The form is patched with transactionProcessingStrategyCode before that variable is computed, causing a null value to be saved; move the computation/assignment of transactionProcessingStrategyCode (use this.loanProductsTemplate.transactionProcessingStrategyCode || this.transactionProcessingStrategyData[0].code) to before the call to this.loanProductSettingsForm.patchValue(...) so that patchValue receives the resolved code from transactionProcessingStrategyCode (refer to transactionProcessingStrategyCode, this.loanProductsTemplate, this.transactionProcessingStrategyData and this.loanProductSettingsForm.patchValue).src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html (2)
799-816:⚠️ Potential issue | 🟡 MinorUse repayment‑event specific tooltip keys.
Lines 805 and 815 reuse the tooltip for “Maximum outstanding loan account balance”, which doesn’t describe the repayment-event fields. Please switch to dedicated tooltip keys and add the translations.
because this repeats prior feedback on the same lines.
🛠️ Suggested fix
- matTooltip="{{ 'tooltips.Maximum outstanding loan account balance' | translate }}" + matTooltip="{{ 'tooltips.DueDaysForRepaymentEvent' | translate }}" ... - matTooltip="{{ 'tooltips.Maximum outstanding loan account balance' | translate }}" + matTooltip="{{ 'tooltips.OverdueDaysForRepaymentEvent' | translate }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html` around lines 799 - 816, The two mat-form-field inputs bound to formControlName="dueDaysForRepaymentEvent" and "overDueDaysForRepaymentEvent" are using the wrong tooltip key; replace the matTooltip string key "'tooltips.Maximum outstanding loan account balance' | translate" with repayment-event specific keys (e.g. "'tooltips.Due days for repayment event' | translate" and "'tooltips.Overdue days for repayment event' | translate") and add the corresponding entries to the translation files so the tooltip text reflects the repayment-event context.
30-41:⚠️ Potential issue | 🟡 MinorFix NPV day count validation and error copy.
Line 32 relies on the HTML
[min]attribute only; the reactive control still accepts0/negative because the TS form control only hasValidators.required. The error message (Lines 39-40) references “Default Principal” and the block uses legacy*ngIfinstead of the new@ifstyle used elsewhere.Please:
- Add
Validators.min(1)to thenpvDayCountcontrol increateLoanProductSettingsForm.- Update the error text to reference NPV day count.
- Switch the error block to
@ifto match the template’s style.because this matches prior feedback on the same lines.
🛠️ Suggested template fix
- <mat-error - *ngIf=" - loanProductSettingsForm.get('npvDayCount')?.hasError('required') && - (loanProductSettingsForm.get('npvDayCount')?.touched || loanProductSettingsForm.get('npvDayCount')?.dirty) - " - > - {{ 'labels.catalogs.Default' | translate }} {{ 'labels.inputs.Principal' | translate }} - {{ 'labels.commons.is required' | translate }} - </mat-error> + `@if` ( + loanProductSettingsForm.get('npvDayCount')?.invalid && + (loanProductSettingsForm.get('npvDayCount')?.touched || loanProductSettingsForm.get('npvDayCount')?.dirty) + ) { + <mat-error> + {{ 'labels.inputs.NPV day count' | translate }} {{ 'labels.commons.is' | translate }} + <strong>{{ 'labels.commons.required' | translate }}</strong> + </mat-error> + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html` around lines 30 - 41, The npvDayCount control currently only has Validators.required and relies on the HTML min attribute; update createLoanProductSettingsForm to add Validators.min(1) to the 'npvDayCount' FormControl so the reactive form rejects 0/negative values, then update the template's error block for the npvDayCount field: replace the legacy *ngIf with the project's `@if` style, and change the error copy to reference "NPV day count" (or the translated key for that label) instead of "Default Principal". Ensure you reference the 'npvDayCount' control name and the createLoanProductSettingsForm function when making these changes.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (1)
321-338:⚠️ Potential issue | 🟠 MajorWorking‑capital rate validators should allow 0.01 minimum.
Lines 321–338 set
Validators.min(1)for min/default/max period payment rate, but the template exposes a0.01minimum. This mismatch blocks valid values (e.g.,0.5). Align the validators to0.01to match the UI.because this is the same min‑value inconsistency raised in the HTML.
🛠️ Suggested fix
- minPeriodPaymentRate: [ - '', - [ - Validators.min(1) - ] - ], + minPeriodPaymentRate: ['', [Validators.min(0.01)]], ... - periodPaymentRate: [ - '', - [ - Validators.required, - Validators.min(1) - ] - ], + periodPaymentRate: ['', [Validators.required, Validators.min(0.01)]], ... - maxPeriodPaymentRate: [ - '', - [ - Validators.min(1) - ] - ], + maxPeriodPaymentRate: ['', [Validators.min(0.01)]],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 321 - 338, The form controls minPeriodPaymentRate, periodPaymentRate and maxPeriodPaymentRate in LoanProductTermsStepComponent currently use Validators.min(1) which conflicts with the UI minimum of 0.01; update those validators to Validators.min(0.01) (and keep Validators.required on periodPaymentRate) so the FormGroup validation matches the template and allows values like 0.5 or 0.01.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (1)
114-149:⚠️ Potential issue | 🟠 MajorAlign period‑payment rate minimums and error fallbacks.
Line 145 uses
[min]="1"while Lines 116 and 125 use0.01. The error fallback values (Lines 119, 139, 148) still show1, which conflicts with the displayed minimum. This blocks valid rates like0.5. Please align all three inputs to0.01and update the error fallback values. Also update the working‑capital form validators inloan-product-terms-step.component.tstoValidators.min(0.01)so UI and reactive validation match.because this matches prior feedback in this file.
🛠️ Suggested template fix
- <input type="number" matInput formControlName="maxPeriodPaymentRate" [min]="1" step="0.01" /> + <input type="number" matInput formControlName="maxPeriodPaymentRate" [min]="0.01" step="0.01" /> ... - <strong>{{ loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong> ... - <strong>{{ loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong> ... - <strong>{{ loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 114 - 149, The three period-payment rate inputs and their error fallbacks are inconsistent: change the maxPeriodPaymentRate input [min] from 1 to 0.01 and update all displayed error fallback values for minPeriodPaymentRate, periodPaymentRate and maxPeriodPaymentRate to show 0.01 instead of 1; then open loan-product-terms-step.component.ts and update the reactive validators for the corresponding FormControls (minPeriodPaymentRate, periodPaymentRate, maxPeriodPaymentRate and any working-capital related controls) to use Validators.min(0.01) so the template UI and the FormGroup validation are aligned.
🧹 Nitpick comments (9)
src/app/products/products.service.ts (1)
32-50: UseLoanProductTypeunion type instead ofstringfor theproductTypeparameter.The
productTypeparameter is passed as a plain string, but the codebase defines aLoanProductTypeunion type insrc/app/products/loan-products/models/loan-product.model.ts. Update the service method signatures to useLoanProductTypeinstead ofstringfor stricter type safety and to align with Angular guidelines. Current usage shows productType is controlled (either''or'workingcapital'), so the path construction works, but explicit typing prevents accidental misuse.Example:
getLoanProducts(productType: LoanProductType): Observable<any>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 32 - 50, Update the productType parameter types in ProductsService methods to use the existing LoanProductType union instead of plain string: change signatures for getLoanProducts, createLoanProduct, getLoanProductsTemplate, getLoanProduct, and updateLoanProduct to accept productType: LoanProductType; import LoanProductType from src/app/products/loan-products/models/loan-product.model.ts at the top of the file; ensure any usages in these methods (e.g., URL template strings) remain unchanged and that the compiler resolves the type import.src/app/products/loan-products/loan-products-template.resolver.ts (1)
28-33: AvoidObservable<any>for the template resolver output.Define a specific response interface (e.g.,
LoanProductTemplate) and use it in the resolver signature to keep API types strict.Based on learnings,
In TypeScript files (e.g., src/app/.../*.ts), avoid using Observable<any> as a project-wide pattern for API responses. Treat typing the full API response layer as a cross-cutting refactor and track it as a separate enhancement backlog item rather than addressing it in individual bug fixes. When you encounter API responses, introduce specific interfaces/types for the response shapes and use proper typing instead of any, updating services and resolvers accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products-template.resolver.ts` around lines 28 - 33, The resolver currently returns Observable<any>; create a specific interface (e.g., LoanProductTemplate) that models the response shape and update the resolver signature from resolve(route: ActivatedRouteSnapshot): Observable<any> to Observable<LoanProductTemplate>; update any related method signatures on productsService.getLoanProductsTemplate to return Observable<LoanProductTemplate> (including both branches where 'workingcapital' or '' are passed) and adjust imports. Ensure the new interface accurately reflects fields used downstream so callers of resolve/LoanProductsTemplate get strict typing.src/app/products/loan-products/view-loan-product/general-tab/general-tab.component.ts (1)
33-38: Typeroute.dataand make the subscription lifecycle-safe.Replace
anywithLoanProductand usetakeUntilDestroyed()(orfirst()) to keep the subscription clean.♻️ Suggested change
-import { Component, OnInit, inject } from '@angular/core'; +import { Component, OnInit, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; ... constructor() { super(); - this.route.data.subscribe((data: { loanProduct: any }) => { + this.route.data.pipe(takeUntilDestroyed()).subscribe((data: { loanProduct: LoanProduct }) => { this.loanProduct = data.loanProduct; this.useDueForRepaymentsConfigurations = !this.loanProduct.dueDaysForRepaymentEvent && !this.loanProduct.overDueDaysForRepaymentEvent; }); }As per coding guidelines,
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/view-loan-product/general-tab/general-tab.component.ts` around lines 33 - 38, In the constructor subscription to route.data, replace the loose any type with the concrete LoanProduct interface for the data payload (change (data: { loanProduct: any }) to (data: { loanProduct: LoanProduct })) and make the Observable subscription lifecycle-safe by piping the Observable through takeUntilDestroyed() (or use first()) before subscribing; update the route.data usage in the constructor that assigns this.loanProduct and computes this.useDueForRepaymentsConfigurations accordingly and add the necessary import for takeUntilDestroyed (or first) so the subscription is automatically torn down.src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts (1)
10-83: Prefer typed forms overUntypedFormGroup/UntypedFormControl.These untyped controls weaken strict type safety; consider migrating this form to typed
FormGroup/FormControlto keep compile-time guarantees.As per coding guidelines,
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts` around lines 10 - 83, The component uses untyped forms (UntypedFormGroup/UntypedFormControl) which bypass TypeScript safety; update LoanProductCurrencyStepComponent by defining a typed form model interface (e.g., LoanCurrencyFormModel) reflecting controls currencyCode, digitsAfterDecimal, inMultiplesOf and optionally installmentAmountInMultiplesOf, replace UntypedFormGroup/UntypedFormControl with FormGroup<LoanCurrencyFormModel> and FormControl<T> types, update the createLoanProductCurrencyForm method to build a typed FormGroup via the injected FormBuilder (or new FormControl instances) and change the loanProductCurrencyForm property type accordingly, update any addControl call (installmentAmountInMultiplesOf) to use the typed FormControl and ensure imports swap Untyped* to typed FormGroup/FormControl from `@angular/forms` and adjust usages (patchValue, validators) to match the typed shape.src/assets/translations/en-US.json (1)
431-433: Add the new keys to other locale files (if supported).
These additions will render as raw keys in non‑English locales unless corresponding entries exist in other translation files.Also applies to: 1164-1166, 2345-2347, 2742-2745, 2822-2828
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/en-US.json` around lines 431 - 433, Add the new translation keys ("Create Loan Product", "Create Working Capital Product", "Create Office" and the other keys noted at 1164-1166, 2345-2347, 2742-2745, 2822-2828) to all supported locale files so non‑English locales don't show raw keys; locate the matching JSON/YAML translation files (e.g., locales/* or src/assets/translations/*) and insert the same keys with appropriate translated values (or placeholders if awaiting translations) under the same nesting structure and key names used in en-US.json, then run the i18n validation script or build to ensure no missing-key warnings remain.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts (1)
182-193: Replace newanyoption arrays with typed interfaces.
Usinganyhere weakens type safety and propagates loosely‑typed data through the mapping layer. Consider a narrow option type or existing model types and apply consistently to the other option arrays in this block.♻️ Suggested typing pattern
- const chargeOffReasonOptions: any = this.loanProductsTemplate.chargeOffReasonOptions || []; - const writeOffReasonOptions: any = this.loanProductsTemplate.writeOffReasonOptions || []; + type OptionLike = { id: number; code?: string; value?: string; name?: string }; + const chargeOffReasonOptions: OptionLike[] = this.loanProductsTemplate.chargeOffReasonOptions || []; + const writeOffReasonOptions: OptionLike[] = this.loanProductsTemplate.writeOffReasonOptions || [];As per coding guidelines, “strict type safety”.
src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts (1)
432-724: ConsidertakeUntilDestroyedfor form subscriptions.There are many
valueChanges.subscribe(...)calls insetConditionalControlswithout a teardown. UsingtakeUntilDestroyed()(or anngOnDestroysubject) will prevent leaks when the component is destroyed.♻️ Suggested pattern
+import { DestroyRef } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; ... +private destroyRef = inject(DestroyRef); this.loanProductSettingsForm .get('allowAttributeConfiguration') - .valueChanges.subscribe((allowAttributeConfiguration: any) => { + .valueChanges.pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((allowAttributeConfiguration: any) => { ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 432 - 724, Multiple valueChanges subscriptions in setConditionalControls (e.g., on allowAttributeConfiguration, daysInYearType, interestCalculationPeriodType, allowVariableInstallments, isInterestRecalculationEnabled, holdGuaranteeFunds, multiDisburseLoan, enableDownPayment, transactionProcessingStrategyCode, useDueForRepaymentsConfigurations) are created without teardown; convert them to use a lifecycle unsubscribe pattern by piping each observable through takeUntilDestroyed() (or pipe(..., takeUntil(this.destroy$))) before subscribe, add a private destroy$ = new Subject<void>() and implement ngOnDestroy() { this.destroy$.next(); this.destroy$.complete(); } if using takeUntil, or import and call takeUntilDestroyed(this) if using the Angular standalone helper; update all valueChanges chains inside setConditionalControls to use this pattern so subscriptions are cleaned up on component destroy.src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts (1)
111-126: Avoid magic-string product types in initialization.Consider normalizing the query param to the
LOAN_PRODUCT_TYPEconstants to avoid drifting string values.As per coding guidelines: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.♻️ Suggested guard
import { LoanProductBaseComponent } from '../common/loan-product-base.component'; +import { LOAN_PRODUCT_TYPE } from '../models/loan-product.model'; @@ - const productType = this.route.snapshot.queryParamMap.get('productType') || 'loan'; - this.loanProductService.initialize(productType); + const rawType = this.route.snapshot.queryParamMap.get('productType'); + const productType = + rawType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL ? rawType : LOAN_PRODUCT_TYPE.LOAN; + this.loanProductService.initialize(productType);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts` around lines 111 - 126, The code reads the raw query param into productType and passes it to loanProductService.initialize, which uses a magic string; normalize the query param against your LOAN_PRODUCT_TYPE enum/constant (e.g., map/validate values) before calling loanProductService.initialize(productType) to ensure only allowed types are used, fall back to LOAN_PRODUCT_TYPE.LOAN (or configured default) if invalid, and update references to the local productType variable and any downstream logic in the constructor and route.data block to use the normalized constant value.src/app/products/loan-products/create-loan-product/create-loan-product.component.ts (1)
107-123: Use LOAN_PRODUCT_TYPE constant for query-param default.Normalizing the query param to the shared constants reduces drift from the model.
As per coding guidelines: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.♻️ Suggested guard
import { LoanProductBaseComponent } from '../common/loan-product-base.component'; +import { LOAN_PRODUCT_TYPE } from '../models/loan-product.model'; @@ - const productType = this.route.snapshot.queryParamMap.get('productType') || 'loan'; - this.loanProductService.initialize(productType); + const rawType = this.route.snapshot.queryParamMap.get('productType'); + const productType = + rawType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL ? rawType : LOAN_PRODUCT_TYPE.LOAN; + this.loanProductService.initialize(productType);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/create-loan-product/create-loan-product.component.ts` around lines 107 - 123, Replace the hardcoded string default 'loan' for the query-param with the shared constant LOAN_PRODUCT_TYPE: in the constructor where productType is set (currently using this.route.snapshot.queryParamMap.get('productType') || 'loan'), import and use LOAN_PRODUCT_TYPE instead and pass that into loanProductService.initialize(productType); ensure the LOAN_PRODUCT_TYPE symbol is imported from the shared/constants module (or appropriate location) and preserve existing logic for handling this.route.data and accountingMappingOptions in the constructor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts`:
- Around line 407-423: The code reuses optionValue and assigns
loanProduct.daysInYearCustomStrategy after the conditional, causing
daysInYearCustomStrategy to be overwritten with daysInYearType when
isAdvancedPaymentAllocation is false; fix by only computing and assigning
optionValue for the custom strategy inside the if block (use
optionDataLookUp(this.loanProduct.daysInYearCustomStrategy,
this.loanProductsTemplate.daysInYearCustomStrategyOptions) and set
this.loanProduct.daysInYearCustomStrategy there), or use a separate local
variable for the custom strategy so the final assignment doesn't run unless
isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id == 1 are
true.
In
`@src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts`:
- Around line 339-383: The getter loanProduct currently always sets
paymentAllocation, creditAllocation, and supportedInterestRefundTypes to empty
arrays which causes non-advanced edits to send empty arrays; change the logic in
get loanProduct so those three fields are only added when
this.isAdvancedPaymentStrategy is true (move their initializations into the
isAdvancedPaymentStrategy block and assign this.paymentAllocation /
this.creditAllocation / this.supportedInterestRefundTypes there), and also
update the submit method to remove or not include paymentAllocation and
creditAllocation when isAdvancedPaymentStrategy is false (similar to the
existing deletion of supportedInterestRefundTypes and daysInYearCustomStrategy)
so the edit flow matches the create flow.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts`:
- Around line 47-51: The current mapping for digitsAfterDecimal treats 0 as
missing which breaks Validators.required and Validators.min(0); update the
ternary in the loan-product-currency-step component so that only undefined or
null decimalPlaces map to '' and preserve 0 as a valid value (i.e., remove the
check for currency.decimalPlaces === 0), leaving digitsAfterDecimal set to
currency.decimalPlaces when it is 0 or any other number.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts`:
- Around line 413-425: The controls added in the toggle handler
(minInterestRatePerPeriod, interestRatePerPeriod, maxInterestRatePerPeriod,
interestRateFrequencyType) are re-created without their original validators when
isLinkedToFloatingInterestRates is set to false; update the toggle logic to
reapply the same validators used in createLoanProductTermsForm (e.g.,
Validators.required on interestRatePerPeriod and interestRateFrequencyType, plus
the min and pattern validators on
minInterestRatePerPeriod/maxInterestRatePerPeriod) so the controls enforce the
same validation rules after being re-added.
In `@src/assets/translations/cs-CS.json`:
- Around line 2718-2723: The Czech translation for the amortizationType key
"Flat" is wrong ("Byt" = apartment); update the value for amortizationType ->
"Flat" to a correct financial term such as "Paušální" (or "Pevná") so the JSON
entry amortizationType["Flat"] reflects the intended amortization meaning rather
than an apartment.
In `@src/assets/translations/de-DE.json`:
- Around line 2721-2723: The translation for the key "Flat" currently set to
"Wohnung" is incorrect for amortization context; update the value of the "Flat"
key in this translations file to a finance-appropriate term such as "Pauschal"
or "Flachzins" and search the repository for any other occurrences of the "Flat"
key or identical translation entries to make them consistent (ensure keys like
"Equal principal payments" and "Effective Interest Rate" remain unchanged while
updating only the "Flat" value).
- Around line 2791-2796: The German labels are literal and awkward; update the
translations for the keys "Flat Percentage Amount", "Delinquency Bucket
Classification", "Discount Default", "Period Payment Frequency", and "Period
Payment Frequency Type" to more natural German—for example replace "Flat
Percentage Amount" with "Fester Prozentsatz", "Delinquency Bucket
Classification" with "Klassifizierung von Zahlungsausfällen", "Discount Default"
with "Voreingestellter Rabatt", "Period Payment Frequency" with
"Zahlungsfrequenz", and "Period Payment Frequency Type" with "Art der
Zahlungsfrequenz"; apply these or equally natural phrasing in
src/assets/translations/de-DE.json for those keys.
In `@src/assets/translations/ko-KO.json`:
- Around line 2722-2724: The translation for the JSON key "Flat" is currently
"평평한" which implies a physical surface; update the value for the "Flat" key in
the translations object to a finance-appropriate term such as "정액" (or "고정" if
you prefer fixed-rate semantics) so it correctly conveys a flat/level
interest/pricing model in Korean.
- Around line 2790-2793: The translation for the key "Flat Percentage Amount" is
contradictory; update the value for the JSON key "Flat Percentage Amount" from
"정액 비율 금액" to the clearer "고정 비율 금액" so it conveys a fixed percentage amount
consistently across the translations file; ensure the modification is applied to
the "Flat Percentage Amount" entry only and keep surrounding formatting intact.
In `@src/assets/translations/lt-LT.json`:
- Around line 2718-2720: The Lithuanian translation for the "Flat" amortization
key is incorrect: replace the value for the "Flat" key in the translation entry
("Flat": "Butas") with "Fiksuota" so it reads "Flat": "Fiksuota"; also scan the
analogous keys in de-DE.json and cs-CS.json for similar mistaken
literal/apartment translations ("Wohnung"/"Byt") and correct them to their
proper amortization terms to keep terminology consistent.
---
Outside diff comments:
In `@src/app/products/loan-products/loan-products.component.ts`:
- Around line 87-98: Replace the untyped control and any-typed options with
specific types: change loanProductSelector (currently UntypedFormControl) to a
typed FormControl<LoanProductType | null> (import FormControl from
`@angular/forms`) and type loanProductOptions as a readonly array (e.g., readonly
loanProductOptions: ReadonlyArray<LoanProductType> = PRODUCT_TYPES) or the exact
type of PRODUCT_TYPES; mark it readonly for immutability. Keep the existing
PRODUCT_TYPES constant as the value and update any usages to match the new types
(references: loanProductSelector, loanProductOptions, PRODUCT_TYPES, and ensure
LoanProductType is imported/defined).
---
Duplicate comments:
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts`:
- Around line 490-502: The code accesses
this.loanProduct.chargeOffBehaviour.value without ensuring chargeOffBehaviour is
defined; update the guard in loan-product-summary.component (inside the
isAdvancedPaymentAllocation block) to check for
this.loanProduct.chargeOffBehaviour truthiness (e.g., if
(!this.loanProduct.chargeOffBehaviour ||
!this.loanProduct.chargeOffBehaviour.value) ) before calling
stringEnumOptionDataLookUp so stringEnumOptionDataLookUp receives a safe value
and no runtime error is thrown.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts`:
- Around line 44-55: The code assumes this.loanProductsTemplate.currencyOptions
and its first element exist before calling
this.loanProductCurrencyForm.patchValue, which will throw if currencyOptions is
empty or undefined; update the initialization around currencyData and currency
(referencing currencyData, loanProductsTemplate.currencyOptions, and currency)
to first guard that this.loanProductsTemplate?.currencyOptions is a non-empty
array (or that this.loanProductsTemplate.currency exists), and if not either
return/skip patching or set a safe default currency object (with code,
decimalPlaces and inMultiplesOf defaults) before calling
this.loanProductCurrencyForm.patchValue so accessing currency.code,
currency.decimalPlaces and currency.inMultiplesOf is always safe.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html`:
- Around line 799-816: The two mat-form-field inputs bound to
formControlName="dueDaysForRepaymentEvent" and "overDueDaysForRepaymentEvent"
are using the wrong tooltip key; replace the matTooltip string key
"'tooltips.Maximum outstanding loan account balance' | translate" with
repayment-event specific keys (e.g. "'tooltips.Due days for repayment event' |
translate" and "'tooltips.Overdue days for repayment event' | translate") and
add the corresponding entries to the translation files so the tooltip text
reflects the repayment-event context.
- Around line 30-41: The npvDayCount control currently only has
Validators.required and relies on the HTML min attribute; update
createLoanProductSettingsForm to add Validators.min(1) to the 'npvDayCount'
FormControl so the reactive form rejects 0/negative values, then update the
template's error block for the npvDayCount field: replace the legacy *ngIf with
the project's `@if` style, and change the error copy to reference "NPV day count"
(or the translated key for that label) instead of "Default Principal". Ensure
you reference the 'npvDayCount' control name and the
createLoanProductSettingsForm function when making these changes.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts`:
- Around line 747-751: The code uses
transactionProcessingStrategyDataBase.some(...) with a callback that performs
side effects without returning a boolean; replace that some call with
transactionProcessingStrategyDataBase.forEach(...) so each CodeName (cn) is
checked via LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code) and
matching cn values are pushed onto transactionProcessingStrategyData, removing
the incorrect use of some and satisfying the linter.
- Around line 126-165: The form is patched with
transactionProcessingStrategyCode before that variable is computed, causing a
null value to be saved; move the computation/assignment of
transactionProcessingStrategyCode (use
this.loanProductsTemplate.transactionProcessingStrategyCode ||
this.transactionProcessingStrategyData[0].code) to before the call to
this.loanProductSettingsForm.patchValue(...) so that patchValue receives the
resolved code from transactionProcessingStrategyCode (refer to
transactionProcessingStrategyCode, this.loanProductsTemplate,
this.transactionProcessingStrategyData and
this.loanProductSettingsForm.patchValue).
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html`:
- Around line 114-149: The three period-payment rate inputs and their error
fallbacks are inconsistent: change the maxPeriodPaymentRate input [min] from 1
to 0.01 and update all displayed error fallback values for minPeriodPaymentRate,
periodPaymentRate and maxPeriodPaymentRate to show 0.01 instead of 1; then open
loan-product-terms-step.component.ts and update the reactive validators for the
corresponding FormControls (minPeriodPaymentRate, periodPaymentRate,
maxPeriodPaymentRate and any working-capital related controls) to use
Validators.min(0.01) so the template UI and the FormGroup validation are
aligned.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts`:
- Around line 321-338: The form controls minPeriodPaymentRate, periodPaymentRate
and maxPeriodPaymentRate in LoanProductTermsStepComponent currently use
Validators.min(1) which conflicts with the UI minimum of 0.01; update those
validators to Validators.min(0.01) (and keep Validators.required on
periodPaymentRate) so the FormGroup validation matches the template and allows
values like 0.5 or 0.01.
In `@src/app/products/loan-products/services/loan-product.service.ts`:
- Around line 24-29: The initialize method currently accepts a raw string and
checks for 'loan'/'working-capital', which allows invalid inputs; change the
initialize signature to accept the LoanProductType type (the enum used is
LOAN_PRODUCT_TYPE) or a validated/mappped value, then remove the raw string
comparisons and directly call this.productType.next(...) with the corresponding
LOAN_PRODUCT_TYPE enum value; update any callers to pass a LoanProductType (or
perform mapping/validation before calling initialize) so the service cannot
enter an unintended state.
In `@src/assets/translations/es-CL.json`:
- Around line 2721-2724: Replace the Spanish translation value for the key
"Equal principal payments" so it matches the existing phrasing used elsewhere;
change its value from "Pagos iguales de principal" to "Pagos iguales de capital"
to ensure consistency across translations.
- Around line 427-428: Change the translation value for the key "Create Working
Capital Product" to use a lowercase "producto" to match the existing style used
for "Create Loan Product"; locate the entry for the "Create Working Capital
Product" key in src/assets/translations/es-CL.json and replace "Crear Producto
de Capital de Trabajo" with "Crear producto de Capital de Trabajo".
- Around line 2790-2795: Update the Spanish translations to match existing
terminology by changing the values for the keys "Delinquency Bucket
Classification", "Period Payment Frequency", and "Period Payment Frequency Type"
so they use the same phrasing with "del período" and the established term for
delinquency groups (e.g., use "Clasificación de Grupos de morosidad" or
"Clasificación de Grupos de morosidad del período" and change "Period Payment
Frequency" -> "Frecuencia de pago del período" and "Period Payment Frequency
Type" -> "Tipo de frecuencia de pago del período"), ensuring consistency with
the existing "Grupos de morosidad" and "del período" labels.
In `@src/assets/translations/lt-LT.json`:
- Around line 2789-2794: The Lithuanian value "Laikotarpis Mokėjimo dažnumas"
should be normalized to the lowercase/genitive phrasing used elsewhere; update
that translation value to "Laikotarpio mokėjimo dažnumas" by replacing the
existing string for the key "Period Payment Frequency" (look for the exact value
"Laikotarpis Mokėjimo dažnumas" and change it to "Laikotarpio mokėjimo
dažnumas") so it matches the genitive/casing pattern used by "Period Payment
Frequency Type".
---
Nitpick comments:
In
`@src/app/products/loan-products/create-loan-product/create-loan-product.component.ts`:
- Around line 107-123: Replace the hardcoded string default 'loan' for the
query-param with the shared constant LOAN_PRODUCT_TYPE: in the constructor where
productType is set (currently using
this.route.snapshot.queryParamMap.get('productType') || 'loan'), import and use
LOAN_PRODUCT_TYPE instead and pass that into
loanProductService.initialize(productType); ensure the LOAN_PRODUCT_TYPE symbol
is imported from the shared/constants module (or appropriate location) and
preserve existing logic for handling this.route.data and
accountingMappingOptions in the constructor.
In
`@src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts`:
- Around line 111-126: The code reads the raw query param into productType and
passes it to loanProductService.initialize, which uses a magic string; normalize
the query param against your LOAN_PRODUCT_TYPE enum/constant (e.g., map/validate
values) before calling loanProductService.initialize(productType) to ensure only
allowed types are used, fall back to LOAN_PRODUCT_TYPE.LOAN (or configured
default) if invalid, and update references to the local productType variable and
any downstream logic in the constructor and route.data block to use the
normalized constant value.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts`:
- Around line 10-83: The component uses untyped forms
(UntypedFormGroup/UntypedFormControl) which bypass TypeScript safety; update
LoanProductCurrencyStepComponent by defining a typed form model interface (e.g.,
LoanCurrencyFormModel) reflecting controls currencyCode, digitsAfterDecimal,
inMultiplesOf and optionally installmentAmountInMultiplesOf, replace
UntypedFormGroup/UntypedFormControl with FormGroup<LoanCurrencyFormModel> and
FormControl<T> types, update the createLoanProductCurrencyForm method to build a
typed FormGroup via the injected FormBuilder (or new FormControl instances) and
change the loanProductCurrencyForm property type accordingly, update any
addControl call (installmentAmountInMultiplesOf) to use the typed FormControl
and ensure imports swap Untyped* to typed FormGroup/FormControl from
`@angular/forms` and adjust usages (patchValue, validators) to match the typed
shape.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts`:
- Around line 432-724: Multiple valueChanges subscriptions in
setConditionalControls (e.g., on allowAttributeConfiguration, daysInYearType,
interestCalculationPeriodType, allowVariableInstallments,
isInterestRecalculationEnabled, holdGuaranteeFunds, multiDisburseLoan,
enableDownPayment, transactionProcessingStrategyCode,
useDueForRepaymentsConfigurations) are created without teardown; convert them to
use a lifecycle unsubscribe pattern by piping each observable through
takeUntilDestroyed() (or pipe(..., takeUntil(this.destroy$))) before subscribe,
add a private destroy$ = new Subject<void>() and implement ngOnDestroy() {
this.destroy$.next(); this.destroy$.complete(); } if using takeUntil, or import
and call takeUntilDestroyed(this) if using the Angular standalone helper; update
all valueChanges chains inside setConditionalControls to use this pattern so
subscriptions are cleaned up on component destroy.
In `@src/app/products/loan-products/loan-products-template.resolver.ts`:
- Around line 28-33: The resolver currently returns Observable<any>; create a
specific interface (e.g., LoanProductTemplate) that models the response shape
and update the resolver signature from resolve(route: ActivatedRouteSnapshot):
Observable<any> to Observable<LoanProductTemplate>; update any related method
signatures on productsService.getLoanProductsTemplate to return
Observable<LoanProductTemplate> (including both branches where 'workingcapital'
or '' are passed) and adjust imports. Ensure the new interface accurately
reflects fields used downstream so callers of resolve/LoanProductsTemplate get
strict typing.
In
`@src/app/products/loan-products/view-loan-product/general-tab/general-tab.component.ts`:
- Around line 33-38: In the constructor subscription to route.data, replace the
loose any type with the concrete LoanProduct interface for the data payload
(change (data: { loanProduct: any }) to (data: { loanProduct: LoanProduct }))
and make the Observable subscription lifecycle-safe by piping the Observable
through takeUntilDestroyed() (or use first()) before subscribing; update the
route.data usage in the constructor that assigns this.loanProduct and computes
this.useDueForRepaymentsConfigurations accordingly and add the necessary import
for takeUntilDestroyed (or first) so the subscription is automatically torn
down.
In `@src/app/products/products.service.ts`:
- Around line 32-50: Update the productType parameter types in ProductsService
methods to use the existing LoanProductType union instead of plain string:
change signatures for getLoanProducts, createLoanProduct,
getLoanProductsTemplate, getLoanProduct, and updateLoanProduct to accept
productType: LoanProductType; import LoanProductType from
src/app/products/loan-products/models/loan-product.model.ts at the top of the
file; ensure any usages in these methods (e.g., URL template strings) remain
unchanged and that the compiler resolves the type import.
In `@src/assets/translations/en-US.json`:
- Around line 431-433: Add the new translation keys ("Create Loan Product",
"Create Working Capital Product", "Create Office" and the other keys noted at
1164-1166, 2345-2347, 2742-2745, 2822-2828) to all supported locale files so
non‑English locales don't show raw keys; locate the matching JSON/YAML
translation files (e.g., locales/* or src/assets/translations/*) and insert the
same keys with appropriate translated values (or placeholders if awaiting
translations) under the same nesting structure and key names used in en-US.json,
then run the i18n validation script or build to ensure no missing-key warnings
remain.
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
Show resolved
Hide resolved
src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts
Show resolved
Hide resolved
...ucts/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts
Outdated
Show resolved
Hide resolved
...n-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
Outdated
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-711/wc-product-configuration
branch
from
b61977c to
4f380ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts (2)
41-63:⚠️ Potential issue | 🟠 MajorMissing cross-field validation: no enforcement that
maximumAgeDays > minimumAgeDays.Individual field validators don't prevent an inverted range (e.g., min=100, max=50). Such a payload will be submitted to the API as valid. A group-level validator is needed to enforce the ordering invariant.
🛡️ Proposed fix — add a group validator
setInputForm(): void { this.delinquencyRangeForm = this.formBuilder.group({ ... - }); + }, { validators: this.maxGreaterThanMinValidator }); } + private maxGreaterThanMinValidator(group: UntypedFormGroup) { + const min = group.get('minimumAgeDays')?.value; + const max = group.get('maximumAgeDays')?.value; + if (min != null && max != null && max !== '' && Number(max) <= Number(min)) { + return { maxNotGreaterThanMin: true }; + } + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts` around lines 41 - 63, The form allows inverted ranges because there's no group-level validator; update setInputForm so the FormGroup returned by this.formBuilder.group (assigned to delinquencyRangeForm) includes a custom validator that reads controls minimumAgeDays and maximumAgeDays and marks the group (or the maximumAgeDays control) invalid when maximum <= minimum (or when maximum is present and not > minimum), providing a descriptive validation error key (e.g., rangeOrder) so the template and submit logic can block submission when the group is invalid.
47-53:⚠️ Potential issue | 🟡 MinorContradictory validators on
minimumAgeDays: pattern allows0butmin(1)rejects it.The regex
'^(0|[1-9][0-9]*)$'explicitly matches0, yetValidators.min(1)marks0as invalid. When a user enters0,patternpasses butminfails, producing confusing or inconsistent error messaging. Either update the pattern to exclude0, or dropValidators.min(1)and rely solely on the pattern.🐛 Proposed fix — align pattern with min(1)
- Validators.pattern('^(0|[1-9][0-9]*)$'), - Validators.min(1) + Validators.pattern('^[1-9][0-9]*$'), + Validators.min(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts` around lines 47 - 53, The minimumAgeDays form control has conflicting validators: Validators.pattern('^(0|[1-9][0-9]*)$') allows "0" but Validators.min(1) rejects it; to fix, make them consistent in create-range.component.ts by updating the minimumAgeDays validators — either remove Validators.min(1) and keep the pattern that allows 0, or (recommended) change the pattern to '^[1-9][0-9]*$' so it excludes 0 and aligns with Validators.min(1); update the Validators.pattern reference and ensure minimumAgeDays remains required.
♻️ Duplicate comments (19)
src/app/organization/currencies/manage-currencies/manage-currencies.component.html (1)
12-12: Switch the Add button totype="button"after removing submit handling.At Line 12,
(ngSubmit)is removed, but at Line 36 the Add button remainstype="submit". This keeps triggering form submit events even though the action is now click-driven at Line 41. Usetype="button"to align behavior and avoid the dead submit path.♻️ Proposed fix
<button - type="submit" + type="button" mat-raised-button class="add-currency-button" color="primary" [disabled]="!currencyForm.valid" (click)="addCurrency()"Also applies to: 35-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/organization/currencies/manage-currencies/manage-currencies.component.html` at line 12, The Add button still uses type="submit" despite removing (ngSubmit) on the form (formRef / [formGroup]="currencyForm"), which causes unintended submit events; change the Add button element (the one labeled "Add" that currently triggers submission) to use type="button" so its click handler is the only trigger for adding currency and no form submit is emitted.src/assets/translations/ko-KO.json (2)
2791-2791:⚠️ Potential issue | 🟡 MinorRefine “Flat Percentage Amount” Korean phrasing.
Line 2791 (“정액 비율 금액”) is awkward/contradictory in Korean finance context; “고정 비율 금액” is clearer.
📝 Suggested fix
- "Flat Percentage Amount": "정액 비율 금액", + "Flat Percentage Amount": "고정 비율 금액",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/ko-KO.json` at line 2791, The translation for the key "Flat Percentage Amount" should be changed from "정액 비율 금액" to the clearer Korean phrase "고정 비율 금액"; update the value for the "Flat Percentage Amount" entry in the translations JSON (key: "Flat Percentage Amount") to "고정 비율 금액".
2722-2724:⚠️ Potential issue | 🟡 MinorTranslate “Flat” with a financial term, not a physical adjective.
Line 2724 uses “평평한”, which reads as “flat surface.” In this context, use a finance term like “정액” (or “고정”).
📝 Suggested fix
- "Flat": "평평한" + "Flat": "정액"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/ko-KO.json` around lines 2722 - 2724, The JSON value for the key "Flat" is translated as a physical adjective ("평평한"); replace it with an appropriate financial term such as "정액" (or "고정") to reflect the finance meaning; update the value for the "Flat" key near "Equal principal payments" and "Effective Interest Rate" so the translation reads "Flat": "정액" (or "고정").src/assets/translations/de-DE.json (2)
2791-2798: Several new German labels are still too literal.Line 2792–Line 2796 are grammatically awkward for native German UI wording and should be polished.
📝 Suggested wording
- "Flat Percentage Amount": "Pauschaler Prozentsatz", - "Delinquency Bucket Classification": "Delinquenz-Klassifizierung", - "Discount Default": "Standardrabatt", - "Period Payment Frequency": "Zahlungsperiode Häufigkeit", - "Period Payment Frequency Type": "Zahlungsperiode Häufigkeit Art", + "Flat Percentage Amount": "Pauschaler Prozentbetrag", + "Delinquency Bucket Classification": "Klassifizierung des Delinquenz-Buckets", + "Discount Default": "Voreingestellter Rabatt", + "Period Payment Frequency": "Zahlungsperiodenhäufigkeit", + "Period Payment Frequency Type": "Typ der Zahlungsperiodenhäufigkeit",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/de-DE.json` around lines 2791 - 2798, Replace the overly literal German strings for the keys "Flat Percentage Amount", "Delinquency Bucket Classification", "Discount Default", "Period Payment Frequency", and "Period Payment Frequency Type" in the de-DE translation bundle with polished, idiomatic UI wording (fix grammar, word order and capitalization to match German UX conventions), ensure the translations read naturally in context and are consistent with existing terminology in the file, and run your i18n/lint checks to catch any formatting or duplicated-entry issues before committing.
2721-2723: Amortization type “Flat” is still mistranslated.Line 2723 uses „Wohnung“, which is incorrect in finance context. Use a term like „Pauschal“ (or „Flachzins“).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/de-DE.json` around lines 2721 - 2723, The translation for the JSON key "Flat" is incorrect ("Wohnung"); update the value for the "Flat" key in src/assets/translations/de-DE.json to a finance-appropriate term such as "Pauschal" (or "Flachzins") so the amortization type is translated correctly.src/assets/translations/cs-CS.json (1)
2720-2722:⚠️ Potential issue | 🟡 MinorFix incorrect Czech translation for amortization type “Flat”.
Line 2722 still uses
"Byt"(apartment), which is incorrect in this financial context. Use"Paušální"(or"Pevná"), consistent with amortization terminology.💡 Proposed fix
- "Flat": "Byt" + "Flat": "Paušální"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` around lines 2720 - 2722, The JSON translation entry for the amortization type key "Flat" currently maps to the incorrect Czech word "Byt"; update the value for the "Flat" key to a correct financial term such as "Paušální" (or "Pevná") so the entry reads "Flat": "Paušální" (or "Pevná") to match amortization terminology and remain consistent with surrounding keys like "Equal principal payments" and "Effective Interest Rate".src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts (2)
45-47:⚠️ Potential issue | 🟠 MajorGuard missing currency options before patching.
If template currency is absent and
currencyOptionsis empty/undefined,currencybecomesundefinedand Line 47 dereferencescurrency.code.Suggested safeguard
- this.currencyData = this.loanProductsTemplate.currencyOptions; - const currency = this.loanProductsTemplate.currency ? this.loanProductsTemplate.currency : this.currencyData[0]; + this.currencyData = this.loanProductsTemplate.currencyOptions ?? []; + const currency = this.loanProductsTemplate.currency ?? this.currencyData[0]; + if (!currency) { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts` around lines 45 - 47, The code assumes a currency exists when computing const currency = this.loanProductsTemplate.currency ? this.loanProductsTemplate.currency : this.currencyData[0] and then calling this.loanProductCurrencyForm.patchValue({ currencyCode: currency.code }); so add a guard to ensure a valid currency is present: check this.loanProductsTemplate.currency and fallback only if this.currencyData && this.currencyData.length > 0, otherwise set a safe default (e.g., null or empty string) for currencyCode or skip patching; update the logic around loanProductsTemplate, currencyData and the this.loanProductCurrencyForm.patchValue call to handle undefined currency without dereferencing currency.code.
49-51:⚠️ Potential issue | 🟠 MajorKeep
decimalPlaces = 0as a valid numeric value.Coercing
0to''breaksValidators.required/Validators.min(0)for zero-decimal currencies.Proposed fix
digitsAfterDecimal: - currency.decimalPlaces === 0 || currency.decimalPlaces === undefined || currency.decimalPlaces === null + currency.decimalPlaces === undefined || currency.decimalPlaces === null ? '' : currency.decimalPlaces,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts` around lines 49 - 51, The current ternary coerces decimalPlaces === 0 into an empty string which breaks Validators.required/Validators.min(0); change the check in loan-product-currency-step.component (the currency.decimalPlaces initializer) to only treat null/undefined as empty (e.g., use a nullish check like currency.decimalPlaces == null or currency.decimalPlaces === undefined || currency.decimalPlaces === null) so that 0 remains a valid numeric value for the form control used with Validators.required and Validators.min.src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.html (1)
94-94:⚠️ Potential issue | 🟠 MajorTrack fund options by a stable id, not object identity.
track fundwill re-create option DOM nodes wheneverfundDatais replaced with new object instances. Usefund.idfor stable rendering.Proposed fix
- `@for` (fund of fundData; track fund) { + `@for` (fund of fundData; track fund.id) {As per coding guidelines:
src/app/**: “For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.html` at line 94, The template currently tracks by object identity with "track fund" on the *ngFor over fundData causing DOM re-creation; change it to track by a stable id by either updating the template to use the fund.id key (e.g., "track fund.id") or, preferably, add a trackBy function named trackByFundId in the LoanProductDetailsStepComponent that returns fund.id and reference it from the template (trackBy: trackByFundId) so the *ngFor over fundData (fund) uses stable identifiers.src/app/products/loan-products/services/loan-product.service.ts (1)
24-30:⚠️ Potential issue | 🟠 MajorNormalize unknown
productTypevalues to avoid stale singleton state.
initializecurrently ignores unexpected strings, so the previousproductTypecan persist in this root-level service. Also, thestringinput weakens type safety.Proposed hardening
- initialize(productType: string): void { - if (productType === 'loan') { - this.productType.next(LOAN_PRODUCT_TYPE.LOAN); - } else if (productType === 'working-capital') { - this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL); - } + initialize(productType: LoanProductType | null | undefined): void { + this.productType.next( + productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL + ? LOAN_PRODUCT_TYPE.WORKING_CAPITAL + : LOAN_PRODUCT_TYPE.LOAN + ); }As per coding guidelines:
src/app/**: “For Angular code: verify ... strict type safety ...”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/services/loan-product.service.ts` around lines 24 - 30, initialize currently takes a plain string and ignores unknown values, allowing stale state; change the method signature to accept a stricter union or the LOAN_PRODUCT_TYPE enum (e.g. productType: LOAN_PRODUCT_TYPE | 'loan' | 'working-capital'), map known inputs to LOAN_PRODUCT_TYPE.LOAN or LOAN_PRODUCT_TYPE.WORKING_CAPITAL inside initialize, and add an explicit else branch that clears or resets the BehaviorSubject (this.productType.next(null) or LOAN_PRODUCT_TYPE.UNKNOWN) to avoid retaining prior state; update any callers to pass the tighter type.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts (2)
749-753:⚠️ Potential issue | 🟠 MajorUse
forEachinstead ofsome()for side-effect iteration.Line 749 uses
some()but never returns a boolean from the callback. This is semantically wrong and matches the static-analysis error.🛠️ Suggested fix
- this.transactionProcessingStrategyDataBase.some((cn: CodeName) => { + this.transactionProcessingStrategyDataBase.forEach((cn: CodeName) => { if (LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)) { this.transactionProcessingStrategyData.push(cn); } });#!/bin/bash # Verify current iterable method usage in the reported block. rg -n "transactionProcessingStrategyDataBase\\.some\\(" src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts -C3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 749 - 753, The callback passed to transactionProcessingStrategyDataBase.some(...) in loan-product-settings-step.component.ts is used for side-effects only and never returns a boolean; replace the use of Array.prototype.some with Array.prototype.forEach to iterate without expecting a truthy return. Update the call site where transactionProcessingStrategyDataBase.some((cn: CodeName) => { if (LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)) { this.transactionProcessingStrategyData.push(cn); } }); to use transactionProcessingStrategyDataBase.forEach and keep the same body, ensuring the code references LoanProducts.isAdvancedPaymentAllocationStrategy and this.transactionProcessingStrategyData remain unchanged.
499-642:⚠️ Potential issue | 🟠 MajorNested
valueChangessubscriptions accumulate without cleanup, causing duplicate form mutations on re-toggle.Line 500 establishes a subscription that registers inner subscriptions (lines 524, 533, 562, 592, 621) every time
isInterestRecalculationEnabledemits true. These nested subscriptions are never unsubscribed, so toggling the form control true→false→true stacks multiple handlers that re-runaddControl/removeControllogic.The component has 16
.valueChanges.subscribe()calls throughout with zero cleanup mechanisms (takeUntilDestroyed,ngOnDestroy, oruntilDestroyedare absent), and the base class provides no lifecycle management. This violates the coding guideline: "For Angular code: verify…clean observable patterns."Fix: Use DestroyRef with takeUntilDestroyed
+import { DestroyRef, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; export class LoanProductSettingsStepComponent extends LoanProductBaseComponent implements OnInit { + private destroyRef = inject(DestroyRef); setConditionalControls() { if (this.loanProductService.isLoanProduct) { this.loanProductSettingsForm .get('isInterestRecalculationEnabled') - .valueChanges.subscribe((isInterestRecalculationEnabled: any) => { + .valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((isInterestRecalculationEnabled: any) => { // Apply to all nested subscriptions similarly }); } } }Additionally, line 749 uses
Array.prototype.some()with a side-effect-only callback that does not return a boolean; useforEach()instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 499 - 642, The nested valueChanges subscriptions off isInterestRecalculationEnabled (the subscription starting at get('isInterestRecalculationEnabled').valueChanges) create inner subscriptions (e.g., interestRecalculationCompoundingMethod, recalculationCompoundingFrequencyType, recalculationCompoundingFrequencyNthDayType, recalculationRestFrequencyType, recalculationRestFrequencyNthDayType) every time the toggle is set true and never unsubscribe; refactor to register each dependant valueChanges only once (move those .get(...).valueChanges subscriptions out of the isInterestRecalculationEnabled handler) or ensure proper teardown by capturing the inner Subscription objects and unsubscribing when isInterestRecalculationEnabled becomes false or on destroy (use Angular DestroyRef + takeUntilDestroyed or an ngOnDestroy that unsubscribes), keep the addControl/removeControl logic idempotent, and also replace the side-effect-only Array.prototype.some usage (the call at/around the code referenced as using .some()) with forEach so callbacks that don't return booleans do not misuse .some().src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (1)
114-150:⚠️ Potential issue | 🟠 MajorUnify period-payment-rate minimum constraints and fallback messages.
Line 145 uses
[min]="1"while Lines 116 and 125 use0.01. Also, Lines 119, 139, and 148 still fallback to1, which makes the validation message inconsistent.🔧 Proposed fix
- <strong>{{ loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong> ... - <strong>{{ loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong> ... - <input type="number" matInput formControlName="maxPeriodPaymentRate" [min]="1" step="0.01" /> + <input type="number" matInput formControlName="maxPeriodPaymentRate" [min]="0.01" step="0.01" /> ... - <strong>{{ loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 1 }}</strong> + <strong>{{ loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 0.01 }}</strong>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 114 - 150, The three numeric inputs for period payment rate are inconsistent: update the min attributes and their error fallback values so all use 0.01 uniformly; specifically change maxPeriodPaymentRate's [min] from 1 to 0.01 and replace any fallback default values in the mat-error expressions for minPeriodPaymentRate, periodPaymentRate, and maxPeriodPaymentRate (currently using || 1) to || 0.01 so the displayed minimum matches the actual validator, referencing the template controls loanProductTermsForm.get('minPeriodPaymentRate'), loanProductTermsForm.get('periodPaymentRate'), and loanProductTermsForm.get('maxPeriodPaymentRate').src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts (2)
497-502:⚠️ Potential issue | 🟠 MajorNull guard is still missing before reading
chargeOffBehaviour.value.Line 497 dereferences
.valuewithout ensuringchargeOffBehaviourexists, which can still throw at runtime.🛡️ Proposed guard
- if (!this.loanProduct.chargeOffBehaviour.value) { + if (!this.loanProduct.chargeOffBehaviour || !this.loanProduct.chargeOffBehaviour.value) { this.loanProduct.chargeOffBehaviour = this.stringEnumOptionDataLookUp( this.loanProduct.chargeOffBehaviour, this.loanProductsTemplate.chargeOffBehaviourOptions ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 497 - 502, Add a null guard before dereferencing chargeOffBehaviour.value: ensure this.loanProduct.chargeOffBehaviour exists (or use optional chaining) before checking its .value in the component where this code runs (referencing this.loanProduct, chargeOffBehaviour, stringEnumOptionDataLookUp and this.loanProductsTemplate.chargeOffBehaviourOptions). If chargeOffBehaviour is falsy or its value is falsy, call this.stringEnumOptionDataLookUp(...) to set this.loanProduct.chargeOffBehaviour; otherwise skip the lookup. This prevents runtime errors when chargeOffBehaviour is null/undefined.
417-423:⚠️ Potential issue | 🟠 Major
daysInYearCustomStrategystill gets overwritten when condition is false.Line 423 assigns
optionValueunconditionally, so when theifin Line 417 is false, it can carry the wrong prior value.🐛 Proposed fix
- if (this.isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id == 1) { - optionValue = this.optionDataLookUp( - this.loanProduct.daysInYearCustomStrategy, - this.loanProductsTemplate.daysInYearCustomStrategyOptions - ); - } - this.loanProduct.daysInYearCustomStrategy = optionValue; + let daysInYearCustomStrategy: OptionData | null = null; + if (this.isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id === 1) { + daysInYearCustomStrategy = this.optionDataLookUp( + this.loanProduct.daysInYearCustomStrategy, + this.loanProductsTemplate.daysInYearCustomStrategyOptions + ); + } + this.loanProduct.daysInYearCustomStrategy = daysInYearCustomStrategy;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 417 - 423, The unconditional assignment to this.loanProduct.daysInYearCustomStrategy at the end overwrites the property even when the if condition fails; update the logic in the component so that optionValue is only computed and assigned when the condition is true (use the existing this.loanProduct.daysInYearCustomStrategy if the condition is false), e.g., call optionDataLookUp and assign to this.loanProduct.daysInYearCustomStrategy only inside the if that checks this.isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id == 1, referencing optionDataLookUp, this.loanProductsTemplate.daysInYearCustomStrategyOptions, and this.loanProduct.daysInYearCustomStrategy.src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts (1)
350-357:⚠️ Potential issue | 🟠 MajorOnly include advanced allocation fields when strategy is advanced.
Defaulting
paymentAllocation/creditAllocation/supportedInterestRefundTypesto[]in edit mode sends those fields for non-advanced strategies.🛠️ Proposed fix
- // Default empty array - loanProduct['paymentAllocation'] = []; - loanProduct['creditAllocation'] = []; - loanProduct['supportedInterestRefundTypes'] = []; if (this.isAdvancedPaymentStrategy) { loanProduct['paymentAllocation'] = this.paymentAllocation; loanProduct['creditAllocation'] = this.creditAllocation; loanProduct['supportedInterestRefundTypes'] = this.supportedInterestRefundTypes; ... }} else { + delete loanProduct['paymentAllocation']; + delete loanProduct['creditAllocation']; delete loanProduct['supportedInterestRefundTypes']; delete loanProduct['daysInYearCustomStrategy']; }Also applies to: 404-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts` around lines 350 - 357, The code is always assigning empty arrays to loanProduct.paymentAllocation, loanProduct.creditAllocation, and loanProduct.supportedInterestRefundTypes which causes non-advanced strategies to send those fields; change the logic so these three properties are only added to loanProduct when this.isAdvancedPaymentStrategy is true (i.e., remove the default assignments and only assign loanProduct['paymentAllocation'] = this.paymentAllocation, loanProduct['creditAllocation'] = this.creditAllocation, and loanProduct['supportedInterestRefundTypes'] = this.supportedInterestRefundTypes inside the isAdvancedPaymentStrategy branch) and apply the same fix to the other similar block that constructs loanProduct further down.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
182-183:⚠️ Potential issue | 🟠 MajorAvoid recreating the terms form after
ngOnChangeshas already configured it.
ngOnChangesruns beforengOnInitfor initial inputs; recreating the form inngOnInitclears controls/state added earlier (e.g., disabled override fields and dynamically added controls).🛠️ Proposed fix
ngOnInit() { - this.createloansAccountTermsForm(); this.maxDate = this.settingsService.maxFutureDate; this.loansAccountTermsData = this.loansAccountProductTemplate;Also applies to: 324-326
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 182 - 183, Remove the unconditional call to createloansAccountTermsForm() from ngOnInit so you don't overwrite the form built/configured in ngOnChanges; instead either delete that call or guard it with a check like if (!this.loansAccountTermsForm) before calling createloansAccountTermsForm(), and apply the same change for the duplicate call found around the createloansAccountTermsForm invocation at lines ~324-326; keep form creation/configuration centralized in ngOnChanges and reference the loansAccountTermsForm, ngOnInit, ngOnChanges, and createloansAccountTermsForm symbols when making the change.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (1)
415-427:⚠️ Potential issue | 🟠 MajorRestore original validators and remove
NaNfallback when re-adding fixed-rate controls.When toggling back from floating rates, the controls are recreated with weaker validation than the initial form, and Line 424 can produce
NaNwhen options are empty.🛠️ Proposed fix
- this.loanProductTermsForm.addControl('minInterestRatePerPeriod', new UntypedFormControl('')); + this.loanProductTermsForm.addControl( + 'minInterestRatePerPeriod', + new UntypedFormControl('', [Validators.min(0), Validators.pattern(/^\d+([.,]\d{1,6})?$/)]) + ); this.loanProductTermsForm.addControl( 'interestRatePerPeriod', - new UntypedFormControl('', Validators.required) + new UntypedFormControl('', [ + Validators.required, + Validators.min(0), + Validators.pattern(/^\d+([.,]\d{1,6})?$/) + ]) ); - this.loanProductTermsForm.addControl('maxInterestRatePerPeriod', new UntypedFormControl('')); + this.loanProductTermsForm.addControl( + 'maxInterestRatePerPeriod', + new UntypedFormControl('', [Validators.min(0), Validators.pattern(/^\d+([.,]\d{1,6})?$/)]) + ); this.loanProductTermsForm.addControl( 'interestRateFrequencyType', new UntypedFormControl( - this.loanProductsTemplate.interestRateFrequencyType?.id ?? +this.interestRateFrequencyTypeData[0]?.id, + this.loanProductsTemplate.interestRateFrequencyType?.id ?? + this.interestRateFrequencyTypeData?.[0]?.id ?? + null, Validators.required ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 415 - 427, Recreate the fixed-rate controls on loanProductTermsForm with the same validators as the original form (restore required/number validators for 'minInterestRatePerPeriod', 'interestRatePerPeriod', and 'maxInterestRatePerPeriod') instead of using empty UntypedFormControl(''), and remove the NaN fallback when setting the default for 'interestRateFrequencyType' — use the template value if present or the first option id only when interestRateFrequencyTypeData[0] exists (avoid +this.interestRateFrequencyTypeData[0]?.id which can yield NaN); update the control creation code that references loanProductTermsForm.addControl and interestRateFrequencyTypeData/loanProductsTemplate to enforce the original validators and safe default selection.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html (1)
805-816:⚠️ Potential issue | 🟡 MinorUse repayment-event tooltip keys for repayment-event fields.
These two inputs still reference the outstanding-balance tooltip key, so the help text is misleading.
🛠️ Proposed fix
- matTooltip="{{ 'tooltips.Maximum outstanding loan account balance' | translate }}" + matTooltip="{{ 'tooltips.DueDaysForRepaymentEvent' | translate }}" ... - matTooltip="{{ 'tooltips.Maximum outstanding loan account balance' | translate }}" + matTooltip="{{ 'tooltips.OverdueDaysForRepaymentEvent' | translate }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html` around lines 805 - 816, Replace the incorrect outstanding-balance tooltip key on the two inputs and use the repayment-event translation keys instead: update the matTooltip for the input with formControlName "dueDaysForRepaymentEvent" and the input with formControlName "overDueDaysForRepaymentEvent" to the appropriate repayment-event tooltip keys (e.g., use the translation keys that correspond to due/overdue days for repayment events rather than "tooltips.Maximum outstanding loan account balance").
🧹 Nitpick comments (11)
src/app/products/loan-products/loan-products.component.scss (1)
17-21: Consider a responsive fallback for.form-container.
flex-direction: rowwith percentage-based child widths can get cramped on narrow screens; consider stacking controls via a small-screen media query (or enabling wrap) for better mobile usability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.scss` around lines 17 - 21, The .form-container currently forces a horizontal layout (display:flex; flex-direction:row; gap:2%) which can break on narrow viewports; update .form-container to either allow wrapping by adding flex-wrap:wrap and adjust child widths (e.g., max-width:100% on narrow sizes) or add a small-screen media query (e.g., `@media` (max-width:600px)) that switches .form-container to flex-direction:column and adjusts gaps/margins so form controls stack cleanly; target the .form-container selector and any child control width rules to implement the responsive fallback.src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts (1)
72-72: Typeanyon API response — introduce a typed interface.
response: anybypasses type safety for thecreateDelinquencyRangeresult. Based on learnings, introduce a specific response interface (e.g.,{ resourceId: number }) and type the observable accordingly rather than usingany.♻️ Proposed refactor
+interface DelinquencyRangeResponse { + resourceId: number; +} - this.productsService.createDelinquencyRange(data).subscribe((response: any) => { + this.productsService.createDelinquencyRange(data).subscribe((response: DelinquencyRangeResponse) => {Also update the
ProductsService.createDelinquencyRangereturn type toObservable<DelinquencyRangeResponse>.Based on learnings: "avoid using
Observable<any>as a project-wide pattern for API responses … introduce specific interfaces/types for the response shapes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts` at line 72, Define a concrete response type (e.g., interface DelinquencyRangeResponse { resourceId: number }) and update ProductsService.createDelinquencyRange to return Observable<DelinquencyRangeResponse> instead of Observable<any>; then update the consumer in create-range.component.ts so the subscribe callback uses (response: DelinquencyRangeResponse) and handle response.resourceId accordingly. Ensure the new interface is exported from a shared types file or next to ProductsService so both the service and the component can import it.src/app/products/products.service.ts (1)
32-50: Use typed contracts for the newly parameterized loan-product APIs.These updated methods still expose
Observable<any>andloanProduct: any. Tightening just this touched surface (request/response interfaces + constrainedproductTypeunion) would materially improve compile-time safety.Based on learnings: “In TypeScript files (e.g., src/app/.../*.ts), avoid using Observable as a project-wide pattern for API responses... introduce specific interfaces/types for the response shapes and use proper typing instead of any...”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 32 - 50, The methods getLoanProducts, createLoanProduct, getLoanProductsTemplate, getLoanProduct, and updateLoanProduct currently use loose types (Observable<any> and loanProduct: any); define strong TypeScript contracts (e.g., type ProductType = 'personal' | 'business' | 'auto' or the project's allowed values, interface LoanProduct { ... } for request/updates, interface LoanProductResponse { ... } for single-resource responses and LoanProductListResponse for arrays, and interface LoanProductTemplate for the template endpoint) and change signatures to use them (e.g., getLoanProducts(productType: ProductType): Observable<LoanProductListResponse>, getLoanProduct(productType: ProductType, loanProductId: string, template = false): Observable<LoanProductResponse | LoanProductTemplate>, createLoanProduct(productType: ProductType, loanProduct: LoanProduct): Observable<LoanProductResponse>, updateLoanProduct(productType: ProductType, loanProductId: string, loanProduct: LoanProduct): Observable<LoanProductResponse>), and update the HttpParams usage for template to reflect the typed return; add or import the new interfaces near the top of the file or in a shared models file and replace any remaining any occurrences in these methods.src/app/products/loan-products/loan-products.ts (1)
76-79: Add defensive null coalescing for charges mapping.Line 79 directly calls
.map()onloanProductData.chargeswithout a guard. While the component initialization ensures charges is always an array in practice, using a null coalescing operator aligns with strict type safety principles (per Angular coding guidelines). The fix also protects against edge cases if the function is called with unexpected input.Suggested improvement
const loanProduct = this.loanProductService.isLoanProduct ? { ...loanProductData, - charges: loanProductData.charges.map((charge: any) => ({ id: charge.id })), + charges: (loanProductData.charges ?? []).map((charge: any) => ({ id: charge.id })), dateFormat, locale }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.ts` around lines 76 - 79, The code builds loanProduct when this.loanProductService.isLoanProduct is true and calls loanProductData.charges.map(...) without guarding for null/undefined; update the construction of loanProduct (the branch using loanProductData and charges) to use a null-coalescing fallback for loanProductData.charges (e.g., (loanProductData.charges ?? []) ) before calling .map so mapping is safe even if charges is null/undefined; apply this change where loanProduct is assembled to ensure strict type safety.src/assets/translations/lv-LV.json (1)
2791-2791: Consider aligning delinquency terminology with existing wording in this locale.
"Kavējumu grupas klasifikācija"introduces another variant for “Delinquency Bucket” in a file that already uses other terms. A single term improves UI consistency.♻️ Optional wording alignment
- "Delinquency Bucket Classification": "Kavējumu grupas klasifikācija", + "Delinquency Bucket Classification": "Likumpārkāpumu kopas klasifikācija",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lv-LV.json` at line 2791, The translation for the key "Delinquency Bucket Classification" uses a different variant ("Kavējumu grupas klasifikācija"); update this value to match the established delinquency term used elsewhere in the lv-LV locale (replace the value for "Delinquency Bucket Classification" with the same phrasing used by other keys for "Delinquency Bucket") so the UI uses a single consistent Latvian term.src/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/payment-allocation-model.ts (1)
143-155: AlignfutureInstallmentAllocationRulenullability in model types and payload construction.Line 143 introduces nullable/absent rule values, and Line 231 serializes that path optionally, but the interfaces still model it as always present. Please make the type contract explicit to keep strict typing reliable.
♻️ Suggested refactor
export interface PaymentAllocation { transactionType: string; paymentAllocationOrder: PaymentAllocationOrder[]; - futureInstallmentAllocationRule: string; + futureInstallmentAllocationRule?: string; } export class AdvancedPaymentAllocation { transaction: PaymentAllocationTransactionType; paymentAllocationOrder: PaymentAllocationOrder[]; - futureInstallmentAllocationRule: FutureInstallmentAllocationRule; + futureInstallmentAllocationRule: FutureInstallmentAllocationRule | null; futureInstallmentAllocationRules: FutureInstallmentAllocationRule[]; } -const futureInstallmentAllocationRule = loanProduct.advancedPaymentAllocationFutureInstallmentAllocationRules - ? loanProduct.advancedPaymentAllocationFutureInstallmentAllocationRules[0] - : null; +const futureInstallmentAllocationRule = + loanProduct.advancedPaymentAllocationFutureInstallmentAllocationRules?.[0] ?? null; paymentAllocations.push({ transactionType: paymentAllocation.transaction.code, paymentAllocationOrder: this.buildPaymentAllocationOrder(paymentAllocation.paymentAllocationOrder), - futureInstallmentAllocationRule: paymentAllocation.futureInstallmentAllocationRule?.code + ...(paymentAllocation.futureInstallmentAllocationRule?.code + ? { futureInstallmentAllocationRule: paymentAllocation.futureInstallmentAllocationRule.code } + : {}) });As per coding guidelines, "For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns."
Also applies to: 231-231
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/payment-allocation-model.ts` around lines 143 - 155, The model currently assigns a possibly null futureInstallmentAllocationRule from advancedPaymentAllocationFutureInstallmentAllocationRules but the interfaces still type it as always present; update the type for futureInstallmentAllocationRule in the payment allocation model/interface to be nullable (e.g., Rule | null or Rule | undefined) and adjust any serializer/payload builder to conditionally include futureInstallmentAllocationRule (or emit null explicitly) when constructing the payload in the block that calls buildPaymentAllocationTransactionOrder and pushes into advancedPaymentAllocation; ensure references to advancedPaymentAllocationFutureInstallmentAllocationRules and the push into advancedPaymentAllocation remain consistent with the new nullable type.src/app/products/loan-products/common/loan-product-base.component.ts (1)
15-15: Remove the empty constructor.
constructor() {}is redundant here and can be dropped.Proposed cleanup
export abstract class LoanProductBaseComponent { protected loanProductService = inject(LoanProductService); - constructor() {} - getProductTypeLabel(upperCase: boolean): string {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-base.component.ts` at line 15, Remove the redundant empty constructor in the LoanProductBaseComponent class: delete the `constructor() {}` declaration from loan-product-base.component.ts so the class relies on the default constructor; ensure no initialization or dependency injection logic is lost from the `LoanProductBaseComponent` when removing it.src/app/products/loan-products/loan-products-template.resolver.ts (1)
28-28: Use a concrete resolver response type instead ofObservable<any>.The new resolver branch is still untyped at the API boundary. Please return the specific template response interface used by
ProductsService.getLoanProductsTemplate(...)to keep route data strongly typed.Based on learnings: “In TypeScript files ... avoid using Observable ... introduce specific interfaces/types for the response shapes and use proper typing instead of any.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products-template.resolver.ts` at line 28, The resolver's resolve method is untyped—replace Observable<any> with the concrete interface returned by ProductsService.getLoanProductsTemplate (e.g., Observable<LoanProductsTemplate> or the exact response type name used by ProductsService), import that interface, update the resolve signature (resolve(route: ActivatedRouteSnapshot): Observable<ThatConcreteType>) and ensure any downstream usages (route.data access) expect that concrete type so route data stays strongly typed.src/app/products/loan-products/loan-products.component.ts (1)
239-243: Extract product-path mapping to one helper to avoid drift.The
loanProductSelector -> API pathmapping is duplicated. Centralizing it prevents future divergence.♻️ Suggested refactor
+ private getSelectedProductApiPath(): string { + return this.loanProductSelector.value === LOAN_PRODUCT_TYPE.LOAN ? '' : 'workingcapital'; + } importLoanProduct(file: File): void { @@ - const productType = this.loanProductSelector.value === LOAN_PRODUCT_TYPE.LOAN ? '' : 'workingcapital'; + const productType = this.getSelectedProductApiPath(); @@ fetchProducts(): void { - const productType: string = this.loanProductSelector.value === LOAN_PRODUCT_TYPE.LOAN ? '' : 'workingcapital'; + const productType: string = this.getSelectedProductApiPath();Also applies to: 267-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` around lines 239 - 243, The product-type to API-path logic is duplicated; extract it into a single helper (e.g., mapLoanProductTypeToPath) and use it wherever productType is computed (replace the inline expression that uses this.loanProductSelector.value and LOAN_PRODUCT_TYPE). Update calls to createLoanProduct and getLoanProducts (and the other occurrence around the get/update flow that also computes productType) to call the helper instead of duplicating the ternary, so both createLoanProduct(productPath, payload) and getLoanProducts(productPath) consistently use the same mapping function.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts (1)
188-193: Tighten types in newly added mapping branches (anyleaks safety here).The new block at lines 188-193 and the mapping branches at lines 261-343 use
anytypes which weakens compile-time type checks. Replaceanywith proper types already imported in this file:
- Lines 188-193: Constants should be typed as
OptionData[](already imported fromoption-data.model)- Lines 261-343: Replace
anyforEach parameter types with their proper counterparts:
feeToIncomeAccountMappings.forEach((m: any)→ChargeToIncomeAccountMappingpenaltyToIncomeAccountMappings.forEach((m: any)→ChargeToIncomeAccountMappingbuydownfeeClassificationToIncomeAccountMappings.forEach((m: any)→ClassificationToIncomeAccountMappingcapitalizedIncomeClassificationToIncomeAccountMappings.forEach((m: any)→ClassificationToIncomeAccountMappingwriteOffReasonsToExpenseMappings.forEach((m: any)→ChargeOffReasonToExpenseAccountMappingNote:
chargeOffReasonToExpenseAccountMappingsalready uses the properChargeOffReasonToExpenseAccountMappingtype—apply the same pattern to the remaining mappings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 188 - 193, Replace the loose any types: change the constants chargeOffReasonOptions, writeOffReasonOptions, buydownFeeClassificationOptions, and capitalizedIncomeClassificationOptions from any to OptionData[] (imported OptionData). Also tighten the mapping foreach parameter types: use ChargeToIncomeAccountMapping for feeToIncomeAccountMappings.forEach and penaltyToIncomeAccountMappings.forEach, use ClassificationToIncomeAccountMapping for buydownfeeClassificationToIncomeAccountMappings.forEach and capitalizedIncomeClassificationToIncomeAccountMappings.forEach, and use ChargeOffReasonToExpenseAccountMapping for writeOffReasonsToExpenseMappings.forEach (matching the already-correct chargeOffReasonToExpenseAccountMappings usage).src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (1)
189-191: Use lifecycle-aware teardown foradvancedTransactionProcessingStrategysubscription.This subscription should be tied to component lifecycle to avoid retaining handlers across recreations.
🛠️ Proposed refactor
-import { Component, OnInit, Input, OnChanges, SimpleChanges, inject } from '@angular/core'; +import { Component, OnInit, Input, OnChanges, SimpleChanges, inject, DestroyRef } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; export class LoanProductTermsStepComponent extends LoanProductBaseComponent implements OnInit, OnChanges { + private destroyRef = inject(DestroyRef); ... - this.processingStrategyService.advancedTransactionProcessingStrategy.subscribe((value: boolean) => { + this.processingStrategyService.advancedTransactionProcessingStrategy + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value: boolean) => { this.isAdvancedTransactionProcessingStrategy = value; - }); + });As per coding guidelines,
src/app/**: For Angular code: verify ... clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 189 - 191, The subscription to processingStrategyService.advancedTransactionProcessingStrategy must be lifecycle-aware: create a private destroy$ Subject in the component, replace the direct subscribe call on processingStrategyService.advancedTransactionProcessingStrategy with a pipe(takeUntil(this.destroy$)) subscription that sets isAdvancedTransactionProcessingStrategy, and implement ngOnDestroy to call this.destroy$.next() and this.destroy$.complete() so the handler is torn down when the component is destroyed.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
src/app/core/shell/toolbar/toolbar.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/organization/currencies/manage-currencies/manage-currencies.component.htmlsrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.htmlsrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.tssrc/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/advance-payment-allocation-tab/advance-payment-allocation-tab.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/advance-payment-allocation-tab/advance-payment-allocation-tab.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/loan-product-payment-strategy-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/payment-allocation-model.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-product.resolver.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.component.scsssrc/app/products/loan-products/loan-products.component.tssrc/app/products/loan-products/loan-products.resolver.tssrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/services/loan-product.service.tssrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.htmlsrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.tssrc/app/products/loan-products/view-loan-product/shared/view-advance-paymeny-allocation/view-advance-paymeny-allocation.component.htmlsrc/app/products/loan-products/view-loan-product/shared/view-advance-paymeny-allocation/view-advance-paymeny-allocation.component.tssrc/app/products/loan-products/view-loan-product/view-loan-product.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.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 with no reviewable changes (1)
- src/app/core/shell/toolbar/toolbar.component.html
🚧 Files skipped from review as they are similar to previous changes (13)
- src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.html
- src/app/products/loan-products/view-loan-product/general-tab/general-tab.component.html
- src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts
- src/app/products/loan-products/loan-products.component.html
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/app/products/loan-products/loan-product.resolver.ts
- src/assets/translations/fr-FR.json
- src/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.ts
- src/assets/translations/lt-LT.json
- src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html
- src/assets/translations/ne-NE.json
- src/app/products/loan-products/loan-products.resolver.ts
- src/assets/translations/es-CL.json
...p/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
Outdated
Show resolved
Hide resolved
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
Outdated
Show resolved
Hide resolved
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
Show resolved
Hide resolved
src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts
Show resolved
Hide resolved
Comment on lines
+34
to
+38
| super(); | ||
| const productType = this.route.snapshot.queryParamMap.get('productType') || null; | ||
| if (productType) { | ||
| this.loanProductService.initialize(productType); | ||
| } |
There was a problem hiding this comment.
Initialize product type unconditionally to prevent stale service state.
At Line 35-Line 38, initialization only happens when productType exists. If this route is opened without that query param after a Working Capital flow, LoanProductService can retain the previous type and render the wrong UI branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/view-loan-product/view-loan-product.component.ts`
around lines 34 - 38, The component currently only calls
this.loanProductService.initialize(productType) when productType is truthy,
which lets LoanProductService keep a stale type; always call initialize with the
retrieved value (even when null/undefined) right after computing productType
(after super() and the const productType =
this.route.snapshot.queryParamMap.get('productType') || null) so the service
resets its state when the query param is absent.
...products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts
Show resolved
Hide resolved
src/assets/translations/pt-PT.json
Outdated
| "Retry": "Tentar novamente" | ||
| "Retry": "Tentar novamente", | ||
| "Flat Percentage Amount": "Valor percentual fixo", | ||
| "Delinquency Bucket Classification": "Classificação de Inadimplência", |
There was a problem hiding this comment.
Keep “Bucket” explicit in delinquency classification text on Line 2790.
Current value drops the bucket concept, which makes this label less specific than the source key.
Suggested wording fix
- "Delinquency Bucket Classification": "Classificação de Inadimplência",
+ "Delinquency Bucket Classification": "Classificação do intervalo de inadimplência",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "Delinquency Bucket Classification": "Classificação de Inadimplência", | |
| "Delinquency Bucket Classification": "Classificação do intervalo de inadimplência", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/pt-PT.json` at line 2790, The translated value for
the key "Delinquency Bucket Classification" currently omits "Bucket" — update
the Portuguese translation for that key (the entry with "Delinquency Bucket
Classification") to explicitly include the word equivalent of "Bucket" (e.g.,
"Classificação de Buckets de Inadimplência" or "Classificação de Inadimplência
por Bucket") so the label matches the source specificity.
268c98c to
d57273b
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/products/loan-products/loan-products.component.ts (1)
87-97: 🛠️ Refactor suggestion | 🟠 MajorReplace
UntypedFormControlwithFormControl<LoanProductType>and typeloanProductOptions.The product type selector controls critical routing and API behavior; using
UntypedFormControlandanytypes bypasses type safety checks that prevent invalid product values from propagating. SinceLoanProductType('loan' | 'working-capital') is already defined in the model, apply it:loanProductSelector = new FormControl<LoanProductType>(LOAN_PRODUCT_TYPE.LOAN); loanProductOptions: readonly typeof PRODUCT_TYPES[number][] = PRODUCT_TYPES;This aligns with the required strict type safety checks for Angular code in
src/app/**.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` around lines 87 - 97, Replace the untyped control and any-typed options with strict generics: change loanProductSelector from UntypedFormControl to FormControl<LoanProductType> and initialize it with a default like LOAN_PRODUCT_TYPE.LOAN, and change loanProductOptions from any to readonly typeof PRODUCT_TYPES[number][] = PRODUCT_TYPES so the selector only accepts valid LoanProductType values; update imports to bring in FormControl and LoanProductType and ensure PRODUCT_TYPES and LOAN_PRODUCT_TYPE are referenced for the default and type.
♻️ Duplicate comments (18)
src/app/organization/currencies/manage-currencies/manage-currencies.component.html (1)
35-41:⚠️ Potential issue | 🟡 Minor
type="submit"still not updated totype="button"after removing(ngSubmit).This was flagged in a previous review and remains unresolved. With
(ngSubmit)now gone (line 12), thetype="submit"button is misleading: Enter-key presses will fire the DOMsubmitevent (suppressed silently byFormGroupDirective), but the(click)handler on line 41 will not fire for keyboard-triggered submissions — making the interaction asymmetric.♻️ Proposed fix
<button - type="submit" + type="button" mat-raised-button class="add-currency-button" color="primary" [disabled]="!currencyForm.valid" (click)="addCurrency()"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/organization/currencies/manage-currencies/manage-currencies.component.html` around lines 35 - 41, The Add Currency button still uses type="submit" despite removing the (ngSubmit) handler, causing Enter-key submits to invoke the native form submit path (and not the (click) -> addCurrency() flow); update the button element that calls addCurrency() to type="button" (and ensure the template's [disabled]="!currencyForm.valid" and (click)="addCurrency()" remain unchanged) so keyboard activation triggers the same click handler as mouse clicks.src/assets/translations/lt-LT.json (1)
2793-2793:⚠️ Potential issue | 🟡 MinorFix Lithuanian grammar/casing for “Period Payment Frequency”.
"Laikotarpis Mokėjimo dažnumas"should be normalized to"Laikotarpio mokėjimo dažnumas"for consistent phrasing.🔧 Proposed fix
- "Period Payment Frequency": "Laikotarpis Mokėjimo dažnumas", + "Period Payment Frequency": "Laikotarpio mokėjimo dažnumas",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lt-LT.json` at line 2793, Update the Lithuanian translation value for the key "Period Payment Frequency" in src/assets/translations/lt-LT.json: replace the current value "Laikotarpis Mokėjimo dažnumas" with the grammatically/case-corrected string "Laikotarpio mokėjimo dažnumas" to match project phrasing and casing conventions.src/assets/translations/pt-PT.json (1)
2720-2722:⚠️ Potential issue | 🟡 MinorUse a finance-specific translation for
Flat.On Line 2722,
"Plano"reads as “plan” instead of flat interest/amortization. Prefer a domain term like “Taxa fixa”.Suggested fix
"Equal principal payments": "Pagamentos de principal iguais", "Effective Interest Rate": "Taxa de juros efetiva", - "Flat": "Plano" + "Flat": "Taxa fixa"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/pt-PT.json` around lines 2720 - 2722, Replace the generic Portuguese translation for the "Flat" translation key (currently "Plano") with a finance-specific term; update the value for the "Flat" key to "Taxa fixa" so it correctly conveys flat interest/amortization in financial contexts.src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts (1)
32-35:⚠️ Potential issue | 🟠 MajorDerive
productTypefrom route in resolver before API call.Line 34 depends on service state that may be stale. This can fetch loan data from the wrong endpoint when opening/editing directly.
Suggested fix
+import { LOAN_PRODUCT_TYPE } from '../models/loan-product.model'; @@ resolve(route: ActivatedRouteSnapshot): Observable<any> { const loanProductId = route.parent.paramMap.get('productId'); + const productType = route.queryParams['productType'] || LOAN_PRODUCT_TYPE.LOAN; + this.loanProductService.initialize(productType); return this.productsService.getLoanProduct(this.loanProductService.loanProductPath, loanProductId, true); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.ts` around lines 32 - 35, The resolver's resolve() currently uses loanProductService.loanProductPath (service state) which can be stale; instead read productType from the route (e.g. route.parent.paramMap.get('productType') or route.paramMap.get('productType')) and derive the correct endpoint/path locally before calling productsService.getLoanProduct; replace the loanProductService.loanProductPath argument with the path computed from that route-derived productType (or fall back to the service value only if the route value is null) so the API call always targets the correct product endpoint in resolve().src/app/products/loan-products/view-loan-product/view-loan-product.component.ts (1)
35-38:⚠️ Potential issue | 🟠 MajorInitialize product type unconditionally to avoid stale state.
At Line 36, initialization is gated by truthiness. If
productTypeis absent,LoanProductServicecan retain the previous value and drive the wrong branch.Proposed fix
const productType = this.route.snapshot.queryParamMap.get('productType') || null; - if (productType) { - this.loanProductService.initialize(productType); - } + this.loanProductService.initialize(productType);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/view-loan-product/view-loan-product.component.ts` around lines 35 - 38, The guard around initializing LoanProductService allows stale state when query param is missing; remove the conditional and always call this.loanProductService.initialize(productType) after retrieving productType from this.route.snapshot.queryParamMap.get('productType') (passing null if absent), and if necessary update LoanProductService.initialize to accept and correctly reset state when given null so previous productType cannot drive logic.src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts (1)
44-47:⚠️ Potential issue | 🟠 MajorGuard missing currency data before
patchValue.At Line 45, fallback to
this.currencyData[0]can still beundefined. Then Line 47 accessescurrency.codeand can throw.Proposed fix
- this.currencyData = this.loanProductsTemplate.currencyOptions; - const currency = this.loanProductsTemplate.currency ? this.loanProductsTemplate.currency : this.currencyData[0]; + this.currencyData = this.loanProductsTemplate.currencyOptions ?? []; + const currency = this.loanProductsTemplate.currency ?? this.currencyData[0]; + if (!currency) { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts` around lines 44 - 47, Guard against undefined currency before calling patchValue: in LoanProductCurrencyStepComponent use a safe lookup for currency (from this.loanProductsTemplate.currency or this.currencyData[0]) and verify it exists before accessing currency.code; if neither is available, set a sensible default (e.g., null or empty string) or skip/return early. Update the logic around this.currencyData, this.loanProductsTemplate, and the call to this.loanProductCurrencyForm.patchValue(...) so you only read currency.code when currency is defined, e.g., compute a safeCurrency variable, check safeCurrency !== undefined before patching, and ensure currencyData is an array with length > 0.src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.html (1)
94-98:⚠️ Potential issue | 🟡 MinorUse stable key tracking for funds.
At Line 94,
track funduses object reference tracking. Use a stable identifier (fund.id) to avoid unnecessary DOM re-creation on refreshed arrays.Proposed fix
- `@for` (fund of fundData; track fund) { + `@for` (fund of fundData; track fund.id) {As per coding guidelines:
src/app/**: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.html` around lines 94 - 98, The template currently tracks the funds by object reference in the loop over fundData; change it to use a stable identifier by replacing the current track usage with Angular's trackBy via a function (e.g., trackByFund) and implement that function on the component (loan-product-details-step.component) to return fund.id; update the template loop (the *ngFor over fundData that renders <mat-option [value]="fund.id">) to use trackBy: trackByFund so DOM elements are preserved when arrays are refreshed.src/assets/translations/es-CL.json (2)
2722-2722:⚠️ Potential issue | 🟡 MinorUse “capital” consistently in amortization wording.
Line 2722 switches to “principal”, but this locale already uses “capital” for the same concept.
✏️ Suggested adjustment
- "Equal principal payments": "Pagos iguales de principal", + "Equal principal payments": "Pagos iguales de capital",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` at line 2722, The translation key/value "Equal principal payments" should use the locale's consistent term "capital" instead of "principal"; update the value for the translation entry "Equal principal payments" to replace "principal" with "capital" (e.g., "Pagos iguales de capital") so it matches other amortization wording in this locale.
428-428:⚠️ Potential issue | 🟡 MinorLowercase “producto” to match nearby button labels.
Line 428 uses title case (“Crear Producto…”), while the adjacent entry (Line 427) uses sentence case (“Crear producto…”). Keeping this consistent avoids UI copy drift.
✏️ Suggested adjustment
- "Create Working Capital Product": "Crear Producto de Capital de Trabajo", + "Create Working Capital Product": "Crear producto de Capital de Trabajo",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` at line 428, Change the Spanish translation for the "Create Working Capital Product" key to match sentence-case used nearby: update the value from "Crear Producto de Capital de Trabajo" to "Crear producto de capital de trabajo" so "producto" (and following words) are lowercase to maintain consistency with adjacent label entries; locate the "Create Working Capital Product" key in the translations file and replace its value accordingly.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts (1)
752-756:⚠️ Potential issue | 🟠 MajorReplace
some()withforEach()in Line 752 to avoid lint failure.This callback never returns a boolean, so
some()is the wrong API and Biome flags it as an error.Suggested fix
- this.transactionProcessingStrategyDataBase.some((cn: CodeName) => { + this.transactionProcessingStrategyDataBase.forEach((cn: CodeName) => { if (LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code)) { this.transactionProcessingStrategyData.push(cn); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts` around lines 752 - 756, The code uses Array.some on transactionProcessingStrategyDataBase with a callback that never returns a boolean; replace the call to some() with forEach() so each CodeName (cn) is tested with LoanProducts.isAdvancedPaymentAllocationStrategy(cn.code) and, when true, pushed onto transactionProcessingStrategyData; keep the existing callback body and ensure no return value is expected from the iterator.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts (3)
501-502:⚠️ Potential issue | 🟠 MajorGuard
chargeOffBehaviourbefore reading.valuein Line 501.
this.loanProduct.chargeOffBehaviourcan be undefined, which will throw at runtime.Suggested fix
- if (!this.loanProduct.chargeOffBehaviour.value) { + if (!this.loanProduct.chargeOffBehaviour || !this.loanProduct.chargeOffBehaviour.value) { this.loanProduct.chargeOffBehaviour = this.stringEnumOptionDataLookUp( this.loanProduct.chargeOffBehaviour, this.loanProductsTemplate.chargeOffBehaviourOptions ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 501 - 502, The check reads this.loanProduct.chargeOffBehaviour.value without guarding against chargeOffBehaviour being undefined; update the condition in the method containing this snippet to first ensure this.loanProduct and this.loanProduct.chargeOffBehaviour exist (e.g. if (!this.loanProduct || !this.loanProduct.chargeOffBehaviour || !this.loanProduct.chargeOffBehaviour.value) ) before calling this.stringEnumOptionDataLookUp and assigning to this.loanProduct.chargeOffBehaviour so it won't throw at runtime when chargeOffBehaviour is undefined.
248-249:⚠️ Potential issue | 🔴 CriticalUse dedicated charge-off account IDs instead of
writeOffAccountIdfor both fields.Both
chargeOffExpenseAccountandchargeOffFraudExpenseAccountare currently mapped from the same source, which drops distinct user selections.Suggested fix
- chargeOffExpenseAccount: this.glAccountLookUp(this.loanProduct.writeOffAccountId, expenseAccountData), - chargeOffFraudExpenseAccount: this.glAccountLookUp(this.loanProduct.writeOffAccountId, expenseAccountData), + chargeOffExpenseAccount: this.glAccountLookUp(this.loanProduct.chargeOffExpenseAccountId, expenseAccountData), + chargeOffFraudExpenseAccount: this.glAccountLookUp( + this.loanProduct.chargeOffFraudExpenseAccountId, + expenseAccountData + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 248 - 249, Both chargeOffExpenseAccount and chargeOffFraudExpenseAccount are using the same source this.loanProduct.writeOffAccountId; update their mappings to use the dedicated charge-off IDs on the loanProduct (e.g. this.loanProduct.chargeOffAccountId and this.loanProduct.chargeOffFraudAccountId) when calling glAccountLookUp with expenseAccountData so each field reflects the distinct user selection; locate and change the two lines that set chargeOffExpenseAccount and chargeOffFraudExpenseAccount in loan-product-summary.component.ts to reference the appropriate loanProduct properties instead of writeOffAccountId.
421-427:⚠️ Potential issue | 🟠 MajorAvoid overwriting
daysInYearCustomStrategywithdaysInYearTypewhen the condition is false.At Line 427,
optionValuecan still containdaysInYearType, sodaysInYearCustomStrategygets an incorrect value.Suggested fix
- if (this.isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id == 1) { - optionValue = this.optionDataLookUp( - this.loanProduct.daysInYearCustomStrategy, - this.loanProductsTemplate.daysInYearCustomStrategyOptions - ); - } - this.loanProduct.daysInYearCustomStrategy = optionValue; + let daysInYearCustomStrategy: OptionData | null = null; + if (this.isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id === 1) { + daysInYearCustomStrategy = this.optionDataLookUp( + this.loanProduct.daysInYearCustomStrategy, + this.loanProductsTemplate.daysInYearCustomStrategyOptions + ); + } + this.loanProduct.daysInYearCustomStrategy = daysInYearCustomStrategy;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 421 - 427, The code overwrites loanProduct.daysInYearCustomStrategy with an optionValue that may still hold daysInYearType when the if condition is false; to fix, initialize optionValue from this.loanProduct.daysInYearCustomStrategy, then only call optionDataLookUp and assign back to this.loanProduct.daysInYearCustomStrategy if the condition (this.isAdvancedPaymentAllocation && this.loanProduct.daysInYearType?.id == 1) is true — update the block around optionDataLookUp and the final assignment to avoid unconditional overwrite (refer to isAdvancedPaymentAllocation, loanProduct.daysInYearType, daysInYearCustomStrategy, optionDataLookUp, and loanProductsTemplate.daysInYearCustomStrategyOptions).src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts (1)
351-355:⚠️ Potential issue | 🟠 MajorDo not include empty
paymentAllocation/creditAllocationfor non-advanced strategies.Line 351-355 prepopulates these fields with
[], and Line 409-412 does not remove them in non-advanced mode.Suggested fix
- // Default empty array - loanProduct['paymentAllocation'] = []; - loanProduct['creditAllocation'] = []; - loanProduct['supportedInterestRefundTypes'] = []; if (this.isAdvancedPaymentStrategy) { loanProduct['paymentAllocation'] = this.paymentAllocation; loanProduct['creditAllocation'] = this.creditAllocation; loanProduct['supportedInterestRefundTypes'] = this.supportedInterestRefundTypes; @@ } else { delete loanProduct['supportedInterestRefundTypes']; + delete loanProduct['paymentAllocation']; + delete loanProduct['creditAllocation']; delete loanProduct['daysInYearCustomStrategy']; }Also applies to: 409-412
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/edit-loan-product/edit-loan-product.component.ts` around lines 351 - 355, The component currently always initializes loanProduct['paymentAllocation'], loanProduct['creditAllocation'], and loanProduct['supportedInterestRefundTypes'] to [] (in edit-loan-product.component.ts) even when this.isAdvancedPaymentStrategy is false; update the logic so these arrays are only created/populated when isAdvancedPaymentStrategy is true and ensure any code that previously removed them in the non-advanced branch also deletes those keys (or skips adding them) when switching to non-advanced mode—specifically adjust the initialization block that currently sets those defaults and the branch around isAdvancedPaymentStrategy (the surrounding code that handles advanced vs non-advanced payment strategy) so that paymentAllocation, creditAllocation, and supportedInterestRefundTypes are omitted for non-advanced strategies.src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html (1)
805-816:⚠️ Potential issue | 🟡 MinorUse repayment-event tooltip keys for repayment-event fields.
Line 805 and Line 815 still point to the outstanding-balance tooltip key, which is unrelated to these inputs.
Suggested fix
- matTooltip="{{ 'tooltips.Maximum outstanding loan account balance' | translate }}" + matTooltip="{{ 'tooltips.Due days for repayment event' | translate }}" @@ - matTooltip="{{ 'tooltips.Maximum outstanding loan account balance' | translate }}" + matTooltip="{{ 'tooltips.OverDue days for repayment event' | translate }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html` around lines 805 - 816, The matTooltip for the inputs bound to formControlName "dueDaysForRepaymentEvent" and "overDueDaysForRepaymentEvent" is incorrectly using the outstanding-balance key; update those matTooltip bindings to use the appropriate repayment-event tooltip keys (i.e. replace 'tooltips.Maximum outstanding loan account balance' with the corresponding repayment-event tooltip keys for the dueDaysForRepaymentEvent and overDueDaysForRepaymentEvent fields) so the tooltip text matches the input purpose.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
329-331:⚠️ Potential issue | 🟠 MajorMissing optional chaining on
loansAccountTemplate.accountNoinngOnInit.
ngOnChangesat line 193 correctly usesthis.loansAccountTemplate?.accountNo, but the equivalent check inngOnInitat line 329 still accesses it directly. SinceloansAccountTemplateandloanIdare independent@Inputbindings with separate initialization timelines,loansAccountTemplatecan beundefinedwhenngOnInitruns if the parent has not yet bound it.🐛 Proposed fix
- if (this.loanId != null && this.loansAccountTemplate.accountNo) { + if (this.loanId != null && this.loansAccountTemplate?.accountNo) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 329 - 331, In ngOnInit, make the same safe check used in ngOnChanges by using optional chaining for loansAccountTemplate: change the condition that currently reads "if (this.loanId != null && this.loansAccountTemplate.accountNo)" to use "this.loansAccountTemplate?.accountNo" so loansAccountTermsData is only set when loansAccountTemplate exists and has accountNo; locate this in the ngOnInit method and update the conditional before assigning loansAccountTermsData.src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts (1)
52-59:⚠️ Potential issue | 🟡 MinorNo upper bound on either field after removing
max()validators.
Validators.max(1000)andValidators.max(10000)were dropped without replacement. There is still no client-side ceiling on either field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.ts` around lines 52 - 59, Reintroduce client-side upper bounds by adding appropriate Validators.max calls back into the form controls: add Validators.max(1000) to the minimumAgeDays control and Validators.max(10000) to the maximumAgeDays control (where the form is built in CreateRangeComponent / the create range form group), ensuring the Validators array for each control includes the existing pattern and min validators plus the respective max validator.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (1)
114-150:⚠️ Potential issue | 🟠 Major
maxPeriodPaymentRateuses[min]="1"while the other two rate fields use[min]="0.01"; error fallbacks show|| 1instead of|| 0.01.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 114 - 150, The maxPeriodPaymentRate control and the error fallback values are inconsistent with the other rate fields: update the <input> for maxPeriodPaymentRate to use [min]="0.01" instead of [min]="1", and change the error fallback expressions (loanProductTermsForm.get('minPeriodPaymentRate')?.errors?.['min']?.min || 1, loanProductTermsForm.get('periodPaymentRate')?.errors?.['min']?.min || 1, and loanProductTermsForm.get('maxPeriodPaymentRate')?.errors?.['min']?.min || 1) to use 0.01 as the fallback so all three controls (minPeriodPaymentRate, periodPaymentRate, maxPeriodPaymentRate) consistently enforce and display a minimum of 0.01.
🧹 Nitpick comments (10)
src/assets/translations/ne-NE.json (1)
2719-2721: NormalizeFlattranslation for consistency.
"Flat"is translated here asसमतल, while existing app usage maps"Flat"toफ्ल्याट. Keeping one term avoids inconsistent UI wording for the same option.♻️ Suggested consistency fix
"amortizationType": { "Equal installments": "बराबर किस्ता", "Equal principal payments": "समान मूल भुक्तानीहरू", "Effective Interest Rate": "प्रभावकारी ब्याजदर", - "Flat": "समतल" + "Flat": "फ्ल्याट" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/ne-NE.json` around lines 2719 - 2721, The translation for the key "Flat" is inconsistent (currently "समतल") and should be normalized to the app's existing term "फ्ल्याट"; update the JSON entry with key "Flat" to use the value "फ्ल्याट" so the UI shows the same wording everywhere (locate the "Flat" key in the translations block alongside "Equal principal payments" and "Effective Interest Rate" and replace its value).src/app/products/loan-products/loan-products.resolver.ts (1)
30-30: Use an explicit product-type constant instead of''.The empty-string argument works but is opaque and brittle. Prefer a named constant/enum value for the default loan-product discriminator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.resolver.ts` at line 30, Replace the opaque empty-string argument to productsService.getLoanProducts('') with a named constant or enum value (e.g. DEFAULT_LOAN_PRODUCT_TYPE or ProductType.DEFAULT_LOAN) to make the default discriminator explicit; define that constant/enum in an appropriate shared constants or product-type enum file and update the resolver call in loan-products.resolver.ts to pass the new constant to getLoanProducts so the intent is clear and robust.src/app/products/loan-products/loan-products.ts (1)
79-79: Replacecharge: anywith a proper type for strict type safety.The charge parameter in the mapper should be narrowly typed. Either use the minimal type
{ id: number }or import and use theChargeinterface fromsrc/app/shared/models/general.model.ts(which already defines the proper structure).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.ts` at line 79, The mapper using loanProductData.charges.map currently types the parameter as charge: any; change this to a strict type by replacing any with either the minimal inline type { id: number } or the imported Charge interface (from src/app/shared/models/general.model.ts) so the expression charges: loanProductData.charges.map((charge: Charge) => ({ id: charge.id })) (or using { id: number }) enforces compile-time safety and prevents accidental access to undeclared properties.src/app/products/loan-products/loan-products.component.scss (1)
17-21: Consider adding wrap behavior for smaller screens.With
flex-direction: rowon Line 19, controls may overflow on narrow widths. Addingflex-wrap: wrapkeeps the layout resilient.Suggested tweak
.form-container { display: flex; flex-direction: row; + flex-wrap: wrap; gap: 2%; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.scss` around lines 17 - 21, The .form-container style currently sets display: flex and flex-direction: row which can cause overflow on narrow viewports; update the .form-container rule to include flex-wrap: wrap to allow items to wrap, and optionally add a small responsive tweak (e.g., a media query switching .form-container to flex-direction: column or adjusting gap) so controls stack cleanly on smaller screens; target the .form-container selector to make these changes.src/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.ts (1)
101-103: Add lifecycle teardown for unmanaged subscriptions.The component has three direct subscriptions without teardown (lines 100, 102, and 116). Without proper cleanup, recreated component instances can retain handlers and trigger duplicate resets. Consider using
takeUntilDestroyedpattern for all subscriptions.Suggested fix
-import { Component, Input, OnInit, inject } from '@angular/core'; +import { Component, DestroyRef, Input, OnInit, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ export class LoanProductChargesStepComponent extends LoanProductBaseComponent implements OnInit { dialog = inject(MatDialog); private translateService = inject(TranslateService); + private destroyRef = inject(DestroyRef); @@ - this.currencyCode.valueChanges.subscribe(() => (this.chargesDataSource = [])); + this.currencyCode.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(() => (this.chargesDataSource = [])); if (this.loanProductService.isLoanProduct && this.multiDisburseLoan) { - this.multiDisburseLoan.valueChanges.subscribe(() => (this.chargesDataSource = [])); + this.multiDisburseLoan.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(() => (this.chargesDataSource = [])); } @@ deleteCharge(charge: any) { const deleteChargeDialogRef = this.dialog.open(DeleteDialogComponent, { data: { deleteContext: this.translateService.instant('labels.inputs.Charge') + ' ' + charge.name } }); - deleteChargeDialogRef.afterClosed().subscribe((response: any) => { + deleteChargeDialogRef.afterClosed() + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((response: any) => { if (response.delete) { this.chargesDataSource.splice(this.chargesDataSource.indexOf(charge), 1); this.chargesDataSource = this.chargesDataSource.concat([]); this.pristine = false; } });Per coding guidelines:
src/app/**: "For Angular code: verify ... clean observable patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.ts` around lines 101 - 103, The component creates direct subscriptions (e.g., subscribing to multiDisburseLoan.valueChanges and two other observables in this component) without teardown; update these to use a lifecycle teardown pattern such as takeUntilDestroyed (or a local destroy$ with ngOnDestroy) so subscriptions tied to this.loanProductService.isLoanProduct and multiDisburseLoan.valueChanges (and the third subscription referenced around the charges reset) are unsubscribed when the component is destroyed; import and apply takeUntilDestroyed (or add a private destroy$ = new Subject<void>() and call next/complete in ngOnDestroy) to each subscription and remove raw subscribe calls to prevent duplicate handlers and memory leaks.src/app/products/products.service.ts (1)
36-50: Consider typing as a separate cross-cutting enhancement.While strict typing for loan-product requests and responses would improve type safety, the full API response layer refactoring is a cross-cutting concern that should be tracked separately. The project currently uses
Observable<any>consistently across all services. Creating isolated payload/response interfaces (e.g.,LoanProductPayload,LoanProductResponse) within this feature PR would introduce inconsistency with the rest of the service surface. This typing work is better suited as a dedicated refactoring task tracked across the entire codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 36 - 50, The service methods createLoanProduct, getLoanProductsTemplate, getLoanProduct, and updateLoanProduct should remain using Observable<any> to stay consistent with existing services; remove any newly added feature-scoped interfaces or typed signatures from this PR and restore the original Observable<any> return types, and add a single-line TODO comment referencing a ticket/issue (or "TRACK-TYPING-REFactor") above these methods to indicate typing should be addressed in a separate, cross-cutting refactor.src/app/products/loan-products/loan-products.component.ts (1)
239-243: Centralize the UI→API product-type mapping.The
'workingcapital'conversion is duplicated in both import and fetch paths. Extract one helper (or service method) and reuse it to avoid divergence.Suggested refactor
+ private toApiProductType(productType: string): '' | 'workingcapital' { + return productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL ? 'workingcapital' : ''; + } - const productType = this.loanProductSelector.value === LOAN_PRODUCT_TYPE.LOAN ? '' : 'workingcapital'; + const productType = this.toApiProductType(this.loanProductSelector.value); - const productType: string = this.loanProductSelector.value === LOAN_PRODUCT_TYPE.LOAN ? '' : 'workingcapital'; + const productType = this.toApiProductType(this.loanProductSelector.value);Also applies to: 267-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-products.component.ts` around lines 239 - 243, Centralize the UI→API product-type mapping used where productType is computed (currently: const productType = this.loanProductSelector.value === LOAN_PRODUCT_TYPE.LOAN ? '' : 'workingcapital') by extracting a single helper (e.g., mapProductType(value) or add mapProductType method to ProductsService) and replacing inline logic in both createLoanProduct and getLoanProducts callers; update references in loan-products.component.ts (where productType is passed to this.productsService.createLoanProduct and .getLoanProducts) and the other location around lines 267-275 to call the new helper/service method so the 'workingcapital' conversion is defined in one place.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (1)
368-382: Redundant innerisLoanProductguards inside an already-gated block.Lines 371 and 379 repeat
if (this.loanProductService.isLoanProduct)insidesetConditionalControls, which is itself already wrapped in an outerif (this.loanProductService.isLoanProduct)at line 364. The inner guards are alwaystruehere and can be removed.♻️ Proposed fix
if (allowApprovedDisbursedAmountsOverApplied) { this.loanProductTermsForm.get('overAppliedCalculationType').enable(); this.loanProductTermsForm.get('overAppliedNumber').enable(); - if (this.loanProductService.isLoanProduct) { - this.loanProductTermsForm.addControl('disallowExpectedDisbursements', new UntypedFormControl(true)); - } + this.loanProductTermsForm.addControl('disallowExpectedDisbursements', new UntypedFormControl(true)); } else { this.loanProductTermsForm.get('overAppliedCalculationType').disable(); this.loanProductTermsForm.get('overAppliedCalculationType').patchValue(null); this.loanProductTermsForm.get('overAppliedNumber').disable(); this.loanProductTermsForm.get('overAppliedNumber').patchValue(null); - if (this.loanProductService.isLoanProduct) { - this.loanProductTermsForm.removeControl('disallowExpectedDisbursements'); - } + this.loanProductTermsForm.removeControl('disallowExpectedDisbursements'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 368 - 382, Remove the redundant inner guards that check this.loanProductService.isLoanProduct inside the setConditionalControls block: when allowApprovedDisbursedAmountsOverApplied is true/false, directly add/remove the 'disallowExpectedDisbursements' control on loanProductTermsForm (using addControl/removeControl) without re-checking isLoanProduct, since the entire setConditionalControls logic is already executed only when this.loanProductService.isLoanProduct is true; update the branches that currently wrap addControl('disallowExpectedDisbursements', ...) and removeControl('disallowExpectedDisbursements') to perform those operations unconditionally within the outer-gated block.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html (1)
62-99: Merge the two adjacent@ifblocks that share the same condition.Lines 62 and 85 both check
loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied. They can be collapsed into a single@ifblock containing bothmat-form-fieldelements, reducing repetition.♻️ Proposed refactor
- `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) { - <mat-form-field class="flex-fill flex-32"> - <mat-label>{{ 'labels.inputs.Over Amount Calculation Type' | translate }}</mat-label> - ... - </mat-form-field> - } - - `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) { - <mat-form-field class="flex-fill flex-32"> - <mat-label>{{ 'labels.inputs.Over Amount' | translate }}</mat-label> - ... - </mat-form-field> - } + `@if` (loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied) { + <mat-form-field class="flex-fill flex-32"> + <mat-label>{{ 'labels.inputs.Over Amount Calculation Type' | translate }}</mat-label> + ... + </mat-form-field> + + <mat-form-field class="flex-fill flex-32"> + <mat-label>{{ 'labels.inputs.Over Amount' | translate }}</mat-label> + ... + </mat-form-field> + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html` around lines 62 - 99, Combine the two adjacent conditional blocks that both check loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied into a single `@if` wrapper so both mat-form-field elements (the mat-select for overAppliedCalculationType iterating over overAppliedCalculationTypeData and the numeric input bound to formControlName "overAppliedNumber") render under the same condition; locate the existing checks referencing loanProductTermsForm.value.allowApprovedDisbursedAmountsOverApplied and move both mat-form-field nodes inside one shared `@if`, preserving their mat-labels, formControlName values, validation logic (mat-error clauses) and the for-loop over overAppliedCalculationTypeData.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
377-389: RedundantremoveControl('maxOutstandingLoanBalance')call inside theallowAddDisbursementDetails()branch.Line 377 already unconditionally removes the control. The additional
removeControlat line 379 (inside theifbranch) is always a no-op.♻️ Proposed fix
this.loansAccountTermsForm.removeControl('maxOutstandingLoanBalance'); if (this.allowAddDisbursementDetails()) { - this.loansAccountTermsForm.removeControl('maxOutstandingLoanBalance'); this.loansAccountTermsForm.addControl( 'maxOutstandingLoanBalance', new UntypedFormControl(this.loansAccountTermsData?.maxOutstandingLoanBalance ?? null, Validators.required) ); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 377 - 389, The code redundantly calls this.loansAccountTermsForm.removeControl('maxOutstandingLoanBalance') twice; remove the inner duplicate call inside the if block (the one immediately after if (this.allowAddDisbursementDetails())) so the form control is removed once up front and then re-added with either Validators.required or without, leaving references to loansAccountTermsForm, allowAddDisbursementDetails, loansAccountTermsData and the 'maxOutstandingLoanBalance' control intact.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
src/app/core/shell/toolbar/toolbar.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/organization/currencies/manage-currencies/manage-currencies.component.htmlsrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.htmlsrc/app/products/loan-products/edit-loan-product/edit-loan-product.component.tssrc/app/products/loan-products/edit-loan-product/loan-product-and-template.resolver.tssrc/app/products/loan-products/loan-product-stepper/loan-product-charges-step/loan-product-charges-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-details-step/loan-product-details-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/advance-payment-allocation-tab/advance-payment-allocation-tab.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/advance-payment-allocation-tab/advance-payment-allocation-tab.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/loan-product-payment-strategy-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/payment-allocation-model.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-product.resolver.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.component.scsssrc/app/products/loan-products/loan-products.component.tssrc/app/products/loan-products/loan-products.resolver.tssrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/services/loan-product.service.tssrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.htmlsrc/app/products/loan-products/view-loan-product/general-tab/general-tab.component.tssrc/app/products/loan-products/view-loan-product/shared/view-advance-paymeny-allocation/view-advance-paymeny-allocation.component.htmlsrc/app/products/loan-products/view-loan-product/shared/view-advance-paymeny-allocation/view-advance-paymeny-allocation.component.tssrc/app/products/loan-products/view-loan-product/view-loan-product.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-range/create-range/create-range.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/edit-range/edit-range.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.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 with no reviewable changes (1)
- src/app/core/shell/toolbar/toolbar.component.html
🚧 Files skipped from review as they are similar to previous changes (10)
- src/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/advance-payment-allocation-tab/advance-payment-allocation-tab.component.html
- src/app/shared/icons.module.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/loan-product-payment-strategy-step.component.ts
- src/assets/translations/de-DE.json
- src/app/products/loan-products/common/loan-product-base.component.ts
- src/app/products/loan-products/models/loan-product.model.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-payment-strategy-step/payment-allocation-model.ts
- src/app/products/loan-products/view-loan-product/shared/view-advance-paymeny-allocation/view-advance-paymeny-allocation.component.ts
- src/assets/translations/es-MX.json
- src/assets/translations/cs-CS.json
Comment on lines
+33
to
+40
| @if ( | ||
| loanProductSettingsForm.get('npvDayCount')?.invalid && | ||
| (loanProductSettingsForm.get('npvDayCount')?.touched || loanProductSettingsForm.get('npvDayCount')?.dirty) | ||
| ) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.NPV day count' | translate }} {{ 'labels.commons.is' | translate }} | ||
| <strong>{{ 'labels.commons.required' | translate }}</strong> | ||
| </mat-error> |
There was a problem hiding this comment.
Show a min-value error for npvDayCount instead of always showing “required”.
When users enter 0 or negative values, the control fails min but currently displays the required message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html`
around lines 33 - 40, The npvDayCount control is showing a "required" message
even when it fails the min validator; update the template logic for
loanProductSettingsForm.get('npvDayCount') to inspect control.errors (e.g.,
errors?.min) and render a specific min-value mat-error when errors.min exists
(using errors.min.min or errors.min.actual as appropriate) and only fall back to
the required message when errors.required is present; keep the touched/dirty
checks and use the same loanProductSettingsForm and npvDayCount identifiers so
the change is limited to the mat-error rendering logic.
Comment on lines
+189
to
+192
| this.processingStrategyService.advancedTransactionProcessingStrategy.subscribe((value: boolean) => { | ||
| this.isAdvancedTransactionProcessingStrategy = value; | ||
| }); | ||
| this.validateAdvancedPaymentStrategyControls(); |
There was a problem hiding this comment.
Unmanaged subscription — potential memory leak.
The advancedTransactionProcessingStrategy subscription at line 189 is never unsubscribed. Use takeUntilDestroyed (Angular 16+) or store and unsubscribe in ngOnDestroy.
♻️ Proposed fix using `takeUntilDestroyed`
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
+import { DestroyRef, inject } from '@angular/core';
export class LoanProductTermsStepComponent extends LoanProductBaseComponent implements OnInit, OnChanges {
+ private destroyRef = inject(DestroyRef);
...
this.processingStrategyService.advancedTransactionProcessingStrategy
+ .pipe(takeUntilDestroyed(this.destroyRef))
.subscribe((value: boolean) => {
this.isAdvancedTransactionProcessingStrategy = value;
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts`
around lines 189 - 192, The subscription to
processingStrategyService.advancedTransactionProcessingStrategy in the
loan-product-terms-step.component is unmanaged and can leak; modify the
component to unsubscribe automatically by piping the observable through
takeUntilDestroyed() (Angular 16+) or by capturing the Subscription and
unsubscribing it in ngOnDestroy; ensure the change affects the existing
subscribe callback that sets isAdvancedTransactionProcessingStrategy and still
calls validateAdvancedPaymentStrategyControls so behavior remains identical.
Comment on lines
+323
to
+341
| minPeriodPaymentRate: [ | ||
| '', | ||
| [ | ||
| Validators.min(1) | ||
| ] | ||
| ], | ||
| periodPaymentRate: [ | ||
| '', | ||
| [ | ||
| Validators.required, | ||
| Validators.min(1) | ||
| ] | ||
| ], | ||
| maxPeriodPaymentRate: [ | ||
| '', | ||
| [ | ||
| Validators.min(1) | ||
| ] | ||
| ], |
There was a problem hiding this comment.
Validators.min(1) contradicts [min]="0.01" in the template for minPeriodPaymentRate and periodPaymentRate.
The form builder sets Validators.min(1) for both minPeriodPaymentRate and periodPaymentRate, but the HTML template renders these fields with [min]="0.01". Any rate between 0.01 and 1 (exclusive) will pass the UI hint but fail the reactive-form validator, silently blocking submission with no visible error matching what the user sees.
maxPeriodPaymentRate consistently uses Validators.min(1) and [min]="1", so the fix should align min/max period payment rate fields to a uniform floor (either all 0.01 or all 1).
🐛 Proposed fix (align to `0.01` matching the HTML)
minPeriodPaymentRate: [
'',
[
- Validators.min(1)
+ Validators.min(0.01)
]
],
periodPaymentRate: [
'',
[
Validators.required,
- Validators.min(1)
+ Validators.min(0.01)
]
],
maxPeriodPaymentRate: [
'',
[
- Validators.min(1)
+ Validators.min(0.01)
]
],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts`
around lines 323 - 341, The reactive form validators for minPeriodPaymentRate,
periodPaymentRate and maxPeriodPaymentRate in the LoanProductTermsStepComponent
conflict with the template min of 0.01; update the FormBuilder control
definitions (symbols: minPeriodPaymentRate, periodPaymentRate,
maxPeriodPaymentRate) to use Validators.min(0.01) (or otherwise make the
template and validators agree) so values between 0.01 and 1 are accepted; ensure
the change is applied where the form is created (the FormBuilder group in
loan-product-terms-step.component.ts) and run unit/manual checks to confirm
validation and template min attribute are consistent.
Comment on lines
32
to
35
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| const productId = route.parent.paramMap.get('productId'); | ||
| return this.productsService.getLoanProduct(productId); | ||
| return this.productsService.getLoanProduct(this.loanProductService.loanProductPath, productId); | ||
| } |
There was a problem hiding this comment.
Initialize product context from route before using loanProductPath.
Line 34 currently relies on pre-existing singleton state. On direct URL loads or refreshes, this can call the wrong product endpoint.
Suggested fix
+import { LOAN_PRODUCT_TYPE } from './models/loan-product.model';
@@
resolve(route: ActivatedRouteSnapshot): Observable<any> {
const productId = route.parent.paramMap.get('productId');
+ const productType = route.queryParams['productType'] || LOAN_PRODUCT_TYPE.LOAN;
+ this.loanProductService.initialize(productType);
return this.productsService.getLoanProduct(this.loanProductService.loanProductPath, productId);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/loan-products/loan-product.resolver.ts` around lines 32 -
35, The resolver uses this.loanProductService.loanProductPath before ensuring
the service was initialized from the route, causing wrong endpoints on direct
loads; modify resolve(route: ActivatedRouteSnapshot) to extract productId and
first initialize the loanProductService with that productId (e.g., call an
initialization method like loanProductService.initialize(productId) or
loanProductService.setActiveProduct(productId)) so the loanProductPath is
derived from the correct product, then call
productsService.getLoanProduct(this.loanProductService.loanProductPath,
productId).
Comment on lines
+76
to
+87
| const loanProduct = this.loanProductService.isLoanProduct | ||
| ? { | ||
| ...loanProductData, | ||
| charges: loanProductData.charges.map((charge: any) => ({ id: charge.id })), | ||
| dateFormat, | ||
| locale | ||
| } | ||
| : { | ||
| ...loanProductData, | ||
| dateFormat, | ||
| locale | ||
| }; |
There was a problem hiding this comment.
Non-loan payload still carries charges through the spread.
...loanProductData in the non-loan branch includes charges when present, so working-capital payloads can still send the raw charges shape unintentionally.
🔧 Proposed fix
- const loanProduct = this.loanProductService.isLoanProduct
- ? {
- ...loanProductData,
- charges: loanProductData.charges.map((charge: any) => ({ id: charge.id })),
- dateFormat,
- locale
- }
- : {
- ...loanProductData,
- dateFormat,
- locale
- };
+ const { charges, ...loanProductDataWithoutCharges } = loanProductData;
+ const loanProduct = this.loanProductService.isLoanProduct
+ ? {
+ ...loanProductDataWithoutCharges,
+ charges: (charges ?? []).map((charge: { id: number }) => ({ id: charge.id })),
+ dateFormat,
+ locale
+ }
+ : {
+ ...loanProductDataWithoutCharges,
+ dateFormat,
+ locale
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/loan-products/loan-products.ts` around lines 76 - 87, The
non-loan branch of the loanProduct creation still spreads ...loanProductData
which can unintentionally include charges; update the non-loan branch (the
branch where this.loanProductService.isLoanProduct is false) to explicitly omit
charges from the payload — e.g., destructure loanProductData to remove charges
(const { charges, ...rest } = loanProductData) and spread rest into the returned
object, then add dateFormat and locale; keep the loan branch unchanged where
charges are mapped.
src/app/products/products.service.ts
Outdated
Comment on lines
+32
to
+50
| getLoanProducts(productType: string): Observable<any> { | ||
| return this.http.get(`/${productType}loanproducts`); | ||
| } | ||
|
|
||
| createLoanProduct(loanProduct: string): Observable<any> { | ||
| return this.http.post('/loanproducts', loanProduct); | ||
| createLoanProduct(productType: string, loanProduct: any): Observable<any> { | ||
| return this.http.post(`/${productType}loanproducts`, loanProduct); | ||
| } | ||
|
|
||
| getLoanProductsTemplate(): Observable<any> { | ||
| return this.http.get('/loanproducts/template'); | ||
| getLoanProductsTemplate(productType: string): Observable<any> { | ||
| return this.http.get(`/${productType}loanproducts/template`); | ||
| } | ||
|
|
||
| getLoanProduct(loanProductId: string, template: boolean = false): Observable<any> { | ||
| getLoanProduct(productType: string, loanProductId: string, template: boolean = false): Observable<any> { | ||
| const httpParams = new HttpParams().set('template', template.toString()); | ||
| return this.http.get(`/loanproducts/${loanProductId}`, { params: httpParams }); | ||
| return this.http.get(`/${productType}loanproducts/${loanProductId}`, { params: httpParams }); | ||
| } | ||
|
|
||
| updateLoanProduct(loanProductId: string, loanProduct: any): Observable<any> { | ||
| return this.http.put(`/loanproducts/${loanProductId}`, loanProduct); | ||
| updateLoanProduct(productType: string, loanProductId: string, loanProduct: any): Observable<any> { | ||
| return this.http.put(`/${productType}loanproducts/${loanProductId}`, loanProduct); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for loan-product type/constant definitions
fd 'loan-product|product.*model' src/app --type f | head -20Repository: openMF/web-app
Length of output: 1862
🏁 Script executed:
# Look for LOAN_PRODUCT_TYPE, PRODUCT_TYPES, or similar constants
rg 'LOAN_PRODUCT_TYPE|PRODUCT_TYPES|enum.*[Pp]roduct|export.*const.*[Pp]roduct' src/app -t ts --max-count 30Repository: openMF/web-app
Length of output: 2487
🏁 Script executed:
# Find actual call sites of the loan-product methods
rg 'getLoanProducts\(|createLoanProduct\(|updateLoanProduct\(|getLoanProductsTemplate\(|getLoanProduct\(' src/app -g '!**/*.spec.ts' -B2 -A2Repository: openMF/web-app
Length of output: 9651
🏁 Script executed:
# Check the actual service file to understand context
cat -n src/app/products/products.service.ts | head -60Repository: openMF/web-app
Length of output: 2381
🏁 Script executed:
cat -n src/app/products/loan-products/services/loan-product.service.ts | head -50Repository: openMF/web-app
Length of output: 1903
🏁 Script executed:
# Check what loanProductPath returns/resolves to
rg 'loanProductPath' src/app/products/loan-products -B2 -A2Repository: openMF/web-app
Length of output: 2274
Constrain productType parameter to the LoanProductType domain type.
Lines 32–50 accept productType: string, allowing any string at compile time. The codebase already exports LoanProductType from the loan-product model; use this type to constrain all loan-product methods to only accept valid product types ('', 'workingcapital').
♻️ Suggested refactor
+ import { LoanProductType } from './loan-products/models/loan-product.model';
+
- getLoanProducts(productType: string): Observable<any> {
+ getLoanProducts(productType: LoanProductType): Observable<any> {
return this.http.get(`/${productType}loanproducts`);
}
- createLoanProduct(productType: string, loanProduct: any): Observable<any> {
+ createLoanProduct(productType: LoanProductType, loanProduct: any): Observable<any> {
return this.http.post(`/${productType}loanproducts`, loanProduct);
}
- getLoanProductsTemplate(productType: string): Observable<any> {
+ getLoanProductsTemplate(productType: LoanProductType): Observable<any> {
return this.http.get(`/${productType}loanproducts/template`);
}
- getLoanProduct(productType: string, loanProductId: string, template: boolean = false): Observable<any> {
+ getLoanProduct(productType: LoanProductType, loanProductId: string, template: boolean = false): Observable<any> {
const httpParams = new HttpParams().set('template', template.toString());
return this.http.get(`/${productType}loanproducts/${loanProductId}`, { params: httpParams });
}
- updateLoanProduct(productType: string, loanProductId: string, loanProduct: any): Observable<any> {
+ updateLoanProduct(productType: LoanProductType, loanProductId: string, loanProduct: any): Observable<any> {
return this.http.put(`/${productType}loanproducts/${loanProductId}`, loanProduct);
}Also, update loanProductPath in loan-product.service.ts line 46 to return LoanProductType instead of string.
Per coding guidelines (src/app/**): strict type safety is required.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/products.service.ts` around lines 32 - 50, Replace the loose
string productType parameter with the domain type LoanProductType: import
LoanProductType from the loan-product model and change signatures of
getLoanProducts, createLoanProduct, getLoanProductsTemplate, getLoanProduct, and
updateLoanProduct to accept productType: LoanProductType (and update any
internal uses accordingly). Also update loanProductPath in
loan-product.service.ts to return LoanProductType instead of string and adjust
callers to match the new type. Ensure compilation by updating any call sites
that currently pass arbitrary strings.
src/assets/translations/en-US.json
Outdated
Comment on lines
+2823
to
+2830
| "Legal Entity": "Legal Entity", | ||
| "Flat Percentage Amount": "Flat Percentage Amount", | ||
| "Delinquency Bucket Classification": "Delinquency Bucket Classification", | ||
| "Discount Default": "Discount Default", | ||
| "Period Payment Frequency": "Period Payment Frequency", | ||
| "Period Payment Frequency Type": "Period Payment Frequency Type", | ||
| "Payment Allocation Transactions": "Payment Allocation Transactions", | ||
| "Net Present Value day count": "Net Present Value day count" |
There was a problem hiding this comment.
Normalize casing for the NPV day-count label.
The value at Line 2830 is mixed-case while neighboring labels are title-case. This will look inconsistent in forms.
📝 Suggested fix
- "Net Present Value day count": "Net Present Value day count"
+ "Net Present Value day count": "Net Present Value Day 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
| "Legal Entity": "Legal Entity", | |
| "Flat Percentage Amount": "Flat Percentage Amount", | |
| "Delinquency Bucket Classification": "Delinquency Bucket Classification", | |
| "Discount Default": "Discount Default", | |
| "Period Payment Frequency": "Period Payment Frequency", | |
| "Period Payment Frequency Type": "Period Payment Frequency Type", | |
| "Payment Allocation Transactions": "Payment Allocation Transactions", | |
| "Net Present Value day count": "Net Present Value day count" | |
| "Legal Entity": "Legal Entity", | |
| "Flat Percentage Amount": "Flat Percentage Amount", | |
| "Delinquency Bucket Classification": "Delinquency Bucket Classification", | |
| "Discount Default": "Discount Default", | |
| "Period Payment Frequency": "Period Payment Frequency", | |
| "Period Payment Frequency Type": "Period Payment Frequency Type", | |
| "Payment Allocation Transactions": "Payment Allocation Transactions", | |
| "Net Present Value day count": "Net Present Value Day Count" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/en-US.json` around lines 2823 - 2830, The JSON
translation entry "Net Present Value day count" uses mixed-case; update its
value to title-case to match neighboring labels—change the value for the "Net
Present Value day count" key to "Net Present Value Day Count" so all labels use
consistent Title Case (e.g., adjust the string for the "Net Present Value day
count" key in src assets translations).
src/assets/translations/fr-FR.json
Outdated
| "Period Payment Frequency": "Fréquence de paiement périodique", | ||
| "Period Payment Frequency Type": "Type de fréquence de paiement périodique", | ||
| "Payment Allocation Transactions": "Transactions d'affectation des paiements", | ||
| "Net Present Value day count": "Valeur actuelle nette nombre de jours" |
There was a problem hiding this comment.
Fix awkward French phrasing for NPV day-count label.
Line 2799 ("Valeur actuelle nette nombre de jours") is grammatically unnatural and may confuse users. Consider a clearer translation.
💬 Suggested update
- "Net Present Value day count": "Valeur actuelle nette nombre de jours"
+ "Net Present Value day count": "Nombre de jours de la valeur actuelle nette"📝 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
| "Net Present Value day count": "Valeur actuelle nette nombre de jours" | |
| "Net Present Value day count": "Nombre de jours de la valeur actuelle nette" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/fr-FR.json` at line 2799, The French value for the
key "Net Present Value day count" is awkward; update the translation for the
JSON key "Net Present Value day count" to a clearer phrase such as "Nombre de
jours pour la valeur actuelle nette" (or "Base de calcul des jours — VAN") by
replacing the current value "Valeur actuelle nette nombre de jours" with the
chosen improved string so the label reads naturally in French.
src/assets/translations/ko-KO.json
Outdated
| "Enter last name": "성을 입력하세요", | ||
| "Sender name": "송금인 이름", | ||
| "Retry": "재시도" | ||
| "Retry": "재시도", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the structure around line 608 and 2790 in the ko-KO.json file
wc -l src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 97
🏁 Script executed:
# Check the content around line 608 to verify the buttons.Retry entry
sed -n '600,620p' src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 676
🏁 Script executed:
# Check the content around line 2790 to verify the inputs.Retry entry
sed -n '2780,2800p' src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 880
🏁 Script executed:
# Search for all occurrences of "Retry" in the translation file to see if there are more duplicates
rg -n '"Retry"' src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 109
🏁 Script executed:
# Search for how the translation keys are consumed - look for references to labels.buttons.Retry and labels.inputs.Retry
rg -n 'labels\.buttons\.Retry|labels\.inputs\.Retry|"Retry"' --type ts --type tsx --type js --type jsx -A1 -B1 | head -50Repository: openMF/web-app
Length of output: 85
🏁 Script executed:
# Search more broadly for how input retry is referenced in the codebase
rg -n 'Retry' --type ts --type tsx --type js --type jsx | grep -i 'input\|labels' | head -30Repository: openMF/web-app
Length of output: 85
🏁 Script executed:
# Get more context to see the section hierarchy around line 608
sed -n '580,620p' src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 1383
🏁 Script executed:
# Get more context to see the section hierarchy around line 2790
sed -n '2770,2810p' src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 1528
🏁 Script executed:
# Search for how "Retry" is used in component code (typescript/javascript files)
rg -n 'Retry' --type ts --type js -B2 -A2 | head -80Repository: openMF/web-app
Length of output: 276
🏁 Script executed:
# Search for translation key patterns to understand how buttons vs inputs sections are consumed
rg -n 'labels\.(buttons|inputs)\.' --type ts --type js | head -40Repository: openMF/web-app
Length of output: 6677
🏁 Script executed:
# Get wider context to see section hierarchy around line 2790
sed -n '2700,2720p' src/assets/translations/ko-KO.jsonRepository: openMF/web-app
Length of output: 626
🏁 Script executed:
# Check the broader structure to find section name for line 2790
sed -n '2750,2795p' src/assets/translations/ko-KO.json | cat -nRepository: openMF/web-app
Length of output: 2234
🏁 Script executed:
# Search for all occurrences of "Retry" in the entire codebase including translation usage
rg -n '"Retry"' --type ts --type jsRepository: openMF/web-app
Length of output: 40
🏁 Script executed:
# Look at the JSON structure more carefully - find parent section for both Retry entries
python3 << 'EOF'
import json
with open('src/assets/translations/ko-KO.json', 'r', encoding='utf-8') as f:
data = json.load(f)
# Check the structure of labels
if 'labels' in data:
labels = data['labels']
# Check if buttons and inputs sections exist and if they contain Retry
if 'buttons' in labels:
if 'Retry' in labels['buttons']:
print(f"✓ labels.buttons.Retry: {labels['buttons']['Retry']}")
else:
print("✗ Retry NOT found in labels.buttons")
if 'inputs' in labels:
if 'Retry' in labels['inputs']:
print(f"✓ labels.inputs.Retry: {labels['inputs']['Retry']}")
else:
print("✗ Retry NOT found in labels.inputs")
if 'catalogs' in labels:
if 'Retry' in labels['catalogs']:
print(f"✓ labels.catalogs.Retry: {labels['catalogs']['Retry']}")
else:
print("✗ Retry NOT found in labels.catalogs")
# Print all sections that contain Retry
print("\nAll sections containing 'Retry':")
for section, content in labels.items():
if isinstance(content, dict) and 'Retry' in content:
print(f" - labels.{section}.Retry: {content['Retry']}")
EOFRepository: openMF/web-app
Length of output: 242
🏁 Script executed:
# Search for any dynamic translation key patterns that might use Retry
rg -n 'labels\.(buttons|inputs)' --type ts --type js | grep -i retryRepository: openMF/web-app
Length of output: 40
🏁 Script executed:
# Search for any pattern where "Retry" might be dynamically resolved
rg -n "buttons\['Retry'\]|inputs\['Retry'\]|buttons\.Retry|inputs\.Retry" --type ts --type jsRepository: openMF/web-app
Length of output: 40
🏁 Script executed:
# Check if there are any error handling or retry components that might use these keys
rg -n -i "retry" --type ts --type js -A3 -B3 | head -60Repository: openMF/web-app
Length of output: 806
🏁 Script executed:
# Look for any components related to form inputs or buttons that might use Retry contextually
rg -n "Retry" src/ --type ts --type js | grep -v node_modules | grep -v ".spec" | head -30Repository: openMF/web-app
Length of output: 40
Remove or consolidate redundant "Retry" translation entries.
Both labels.buttons.Retry (line 608) and labels.inputs.Retry (line 2790) exist with identical values ("재시도"), yet neither is used anywhere in the codebase. This duplication creates unnecessary maintenance burden. Either remove the unused entry entirely or consolidate both sections to use a single shared key if a Retry action is needed in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/ko-KO.json` at line 2790, Remove the duplicate
translation key by consolidating or deleting one of the unused entries: either
remove labels.inputs.Retry (or labels.buttons.Retry) from the ko-KO JSON and
keep a single shared key for "Retry", or consolidate both into one canonical key
(e.g., labels.buttons.Retry) and update any usages to reference that single key;
ensure you run the i18n key search after the change to confirm neither
labels.buttons.Retry nor labels.inputs.Retry is referenced elsewhere and update
references if you choose to rename the canonical key.
src/assets/translations/ko-KO.json
Outdated
| "Discount Default": "할인 기본값", | ||
| "Period Payment Frequency": "기간별 지급 빈도", | ||
| "Period Payment Frequency Type": "기간별 지급 빈도 유형", | ||
| "Payment Allocation Transactions": "지불 배분 거래", |
There was a problem hiding this comment.
Inconsistent Korean term for "allocation" between related keys.
"Payment Allocation Transactions": "지불 배분 거래" uses 배분 for "allocation", while the existing "Advanced Payment Allocation Transactions": "선지급 할당 거래" (line 1453) uses 할당. Both terms are valid Korean, but mixing them for the same English concept ("allocation") within the same domain reduces translation consistency.
💬 Suggested fix to align terminology
- "Payment Allocation Transactions": "지불 배분 거래",
+ "Payment Allocation Transactions": "지불 할당 거래",📝 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
| "Payment Allocation Transactions": "지불 배분 거래", | |
| "Payment Allocation Transactions": "지불 할당 거래", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/ko-KO.json` at line 2796, The translation for the key
"Payment Allocation Transactions" currently uses "지불 배분 거래" which is
inconsistent with "Advanced Payment Allocation Transactions" ("선지급 할당 거래");
update the value for "Payment Allocation Transactions" to use the same Korean
term for allocation (e.g., "지불 할당 거래") and scan other related keys (e.g.,
"Advanced Payment Allocation Transactions") to ensure the term "할당" is used
consistently across all allocation-related keys.
b299c9f to
38f4e2f
Compare
alberto-art3ch
force-pushed
the
WEB-711/wc-product-configuration
branch
from
38f4e2f to
2b95068
Compare
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A new and dedicated product configuration will be introduced, providing users with the flexibility to configure Working Capital loan products without impacting existing loan products
Related issues and discussion
WEB-711
Screenshots
Screen.Recording.2026-02-23.at.11.16.41.AM.mov
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
Improvements
Bug Fixes
Usability