WEB-813: Working Capital loan account creation#3257
Open
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Open
WEB-813: Working Capital loan account creation#3257alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
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 |
|---|---|
Product base & models src/app/products/loan-products/common/loan-product-base.component.ts, src/app/products/loan-products/services/loan-product.service.ts, src/app/loans/models/loan-product.model.ts |
Add product-type helpers, productTypeLabel and loanAccountPath, and new LoanProductBasicDetails model. |
Resolvers & routing src/app/loans/common-resolvers/loan-products.resolver.ts, src/app/loans/common-resolvers/loan-details.resolver.ts, src/app/loans/loans-routing.module.ts |
Add LoanProductsResolver, read productType from query params in loan-details resolver, initialize product context, and wire resolver into create/modify/edit routes. |
Products service src/app/products/products.service.ts |
Add getLoanProductsBasicDetails() endpoint used by resolver and product selection. |
Loans service (working-capital) src/app/loans/loans.service.ts |
Add working-capital endpoints/templates and change createLoansAccount to accept productType and post to product-specific path (loans vs working-capital-loans). |
Create/edit account & stepper src/app/loans/create-loans-account/*, src/app/loans/edit-loans-account/*, src/app/loans/loans-account-stepper/.../* |
Make creation/edit flows product-aware: conditional steps, new inputs/outputs for loanProductsBasicDetails, product-type initialization, separate term models and submit flows for loan vs working-capital, and related template/control changes. |
Details & preview steps src/app/loans/loans-account-stepper/loans-account-details-step/*, .../loans-account-preview-step/* |
Extend base component, accept loanProductsBasicDetails, add product-driven form controls/outputs, product option rendering, and helper methods. |
Loans view & tabs src/app/loans/loans-view/loans-view.component.ts/html, src/app/loans/loans-view/* |
Make LoansView and child views extend product base, gate tabs/sections by product type/feature flags, include productType queryParams on tab links, and adjust data gating/display. |
Client general tab src/app/clients/clients-view/general-tab/general-tab.component.ts/html |
Add Product Type column, preserve productType in row navigations via queryParams, aggregate working-capital and loan accounts, and conditionally render type icon only for loan product; removed print action. |
Loan account actions base src/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.ts |
Add new abstract base providing injected route/router/services, loanId, isLoanProduct/isWorkingCapital getters, and navigation helpers that append productType query param. |
Refactor: loan action components src/app/loans/loans-view/loan-account-actions/**/**.component.ts, src/app/loans/loans-view/loan-account-actions/**/*.html |
~30+ loan-action components converted to extend LoanAccountActionsBaseComponent: removed local route/service injections and many @Input fields; templates changed to call gotoLoanDefaultView/gotoLoanView; service calls use base-provided loanService. |
UI, utilities & small fixes src/app/core/shell/breadcrumb/breadcrumb.component.ts, src/app/shared/footer/footer.component.ts, src/app/shared/long-text/* |
Minor guards and bugfixes: printableValue falsy guard, clear footer polling timer when not relevant, consolidate long-text rendering and adjust expansion threshold. |
Translations src/assets/translations/*.{json} (many locales) |
Add translations for "Loan Amounts", "Product Type", "Total Payment", "Discount", "No Loan Product was found" across locales and minor Swahili text fix. |
Styling src/app/loans/loans-account-stepper/loans-account-details-step/*.scss, src/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scss |
Add product-option styles for product selector; remove some dashboard header gradient/white color rules. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant CreateComponent as CreateLoansAccount<br/>Component
participant LoanProductsResolver as LoanProducts<br/>Resolver
participant ProductsService as Products<br/>Service
participant LoanProductService as LoanProduct<br/>Service
participant LoansService as Loans<br/>Service
participant Backend as Backend API
User->>CreateComponent: Navigate to create loan
CreateComponent->>LoanProductsResolver: route resolve
LoanProductsResolver->>ProductsService: getLoanProductsBasicDetails()
ProductsService->>Backend: GET /loanproducts/basic-details
Backend-->>ProductsService: products
ProductsService-->>LoanProductsResolver: products
LoanProductsResolver-->>CreateComponent: resolved products
User->>CreateComponent: Select product
CreateComponent->>LoanProductService: initialize(productType)
CreateComponent->>LoansService: get template (path depends on productType)
LoansService->>Backend: GET /{loans|working-capital-loans}/template
Backend-->>LoansService: template
User->>CreateComponent: Submit
CreateComponent->>LoansService: createLoansAccount(productType, payload)
LoansService->>Backend: POST /{productType}
Backend-->>LoansService: success
CreateComponent->>CreateComponent: navigate with ?productType
sequenceDiagram
participant User
participant ActionComponent as Loan Action<br/>Component
participant Base as LoanAccountActions<br/>BaseComponent
participant Route as ActivatedRoute
participant LoanProductService as LoanProduct<br/>Service
participant Router as Router
User->>ActionComponent: Open action
ActionComponent->>Base: constructor() -> super()
Base->>Route: read params (loanId)
Base->>LoanProductService: initialize / get productType
User->>ActionComponent: Submit action
ActionComponent->>Base: perform action (calls loanService)
ActionComponent->>Base: gotoLoanView(tab)
Base->>Router: navigate relative, include productType query param
Router-->>User: navigated to tab
Estimated code review effort
🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
- WEB-711: Working Capital product #3159: Large overlap implementing working-capital vs loan split and LoanProductService/base integration.
- WEB-654 fix: show only product-specific datatables in loan account view #3105: Related datatables filtering and LoansView product-aware datatables changes.
- WEB-795 Fix the Loan Information section #3236: Overlapping changes to loans header/metadata rendering in LoansView components.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-account-creation
branch
2 times, most recently
from
8b3a63e to
2778bee
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: 11
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 (7)
src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
31-33:⚠️ Potential issue | 🔴 CriticalRemove the
loanIdredeclaration to avoid shadowing the base class property.Line 32 redeclares
loanId: string;without initialization, which shadows the base class's properly-initializedprotected loanId. This causesthis.loanIdat line 122 to resolve to the child's undefined property instead of the inherited initialized value, passingundefinedto the API call. Delete the local declaration and use the inherited property fromLoanAccountActionsBaseComponent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` around lines 31 - 33, Remove the local redeclaration of loanId in AdjustLoanChargeComponent so it no longer shadows the protected loanId from LoanAccountActionsBaseComponent; specifically delete the line "loanId: string;" in adjust-loan-charge.component.ts, keep chargeId, and ensure all references in methods (e.g., where this.loanId is used to build the API call in the component) now use the inherited protected loanId so undefined is not passed to the API.src/app/loans/loans-view/loan-account-actions/loan-account-actions.component.ts (1)
191-207:⚠️ Potential issue | 🟠 MajorMove
actionButtonDataassignment into the params subscription to stay in sync with route changes.When the
:actionroute parameter changes,this.actionNameupdates inside the params subscription (line 196), butactionButtonData.actionNameis assigned only once at line 206 (outside subscriptions). Child components relying on[dataObject]="actionButtonData"receive stale metadata if the route reuses this component for a different action.♻️ Suggested fix
this.route.data.subscribe((data: { actionButtonData: any }) => { this.actionButtonData = data.actionButtonData ? data.actionButtonData : {}; }); this.route.params.subscribe((params) => { this.actionName = params['action']; if (this.actionName === 'Change Loan Officer') { this.actionName = 'Assign Loan Officer'; } + this.actionButtonData['actionName'] = this.actionName; + this.actionButtonData['productType'] = LoanProductBaseComponent.resolveProductTypeDefault(this.route, 'loan'); for (const key of Object.keys(this.actions)) { this.actions[key as keyof typeof this.actions] = false; } this.actions[this.actionName as keyof typeof this.actions] = true; }); - - this.actionButtonData['actionName'] = this.actionName; - this.actionButtonData['productType'] = LoanProductBaseComponent.resolveProductTypeDefault(this.route, 'loan');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/loan-account-actions.component.ts` around lines 191 - 207, The assignment to actionButtonData['actionName'] and actionButtonData['productType'] is done outside the route.params subscription so it becomes stale when the :action param changes; move both assignments into the this.route.params.subscribe callback (after this.actionName is normalized and this.actions are updated) so that actionButtonData.actionName and actionButtonData.productType (use LoanProductBaseComponent.resolveProductTypeDefault(this.route, 'loan')) are updated whenever params change, ensuring child components bound to [dataObject]="actionButtonData" receive the updated metadata.src/app/clients/clients-view/general-tab/general-tab.component.html (2)
307-337:⚠️ Potential issue | 🟠 MajorGuard the closed-loan Type cell for non-
loanproducts.The open table already fixes this on Line 189, but the closed table still reads
element.loanType.valueunconditionally. A closed working-capital account will hit this branch with noloanTypeand break the row render.🛡️ Suggested fix
<td mat-cell class="center" *matCellDef="let element"> - <i - class="fa fa-large" - [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" - matTooltip=" {{ element.loanType.value }}" - matTooltipPosition="above" - ></i> + `@if` (element.productType === 'loan' && element.loanType) { + <i + class="fa fa-large" + [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" + matTooltip="{{ element.loanType.value }}" + matTooltipPosition="above" + ></i> + } </td>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.html` around lines 307 - 337, The closed-table Type cell is accessing element.loanType.value without guarding for non-loan products; update the template in general-tab.component.html to guard that access (e.g., use the safe navigation operator element.loanType?.value or a conditional like element.loanType ? element.loanType.value : '') and apply the same guard in the [ngClass] expression so the icon lookup uses element.loanType?.value (or falls back) to avoid runtime errors for working-capital/non-loan products.
203-241:⚠️ Potential issue | 🟠 MajorCarry
productTypethrough the action buttons too.Line 278 now preserves
productTypewhen the row itself is clicked, but the loan action buttons on Lines 210, 225, and 241 still navigate without it. Working-capital accounts will lose the new product context as soon as the user chooses an action from this table.🧭 Suggested fix
[routerLink]="['../', 'loans-accounts', element.id, 'actions', 'Make Repayment']" + [queryParams]="{ productType: element.productType }" @@ [routerLink]="['../', 'loans-accounts', element.id, 'actions', 'Approve']" + [queryParams]="{ productType: element.productType }" @@ [routerLink]="['../', 'loans-accounts', element.id, 'actions', 'Disburse']" + [queryParams]="{ productType: element.productType }"Also applies to: 274-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.html` around lines 203 - 241, The action buttons using [routerLink] (the Make Repayment, Approve, and Disburse buttons) drop the productType; update their [routerLink]s to include the same productType propagation used by the row click handler (routeEdit($event)) so working-capital accounts keep context. Locate the three buttons that call routeEdit($event) and change their [routerLink] arrays to include the productType value (e.g. append element.productType or add the same queryParams { productType: productType } depending on how row click preserves it) so the route segments or query params match the row navigation.src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts (1)
101-112:⚠️ Potential issue | 🟡 MinorPotential null reference when dialog is dismissed.
If the dialog is dismissed (e.g., clicking outside or pressing Escape),
responsemay beundefinedornull, causingresponse.deleteto throw an error.🛡️ Proposed fix with null check
deleteGuarantorDialogRef.afterClosed().subscribe((response: any) => { - if (response.delete) { + if (response?.delete) { this.loanService.deleteGuarantor(this.loanId, id).subscribe(() => { this.reload(); }); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts` around lines 101 - 112, The deleteGuarantor method can dereference a null/undefined response from deleteGuarantorDialogRef.afterClosed(), so update the handler in deleteGuarantor to guard against a dismissed dialog by checking the response before accessing response.delete (e.g., use a truthy check or optional chaining) and only call loanService.deleteGuarantor(this.loanId, id).subscribe(...) and reload() when the response indicates deletion; references: deleteGuarantor, deleteGuarantorDialogRef, DeleteDialogComponent, loanService.deleteGuarantor, reload.src/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.ts (1)
56-64:⚠️ Potential issue | 🟡 MinorIncorrect date fallback logic.
The expression
new Date(this.dataObject.date) || new Date()won't fall back tonew Date()whenthis.dataObject.dateis invalid because an Invalid Date object is truthy. The fallback only triggers whendataObject.dateis falsy.🛡️ Proposed fix for proper date validation
createCloseForm() { + const transactionDate = this.dataObject.date ? new Date(this.dataObject.date) : new Date(); + const validDate = !isNaN(transactionDate.getTime()) ? transactionDate : new Date(); this.closeLoanForm = this.formBuilder.group({ - transactionDate: [ - new Date(this.dataObject.date) || new Date(), - Validators.required - ], + transactionDate: [validDate, Validators.required], note: [] }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.ts` around lines 56 - 64, In createCloseForm(), the date fallback is incorrect because an Invalid Date is truthy; fix by explicitly validating the parsed date: create a Date from this.dataObject.date (e.g., const parsed = new Date(this.dataObject.date)), check validity with isNaN(parsed.getTime()) (or Number.isNaN) and if invalid use new Date(), then set that valid Date as the transactionDate initial value when building this.closeLoanForm so Validators.required still applies.src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.ts (1)
122-143:⚠️ Potential issue | 🟠 MajorUse product id consistently through the selection flow.
ngOnInit()patchesproductIdwithloanProductId, butproductChange()resolves the selection byshortNameand the getter rewrites it back toid. That breaks restoring a preselected product and makes duplicate short names ambiguous.Use `id` end-to-end
- const productShortName = event.value; + const productId = event.value; this.productSelected = this.loanProductsBasicDetails.find( - (p: LoanProductBasicDetails) => p.shortName === productShortName + (p: LoanProductBasicDetails) => p.id === productId );The select options should pass
product.idas the control value as well.Also applies to: 197-205, 212-216
🤖 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-details-step/loans-account-details-step.component.ts` around lines 122 - 143, The component currently patches productId using loansAccountDetailsForm.patchValue with loanProductId but productChange() and the selected product getter use shortName, causing mismatches; update the selection flow to use product.id end-to-end: ensure the select control options bind product.id (not product.shortName), change productChange() to accept and resolve by id (look up product via productList or loanProductsBasicDetails by id), and update any getter that maps control value back to a product (e.g., selectedProduct or similar) to expect an id and return the product by id; also update the places noted around the other blocks (lines ~197-205 and 212-216) to use id consistently.
🟡 Minor comments (20)
src/assets/translations/it-IT.json-1147-1147 (1)
1147-1147:⚠️ Potential issue | 🟡 MinorUse a more natural Italian heading here.
Importi di prestitosounds literal. For a UI section title,Importi del prestitoreads more naturally and matches the surrounding loan terminology better.✏️ Suggested wording
- "Loan Amounts": "Importi di prestito", + "Loan Amounts": "Importi del prestito",🤖 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 1147, Replace the current Italian translation value for the "Loan Amounts" key from "Importi di prestito" to the more natural phrasing "Importi del prestito" so the UI section title reads correctly and matches surrounding loan terminology; update the value for the "Loan Amounts" entry in src/assets/translations/it-IT.json accordingly.src/assets/translations/lv-LV.json-3440-3440 (1)
3440-3440:⚠️ Potential issue | 🟡 MinorUse a product-neutral empty-state here.
This PR adds working-capital product selection alongside regular loan products, so
No Loan Product was foundis too narrow for the new flow and can mislead users when no mixed product matches are returned.💡 Suggested wording
- "No Loan Product was found": "Nav atrasts neviens aizdevuma produkts", + "No Loan Product was found": "Nav atrasts neviens produkts",🤖 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 3440, Replace the loan-specific translation value for the key "No Loan Product was found" in the translations JSON with a product-neutral empty-state message (e.g., "Nav atrasts atbilstošs produkts" or similar) so the string covers both working-capital and regular loan product flows; update the value for the exact key "No Loan Product was found" in src/assets/translations/lv-LV.json accordingly.src/assets/translations/lt-LT.json-3440-3440 (1)
3440-3440:⚠️ Potential issue | 🟡 MinorUse product-neutral empty-state copy here.
This PR introduces Working Capital products into the same creation flow, so showing “Nerasta jokių paskolos produktų” will be misleading when the selector is empty for non-loan products too. Please switch this locale entry to neutral wording, e.g. “Nerasta jokių produktų” or equivalent.
🤖 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 3440, The Lithuanian translation for the key "No Loan Product was found" is loan-specific and should be made product-neutral; update the value for "No Loan Product was found" to a neutral phrase like "Nerasta jokių produktų" (or equivalent neutral wording) so the selector copy works for loans and non-loan Working Capital products as well.src/assets/translations/de-DE.json-1148-1148 (1)
1148-1148:⚠️ Potential issue | 🟡 MinorTighten the financial wording for these new labels.
These translations are understandable, but they read a bit generic for a lending UI. In particular, Line 2622 maps
Total PaymenttoGesamtbetrag, which changes the meaning from a payment to a generic amount. Consider more domain-specific wording such asGesamtzahlung, and verify whetherDarlehensbeträge/Rabattmatch the terminology used elsewhere in the German loan flow.Also applies to: 1786-1786, 2622-2622
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/de-DE.json` at line 1148, The translations use generic wording for lending UI keys; update the entries for "Loan Amounts" and "Total Payment" to more domain-specific German: replace the current value for "Loan Amounts" (key "Loan Amounts") with a context-appropriate term (e.g., "Darlehensbetrag" or "Darlehensbeträge" depending on singular/plural usage in the UI) and change the value for "Total Payment" (key "Total Payment") to "Gesamtzahlung"; also review occurrences of "Rabatt" and ensure they match terminology used elsewhere in the loan flow and adjust to the consistent term if needed.src/assets/translations/fr-FR.json-1789-1789 (1)
1789-1789:⚠️ Potential issue | 🟡 MinorUse the existing
Remiseterminology for discounts.
Rabaisis inconsistent with the rest of this locale file (Discount Defaultalready usesRemise) and reads more retail/promo than financial UI. Reusing the same term avoids mixed labels in the loan flow.Suggested change
- "Discount": "Rabais", + "Discount": "Remise",🤖 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 1789, The translation for the key "Discount" currently uses "Rabais" which is inconsistent with the existing "Discount Default" label; update the value for the "Discount" key to "Remise" to match the rest of the locale and keep consistent financial terminology across keys like "Discount Default" and "Discount".src/assets/translations/fr-FR.json-3442-3442 (1)
3442-3442:⚠️ Potential issue | 🟡 MinorMake the empty-state message product-type neutral.
This PR expands the picker to working capital products as well, so
Aucun produit de prêt...becomes misleading when the user is actually looking at a combined product list.Suggested change
- "No Loan Product was found": "Aucun produit de prêt n'a été trouvé.", + "No Loan Product was found": "Aucun produit n'a été trouvé.",🤖 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 3442, The French translation for the key "No Loan Product was found" is too specific to loans; update the translation value for that key (the string "No Loan Product was found") to a product-type neutral phrase (e.g., "Aucun produit trouvé." or similar neutral French) so the empty-state works for combined loan and working-capital pickers.src/assets/translations/en-US.json-3559-3559 (1)
3559-3559:⚠️ Potential issue | 🟡 MinorUse a product-agnostic empty-state message.
Line 3559 only mentions “Loan Product”, but this flow now exposes both loan and working capital products. That will read incorrectly when no working capital products match.
📝 Suggested copy
- "No Loan Product was found": "No Loan Product was found", + "No Loan Product was found": "No product was found",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/en-US.json` at line 3559, Replace the product-specific empty-state string "No Loan Product was found" with a product-agnostic message (e.g., "No products were found") in the translations JSON: update both the translation key and its value for the entry currently containing "No Loan Product was found" so the UI reads correctly for both loan and working capital flows; ensure any code that references the exact key is updated to the new key if necessary (search for the string "No Loan Product was found" to locate usages).src/assets/translations/pt-PT.json-3440-3440 (1)
3440-3440:⚠️ Potential issue | 🟡 MinorMake the empty-state copy product-type neutral.
This flow now surfaces both standard loan products and working capital products, so
"Nenhum produto de empréstimo foi encontrado."becomes misleading when no working capital product matches. A neutral message like"Nenhum produto foi encontrado."would avoid showing the wrong product type here.💡 Suggested change
- "No Loan Product was found": "Nenhum produto de empréstimo foi encontrado.", + "No Loan Product was found": "Nenhum produto foi encontrado.",🤖 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 3440, Update the translation value for the key "No Loan Product was found" to a product-type neutral Portuguese message; replace "Nenhum produto de empréstimo foi encontrado." with "Nenhum produto foi encontrado." so the empty-state copy correctly covers both loan and working capital products.src/assets/translations/ne-NE.json-3437-3437 (1)
3437-3437:⚠️ Potential issue | 🟡 MinorGeneralize this empty-state message for the new multi-product flow.
Line 3437 still says "Loan Product", but this PR adds working-capital products into the same selector. If the filtered list is empty, this message will be misleading for users who are browsing non-loan product types too. A neutral copy like "No matching product was found" would avoid that mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/ne-NE.json` at line 3437, Update the empty-state copy keyed by "No Loan Product was found" to a neutral message (e.g., change the key to "No matching product was found") and replace the Nepali translation value with the corresponding neutral text so it no longer references only loans; also update any usages that reference the old key to the new key so the app shows the generalized message for all product types.src/assets/translations/es-MX.json-3473-3473 (1)
3473-3473:⚠️ Potential issue | 🟡 MinorMake this empty-state message product-type neutral.
This PR adds working capital products to the same creation flow, but the text still says only
producto de crédito. When the mixed product picker returns no results, that message will be misleading. A neutral translation here avoids showing the wrong product type.📝 Suggested wording
- "No Loan Product was found": "No se encontró ningún producto de crédito", + "No Loan Product was found": "No se encontró ningún producto",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 3473, The translation value for the key "No Loan Product was found" is specific to credit products; update its Spanish value to a product-type neutral phrase (e.g., "No se encontró ningún producto" or "No se encontraron productos") so the mixed product picker shows a correct empty-state message; modify the value associated with the "No Loan Product was found" JSON key in src/assets/translations/es-MX.json accordingly.src/assets/translations/cs-CS.json-3440-3440 (1)
3440-3440:⚠️ Potential issue | 🟡 MinorMake the empty-state copy product-type neutral.
This PR adds working-capital products to the same selection flow, so
Nebyl nalezen žádný úvěrový produktwill be misleading when no working-capital product matches either. A neutral Czech string avoids confusing users in that state.✏️ Proposed copy update
- "No Loan Product was found": "Nebyl nalezen žádný úvěrový produkt", + "No Loan Product was found": "Nebyl nalezen žádný produkt",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` at line 3440, Update the Czech translation for the key "No Loan Product was found" in cs-CS.json so the message is product-type neutral (it currently says "Nebyl nalezen žádný úvěrový produkt" which mentions loans); change the value to a neutral string such as "Nebyl nalezen žádný produkt" or "Žádné dostupné produkty nebyly nalezeny" to avoid referencing only loan products wherever the same selection flow includes working-capital products.src/app/shared/long-text/long-text.component.ts-49-53 (1)
49-53:⚠️ Potential issue | 🟡 Minor
chars >= 1000still makes the toggle inert.Line 50 uses
printChars < 1000as the expansion state check, so any consumer that passescharsas1000or more will hit theelsebranch immediately and keep reassigning the same collapsed value. Please track expanded/collapsed state explicitly instead of inferring it from the truncation length.Suggested fix
export class LongTextComponent implements OnInit { `@Input`() textValue: string; `@Input`() chars = 30; iconVisible = false; displayL = false; displayR = true; emptyValue = false; + expanded = false; printChars = 30; @@ - showValue() { - if (this.printChars < 1000) { - this.printChars = 1000; - } else { - this.printChars = this.chars; - } - } + showValue(): void { + this.expanded = !this.expanded; + this.printChars = this.expanded ? (this.textValue?.length ?? this.chars) : this.chars; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/long-text/long-text.component.ts` around lines 49 - 53, The toggle logic in showValue() wrongly infers expanded state from printChars (<1000) so when chars >= 1000 the toggle is inert; add an explicit boolean (e.g., isExpanded) on LongTextComponent, initialize it false, and update showValue() to flip isExpanded and set printChars = isExpanded ? this.chars : 1000; also update any template bindings that depend on printChars to use isExpanded if needed (e.g., button label or aria-expanded) so expansion/collapse no longer depends on the numeric threshold.src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss-79-79 (1)
79-79:⚠️ Potential issue | 🟡 MinorCap the panel width to the viewport.
The hard 600px minimum will overflow on smaller viewports, which can clip the product list horizontally. Consider capping it with a viewport-aware max width.
Possible tweak
- min-width: max(600px, 100%) !important; + min-width: min(max(600px, 100%), calc(100vw - 32px)) !important; + max-width: calc(100vw - 32px) !important;🤖 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-details-step/loans-account-details-step.component.scss` at line 79, The CSS rule "min-width: max(600px, 100%) !important;" forces the panel wider than the viewport on small screens; update the rule to be viewport-aware by replacing it with a combination that caps width to the viewport (e.g., use min-width: min(600px, 100%) and/or add max-width: 100vw) so the panel never overflows on small viewports; target the same rule in loans-account-details-step.component.scss where the current min-width declaration appears.src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss-78-89 (1)
78-89:⚠️ Potential issue | 🟡 MinorMove this overlay override to a global stylesheet.
The custom class
loan-product-select-panelis assigned viapanelClass, which allows this styling to live in a global stylesheet without::ng-deep. This improves maintainability and follows the codebase pattern for overlay customization.-::ng-deep .loan-product-select-panel { - min-width: max(600px, 100%) !important; - - .mat-mdc-option { - height: auto !important; - padding: 10px 16px !important; - } - - .mdc-list-item__primary-text { - width: 100%; - } -}Add to
src/assets/styles/_misc.scssor similar:.loan-product-select-panel { min-width: max(600px, 100%) !important; .mat-mdc-option { height: auto !important; padding: 10px 16px !important; } .mdc-list-item__primary-text { width: 100%; } }🤖 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-details-step/loans-account-details-step.component.scss` around lines 78 - 89, Remove the ::ng-deep overlay override from loans-account-details-step.component.scss and move the rules for the custom overlay class loan-product-select-panel into a global stylesheet (used by your app's global styles), because the overlay is applied via panelClass on the select; keep the exact selectors (.loan-product-select-panel, .mat-mdc-option, .mdc-list-item__primary-text) and the same declarations (min-width, .mat-mdc-option height/padding, primary-text width) but without ::ng-deep so the overlay styling applies globally, then remove the moved rules from the component SCSS.src/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.ts-100-101 (1)
100-101:⚠️ Potential issue | 🟡 MinorAdd null safety check for
route.parent.Accessing
this.route.parent.datawithout a null check could cause a runtime error if this component is ever used at a route level where no parent exists.🛡️ Suggested defensive check
- this.route.parent.data.subscribe((data: { loanDetailsData: any }) => { + this.route.parent?.data.subscribe((data: { loanDetailsData: any }) => { this.loanId = data.loanDetailsData.id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.ts` around lines 100 - 101, The code subscribes directly to this.route.parent.data (in LoanTermVariationsTabComponent) which can throw if route.parent is null; add a null-safety guard before subscribing — e.g., check if this.route.parent exists (this.route.parent != null) or use optional chaining (this.route.parent?.data) and only subscribe when present, and handle the absent-parent case (e.g., set loanId to null or skip subscription) so accessing data.loanDetailsData.id is safe.src/app/loans/loans.service.ts-198-201 (1)
198-201:⚠️ Potential issue | 🟡 MinorFix typo in method name:
getWorkingCapitalLoannDetailsshould begetWorkingCapitalLoanDetails.The method has a double 'n' in "Loann". This will require updating the definition in
loans.service.tsand the single call site inloan-details.resolver.ts:45.🐛 Changes required
- getWorkingCapitalLoannDetails(loanId: string) { + getWorkingCapitalLoanDetails(loanId: string) { const httpParams = new HttpParams().set('associations', 'all'); return this.http.get(`/working-capital-loans/${loanId}`, { params: httpParams }); }Also update the call in
loan-details.resolver.ts:45:- return this.loansService.getWorkingCapitalLoannDetails(loanId); + return this.loansService.getWorkingCapitalLoanDetails(loanId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans.service.ts` around lines 198 - 201, Rename the misspelled method getWorkingCapitalLoannDetails to getWorkingCapitalLoanDetails in the LoansService implementation, and update any callers to the new name (notably the call from the LoanDetailsResolver where the old name is used). Ensure the method signature and exported symbol name are updated so TypeScript references resolve, then run the build/TS checks to catch any remaining references to getWorkingCapitalLoannDetails.src/assets/translations/es-CL.json-3441-3441 (1)
3441-3441:⚠️ Potential issue | 🟡 MinorBroaden this empty-state copy for the mixed product picker.
This flow now includes working-capital products too, so
producto de créditois too narrow when the list is empty. If this key is reused in loan-only screens, introduce a flow-specific key instead of widening the shared one.💬 Suggested wording
- "No Loan Product was found": "No se encontró ningún producto de crédito", + "No Loan Product was found": "No se encontró ningún producto de crédito o 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 3441, The current translation key "No Loan Product was found" is too narrow for the mixed product picker; either broaden its value to a neutral term like "No se encontraron productos de financiamiento" or (preferable) add a new flow-specific key (e.g., "No Financing Product was found" or "No Products Found - financing_mixed") and update the mixed-picker code to use that new key while leaving the existing "No Loan Product was found" value intact for loan-only screens; locate usages of the JSON key in the mixed product picker component and replace them to reference the new key.src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts-78-78 (1)
78-78:⚠️ Potential issue | 🟡 MinorInitialize input with default empty array.
Without a default value,
loanProductsBasicDetailscould beundefinedif the parent doesn't provide it, causing errors inloanProductName().🛡️ Proposed fix
- `@Input`() loanProductsBasicDetails: LoanProductBasicDetails[]; + `@Input`() loanProductsBasicDetails: LoanProductBasicDetails[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts` at line 78, The Input property loanProductsBasicDetails on LoansAccountPreviewStepComponent can be undefined when the parent omits it, causing loanProductName() to throw; initialize the input with a safe default (set loanProductsBasicDetails: LoanProductBasicDetails[] = []) and, while here, ensure loanProductName() uses the array defensively (e.g., iterate/lookup only if loanProductsBasicDetails exists) so lookups never operate on undefined.src/app/clients/clients-view/general-tab/general-tab.component.ts-432-439 (1)
432-439:⚠️ Potential issue | 🟡 MinorUse
forEachinstead ofmapwhen not returning values.The
map()method is used here, but the callback doesn't return anything. Since you're only performing side effects (pushing toloanAccounts), useforEach()instead.🐛 Proposed fix
private processLoanAccounts(accounts: any[], productType: string): void { - accounts.map((account: any) => { + accounts.forEach((account: any) => { this.loanAccounts.push({ productType: productType, ...account }); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around lines 432 - 439, The method processLoanAccounts uses Array.prototype.map for side effects; change the iteration to use forEach (or a for...of) over the accounts parameter and push each transformed object into this.loanAccounts instead of calling map without returning values; update the function body around processLoanAccounts and its use of accounts and this.loanAccounts accordingly to remove the incorrect map usage.src/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.ts-87-92 (1)
87-92:⚠️ Potential issue | 🟡 MinorSilent no-op when
isLoanProductis false.The
submit()method only executes whenisLoanProductis true. If the product type is working capital or unset, clicking Submit does nothing with no user feedback. Consider either:
- Adding handling for working capital products
- Throwing an error or logging for unexpected product types
- Disabling the submit button when not applicable
🛡️ Proposed fix to handle unexpected cases
if (this.isLoanProduct) { this.loanService.loanActionButtons(this.loanId, 'reject', data).subscribe((response: any) => { this.gotoLoanDefaultView(); }); + } else { + console.warn('Reject action is only available for loan products'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.ts` around lines 87 - 92, The submit() method currently only acts when isLoanProduct is true, causing a silent no-op otherwise; update the submit() implementation to handle other product types or fail loudly by adding an else branch: for working-capital products call the appropriate service method (e.g., loanService.loanActionButtons(this.loanId, 'reject', data) or a dedicated workingCapitalReject method) and navigate/notify on success (reuse gotoLoanDefaultView()), or at minimum log the unexpected case and show user feedback (e.g., use a notification/toast service to display an error message) so users aren’t left with no response; alternatively ensure the template disables the submit button when isLoanProduct is false (update template bindings) — reference symbols: submit(), isLoanProduct, loanService.loanActionButtons, loanId, gotoLoanDefaultView.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3f77d8d-fbb1-43d9-b815-03af7dc11a19
📒 Files selected for processing (109)
src/app/clients/clients-view/general-tab/general-tab.component.htmlsrc/app/clients/clients-view/general-tab/general-tab.component.tssrc/app/core/shell/breadcrumb/breadcrumb.component.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/common-resolvers/loan-products.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.htmlsrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.tssrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/account-details/account-details.component.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.tssrc/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.tssrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.tssrc/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.htmlsrc/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.tssrc/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.htmlsrc/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.tssrc/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.htmlsrc/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.tssrc/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.htmlsrc/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.tssrc/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.htmlsrc/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.tssrc/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.tssrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.tssrc/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.tssrc/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.htmlsrc/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.tssrc/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.tssrc/app/loans/loans-view/loan-account-actions/loan-account-actions.component.tssrc/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.tssrc/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.htmlsrc/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.tssrc/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.tssrc/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.tssrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.tssrc/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.tssrc/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.tssrc/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.htmlsrc/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.tssrc/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.htmlsrc/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.tssrc/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.htmlsrc/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.tssrc/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scsssrc/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans.service.tssrc/app/loans/models/loan-product.model.tssrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/products/loan-products/services/loan-product.service.tssrc/app/products/products.service.tssrc/app/shared/footer/footer.component.tssrc/app/shared/long-text/long-text.component.htmlsrc/app/shared/long-text/long-text.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
💤 Files with no reviewable changes (2)
- src/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scss
- src/app/shared/long-text/long-text.component.html
src/app/loans/create-loans-account/create-loans-account.component.ts
Outdated
Show resolved
Hide resolved
...ans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.ts
Outdated
Show resolved
Hide resolved
...ans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts
Show resolved
Hide resolved
...loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
Show resolved
Hide resolved
...p/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
Outdated
Show resolved
Hide resolved
src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts
Show resolved
Hide resolved
Comment on lines
24
to
31
| 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); | ||
| if (productType !== null) { | ||
| if (productType === 'loan') { | ||
| this.productType.next(LOAN_PRODUCT_TYPE.LOAN); | ||
| } else if (productType === 'working-capital') { | ||
| this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL); | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't let unknown product types bleed into working-capital state.
LoanProductService is a root singleton. With the new initialize() branch, undefined/unexpected values leave the previous productType in place, and productTypeLabel() treats every non-loan value as Working Capital. A missing or malformed productType can therefore generate the wrong labels/routes for standard loan screens. Reset nullish/unknown inputs to a safe default instead of silently reusing or assuming working capital.
Suggested fix
- initialize(productType: string): void {
- if (productType !== null) {
- 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
+ );
+ }
@@
- static productTypeLabel(productType: string): string {
- return productType === 'loan' ? 'labels.heading.Loan Product' : 'labels.heading.Working Capital Product';
- }
+ static productTypeLabel(productType: LoanProductType | null | undefined): string {
+ return productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL
+ ? 'labels.heading.Working Capital Product'
+ : 'labels.heading.Loan Product';
+ }Also applies to: 56-57
🤖 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 - 31, initialize currently leaves the previous productType when passed
null/undefined or any unrecognized string, causing non-loan values to be treated
as WORKING_CAPITAL; update LoanProductService.initialize (and the other branch
around lines 56-57) to explicitly handle nullish or unknown inputs by resetting
this.productType.next(LOAN_PRODUCT_TYPE.LOAN) as a safe default, and change the
conditional to validate recognized values (e.g., 'loan' or 'working-capital')
and otherwise set LOAN_PRODUCT_TYPE.LOAN so productTypeLabel()/routing won’t
incorrectly assume working-capital.
2778bee to
b2146d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts (1)
60-78:⚠️ Potential issue | 🟠 MajorAvoid reloading guarantors through a client-only route.
After this refactor,
deleteGuarantor()still finishes by callingreload(), and that helper rebuilds the page via/clients/${clientId}/loans-accounts. For group-owned or working-capital loans,dataObject.clientIdis not guaranteed, so the refresh can jump to an invalid route right after a successful delete. Please refetch the guarantors in place or derive the refresh navigation from the current account route instead ofclientId.Also applies to: 107-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts` around lines 60 - 78, The reload() call at the end of deleteGuarantor() uses dataObject.clientId to navigate to /clients/${clientId}/loans-accounts, which breaks for group-owned or working-capital loans; modify deleteGuarantor() (and any other callers around lines 107-109) to avoid client-only navigation: after successful delete, either re-fetch the guarantors list in-place (call the existing guarantor-fetching method used by this component) or compute the refresh navigation from the current activated route/account identifier instead of dataObject.clientId so the component stays on the correct account route; remove or replace any direct calls to reload() that rebuild the page via /clients/${clientId}/loans-accounts.src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
27-55:⚠️ Potential issue | 🔴 CriticalRemove the shadowing
loanIdproperty from the child class.The
AdjustLoanChargeComponentdeclares a publicloanIdon line 32 that shadows the protectedloanIdinherited fromLoanAccountActionsBaseComponent. The child'sloanIdis never initialized, remainingundefined. Whensubmit()callsexecuteLoansAccountChargesCommand(this.loanId, ...)on line 122, it passesundefinedinstead of the properly initialized value from the parent class.Remove the
loanId: string;declaration on line 32 to use the inherited protected property instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` around lines 27 - 55, AdjustLoanChargeComponent declares a public loanId that shadows the protected loanId from LoanAccountActionsBaseComponent, causing undefined to be passed in submit(); remove the child declaration "loanId" from AdjustLoanChargeComponent so the component uses the inherited protected loanId (ensure submit() and any references like executeLoansAccountChargesCommand(this.loanId, ...) rely on the parent's initialized property).src/app/clients/clients-view/general-tab/general-tab.component.html (1)
203-245:⚠️ Potential issue | 🟠 MajorAdd
productTypequeryParams to action buttons.The action buttons (
Make Repayment,Approve,Disburse) currently navigate without carrying theproductTypecontext. The row-level[queryParams]="{ productType: row.productType }"is bypassed whenrouteEdit($event)stops propagation, so the button's own[routerLink]takes control—but it lacks the query parameter. This causes working-capital rows to lose product context during navigation.Add
[queryParams]="{ productType: element.productType }"to all three action buttons to maintain consistency with the row-level routing and preserve product type information.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.html` around lines 203 - 245, The action buttons ("Make Repayment", "Approve", "Disburse") navigate via their own [routerLink] but don't forward productType; update each button element in the template that calls routeEdit($event) to include [queryParams]="{ productType: element.productType }" (i.e., add this attribute to the button with the fa-dollar icon, the button with fa-check, and the button with fa-flag) so the productType from element is passed on navigation.src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
280-293:⚠️ Potential issue | 🟠 MajorInclude working-capital loans in the derived client metrics.
You now merge
workingCapitalLoanAccountsinto the displayedloanAccounts, butcomputePerformanceHistory()still receives the raw resolver payload and only countsloanAccounts. That leaves loan cycle, active loans, and last loan amount wrong for clients whose new accounts are working-capital loans.🐛 Proposed fix
- this.computePerformanceHistory(data.clientAccountsData ?? { loanAccounts: [], savingsAccounts: [] }); + this.computePerformanceHistory({ + ...(data.clientAccountsData ?? {}), + loanAccounts: [ + ...(data.clientAccountsData?.loanAccounts ?? []), + ...(data.clientAccountsData?.workingCapitalLoanAccounts ?? []) + ] + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around lines 280 - 293, computePerformanceHistory currently receives the raw resolver payload (data.clientAccountsData) so it only sees the original loanAccounts and ignores workingCapitalLoanAccounts that you merged into this.loanAccounts; update the call so computePerformanceHistory receives the merged accounts (e.g., pass an object where loanAccounts is this.loanAccounts and preserve savingsAccounts from data.clientAccountsData) so the derived metrics (loan cycle, active loans, last loan amount) include working-capital loans; refer to computePerformanceHistory, processLoanAccounts, this.loanAccounts and workingCapitalLoanAccounts to locate and adjust the invocation.src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts (1)
171-175:⚠️ Potential issue | 🟡 MinorInconsistent navigation pattern - missing productType query param.
The
submitRepayment()method uses directrouter.navigatewithout theproductTypequery parameter, whilesubmitContractTermination()correctly usesgotoLoanDefaultView(). This inconsistency could cause navigation issues for non-loan products.🐛 Proposed fix to use consistent navigation
this.loanService.submitLoanActionButton(this.loanId, data, 'repayment').subscribe((response: any) => { - this.router.navigate(['../../general'], { relativeTo: this.route }); + this.gotoLoanDefaultView(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts` around lines 171 - 175, The submitRepayment() flow is navigating with router.navigate(...) directly and omits the productType query param, causing inconsistency with submitContractTermination() which uses gotoLoanDefaultView(); update submitRepayment() (after loanService.submitLoanActionButton(...) subscription) to call the existing gotoLoanDefaultView(...) helper (or call router.navigate with the same productType query param) so the productType is preserved—locate submitRepayment(), submitContractTermination(), gotoLoanDefaultView(), and the router.navigate call in prepay-loan.component.ts and replace the direct router.navigate with a call to gotoLoanDefaultView(this.productType) (or equivalent navigation that includes productType).src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.ts (2)
176-189:⚠️ Potential issue | 🟠 MajorKeep product selection in the first-step validity check.
Line 178 dropped the validator, but
loansAccountDetailsstill returnsnulluntilproductSelectedis set. That lets the first step validate and the wizard advance with no selected product.Also applies to: 195-205
🤖 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-details-step/loans-account-details-step.component.ts` around lines 176 - 189, The product selection validator was removed from createLoansAccountDetailsForm causing loansAccountDetails to remain null until productSelected is set and allowing the wizard to advance without a product; restore Validators.required on the productId control (and similarly on fundId if it's required for first-step validity) in createLoansAccountDetailsForm and in the duplicate form setup referenced around the 195-205 block so loansAccountDetails and productSelected are enforced before advancing the step.
121-142:⚠️ Potential issue | 🟠 MajorPreselected products never execute the new initialization path.
When
loansAccountTemplate.loanProductIdis already present, this branch only patches the form. It never resolvesproductSelected, emitsloansProductType, or loads the product template, so prefilled create-account flows stay uninitialized until the user manually reselects the product.🤖 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-details-step/loans-account-details-step.component.ts` around lines 121 - 142, When loansAccountTemplate.loanProductId is present the code only patches the form and skips initializing the selected product: set the component's productSelected to the preselected id, emit the loansProductType EventEmitter with the selected product (or id) and call the product template loader (e.g., loadProductTemplate or the existing method that loads product details) so the component initializes the same way as a user selection; update the branch where this.loansAccountTemplate.loanProductId is truthy to assign this.productSelected = this.loansAccountTemplate.loanProductId, this.loansProductType.emit(...) with the appropriate payload, and invoke the existing product-loading method to populate template-dependent fields.
♻️ Duplicate comments (4)
src/app/loans/loans-view/loans-view.component.html (1)
59-61:⚠️ Potential issue | 🟠 MajorGuard the working-capital client fallback before dereferencing
client.Group-owned working-capital loans can omit
client, soloanDetailsData.client.displayNameandloanDetailsData.client.accountNothrow before the|| loanDetailsData.group?.…fallback is evaluated. Use optional chaining on theclientside here.♻️ Suggested fix
- {{ loanDetailsData.client.displayName || loanDetailsData.group?.name }} - ({{ loanDetailsData.client.accountNo || loanDetailsData.group?.accountNo }}) + {{ loanDetailsData.client?.displayName || loanDetailsData.group?.name }} + ({{ loanDetailsData.client?.accountNo || loanDetailsData.group?.accountNo }})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loans-view.component.html` around lines 59 - 61, The template dereferences loanDetailsData.client (loanDetailsData.client.displayName and loanDetailsData.client.accountNo) without guarding against client being undefined for group-owned working-capital loans; change those references to use optional chaining (e.g., loanDetailsData.client?.displayName and loanDetailsData.client?.accountNo) so the left side is safely evaluated before falling back to loanDetailsData.group?.name and loanDetailsData.group?.accountNo, leaving the surrounding `@if` (loanProductService.isWorkingCapital) block intact.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
320-329:⚠️ Potential issue | 🟠 MajorRebuild and fully reset the working-capital form when the selected product changes.
This component now has two different form shapes, but
createloansAccountTermsForm()still runs only during construction/init. If the user switches between loan-product and working-capital products after the step is mounted, the old schema stays alive and submission can carry the wrong fields. Even within the working-capital path, only four controls are repatched here, sototalPaymentanddiscountfrom the previous selection are left behind. Please recreate/reset the working-capital form in the change path and initialize or clear every working-capital control from the new template.Also applies to: 408-415, 513-603
🤖 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 320 - 329, When switching to a working-capital product you must fully reset and re-create the form instead of only patching a few controls: call the form factory (createloansAccountTermsForm or equivalent) or reconstruct this.loansAccountTermsForm, then reset it (this.loansAccountTermsForm.reset()) and initialize every working-capital control from this.loansAccountProductTemplate (including totalPayment, discount, and any other fields not currently patched) before patching values; update the change-path blocks (the branch with this.loanProductService.isWorkingCapital and the other mentioned ranges) to always recreate/reset the form and then populate all controls from loansAccountProductTemplate so stale controls from the previous product shape cannot persist.src/app/products/loan-products/services/loan-product.service.ts (1)
24-31:⚠️ Potential issue | 🟠 MajorDefault unknown product types back to
LOAN.
LoanProductServiceis a root singleton.initialize()still leaves the previous value in place fornull/undefined/unexpected input, andproductTypeLabel()still maps every non-loanvalue to Working Capital. That lets a stale WC selection leak into normal loan labels/routes.Suggested fix
- initialize(productType: string): void { - if (productType !== null) { - 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 + ); + } @@ - static productTypeLabel(productType: string): string { - return productType === 'loan' ? 'labels.heading.Loan Product' : 'labels.heading.Working Capital Product'; - } + static productTypeLabel(productType: LoanProductType | null | undefined): string { + return productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL + ? 'labels.heading.Working Capital Product' + : 'labels.heading.Loan Product'; + }Also applies to: 56-57
🤖 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 - 31, The initialize method in LoanProductService should not leave a stale value when given null/undefined or unexpected productType; update initialize(productType: string) to treat any non-'loan' explicit value other than 'working-capital' as default LOAN by calling this.productType.next(LOAN_PRODUCT_TYPE.LOAN) for null/undefined/unrecognized inputs, and likewise update productTypeLabel (and any other mappings mentioned around lines 56–57) so that only 'working-capital' maps to WORKING_CAPITAL and every other/unknown value maps to LOAN; use the existing LOAN_PRODUCT_TYPE enum and the productType.next(...) calls to implement the safe default.src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts (1)
125-129:⚠️ Potential issue | 🟠 MajorDon't leave
submit()as a silent no-op for working-capital accounts.When
isLoanProductis false, clicking Approve does nothing: no API call, no navigation, and no feedback. Either handle the working-capital branch here or gate this action upstream so this screen is unreachable for that product type.Run this read-only check to confirm the action is actually gated upstream. Expected result: either the approve action/component is hidden for working-capital accounts, or there is a non-loan submit branch elsewhere in the flow.
#!/bin/bash set -euo pipefail echo "=== approve-loan submit() ===" fd 'approve-loan.component.ts$' src/app --exec sed -n '107,132p' {} echo echo "=== loan-account-actions host/template ===" fd 'loan-account-actions.component\.(ts|html)$' src/app --exec sed -n '1,260p' {} echo echo "=== product-type gating references ===" rg -n -C3 "ApproveLoanComponent|mifosx-approve-loan|isLoanProduct|working-capital|actions\\['Approve'\\]|actions\\[\"Approve\"\\]" src/app/loans --glob '!**/*.spec.ts'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts` around lines 125 - 129, The submit() currently only acts when this.isLoanProduct is true and is a silent no-op for working-capital accounts; update submit() in ApproveLoanComponent to handle the else branch (when isLoanProduct is false) by either calling the appropriate API and handling its response (e.g., call the working-capital endpoint/service analogous to loanService.loanActionButtons and on success call gotoLoanDefaultView() and show user feedback) or explicitly throw/return an error and ensure the UI is hidden upstream; locate submit() and the current this.loanService.loanActionButtons(...) usage and add the matching branch for non-loan products (or add a guard that prevents rendering the ApproveLoanComponent when isLoanProduct is false).
🧹 Nitpick comments (9)
src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss (2)
78-89: Minor: Adjust padding to align with 8px grid.The
::ng-deepusage is acceptable here—it's necessary to style the Material select panel that renders outside the component's view encapsulation. The Stylelint warning is a false positive in the Angular context.However, the vertical padding
10pxdoesn't align with the 8px grid system.♻️ Suggested fix
.mat-mdc-option { height: auto !important; - padding: 10px 16px !important; + padding: 8px 16px !important; }As per coding guidelines: "Stick to the 8px grid system for visual design and spacing."
🤖 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-details-step/loans-account-details-step.component.scss` around lines 78 - 89, The vertical padding for the Material select options (.mat-mdc-option) uses 10px which violates the 8px grid; update the padding in loans-account-details-step.component.scss for the .mat-mdc-option rule to use an 8px-multiple vertical value (e.g., 8px 16px) so spacing aligns with the design system, leaving the ::ng-deep usage as-is to target the overlay.
38-76: Spacing values should align with the 8px grid system.Several spacing values deviate from the 8px grid:
gap: 4px(lines 42, 62) → consider8pxpadding: 4px 0(line 44) → consider8px 0gap: 12px(line 56) → consider8pxor16pxAdditionally, the hardcoded color fallback
#1074b9in.product-option-nameshould leverage SCSS variables from the theme files for consistency.♻️ Suggested changes for 8px grid alignment
.product-option-row { display: flex; flex-direction: column; align-items: flex-start; - gap: 4px; + gap: 8px; width: 100%; - padding: 4px 0; + padding: 8px 0; }.product-option-meta { display: flex; - gap: 12px; + gap: 16px; }.meta-item { display: flex; flex-direction: column; - gap: 4px; + gap: 8px; }As per coding guidelines: "Stick to the 8px grid system for visual design and spacing" and "leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssinstead of generating custom classes and explicit pixel values."🤖 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-details-step/loans-account-details-step.component.scss` around lines 38 - 76, Update spacing to follow the 8px grid and replace the hardcoded color fallback with the theme SCSS variable: change .product-option-row gap from 4px to 8px and padding from 4px 0 to 8px 0; change .product-option-meta gap from 12px to an 8px (or 16px if more separation desired) value consistent with the design system; keep .meta-item gap at 8px instead of 4px; and update .product-option-name to use the theme SCSS color variable (e.g., the primary color variable defined in your theme files) instead of the hardcoded `#1074b9` so it inherits theming.src/app/core/shell/breadcrumb/breadcrumb.component.ts (1)
228-231: Widen the helper signature to match the new null-handling.
printableValuenow intentionally accepts missing values at runtime, sovalue: stringis no longer the real contract. Updating the signature keeps this change type-safe and avoids hiding nullable call sites behind an overly narrow type.Suggested diff
- printableValue(value: string): string { + printableValue(value: string | null | undefined): string { if (!value) { return ''; }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/core/shell/breadcrumb/breadcrumb.component.ts` around lines 228 - 231, The method printableValue currently accepts only string but is called with possibly null or undefined values at runtime. Update its parameter type from string to string | null | undefined to reflect this and maintain type safety. This change aligns the method signature with its actual usage and avoids misrepresenting the contract in breadcrumb.component's printableValue method.src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
10-10: Prefer typed Angular forms in touched code.This refactor keeps extending the
UntypedForm*surface, so the compiler still cannot help with missing or mistyped controls onadjustLoanChargeForm. Please switch this component to typedFormBuilder/FormControl/FormGroupwhile you are already modifying the form path here.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/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` at line 10, The component currently imports and uses UntypedFormBuilder/UntypedFormControl/UntypedFormGroup which prevents compile-time checking of adjustLoanChargeForm; replace those imports with typed FormBuilder/FormControl/FormGroup from `@angular/forms`, declare a small interface (e.g. AdjustLoanChargeForm) that lists each control name and its control type (FormControl<number> | FormControl<string> etc.), change the adjustLoanChargeForm declaration to FormGroup<AdjustLoanChargeForm> and instantiate via FormBuilder.group with typed controls (this.formBuilder.group<AdjustLoanChargeForm>({...})), and update any usages of adjustLoanChargeForm.get(...) to use typed access (this.adjustLoanChargeForm.controls.myControl or typed .get with non-null assertions) so the compiler can validate control names and types.src/app/loans/common-resolvers/loan-details.resolver.ts (1)
31-44: Give the resolver a typed result instead ofObservable<any>.This resolver now multiplexes two detail endpoints, so
anyhides the shape differences exactly where route-data consumers need compile-time help. A small shared interface or union type here would make the new product-type split much safer to use downstream.Based on learnings: In TypeScript files (e.g.,
src/app/.../*.ts), avoid usingObservable<any>as a project-wide pattern for API responses; introduce specific interfaces/types for the response shapes and use proper typing instead ofany.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-details.resolver.ts` around lines 31 - 44, The resolver currently returns Observable<any>; define specific response types (e.g., LoanAccountAssociationDetails and WorkingCapitalLoanDetails) and a union type or shared interface (e.g., LoanDetails = LoanAccountAssociationDetails | WorkingCapitalLoanDetails), then update the resolve method signature from resolve(route: ActivatedRouteSnapshot): Observable<any> to resolve(route: ActivatedRouteSnapshot): Observable<LoanDetails>; ensure the implementations that call loansService.getLoanAccountAssociationDetails(loanId) and loansService.getWorkingCapitalLoannDetails(loanId) are typed to return the matching concrete types (or the new union) and adjust any consumers to use the typed union/interface.src/app/loans/loans.service.ts (1)
198-201: RenamegetWorkingCapitalLoannDetailsbefore callers spread it.The extra
nin this new public API will make usages harder to grep and easy to copy forward.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans.service.ts` around lines 198 - 201, The public method name getWorkingCapitalLoannDetails has an extra "n" — rename the method to getWorkingCapitalLoanDetails in loans.service.ts (keep the signature: (loanId: string) and existing HttpParams logic) and update all callers, imports, tests, and any exported references to use the corrected name so callers don't break or keep the typo; ensure no duplicate overloads remain and run tests/grep to catch remaining occurrences.src/app/loans/loans-view/account-details/account-details.component.html (1)
13-18: Use Material primitives for the new detail rows.These additions keep expanding the hand-rolled
div/spanlayout instead of using Angular Material detail/list primitives. Please keep new visual sections on Material components so the view follows the repo rule.As per coding guidelines,
**/*.{html,ts,tsx}: Use Angular Material elements (e.g.,<mat-card>,<mat-select>) instead of native HTML for all visual components.Also applies to: 45-180, 192-214, 236-341
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/account-details/account-details.component.html` around lines 13 - 18, Replace the hand-rolled div/span detail rows with Angular Material list/card primitives: wrap the detail rows that render loanDetails.transactionProcessingStrategyName (and the other similar detail sections) in a <mat-list> (or <mat-card>) and use <mat-list-item> with <div mat-line> for the label and value, preserving the translate and translateKey pipes; remove the flex div/span pair and apply the repo's Material styling and accessibility attributes so the visual sections follow the Angular Material components used elsewhere (apply the same change to the other detail blocks that render similar loanDetails fields).src/app/loans/loans-view/loans-view.component.ts (1)
197-255: Consider unsubscribing from the observable to prevent potential memory leaks.The
filterDatatablesByProductmethod subscribes tothis.loansService.getEntityDataTableChecks()without storing the subscription for cleanup. While the current code structure (called once during route data subscription) may not cause issues, adding proper subscription management would be more robust.♻️ Suggested improvement
+import { Subject, takeUntil } from 'rxjs'; export class LoansViewComponent extends LoanProductBaseComponent implements OnInit { + private destroy$ = new Subject<void>(); // ... other members filterDatatablesByProduct(): void { // ... early returns - this.loansService.getEntityDataTableChecks().subscribe({ + this.loansService.getEntityDataTableChecks().pipe( + takeUntil(this.destroy$) + ).subscribe({ // ... handlers }); } + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loans-view.component.ts` around lines 197 - 255, filterDatatablesByProduct currently calls this.loansService.getEntityDataTableChecks().subscribe(...) without cleanup; either make the observable auto-complete (preferred) by piping take(1) or first() before subscribe (e.g., this.loansService.getEntityDataTableChecks().pipe(take(1)).subscribe(...)), or capture the Subscription returned by subscribe into a component-level collection (e.g., this.subscriptions) and unsubscribe in ngOnDestroy; update the component to import rxjs/operators take(1) or manage subscriptions and ensure ngOnDestroy unsubscribes all to prevent memory leaks.src/app/loans/create-loans-account/create-loans-account.component.html (1)
122-128: Consider using Angular Material for the alert component.The no-product alert uses native HTML elements (
<div class="alert">). Per coding guidelines, Angular Material elements should be preferred for visual components.♻️ Suggested Material-based alert
- `@if` (loanProductsBasicDetails.length === 0) { - <div class="alert"> - <div class="message"> - <i class="fa fa-exclamation-circle alert-check"></i> - {{ 'labels.text.No Loan Product was found' | translate }} - </div> - </div> - } + `@if` (loanProductsBasicDetails.length === 0) { + <mat-card class="alert-card"> + <mat-card-content> + <div class="message"> + <mat-icon color="warn">error_outline</mat-icon> + {{ 'labels.text.No Loan Product was found' | translate }} + </div> + </mat-card-content> + </mat-card> + }As per coding guidelines: "Use Angular Material elements (e.g.,
<mat-card>,<mat-select>) instead of native HTML for all visual components".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.html` around lines 122 - 128, The template uses plain <div class="alert"> for the empty-product state; replace that block (the conditional that checks loanProductsBasicDetails.length === 0 in create-loans-account.component.html) with an Angular Material visual component (for example a <mat-card> or <mat-toolbar> containing the icon and the translated message) and remove the custom alert class; also ensure the owning module imports the required Material modules (e.g., MatCardModule and MatIconModule) so the new Material components render correctly while preserving the translate pipe and icon semantics.
🤖 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/clients/clients-view/general-tab/general-tab.component.html`:
- Around line 189-196: The closed-table Type cell dereferences
element.loanType.value unconditionally; wrap the existing icon markup for that
cell in the same guard used in the open table (check element.productType ===
'loan') so you only reference element.loanType when productType is 'loan' (i.e.,
protect the icon/ matTooltip block that uses element.loanType.value and the
fa-user/fa-group ngClass).
In `@src/app/clients/clients-view/general-tab/general-tab.component.ts`:
- Around line 432-438: processLoanAccounts currently uses Array.map for side
effects and places the spread after productType so account.productType can
overwrite it; change it to return the mapped accounts and ensure the explicit
productType wins by putting it last. Specifically, in
processLoanAccounts(accounts, productType) replace the side-effect map with
const mapped = accounts.map(account => ({ ...account, productType })); then
merge/assign that result into this.loanAccounts (e.g., this.loanAccounts =
this.loanAccounts.concat(mapped) or return mapped to be used by the caller) and
update any callers if you choose to return the array.
In `@src/app/loans/create-loans-account/create-loans-account.component.ts`:
- Around line 178-180: The getter loanPrincipal is reading the old control name
and returns undefined because loansAccountTermsForm now exposes principalAmount;
update the getter in create-loans-account.component (loanPrincipal) to read
loansAccountTermsStep?.loansAccountTermsForm.value.principalAmount instead of
.principal so consumers see the entered amount; keep the null-safe access and
return behavior unchanged.
- Around line 119-138: In setTemplate, clear/reset loan-specific state before
the isLoanProduct branch so switching products cannot leak
tranche/datatable/collateral state: explicitly reset this.multiDisburseLoan
(e.g., false), clear this.collateralOptions (empty array/null) and clear any
datatable/tranche structures that setDatatables manipulates (reset the variables
it populates) prior to checking this.loanProductService.isLoanProduct; then keep
the existing behaviour inside the branch to repopulate when a loan product is
selected. Ensure you update references to this.productId/this.currencyCode
remain unchanged.
In
`@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html`:
- Around line 12-17: The preview currently accesses
loansAccountProductTemplate.client.displayName and always shows the "Client"
label, which breaks for group loans; change the template to safely handle
missing client and group applicants by using the safe navigation operator and
applicant list: use loansAccountProductTemplate.clientName ||
loansAccountProductTemplate.client?.displayName ||
(loansAccountProductTemplate.applicants?.map(a => a.displayName).join(', ')),
and make the label conditional (e.g. use a translate key like
'labels.inputs.Applicants' or 'labels.inputs.Group' when
loansAccountProductTemplate.applicants?.length > 1) so the UI won’t throw and
the label reflects group loans.
- Around line 405-415: The template uses a hardcoded 'Unassigned' fallback for
charge.dueDate and charge.feeOnMonthDay which bypasses ngx-translate; replace
those literal strings with a translation key (e.g., a new key like
"COMMON.UNASSIGNED") and use the translate pipe for the fallback (apply the
translate pipe where you currently render 'Unassigned' for charge.dueDate and
charge.feeOnMonthDay in the loans-account-preview-step component template), then
add the new translation key to your i18n source files and run npm run
translations:extract to update translation catalogs.
In
`@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts`:
- Line 78: The component assumes loanProductsBasicDetails is defined when
calling .find() in loanProductName(), which can throw if the `@Input` is missing;
initialize loanProductsBasicDetails to an empty array (e.g., default value on
the property) or defensively guard in loanProductName() by checking
Array.isArray(loanProductsBasicDetails) before .find(); update any other places
that access loanProductsBasicDetails (the usages around the 135-138 logic) to
use the same defensive pattern or default so tests and absent bindings don't
error.
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`:
- Around line 543-566: The template currently renders raw English fragments "on"
and "on day" inside LoansAccountTermsStep markup (see
loansAccountTermsData.interestRecalculationData.* usage); replace those literal
strings with ngx-translate keys (e.g. use translate pipe: {{
'loans.recalculation.on' | translate }} and {{ 'loans.recalculation.onDay' |
translate }} or equivalent TranslateService usage) for the three places shown
(the single weekday branch, the "on day" numeric branch, and the "on <nth>
<weekday>" branch), add the new keys to your i18n extraction files, and run npm
run translations:extract to update translation catalogs.
In `@src/app/loans/loans-view/account-details/account-details.component.html`:
- Around line 251-256: The template currently renders an anchor with href="#"
for the closure loan which is a misleading placeholder; update the block guarded
by loanDetails.isTopup to avoid the dead link by rendering the closure loan
account number as plain text (e.g., a span) until a real navigation target
exists, or if you do have an app route, replace the placeholder with a proper
routerLink pointing to the closure loan using loanDetails.closureLoanAccountNo
(e.g., the component’s loans detail route) and remove href="#" to prevent the
page jump.
In `@src/app/loans/loans-view/general-tab/general-tab.component.ts`:
- Around line 290-293: The loanProductType() method re-derives the product type
from loanDetails.loanType creating a second source of truth; update
loanProductType() to read the initialized product context directly from
LoanProductService (e.g. use its current product property or getter such as
productType/getProductType()) and pass that value into
LoanProductService.productTypeLabel, preserving an appropriate fallback if the
service returns undefined; change only the source used in loanProductType() so
it relies on LoanProductService rather than loanDetails.
In
`@src/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.ts`:
- Around line 110-111: The submit success handler currently calls
router.navigate(['../../general'], { relativeTo: this.route }) after
loanService.loanActionButtons(...), which hard-codes the post-submit route;
replace that direct router.navigate call with the product-aware post-submit
navigation helper provided by the base class (LoanAccountActionsBaseComponent)
so post-submit routing uses the product-type-aware logic—i.e., call the base
component's navigation helper method instead of router.navigate in the
loanService.loanActionButtons(...) subscribe success callback.
In
`@src/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.ts`:
- Around line 100-103: The subscription to this.route.parent.data sets
loanTermVariationsData but doesn't reset the per-type bucket arrays, causing
duplicates when the observable emits again; update the handler used in the
this.route.parent.data.subscribe callback (the block that sets this.loanId,
this.loanTermVariationsData, and iterates
data.loanDetailsData.loanTermVariations) to also clear all per-term arrays
(e.g., any fields like principalRepaidTermVariations,
interestChargedTermVariations, feeTermVariations or similarly named arrays on
the component) before rebuilding the view model so each emission starts from
empty buckets.
In `@src/assets/translations/en-US.json`:
- Line 3580: Update the empty-state translation entry so it is product-type
neutral: replace the key and value "No Loan Product was found" with "No product
was found" (i.e., change the translation key "No Loan Product was found" and its
value to "No product was found") so the message covers both loan and
working-capital products in the selector.
In `@src/assets/translations/es-CL.json`:
- Line 3462: The Spanish string for the key "No Loan Product was found" is too
specific to loans; update its value in the es-CL.json entry "No Loan Product was
found" to a broader, product-type-agnostic translation (for example "No se
encontró ningún producto" or similar) so it reads correctly for both standard
loan and working-capital product searches.
In `@src/assets/translations/pt-PT.json`:
- Line 3461: The current Portuguese translation for the key "No Loan Product was
found" is loan-specific; update translations to be product-agnostic or provide
product-specific keys and adjust callers to use them: either change the value of
"No Loan Product was found" to "Nenhum produto foi encontrado."
(product-agnostic), or add two new keys (e.g., "No Loan Product was found" ->
"Nenhum produto de empréstimo foi encontrado." and "No Working Capital Product
was found" -> "Nenhum produto de capital de giro foi encontrado.") and update
the component that reads the translation to choose the appropriate key based on
productType.
---
Outside diff comments:
In `@src/app/clients/clients-view/general-tab/general-tab.component.html`:
- Around line 203-245: The action buttons ("Make Repayment", "Approve",
"Disburse") navigate via their own [routerLink] but don't forward productType;
update each button element in the template that calls routeEdit($event) to
include [queryParams]="{ productType: element.productType }" (i.e., add this
attribute to the button with the fa-dollar icon, the button with fa-check, and
the button with fa-flag) so the productType from element is passed on
navigation.
In `@src/app/clients/clients-view/general-tab/general-tab.component.ts`:
- Around line 280-293: computePerformanceHistory currently receives the raw
resolver payload (data.clientAccountsData) so it only sees the original
loanAccounts and ignores workingCapitalLoanAccounts that you merged into
this.loanAccounts; update the call so computePerformanceHistory receives the
merged accounts (e.g., pass an object where loanAccounts is this.loanAccounts
and preserve savingsAccounts from data.clientAccountsData) so the derived
metrics (loan cycle, active loans, last loan amount) include working-capital
loans; refer to computePerformanceHistory, processLoanAccounts,
this.loanAccounts and workingCapitalLoanAccounts to locate and adjust the
invocation.
In
`@src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.ts`:
- Around line 176-189: The product selection validator was removed from
createLoansAccountDetailsForm causing loansAccountDetails to remain null until
productSelected is set and allowing the wizard to advance without a product;
restore Validators.required on the productId control (and similarly on fundId if
it's required for first-step validity) in createLoansAccountDetailsForm and in
the duplicate form setup referenced around the 195-205 block so
loansAccountDetails and productSelected are enforced before advancing the step.
- Around line 121-142: When loansAccountTemplate.loanProductId is present the
code only patches the form and skips initializing the selected product: set the
component's productSelected to the preselected id, emit the loansProductType
EventEmitter with the selected product (or id) and call the product template
loader (e.g., loadProductTemplate or the existing method that loads product
details) so the component initializes the same way as a user selection; update
the branch where this.loansAccountTemplate.loanProductId is truthy to assign
this.productSelected = this.loansAccountTemplate.loanProductId,
this.loansProductType.emit(...) with the appropriate payload, and invoke the
existing product-loading method to populate template-dependent fields.
In
`@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts`:
- Around line 27-55: AdjustLoanChargeComponent declares a public loanId that
shadows the protected loanId from LoanAccountActionsBaseComponent, causing
undefined to be passed in submit(); remove the child declaration "loanId" from
AdjustLoanChargeComponent so the component uses the inherited protected loanId
(ensure submit() and any references like
executeLoansAccountChargesCommand(this.loanId, ...) rely on the parent's
initialized property).
In
`@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts`:
- Around line 171-175: The submitRepayment() flow is navigating with
router.navigate(...) directly and omits the productType query param, causing
inconsistency with submitContractTermination() which uses gotoLoanDefaultView();
update submitRepayment() (after loanService.submitLoanActionButton(...)
subscription) to call the existing gotoLoanDefaultView(...) helper (or call
router.navigate with the same productType query param) so the productType is
preserved—locate submitRepayment(), submitContractTermination(),
gotoLoanDefaultView(), and the router.navigate call in prepay-loan.component.ts
and replace the direct router.navigate with a call to
gotoLoanDefaultView(this.productType) (or equivalent navigation that includes
productType).
In
`@src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts`:
- Around line 60-78: The reload() call at the end of deleteGuarantor() uses
dataObject.clientId to navigate to /clients/${clientId}/loans-accounts, which
breaks for group-owned or working-capital loans; modify deleteGuarantor() (and
any other callers around lines 107-109) to avoid client-only navigation: after
successful delete, either re-fetch the guarantors list in-place (call the
existing guarantor-fetching method used by this component) or compute the
refresh navigation from the current activated route/account identifier instead
of dataObject.clientId so the component stays on the correct account route;
remove or replace any direct calls to reload() that rebuild the page via
/clients/${clientId}/loans-accounts.
---
Duplicate comments:
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 320-329: When switching to a working-capital product you must
fully reset and re-create the form instead of only patching a few controls: call
the form factory (createloansAccountTermsForm or equivalent) or reconstruct
this.loansAccountTermsForm, then reset it (this.loansAccountTermsForm.reset())
and initialize every working-capital control from
this.loansAccountProductTemplate (including totalPayment, discount, and any
other fields not currently patched) before patching values; update the
change-path blocks (the branch with this.loanProductService.isWorkingCapital and
the other mentioned ranges) to always recreate/reset the form and then populate
all controls from loansAccountProductTemplate so stale controls from the
previous product shape cannot persist.
In
`@src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts`:
- Around line 125-129: The submit() currently only acts when this.isLoanProduct
is true and is a silent no-op for working-capital accounts; update submit() in
ApproveLoanComponent to handle the else branch (when isLoanProduct is false) by
either calling the appropriate API and handling its response (e.g., call the
working-capital endpoint/service analogous to loanService.loanActionButtons and
on success call gotoLoanDefaultView() and show user feedback) or explicitly
throw/return an error and ensure the UI is hidden upstream; locate submit() and
the current this.loanService.loanActionButtons(...) usage and add the matching
branch for non-loan products (or add a guard that prevents rendering the
ApproveLoanComponent when isLoanProduct is false).
In `@src/app/loans/loans-view/loans-view.component.html`:
- Around line 59-61: The template dereferences loanDetailsData.client
(loanDetailsData.client.displayName and loanDetailsData.client.accountNo)
without guarding against client being undefined for group-owned working-capital
loans; change those references to use optional chaining (e.g.,
loanDetailsData.client?.displayName and loanDetailsData.client?.accountNo) so
the left side is safely evaluated before falling back to
loanDetailsData.group?.name and loanDetailsData.group?.accountNo, leaving the
surrounding `@if` (loanProductService.isWorkingCapital) block intact.
In `@src/app/products/loan-products/services/loan-product.service.ts`:
- Around line 24-31: The initialize method in LoanProductService should not
leave a stale value when given null/undefined or unexpected productType; update
initialize(productType: string) to treat any non-'loan' explicit value other
than 'working-capital' as default LOAN by calling
this.productType.next(LOAN_PRODUCT_TYPE.LOAN) for null/undefined/unrecognized
inputs, and likewise update productTypeLabel (and any other mappings mentioned
around lines 56–57) so that only 'working-capital' maps to WORKING_CAPITAL and
every other/unknown value maps to LOAN; use the existing LOAN_PRODUCT_TYPE enum
and the productType.next(...) calls to implement the safe default.
---
Nitpick comments:
In `@src/app/core/shell/breadcrumb/breadcrumb.component.ts`:
- Around line 228-231: The method printableValue currently accepts only string
but is called with possibly null or undefined values at runtime. Update its
parameter type from string to string | null | undefined to reflect this and
maintain type safety. This change aligns the method signature with its actual
usage and avoids misrepresenting the contract in breadcrumb.component's
printableValue method.
In `@src/app/loans/common-resolvers/loan-details.resolver.ts`:
- Around line 31-44: The resolver currently returns Observable<any>; define
specific response types (e.g., LoanAccountAssociationDetails and
WorkingCapitalLoanDetails) and a union type or shared interface (e.g.,
LoanDetails = LoanAccountAssociationDetails | WorkingCapitalLoanDetails), then
update the resolve method signature from resolve(route: ActivatedRouteSnapshot):
Observable<any> to resolve(route: ActivatedRouteSnapshot):
Observable<LoanDetails>; ensure the implementations that call
loansService.getLoanAccountAssociationDetails(loanId) and
loansService.getWorkingCapitalLoannDetails(loanId) are typed to return the
matching concrete types (or the new union) and adjust any consumers to use the
typed union/interface.
In `@src/app/loans/create-loans-account/create-loans-account.component.html`:
- Around line 122-128: The template uses plain <div class="alert"> for the
empty-product state; replace that block (the conditional that checks
loanProductsBasicDetails.length === 0 in create-loans-account.component.html)
with an Angular Material visual component (for example a <mat-card> or
<mat-toolbar> containing the icon and the translated message) and remove the
custom alert class; also ensure the owning module imports the required Material
modules (e.g., MatCardModule and MatIconModule) so the new Material components
render correctly while preserving the translate pipe and icon semantics.
In
`@src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss`:
- Around line 78-89: The vertical padding for the Material select options
(.mat-mdc-option) uses 10px which violates the 8px grid; update the padding in
loans-account-details-step.component.scss for the .mat-mdc-option rule to use an
8px-multiple vertical value (e.g., 8px 16px) so spacing aligns with the design
system, leaving the ::ng-deep usage as-is to target the overlay.
- Around line 38-76: Update spacing to follow the 8px grid and replace the
hardcoded color fallback with the theme SCSS variable: change
.product-option-row gap from 4px to 8px and padding from 4px 0 to 8px 0; change
.product-option-meta gap from 12px to an 8px (or 16px if more separation
desired) value consistent with the design system; keep .meta-item gap at 8px
instead of 4px; and update .product-option-name to use the theme SCSS color
variable (e.g., the primary color variable defined in your theme files) instead
of the hardcoded `#1074b9` so it inherits theming.
In `@src/app/loans/loans-view/account-details/account-details.component.html`:
- Around line 13-18: Replace the hand-rolled div/span detail rows with Angular
Material list/card primitives: wrap the detail rows that render
loanDetails.transactionProcessingStrategyName (and the other similar detail
sections) in a <mat-list> (or <mat-card>) and use <mat-list-item> with <div
mat-line> for the label and value, preserving the translate and translateKey
pipes; remove the flex div/span pair and apply the repo's Material styling and
accessibility attributes so the visual sections follow the Angular Material
components used elsewhere (apply the same change to the other detail blocks that
render similar loanDetails fields).
In
`@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts`:
- Line 10: The component currently imports and uses
UntypedFormBuilder/UntypedFormControl/UntypedFormGroup which prevents
compile-time checking of adjustLoanChargeForm; replace those imports with typed
FormBuilder/FormControl/FormGroup from `@angular/forms`, declare a small interface
(e.g. AdjustLoanChargeForm) that lists each control name and its control type
(FormControl<number> | FormControl<string> etc.), change the
adjustLoanChargeForm declaration to FormGroup<AdjustLoanChargeForm> and
instantiate via FormBuilder.group with typed controls
(this.formBuilder.group<AdjustLoanChargeForm>({...})), and update any usages of
adjustLoanChargeForm.get(...) to use typed access
(this.adjustLoanChargeForm.controls.myControl or typed .get with non-null
assertions) so the compiler can validate control names and types.
In `@src/app/loans/loans-view/loans-view.component.ts`:
- Around line 197-255: filterDatatablesByProduct currently calls
this.loansService.getEntityDataTableChecks().subscribe(...) without cleanup;
either make the observable auto-complete (preferred) by piping take(1) or
first() before subscribe (e.g.,
this.loansService.getEntityDataTableChecks().pipe(take(1)).subscribe(...)), or
capture the Subscription returned by subscribe into a component-level collection
(e.g., this.subscriptions) and unsubscribe in ngOnDestroy; update the component
to import rxjs/operators take(1) or manage subscriptions and ensure ngOnDestroy
unsubscribes all to prevent memory leaks.
In `@src/app/loans/loans.service.ts`:
- Around line 198-201: The public method name getWorkingCapitalLoannDetails has
an extra "n" — rename the method to getWorkingCapitalLoanDetails in
loans.service.ts (keep the signature: (loanId: string) and existing HttpParams
logic) and update all callers, imports, tests, and any exported references to
use the corrected name so callers don't break or keep the typo; ensure no
duplicate overloads remain and run tests/grep to catch remaining occurrences.
Comment on lines
+432
to
+438
| private processLoanAccounts(accounts: any[], productType: string): void { | ||
| accounts.map((account: any) => { | ||
| this.loanAccounts.push({ | ||
| productType: productType, | ||
| ...account | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Return mapped accounts instead of using map() for side effects.
Biome already flags this callback because it never returns. It also lets an existing account.productType overwrite the explicit type because the spread comes last.
♻️ Proposed fix
private processLoanAccounts(accounts: any[], productType: string): void {
- accounts.map((account: any) => {
- this.loanAccounts.push({
- productType: productType,
- ...account
- });
- });
+ this.loanAccounts.push(
+ ...accounts.map((account: any) => ({
+ ...account,
+ productType
+ }))
+ );
}🧰 Tools
🪛 Biome (2.4.6)
[error] 433-433: This callback passed to map() iterable method should always return a value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around
lines 432 - 438, processLoanAccounts currently uses Array.map for side effects
and places the spread after productType so account.productType can overwrite it;
change it to return the mapped accounts and ensure the explicit productType wins
by putting it last. Specifically, in processLoanAccounts(accounts, productType)
replace the side-effect map with const mapped = accounts.map(account => ({
...account, productType })); then merge/assign that result into
this.loanAccounts (e.g., this.loanAccounts = this.loanAccounts.concat(mapped) or
return mapped to be used by the caller) and update any callers if you choose to
return the array.
Comment on lines
+119
to
+138
| setTemplate($event: any): void { | ||
| this.loansAccountProductTemplate = $event; | ||
| this.currencyCode = this.loansAccountProductTemplate.currency.code; | ||
| const clientId = this.loansAccountTemplate.clientId; | ||
| if (!!clientId) { | ||
| this.clientService.getCollateralTemplate(clientId).subscribe((response: any) => { | ||
| this.collateralOptions = response; | ||
| }); | ||
| } else { | ||
| // Fineract API doesn't have "Group Collateral Management" endpoint; from the obsolete | ||
| // community app it appears getCollateralTemplate(clientId) is called as well, but it's not clear how | ||
| // the clientId is selected from the clientIds that belong to the group. | ||
| console.error('No collateral data requested from Fineract, collateral might misbehave'); | ||
| this.productId = this.loansAccountProductTemplate.product.id; | ||
| if (this.loanProductService.isLoanProduct) { | ||
| const clientId = this.loansAccountTemplate.clientId; | ||
| if (!!clientId) { | ||
| this.clientService.getCollateralTemplate(clientId).subscribe((response: any) => { | ||
| this.collateralOptions = response; | ||
| }); | ||
| } else { | ||
| // Fineract API doesn't have "Group Collateral Management" endpoint; from the obsolete | ||
| // community app it appears getCollateralTemplate(clientId) is called as well, but it's not clear how | ||
| // the clientId is selected from the clientIds that belong to the group. | ||
| console.error('No collateral data requested from Fineract, collateral might misbehave'); | ||
| } | ||
| this.multiDisburseLoan = this.loansAccountProductTemplate.multiDisburseLoan; | ||
| this.setDatatables(); | ||
| } | ||
| const entityId = this.loansAccountTemplate.clientId | ||
| ? this.loansAccountTemplate.clientId | ||
| : this.loansAccountTemplate.group.id; | ||
| const isGroup = this.loansAccountTemplate.clientId ? false : true; | ||
| const productId = this.loansAccountProductTemplate.loanProductId; | ||
| this.loansService.getLoansAccountTemplateResource(entityId, isGroup, productId).subscribe((response: any) => { | ||
| this.multiDisburseLoan = response.multiDisburseLoan; | ||
| }); | ||
| this.setDatatables(); | ||
| } |
There was a problem hiding this comment.
Reset loan-only state when the selected product changes.
Line 123 only initializes multiDisburseLoan and datatables in the loan-product branch. If a user selects a loan product and then switches to working capital, the old tranche/datatable state is left behind and can leak into the new flow.
♻️ Minimal reset before branching
setTemplate($event: any): void {
this.loansAccountProductTemplate = $event;
this.currencyCode = this.loansAccountProductTemplate.currency.code;
this.productId = this.loansAccountProductTemplate.product.id;
+ this.multiDisburseLoan = false;
+ this.datatables = [];
if (this.loanProductService.isLoanProduct) {
const clientId = this.loansAccountTemplate.clientId;
if (!!clientId) {
this.clientService.getCollateralTemplate(clientId).subscribe((response: any) => {
this.collateralOptions = response;
});
} else {
// Fineract API doesn't have "Group Collateral Management" endpoint; from the obsolete
// community app it appears getCollateralTemplate(clientId) is called as well, but it's not clear how
// the clientId is selected from the clientIds that belong to the group.
console.error('No collateral data requested from Fineract, collateral might misbehave');
}
this.multiDisburseLoan = this.loansAccountProductTemplate.multiDisburseLoan;
this.setDatatables();
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around
lines 119 - 138, In setTemplate, clear/reset loan-specific state before the
isLoanProduct branch so switching products cannot leak
tranche/datatable/collateral state: explicitly reset this.multiDisburseLoan
(e.g., false), clear this.collateralOptions (empty array/null) and clear any
datatable/tranche structures that setDatatables manipulates (reset the variables
it populates) prior to checking this.loanProductService.isLoanProduct; then keep
the existing behaviour inside the branch to repopulate when a loan product is
selected. Ensure you update references to this.productId/this.currencyCode
remain unchanged.
Comment on lines
178
to
180
| get loanPrincipal() { | ||
| return this.loansAccountTermsStep.loansAccountTermsForm.value.principal; | ||
| return this.loansAccountTermsStep?.loansAccountTermsForm.value.principal; | ||
| } |
There was a problem hiding this comment.
Read the renamed principal control here.
The terms step now binds the amount to principalAmount, so Line 179 always returns undefined. Anything consuming loanPrincipal will stop seeing the entered amount.
🐛 Minimal fix
get loanPrincipal() {
- return this.loansAccountTermsStep?.loansAccountTermsForm.value.principal;
+ return this.loansAccountTermsStep?.loansAccountTermsForm.value.principalAmount;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around
lines 178 - 180, The getter loanPrincipal is reading the old control name and
returns undefined because loansAccountTermsForm now exposes principalAmount;
update the getter in create-loans-account.component (loanPrincipal) to read
loansAccountTermsStep?.loansAccountTermsForm.value.principalAmount instead of
.principal so consumers see the entered amount; keep the null-safe access and
return behavior unchanged.
Comment on lines
+12
to
+17
| <div class="flex-fill"> | ||
| <span class="flex-40">{{ 'labels.inputs.Client' | translate }}:</span> | ||
| <span class="flex-60"> | ||
| {{ loansAccountProductTemplate.clientName || loansAccountProductTemplate.client.displayName }} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
Handle group applicants in this summary block too.
Line 15 falls back to loansAccountProductTemplate.client.displayName, but the create-account flow still supports group loans. On that path client can be absent, so this preview can throw, and the hard-coded Client label is wrong even when it does not.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html`
around lines 12 - 17, The preview currently accesses
loansAccountProductTemplate.client.displayName and always shows the "Client"
label, which breaks for group loans; change the template to safely handle
missing client and group applicants by using the safe navigation operator and
applicant list: use loansAccountProductTemplate.clientName ||
loansAccountProductTemplate.client?.displayName ||
(loansAccountProductTemplate.applicants?.map(a => a.displayName).join(', ')),
and make the label conditional (e.g. use a translate key like
'labels.inputs.Applicants' or 'labels.inputs.Group' when
loansAccountProductTemplate.applicants?.length > 1) so the UI won’t throw and
the label reflects group loans.
...ew/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+100
to
103
| this.route.parent.data.subscribe((data: { loanDetailsData: any }) => { | ||
| this.loanId = data.loanDetailsData.id; | ||
| this.loanTermVariationsData = []; | ||
| data.loanDetailsData.loanTermVariations?.forEach((item: any) => { |
There was a problem hiding this comment.
Reset the per-term buckets before rebuilding the view model.
this.route.parent.data can emit more than once, but only loanTermVariationsData is cleared here. On the next emission, the per-type arrays still contain the previous items, so the tab renders duplicated variations.
🐛 Suggested fix
this.route.parent.data.subscribe((data: { loanDetailsData: any }) => {
this.loanId = data.loanDetailsData.id;
this.loanTermVariationsData = [];
+ this.emiAmountData = [];
+ this.interestRateData = [];
+ this.dueDateData = [];
+ this.deleteInstallmentData = [];
+ this.insertInstallmentData = [];
+ this.principalAmountData = [];
+ this.graceOnInterestData = [];
+ this.graceOnPrincipalData = [];
+ this.extendRepaymentPeriodData = [];
+ this.interestRateFromInstallmentData = [];
+ this.interestPausesData = [];
+ this.invalidData = [];
data.loanDetailsData.loanTermVariations?.forEach((item: any) => {📝 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
| this.route.parent.data.subscribe((data: { loanDetailsData: any }) => { | |
| this.loanId = data.loanDetailsData.id; | |
| this.loanTermVariationsData = []; | |
| data.loanDetailsData.loanTermVariations?.forEach((item: any) => { | |
| this.route.parent.data.subscribe((data: { loanDetailsData: any }) => { | |
| this.loanId = data.loanDetailsData.id; | |
| this.loanTermVariationsData = []; | |
| this.emiAmountData = []; | |
| this.interestRateData = []; | |
| this.dueDateData = []; | |
| this.deleteInstallmentData = []; | |
| this.insertInstallmentData = []; | |
| this.principalAmountData = []; | |
| this.graceOnInterestData = []; | |
| this.graceOnPrincipalData = []; | |
| this.extendRepaymentPeriodData = []; | |
| this.interestRateFromInstallmentData = []; | |
| this.interestPausesData = []; | |
| this.invalidData = []; | |
| data.loanDetailsData.loanTermVariations?.forEach((item: any) => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.ts`
around lines 100 - 103, The subscription to this.route.parent.data sets
loanTermVariationsData but doesn't reset the per-type bucket arrays, causing
duplicates when the observable emits again; update the handler used in the
this.route.parent.data.subscribe callback (the block that sets this.loanId,
this.loanTermVariationsData, and iterates
data.loanDetailsData.loanTermVariations) to also clear all per-term arrays
(e.g., any fields like principalRepaidTermVariations,
interestChargedTermVariations, feeTermVariations or similarly named arrays on
the component) before rebuilding the view model so each emission starts from
empty buckets.
| "NoDocuments": "No documents available.", | ||
| "NoFileSelected": "No file selected", | ||
| "NoNotesAvailable": "No notes available", | ||
| "No Loan Product was found": "No Loan Product was found", |
There was a problem hiding this comment.
Broaden this empty-state message.
After this PR, the selector can return both loan and working-capital products, so "No Loan Product was found" is misleading. A neutral message like "No product was found" would match the new behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/en-US.json` at line 3580, Update the empty-state
translation entry so it is product-type neutral: replace the key and value "No
Loan Product was found" with "No product was found" (i.e., change the
translation key "No Loan Product was found" and its value to "No product was
found") so the message covers both loan and working-capital products in the
selector.
| "NoDocuments": "No hay documentos disponibles.", | ||
| "NoFileSelected": "No se ha seleccionado ningún archivo", | ||
| "NoNotesAvailable": "No hay notas disponibles", | ||
| "No Loan Product was found": "No se encontró ningún producto de crédito", |
There was a problem hiding this comment.
Broaden this empty-state message for product-type-aware searches.
This string now sits in a flow that can return both standard loan products and working-capital products, so "producto de crédito" is too narrow and will read incorrectly when the selected type is working capital. A generic translation here is safer.
💬 Suggested translation
- "No Loan Product was found": "No se encontró ningún producto de crédito",
+ "No Loan Product was found": "No se encontró ningún producto",📝 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
| "No Loan Product was found": "No se encontró ningún producto de crédito", | |
| "No Loan Product was found": "No se encontró ningún producto", |
🤖 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 3462, The Spanish string for the
key "No Loan Product was found" is too specific to loans; update its value in
the es-CL.json entry "No Loan Product was found" to a broader,
product-type-agnostic translation (for example "No se encontró ningún producto"
or similar) so it reads correctly for both standard loan and working-capital
product searches.
| "NoDocuments": "Nenhum documento disponível.", | ||
| "NoFileSelected": "Nenhum arquivo selecionado", | ||
| "NoNotesAvailable": "Nenhuma nota disponível", | ||
| "No Loan Product was found": "Nenhum produto de empréstimo foi encontrado.", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Usages of the translated key ==\n'
rg -n -C2 --glob '*.{ts,html,json}' 'No Loan Product was found'
printf '\n== Product-type-aware loan creation code ==\n'
rg -n -C3 --glob '*.{ts,html}' 'productType|Working Capital|working capital'Repository: openMF/web-app
Length of output: 50370
Generalize the empty-state message for product-type-aware flows.
This component handles both loan and working-capital product creation. If the productType affects which products are displayed, the message should reflect that:
- Use
"Nenhum produto foi encontrado."(product-agnostic), or - Conditionally display based on
productTypeto show loan-specific or working-capital-specific copy.
Currently, "Nenhum produto de empréstimo foi encontrado." (loan-specific) displays regardless of product type.
🤖 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 3461, The current Portuguese
translation for the key "No Loan Product was found" is loan-specific; update
translations to be product-agnostic or provide product-specific keys and adjust
callers to use them: either change the value of "No Loan Product was found" to
"Nenhum produto foi encontrado." (product-agnostic), or add two new keys (e.g.,
"No Loan Product was found" -> "Nenhum produto de empréstimo foi encontrado."
and "No Working Capital Product was found" -> "Nenhum produto de capital de giro
foi encontrado.") and update the component that reads the translation to choose
the appropriate key based on productType.
b2146d7 to
71786a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
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 (6)
src/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.ts (1)
89-93:⚠️ Potential issue | 🟡 MinorHardcoded user-facing strings violate i18n guidelines.
The error message strings passed to
alertService.alert()are hardcoded in English. As per coding guidelines, all user-facing strings should use i18n variables from@ngx-translate/core.🌐 Proposed fix using i18n
this.alertService.alert({ - type: 'Undo Write-off Failed', - message: - 'An error occurred while processing the undo write-off transaction. Please try again or contact support if the problem persists.' + type: 'labels.errors.undoWriteOffFailed', + message: 'labels.errors.undoWriteOffFailedMessage' });Then add the corresponding keys to the translation files and run
npm run translations:extract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.ts` around lines 89 - 93, The alert call in UndoWriteOffComponent uses hardcoded English strings in this.alertService.alert(...) which violates i18n rules; replace both the `type` and `message` arguments with translated keys (e.g. use TranslateService.instant or a translation helper like this.translate.instant('loans.undoWriteOff.error.title') and this.translate.instant('loans.undoWriteOff.error.message')) and update the component to inject TranslateService if not present; then add the corresponding keys to your translation JSON files and run the translations extraction script (npm run translations:extract).src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.ts (1)
54-62:⚠️ Potential issue | 🟡 MinorIneffective date fallback logic.
new Date(this.dataObject.date)always returns a Date object (even if invalid), so|| new Date()never executes. Ifthis.dataObject.dateis undefined, this results in an Invalid Date being set.🐛 Proposed fix
createCloseForm() { this.closeLoanForm = this.formBuilder.group({ transactionDate: [ - new Date(this.dataObject.date) || new Date(), + this.dataObject.date ? new Date(this.dataObject.date) : new Date(), Validators.required ], note: [] }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.ts` around lines 54 - 62, The transactionDate fallback in createCloseForm is ineffective because new Date(this.dataObject.date) can produce an invalid Date yet still be truthy; update createCloseForm to explicitly construct the date from this.dataObject.date (using optional chaining) and check its validity (e.g., via isNaN(date.getTime()) or similar) and if invalid or missing, use new Date(); then pass that validated Date into this.closeLoanForm (the transactionDate control) so it never receives an Invalid Date.src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (1)
111-117:⚠️ Potential issue | 🟡 MinorIdentical if/else branches - likely unintended.
Both branches execute the same code, making the conditional redundant. Was different behavior intended for the
isCapitalizedIncome()case?🔧 Proposed fix to remove redundant conditional
- if (this.isCapitalizedIncome()) { - this.repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])); - this.updateTransactionAmountValidators(false); - } else { - this.repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])); - this.updateTransactionAmountValidators(false); - } + this.repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])); + this.updateTransactionAmountValidators(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts` around lines 111 - 117, The if/else in make-repayment.component.ts around isCapitalizedIncome() is redundant because both branches call repaymentLoanForm.addControl('transactionAmount', ...) and updateTransactionAmountValidators(false); remove the conditional and move repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])) and updateTransactionAmountValidators(false) out of the if/else so they execute once, or if different behavior was intended for the isCapitalizedIncome() case, implement the correct divergent logic there (e.g., different initial value or validator flag) instead of duplicating the same calls; references: isCapitalizedIncome(), repaymentLoanForm.addControl, updateTransactionAmountValidators().src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
31-55:⚠️ Potential issue | 🔴 CriticalRemove redundant
loanIddeclaration or initialize it in the constructor.Line 32 declares
loanId: stringbut the constructor never initializes it. Thesubmit()method on line 122 uses this undefined property. AlthoughLoanAccountActionsBaseComponentinitializes a protectedloanIdin its constructor, the child class's public declaration shadows it. Either remove the local declaration to use the base class property, or initialize it in the constructor withthis.loanId = this.route.snapshot.params['loanId'];(following the pattern in sibling components likeadd-loan-charge.component.ts).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` around lines 31 - 55, The file declares a public loanId property that shadows the protected loanId from LoanAccountActionsBaseComponent and remains uninitialized, causing submit() to use undefined; fix by removing the local loanId declaration so the component inherits the base class's protected loanId, or if you prefer explicit init, set this.loanId in the constructor (e.g., from this.route.snapshot.params['loanId']) and ensure submit() references that same loanId; locate the loanId declaration near the top of the class and the constructor/submit() methods to apply the change.src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.html (1)
83-88:⚠️ Potential issue | 🟡 MinorPre-existing bug: Duplicate "is" text in error message.
Line 86 contains
{{ 'labels.commons.is' | translate }}twice, which will display redundant "is is" text in the UI. The same issue exists on line 96 for the Last Name field.🐛 Proposed fix
`@if` (newGuarantorForm.controls.firstname.hasError('required')) { <mat-error> {{ 'labels.inputs.First Name' | translate }} {{ 'labels.commons.is' | translate }} - {{ 'labels.commons.is' | translate }} <strong>{{ 'labels.commons.required' | translate }}</strong> + <strong>{{ 'labels.commons.required' | translate }}</strong> </mat-error> }And similarly for lines 93-98:
`@if` (newGuarantorForm.controls.lastname.hasError('required')) { <mat-error> {{ 'labels.inputs.Last Name' | translate }} {{ 'labels.commons.is' | translate }} - {{ 'labels.commons.is' | translate }} <strong>{{ 'labels.commons.required' | translate }}</strong> + <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/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.html` around lines 83 - 88, The mat-error blocks for the firstname and lastname controls in the CreateGuarantor template (newGuarantorForm.controls.firstname and newGuarantorForm.controls.lastname) contain a duplicated translation token {{ 'labels.commons.is' | translate }}; remove the extra occurrence in each error block so the message renders "First Name is required" / "Last Name is required" with only a single 'is' token.src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts (1)
158-175:⚠️ Potential issue | 🟠 MajorRepayment submit still bypasses product-aware navigation.
Line 173 still uses direct
router.navigate(...), while the component now relies on base navigation helpers. This can drop product-context query state after repayment.♻️ Suggested fix
this.loanService.submitLoanActionButton(this.loanId, data, 'repayment').subscribe((response: any) => { - this.router.navigate(['../../general'], { relativeTo: this.route }); + this.gotoLoanDefaultView(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts` around lines 158 - 175, In submitRepayment(), replace the direct router.navigate call that drops product context with the component's inherited/base navigation helper so product-aware query state is preserved; locate the line using this.router.navigate(['../../general'], { relativeTo: this.route }) and call the existing navigation helper used elsewhere in this component (the base helper method your component inherits—e.g., this.navigate or this.navigateToGeneral) with the same target path and options (relative route and any query/state) instead of router.navigate, ensuring you pass the identical route array and preserve queryParams/state.
♻️ Duplicate comments (10)
src/app/products/loan-products/services/loan-product.service.ts (1)
24-31:⚠️ Potential issue | 🟠 MajorNormalize unknown/null product types to a safe default.
Current logic can keep stale service state and incorrectly classify unknown values as working-capital, which can misroute UI labels/paths.
Suggested fix
- initialize(productType: string): void { - if (productType !== null) { - 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 + ); + } @@ - static productTypeLabel(productType: string): string { - return productType === 'loan' ? 'labels.heading.Loan Product' : 'labels.heading.Working Capital Product'; - } + static productTypeLabel(productType: LoanProductType | null | undefined): string { + return productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL + ? 'labels.heading.Working Capital Product' + : 'labels.heading.Loan Product'; + }Also applies to: 56-57
🤖 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 - 31, The initialize method leaves productType unchanged for null/unknown inputs and can misclassify or keep stale state; update initialize(productType: string) (and the similar logic around lines 56-57) to normalize input by handling null/undefined/unknown values explicitly: if productType is 'loan' set this.productType.next(LOAN_PRODUCT_TYPE.LOAN), else if 'working-capital' set WORKING_CAPITAL, otherwise set a safe default (e.g., LOAN_PRODUCT_TYPE.UNKNOWN or LOAN_PRODUCT_TYPE.LOAN) to ensure the service state is deterministic and not left stale; reference the initialize function and this.productType/LOAN_PRODUCT_TYPE symbols when making the change.src/assets/translations/es-CL.json (1)
3465-3465:⚠️ Potential issue | 🟡 MinorUse a product-agnostic empty-state message.
Line 3465 is still loan-specific (
producto de crédito) in a flow that can include working-capital products.Suggested fix
- "No Loan Product was found": "No se encontró ningún producto de crédito", + "No Loan Product was found": "No se encontró ningún producto",🤖 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 3465, The Spanish translation for the key "No Loan Product was found" is loan-specific; update the value for the JSON key "No Loan Product was found" to a product-agnostic message (e.g., change "No se encontró ningún producto de crédito" to a neutral string such as "No se encontró ningún producto" or similar) so the empty-state works for both loan and working-capital products; ensure you only modify the value for that exact key in src/assets/translations/es-CL.json.src/app/clients/clients-view/general-tab/general-tab.component.html (1)
329-337:⚠️ Potential issue | 🟠 MajorGuard the closed-table Type cell for working-capital rows.
This cell still dereferences
element.loanType.valueunconditionally. The open table already skips that icon for non-loan products, so switching to the closed view can still throw as soon as a working-capital row is rendered.♻️ Proposed fix
<ng-container matColumnDef="Type"> <th mat-header-cell class="center" *matHeaderCellDef>{{ 'labels.inputs.Type' | translate }}</th> <td mat-cell class="center" *matCellDef="let element"> - <i - class="fa fa-large" - [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" - matTooltip=" {{ element.loanType.value }}" - matTooltipPosition="above" - ></i> + `@if` (element.productType === 'loan') { + <i + class="fa fa-large" + [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" + matTooltip="{{ element.loanType.value }}" + matTooltipPosition="above" + ></i> + } </td> </ng-container>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.html` around lines 329 - 337, The Type cell currently dereferences element.loanType.value unconditionally in the matColumnDef "Type" cell, which can throw for working-capital/non-loan rows; update the template to guard the icon rendering by checking the presence/type before access (e.g. only render the <i> when element.loanType and element.loanType.value exist or when element.productType === 'Loan'), or move the ternary into a safe-navigation check (element.loanType?.value) and add an *ngIf around the <i> to skip rendering for non-loan products so closed-table rows do not dereference a missing loanType.src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
432-438:⚠️ Potential issue | 🟡 MinorStop using
map()for side effects here.This still triggers
useIterableCallbackReturn, and the spread order lets an existingaccount.productTypeoverwrite the explicit type you are adding. Build the mapped objects first and putproductTypelast.♻️ Proposed fix
private processLoanAccounts(accounts: any[], productType: string): void { - accounts.map((account: any) => { - this.loanAccounts.push({ - productType: productType, - ...account - }); - }); + this.loanAccounts.push( + ...accounts.map((account: any) => ({ + ...account, + productType + })) + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around lines 432 - 438, In processLoanAccounts, stop using Array.prototype.map for side effects and avoid letting account.productType overwrite the intended value: iterate accounts with forEach (or a for/of loop), build each mapped object first from the account data then set productType last (i.e., spread account before adding productType) and push that object into this.loanAccounts so productType takes precedence over any existing account.productType.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
319-323:⚠️ Potential issue | 🟠 MajorWorking-capital product changes still leave stale form values.
Line 320 updates
loansAccountTermsData, but this branch does not repatchloansAccountTermsForm. If users switch working-capital products after mount, old term values can remain in the form.♻️ Suggested fix
} else if (this.loanProductService.isWorkingCapital && this.loansAccountProductTemplate) { this.loansAccountTermsData = this.loansAccountProductTemplate; this.currency = this.loansAccountTermsData.currency; this.termFrequencyTypeData = this.loansAccountTermsData.periodFrequencyTypeOptions; + this.loansAccountTermsForm.patchValue({ + principalAmount: this.loansAccountTermsData.principal, + periodPaymentRate: this.loansAccountTermsData.periodPaymentRate, + repaymentEvery: this.loansAccountTermsData.repaymentEvery, + repaymentFrequencyType: this.loansAccountTermsData.repaymentFrequencyType.id + }); }🤖 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 319 - 323, The branch handling working-capital products updates loansAccountTermsData (using loansAccountProductTemplate) but fails to repatch the loansAccountTermsForm, leaving stale values in the UI; update this branch to patch/reset the form with the new loansAccountTermsData (set currency and termFrequencyTypeData as done) using loansAccountTermsForm.patchValue(...) or reset(...) with the template values and then call updateValueAndValidity() (and re-apply any validators if the other branch does so) so the form fields reflect the new working-capital product state.src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html (2)
13-16:⚠️ Potential issue | 🔴 CriticalPreview applicant header is not safe for non-client ownership.
Line 15 directly reads
loansAccountProductTemplate.client.displayName; this can fail for group/applicant-based flows and keeps a fixed “Client” label.🛡️ Suggested fix
- <span class="flex-30">{{ 'labels.inputs.Client' | translate }}:</span> + <span class="flex-30">{{ 'labels.inputs.Applicants' | translate }}:</span> <span class="flex-70"> - {{ loansAccountProductTemplate.clientName || loansAccountProductTemplate.client.displayName }} + {{ + loansAccountProductTemplate.clientName || + loansAccountProductTemplate.client?.displayName || + loansAccountProductTemplate.group?.name + }} </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html` around lines 13 - 16, The template directly accesses loansAccountProductTemplate.client.displayName and uses a fixed "Client" label which breaks for non-client ownership (groups/applicants); update the preview to safely resolve the display name by checking loansAccountProductTemplate.clientName first, then loansAccountProductTemplate.client?.displayName, then loansAccountProductTemplate.applicant?.displayName or loansAccountProductTemplate.group?.name as fallbacks, and also replace the hardcoded "labels.inputs.Client" label with a contextual label (e.g., choose a translation key based on whether client, applicant, or group is present) so the UI and bindings never read undefined properties.
400-406:⚠️ Potential issue | 🟡 MinorTranslate the
Unassignedfallback text in charges dates.Line 400 and Line 405 use hardcoded English fallback text in a localized table.
As per coding guidelines, "Use proper i18n variables from
@ngx-translate/corefor all user-facing strings instead of hardcoded text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html` around lines 400 - 406, Replace the hardcoded "Unassigned" fallbacks in the template with an i18n translation key using `@ngx-translate/core`; specifically update the spans that render {{ (charge.dueDate | dateFormat) || 'Unassigned' }} and {{ (charge.feeOnMonthDay | dateFormat) || 'Unassigned' }} to use a translation key (e.g. 'COMMON.UNASSIGNED' or an existing loans-specific key) via the translate pipe (e.g. use the translate pipe for the fallback), ensuring the template uses the translate pipe for the fallback value so both charge.dueDate and charge.feeOnMonthDay display the localized string when empty.src/app/loans/loans-view/loans-view.component.html (1)
59-61:⚠️ Potential issue | 🟠 MajorGuard working-capital client fields before fallback.
Line 60 and Line 61 access
loanDetailsData.clientdirectly. For group-owned accounts, this can crash before thegroupfallback is evaluated.🛡️ Suggested fix
`@if` (loanProductService.isWorkingCapital) { - {{ loanDetailsData.client.displayName || loanDetailsData.group?.name }} - ({{ loanDetailsData.client.accountNo || loanDetailsData.group?.accountNo }}) + {{ loanDetailsData.client?.displayName || loanDetailsData.group?.name }} + ({{ loanDetailsData.client?.accountNo || loanDetailsData.group?.accountNo }}) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loans-view.component.html` around lines 59 - 61, The template accesses loanDetailsData.client properties directly and can throw for group-owned accounts; update the working-capital branch (condition loanProductService.isWorkingCapital) to guard client fields using safe navigation or explicit checks—replace occurrences of loanDetailsData.client.displayName and loanDetailsData.client.accountNo with loanDetailsData.client?.displayName and loanDetailsData.client?.accountNo (keeping fallback to loanDetailsData.group?.name and loanDetailsData.group?.accountNo) so the group fallback is evaluated safely.src/app/loans/loans-view/account-details/account-details.component.html (1)
276-281:⚠️ Potential issue | 🟡 MinorReplace placeholder top-up anchor.
Line 280 uses
href="#", which looks navigable but is a dead link and only jumps to top.♻️ Suggested fix
- <a href="#">{{ loanDetails.closureLoanAccountNo }}</a> + {{ loanDetails.closureLoanAccountNo }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/account-details/account-details.component.html` around lines 276 - 281, The anchor in the AccountDetails template using href="#" is a dead link; update the <a> that displays loanDetails.closureLoanAccountNo to either (a) navigate to the actual account/loan details using the app's routing (e.g., use routerLink or the component's navigation method and pass loanDetails.closureLoanAccountNo) or (b) if it shouldn't navigate, replace the <a> with a non-interactive element (span/button with proper aria attributes) to avoid a misleading dead link. Target the anchor in account-details.component.html that renders loanDetails.closureLoanAccountNo.src/app/loans/loans-view/general-tab/general-tab.component.ts (1)
290-293:⚠️ Potential issue | 🟡 MinorUse the initialized product context as the source of truth.
This method re-derives the label from
loanDetails.loanTypeeven though the component already stores product context inLoanProductService. Keeping both sources makes the new Product Type row easy to drift out of sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/general-tab/general-tab.component.ts` around lines 290 - 293, The loanProductType() method should read the product type from the already-initialized product context in LoanProductService instead of re-checking loanDetails.loanType; update loanProductType() to consult the product context on LoanProductService (for example the service's productContext or a getter like getProductType()/currentProductType) and then map that to the label via LoanProductService.productTypeLabel('loan'|'working-capital'), removing any direct reliance on loanDetails.loanType to avoid state drift.
🟡 Minor comments (8)
src/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.ts-83-85 (1)
83-85:⚠️ Potential issue | 🟡 MinorAdd error handling to the subscription.
The
subscribe()call lacks error handling. If the API call fails, the user won't receive any feedback. Also,response: anyshould be properly typed.🛡️ Proposed fix
- this.loanService.loanActionButtons(this.loanId, 'assignLoanOfficer', data).subscribe((response: any) => { - this.gotoLoanDefaultView(); - }); + this.loanService.loanActionButtons(this.loanId, 'assignLoanOfficer', data).subscribe({ + next: () => { + this.gotoLoanDefaultView(); + }, + error: (error) => { + console.error('Failed to assign loan officer:', error); + // Consider showing user feedback via MatSnackBar or similar + } + });Based on learnings: "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/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.ts` around lines 83 - 85, The subscribe call to loanService.loanActionButtons(this.loanId, 'assignLoanOfficer', data) in assign-loan-officer.component is missing error handling and uses response: any; create a proper response interface (e.g., AssignLoanOfficerResponse) that matches the API shape, update the subscription to use typed response (subscribe((res: AssignLoanOfficerResponse) => { this.gotoLoanDefaultView(); }, (err) => { /* show user-friendly error via notification/toast and log details */ })) and optionally add a finalize block to re-enable UI state; ensure you reference the loanService.loanActionButtons method and the gotoLoanDefaultView() call when implementing the fix.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts-140-145 (1)
140-145:⚠️ Potential issue | 🟡 MinorMissing error handling in
submitReschedule.The subscription to
loanService.submitRescheduleData()lacks an error handler. If the API call fails, the error will propagate unhandled, potentially leaving the user without feedback. Other methods in this component (e.g.,loadPenalties, penalty waiving) properly handle errors.🛡️ Proposed fix to add error handling
private submitReschedule(data: any) { - this.loanService.submitRescheduleData(data).subscribe((response: any) => { - this.gotoLoanDefaultView(); + this.loanService.submitRescheduleData(data).subscribe({ + next: () => { + this.gotoLoanDefaultView(); + }, + error: (error: any) => { + console.error('Error submitting reschedule:', error); + } }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts` around lines 140 - 145, The submitReschedule method subscribes to loanService.submitRescheduleData(...) without an error callback; add an error handler to the subscribe call in submitReschedule to handle API failures, prevent navigating to gotoLoanDefaultView() on error, and surface feedback (e.g., call the component's existing error handling/notification method such as handleError or showError/toast) and clear any loading state; locate the submitReschedule function in the component and update its subscribe to include both next and error callbacks referencing loanService.submitRescheduleData and gotoLoanDefaultView.src/assets/translations/fr-FR.json-1801-1801 (1)
1801-1801:⚠️ Potential issue | 🟡 MinorUse consistent French terminology for “Discount”.
This file already uses “Remise” terminology (for example,
Discount Default). UsingRabaishere introduces inconsistent UX wording.💡 Suggested wording adjustment
- "Discount": "Rabais", + "Discount": "Remise",🤖 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 1801, The translation entry for the key "Discount" currently uses "Rabais" which is inconsistent with the rest of the file (e.g., "Discount Default" uses "Remise"); update the value for the "Discount" key from "Rabais" to "Remise" so all occurrences use the same French terminology, and scan nearby translation keys (like "Discount Default") to ensure consistency across related keys.src/assets/translations/lt-LT.json-3464-3464 (1)
3464-3464:⚠️ Potential issue | 🟡 MinorMake the empty-state text product-agnostic.
This selector now includes working-capital products too, so “No Loan Product was found” is misleading. A generic message like “No matching product was found” fits both product types.
🤖 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 3464, Update the product-empty-state translation for the JSON key "No Loan Product was found" to a product-agnostic phrase; replace the English with "No matching product was found" and update the Lithuanian value from "Nerasta jokių paskolos produktų" to a generic translation like "Nerastas atitinkamas produktas" in src/assets/translations/lt-LT.json so the message fits both loan and working-capital products.src/app/loans/loans-view/account-details/account-details.component.html-360-362 (1)
360-362:⚠️ Potential issue | 🟡 MinorLocalize the inline
Max:label.Line 360 introduces hardcoded text (
Max:) in a translated UI block.As per coding guidelines, "Use proper i18n variables from
@ngx-translate/corefor all user-facing strings instead of hardcoded text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/account-details/account-details.component.html` around lines 360 - 362, Replace the hardcoded "Max:" label with a translated key in the AccountDetailsComponent template: update the inline text near loanDetails.minimumGap/maximumGap to use the translate pipe (e.g., replace "Max:" with a translation lookup like 'labels.text.Max' via | translate) so all user-facing strings use `@ngx-translate/core`; ensure spacing/punctuation remains correct and add the new 'labels.text.Max' entry to your translation files.src/app/loans/create-loans-account/create-loans-account.component.ts-270-286 (1)
270-286:⚠️ Potential issue | 🟡 MinorSame error handling gap in
submitWorkingCapitalProduct.Consistent with the loan product submission, this call also lacks error handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around lines 270 - 286, The subscribe call inside submitWorkingCapitalProduct that invokes this.loansService.createLoansAccount(this.loanProductService.loanAccountPath, payload) needs error handling: modify the subscription to provide an error callback (or use pipe + catchError) so failures do not silently fail; on error, surface a user-facing message (e.g., via the existing notification/toast service) and avoid navigation, while keeping the success behavior that calls this.router.navigate(['../', response.resourceId, 'general'], { queryParams: { productType: this.loanProductService.productType.value }, relativeTo: this.route }); ensure you reference the createLoansAccount call, submitWorkingCapitalProduct method, loanProductService.loanAccountPath, payload, router.navigate and route when applying the fix.src/app/loans/create-loans-account/create-loans-account.component.ts-232-248 (1)
232-248:⚠️ Potential issue | 🟡 MinorMissing error handling for the loan creation API call.
If
createLoansAccountfails, the observable error is silently ignored and the user is left on the form without feedback. Consider adding.pipe(catchError(...))or an error callback to display a notification.🛡️ Proposed fix to add error handling
+import { catchError } from 'rxjs/operators'; +import { EMPTY } from 'rxjs'; this.loansService .createLoansAccount(this.loanProductService.loanAccountPath, payload) + .pipe( + catchError((error) => { + // Consider injecting a snackbar/notification service to display error + console.error('Failed to create loan account', error); + return EMPTY; + }) + ) .subscribe((response: any) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around lines 232 - 248, The createLoansAccount observable subscription currently only handles success and ignores errors; update the call to handle failures by adding an error path (either add a second callback to subscribe or use .pipe(catchError(...)) before subscribe) on loansService.createLoansAccount(...) so API errors are handled, surface a user-visible error (e.g., call the notification/toast service or set a component error state) and ensure any loading indicators are cleared; reference the createLoansAccount call, the existing subscribe success block that calls router.navigate([...], { queryParams: { productType: this.loanProductService.productType.value }, relativeTo: this.route }), and the loansService so you add error handling alongside that success handling.src/app/loans/loans-view/general-tab/general-tab.component.ts-227-231 (1)
227-231:⚠️ Potential issue | 🟡 Minor
Loan Purposedrops out of the summary path.
setloanNonDetailsTableData()still addsLoan Purposefor standard loans, but this branch only re-addsLoan Officer. As a result, accounts that havesummarydata lose the Loan Purpose row after this change.♻️ Suggested fix
if (this.loanProductService.isLoanProduct) { + this.loanDetailsTableData.push({ + key: 'Loan Purpose' + }); this.loanDetailsTableData.push({ key: 'Loan Officer' }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/general-tab/general-tab.component.ts` around lines 227 - 231, The branch that runs when loanProductService.isLoanProduct only re-adds the "Loan Officer" row and thus omits the "Loan Purpose" row for accounts with summary data; inside the same conditional where you push into loanDetailsTableData (the branch referencing loanProductService.isLoanProduct and loanDetailsTableData), also push the "Loan Purpose" entry (matching the shape used elsewhere by setloanNonDetailsTableData) so Loan Purpose is preserved for loan-product accounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e368511-6f8c-4276-802e-38b402afeb0f
📒 Files selected for processing (111)
src/app/clients/clients-view/general-tab/general-tab.component.htmlsrc/app/clients/clients-view/general-tab/general-tab.component.tssrc/app/core/shell/breadcrumb/breadcrumb.component.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/common-resolvers/loan-products.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.htmlsrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/edit-loans-account/edit-loans-account.component.htmlsrc/app/loans/edit-loans-account/edit-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.tssrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/account-details/account-details.component.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.tssrc/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.tssrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.tssrc/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.htmlsrc/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.tssrc/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.htmlsrc/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.tssrc/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.htmlsrc/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.tssrc/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.htmlsrc/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.tssrc/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.htmlsrc/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.tssrc/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.tssrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.tssrc/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.tssrc/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.htmlsrc/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.tssrc/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.tssrc/app/loans/loans-view/loan-account-actions/loan-account-actions.component.tssrc/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.tssrc/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.htmlsrc/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.tssrc/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.tssrc/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.tssrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.tssrc/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.tssrc/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.tssrc/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.htmlsrc/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.tssrc/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.htmlsrc/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.tssrc/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.htmlsrc/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.tssrc/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scsssrc/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans.service.tssrc/app/loans/models/loan-product.model.tssrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/products/loan-products/services/loan-product.service.tssrc/app/products/products.service.tssrc/app/shared/footer/footer.component.tssrc/app/shared/long-text/long-text.component.htmlsrc/app/shared/long-text/long-text.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
💤 Files with no reviewable changes (2)
- src/app/shared/long-text/long-text.component.html
- src/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scss
🚧 Files skipped from review as they are similar to previous changes (32)
- src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.html
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html
- src/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.html
- src/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.html
- src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html
- src/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.html
- src/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.html
- src/app/loans/loans-view/loan-account-actions/loan-account-actions.component.ts
- src/assets/translations/it-IT.json
- src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.html
- src/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.html
- src/assets/translations/en-US.json
- src/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.html
- src/app/products/loan-products/common/loan-product-base.component.ts
- src/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.ts
- src/assets/translations/sw-SW.json
- src/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.html
- src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.html
- src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.html
- src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.html
- src/app/loans/common-resolvers/loan-products.resolver.ts
- src/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.ts
- src/assets/translations/cs-CS.json
- src/assets/translations/pt-PT.json
- src/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.ts
- src/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.html
- src/assets/translations/es-MX.json
- src/app/loans/loans.service.ts
- src/app/core/shell/breadcrumb/breadcrumb.component.ts
- src/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.ts
- src/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.html
- src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.html
Comment on lines
33
to
46
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| const productType = route.queryParams['productType']; | ||
| const resolvedProductType = | ||
| productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL ? LOAN_PRODUCT_TYPE.WORKING_CAPITAL : LOAN_PRODUCT_TYPE.LOAN; | ||
| this.loanProductService.initialize(resolvedProductType); | ||
| if (!isNaN(+loanId)) { | ||
| return this.loansService.getLoanAccountAssociationDetails(loanId); | ||
| if (resolvedProductType === LOAN_PRODUCT_TYPE.LOAN) { | ||
| return this.loansService.getLoanAccountAssociationDetails(loanId); | ||
| } else { | ||
| return this.loansService.getWorkingCapitalLoannDetails(loanId); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing return path when loanId is invalid.
When loanId is NaN, the resolver implicitly returns undefined. This could cause downstream issues as the route expects an Observable. Consider returning an empty observable or throwing an error for invalid loan IDs.
🛡️ Proposed fix to handle invalid loanId
+import { EMPTY } from 'rxjs';
+
resolve(route: ActivatedRouteSnapshot): Observable<any> {
const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId');
const productType = route.queryParams['productType'];
const resolvedProductType =
productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL ? LOAN_PRODUCT_TYPE.WORKING_CAPITAL : LOAN_PRODUCT_TYPE.LOAN;
this.loanProductService.initialize(resolvedProductType);
if (!isNaN(+loanId)) {
if (resolvedProductType === LOAN_PRODUCT_TYPE.LOAN) {
return this.loansService.getLoanAccountAssociationDetails(loanId);
} else {
return this.loansService.getWorkingCapitalLoannDetails(loanId);
}
}
+ return EMPTY;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/common-resolvers/loan-details.resolver.ts` around lines 33 -
46, The resolver's resolve method can return undefined when loanId is invalid;
update resolve in loan-details.resolver.ts to always return an Observable by
handling the NaN path—after calling
loanProductService.initialize(resolvedProductType) check the parsed loanId and
if invalid return an empty Observable (e.g., rxjs EMPTY) or a thrown error
Observable (e.g., throwError) instead of falling through; keep the existing
branches that call loansService.getLoanAccountAssociationDetails(loanId) and
loansService.getWorkingCapitalLoannDetails(loanId) for valid IDs so the method
consistently returns an Observable.
Comment on lines
13
to
18
| <mat-select | ||
| required | ||
| matTooltip="{{ 'tooltips.Name of the loan product' | translate }}" | ||
| formControlName="productId" | ||
| panelClass="loan-product-select-panel" | ||
| (selectionChange)="productChange($event)" | ||
| > |
There was a problem hiding this comment.
Keep productId bound to the actual product id.
Line 35 switches the option value to product.shortName, but this control is still modeled as productId, and the preview step resolves selected products by id. That changes the payload contract from numeric id to string short name and can break preview/submission.
🔧 Suggested fix
- <mat-option [value]="product.shortName" class="product-option">
+ <mat-option [value]="product.id" class="product-option">Also applies to: 34-36
🤖 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-details-step/loans-account-details-step.component.html`
around lines 13 - 18, The mat-select is currently binding the form control named
productId to product.shortName, changing the payload shape; update the template
so the option values remain the actual product id (keep
formControlName="productId" bound to product.id) and do not replace it with
product.shortName, and if productChange(product) needs the short name, derive it
inside the productChange handler by looking up the selected product by id or by
passing the whole product object separately (e.g., emit the selected product to
productChange) so the productId control remains a numeric id for
preview/submission.
Comment on lines
+78
to
+89
| ::ng-deep .loan-product-select-panel { | ||
| min-width: max(600px, 100%) !important; | ||
|
|
||
| .mat-mdc-option { | ||
| height: auto !important; | ||
| padding: 10px 16px !important; | ||
| } | ||
|
|
||
| .mdc-list-item__primary-text { | ||
| width: 100%; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and check its content
cat -n src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss | sed -n '70,100p'Repository: openMF/web-app
Length of output: 504
🏁 Script executed:
# Check if stylelint is configured and what its rules are
if [ -f ".stylelintrc" ] || [ -f ".stylelintrc.json" ] || [ -f ".stylelintrc.js" ]; then
ls -la .stylelint* 2>/dev/null || true
head -50 .stylelintrc* 2>/dev/null || true
fi
# Also check package.json for stylelint config
grep -A 20 '"stylelint"' package.json 2>/dev/null || trueRepository: openMF/web-app
Length of output: 3388
🏁 Script executed:
# Search for other uses of ::ng-deep in the codebase to understand patterns
rg '::ng-deep' src/app --type scss -n | head -20Repository: openMF/web-app
Length of output: 86
🏁 Script executed:
# Look for any global stylesheet setup or overlay styles
find src -name "*global*" -o -name "*theme*" -o -name "*overlay*" | grep -E '\.(scss|css)$'Repository: openMF/web-app
Length of output: 638
🏁 Script executed:
# Run stylelint on the specific file
cd src/app/loans/loans-account-stepper/loans-account-details-step && npx stylelint loans-account-details-step.component.scss 2>&1Repository: openMF/web-app
Length of output: 1143
🏁 Script executed:
# Search for other overlay or panel style patterns using grep instead
rg 'ng-deep|overlay|panel' src/app --type-list | grep -i scss
rg '(overlay|panel|CDK)' src --glob='*.scss' -l | head -20Repository: openMF/web-app
Length of output: 1223
🏁 Script executed:
# Look for mat-select-panel or similar Material overlay styling patterns
rg 'mat-select-panel|mat-autocomplete-panel|panelClass|CDK' src/app --glob='*.scss' -B 2 -A 2 | head -50Repository: openMF/web-app
Length of output: 419
🏁 Script executed:
# Check if there's a global styles file or if Material overrides go elsewhere
find src -name "styles.scss" -o -name "global*" -o -name "*overlays*" | grep -E '\.(scss|css)$'Repository: openMF/web-app
Length of output: 154
🏁 Script executed:
# Look for panelClass usage in components to understand the styling approach
rg 'panelClass|panelWidth' src/app --glob='*.ts' -B 1 -A 1 | head -30Repository: openMF/web-app
Length of output: 1912
🌐 Web query:
Angular Material CDK overlay panel styling best practices ::ng-deep
💡 Result:
Best practices for styling Angular Material/CDK overlays (without ::ng-deep)
-
Prefer
panelClass/backdropClassover CSS piercing- CDK overlays support adding custom CSS classes directly to the overlay pane/backdrop via
panelClass/backdropClass(oraddPanelClass). Style those classes from a global stylesheet. [3] - With
cdkConnectedOverlay, use itspanelClass/backdropClassinputs (same idea: attach a class, then style it globally). [3]
- CDK overlays support adding custom CSS classes directly to the overlay pane/backdrop via
-
Put overlay styles in a global stylesheet (e.g.,
styles.scss)- Overlay content is rendered in the overlay container (typically attached to the end of
body), so component-scoped styles often won’t reach it reliably—global styles + a uniquepanelClassare the clean approach. [3]
- Overlay content is rendered in the overlay container (typically attached to the end of
-
Include the CDK overlay structural CSS
- If you’re using CDK overlay without Angular Material’s theme bundle, import the required prebuilt overlay styles globally:
@import '@angular/cdk/overlay-prebuilt.css';[2]
- If you’re using CDK overlay without Angular Material’s theme bundle, import the required prebuilt overlay styles globally:
-
Use
::ng-deeponly as a last resort—and scope it- Angular strongly discourages new use of
::ng-deep; it effectively turns the part after it into global matching. [1] - If you must use it, scope it as tightly as possible (e.g.
:host ::ng-deep ...) so it only applies under that component’s host element. [1]
- Angular strongly discourages new use of
-
If you truly need “global” behavior, be explicit
ViewEncapsulation.Nonemakes a component’s styles global—use sparingly and intentionally (it’s clearer than sprinkling deep selectors everywhere). [1]
Rule of thumb: for overlays/menus/dialogs/tooltips, use panelClass + global CSS; reserve :host ::ng-deep for edge cases where the API doesn’t expose a styling hook. [1][3]
Sources: [1] Angular styling guide, [2] CDK overlay overview, [3] CDK overlay API docs.
🌐 Web query:
stylelint-config-standard-scss ::ng-deep selector-pseudo-element rule
💡 Result:
stylelint-config-standard-scss enables Stylelint’s selector-pseudo-element-no-unknown, so Angular’s non‑standard ::ng-deep gets flagged as “Unexpected unknown pseudo-element selector”. Angular documents ::ng-deep as a custom selector used to pierce view encapsulation. [2]
Allow it by overriding the rule and ignoring that pseudo-element name:
{
"extends": ["stylelint-config-standard-scss"],
"rules": {
"selector-pseudo-element-no-unknown": [true, { "ignorePseudoElements": ["ng-deep"] }]
}
}ignorePseudoElements is an official option of selector-pseudo-element-no-unknown. [1] This exact ignore pattern is commonly used for ::ng-deep. [3]
Sources: [1] https://stylelint.io/user-guide/rules/selector-pseudo-element-no-unknown • [2] https://angular.dev/guide/components/styling • [3] https://stackoverflow.com/questions/61500435/unexpected-unknown-pseudo-element-selector-ng-deep
🏁 Script executed:
# Check what the npm run lint script actually does
grep -A 5 '"lint"' package.jsonRepository: openMF/web-app
Length of output: 442
🏁 Script executed:
# Look at the component TypeScript to understand how the panel is styled
cat -n src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.ts | head -100Repository: openMF/web-app
Length of output: 4613
🏁 Script executed:
# Check if there's a mat-select panelClass being used for this component
rg 'loan-product-select|panelClass' src/app/loans --glob='*.ts' -B 2 -A 2 | head -40Repository: openMF/web-app
Length of output: 40
Move the loan-product-select-panel styles out of ::ng-deep and style via panelClass in a global stylesheet.
The ::ng-deep pseudo-element will fail stylelint's selector-pseudo-element-no-unknown rule (enabled by stylelint-config-standard-scss in .stylelintrc.json), blocking npm run lint. Instead, add panelClass="loan-product-select-panel" to the <mat-select> element in the template, then move these styles to a global stylesheet (e.g., src/main.scss or src/theme/mifosx-theme.scss) since CDK overlays are rendered outside the component's DOM.
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 78-78: Unexpected unknown pseudo-element selector "::ng-deep" (selector-pseudo-element-no-unknown)
(selector-pseudo-element-no-unknown)
🤖 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-details-step/loans-account-details-step.component.scss`
around lines 78 - 89, Remove the ::ng-deep .loan-product-select-panel block from
loans-account-details-step.component.scss, add
panelClass="loan-product-select-panel" to the <mat-select> in the
loans-account-details-step component template, and move the CSS rules for
.loan-product-select-panel (including nested .mat-mdc-option and
.mdc-list-item__primary-text rules) into a global stylesheet used for overlays
(e.g., your main/theme global SCSS) so the CDK overlay options are styled
without using ::ng-deep and without triggering stylelint.
Comment on lines
+95
to
+98
| const productType = this.route.snapshot.queryParamMap.get('productType') || null; | ||
| if (productType) { | ||
| this.loanProductService.initialize(productType); | ||
| } |
There was a problem hiding this comment.
Initialize the product context unconditionally.
Lines 95-98 only seed LoanProductService when the query param exists, but the rest of this component later branches on loanProductService.isLoanProduct. On direct loads or bookmarked URLs without productType, the rendered rows depend on service defaults/prior state instead of the current loan.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/general-tab/general-tab.component.ts` around lines
95 - 98, The component only calls LoanProductService.initialize when the query
param exists, causing stale/default product context; always call
this.loanProductService.initialize(...) with the productType value retrieved
from this.route.snapshot.queryParamMap.get('productType') (which may be null) so
the service is seeded unconditionally; update the logic around the productType
variable in general-tab.component.ts to invoke
loanProductService.initialize(productType) regardless of whether productType is
truthy so subsequent branches that check loanProductService.isLoanProduct
reflect the current loan context.
src/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.ts
Show resolved
Hide resolved
Comment on lines
+92
to
94
| this.loanService.submitLoanActionButton(this.loanId, data, 'waiveinterest').subscribe((response: any) => { | ||
| this.gotoLoanDefaultView(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify typing/error-handling conventions for loan action submissions
rg -nP --type=ts -C3 'submitLoanActionButton\s*\(' src/app/loans/loans-view/loan-account-actions
rg -nP --type=ts -C3 '\.subscribe\s*\(' src/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.ts
# Inspect base component contract used by this component
rg -nP --type=ts -C3 'class\s+LoanAccountActionsBaseComponent|loanId|loanService|gotoLoanDefaultView|dataObject' \
src/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.tsRepository: openMF/web-app
Length of output: 11934
Add error handling to the submit callback.
The subscription on line 92 lacks error handling, causing failed submissions to fail silently. Switch to the observer form with an error callback to ensure failures are surfaced via the existing error/notification flow. The response parameter is unused; omit it in the next callback.
Suggested patch
- this.loanService.submitLoanActionButton(this.loanId, data, 'waiveinterest').subscribe((response: any) => {
- this.gotoLoanDefaultView();
- });
+ this.loanService.submitLoanActionButton(this.loanId, data, 'waiveinterest').subscribe({
+ next: () => this.gotoLoanDefaultView(),
+ error: () => {
+ // Surface API failure via the existing notification/error flow.
+ }
+ });📝 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
| this.loanService.submitLoanActionButton(this.loanId, data, 'waiveinterest').subscribe((response: any) => { | |
| this.gotoLoanDefaultView(); | |
| }); | |
| this.loanService.submitLoanActionButton(this.loanId, data, 'waiveinterest').subscribe({ | |
| next: () => this.gotoLoanDefaultView(), | |
| error: () => { | |
| // Surface API failure via the existing notification/error flow. | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.ts`
around lines 92 - 94, The subscription to loanService.submitLoanActionButton in
waive-interest.component.ts currently uses only next and ignores errors; change
it to the observer form subscribe({ next: () => { this.gotoLoanDefaultView(); },
error: (err) => { /* use existing error/notification flow to surface the failure
*/ } }) so failures are handled instead of failing silently, remove the unused
response parameter in the next callback, and call the component's existing
error/notification handler (or process the error via the same service) inside
the error callback.
Comment on lines
+10
to
+11
| import { Component, OnInit, inject } from '@angular/core'; | ||
| import { UntypedFormGroup, UntypedFormBuilder, Validators } from '@angular/forms'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify strict-typing gaps in this component (expect no matches after fix).
rg -n "UntypedFormBuilder|UntypedFormGroup|: any\\b" src/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.tsRepository: openMF/web-app
Length of output: 364
Migrate to typed reactive forms in this refactored component.
The component retains UntypedFormBuilder and UntypedFormGroup alongside the new base-class structure. Replace with typed forms to maintain consistency with strict typing conventions. Also remove the any type from the submit subscription callback.
Suggested migration
-import { UntypedFormGroup, UntypedFormBuilder, Validators } from '@angular/forms';
+import { FormBuilder, FormControl, FormGroup, Validators } from '@angular/forms';
...
- private formBuilder = inject(UntypedFormBuilder);
+ private formBuilder = inject(FormBuilder);
...
- withdrawnByClientLoanForm: UntypedFormGroup;
+ withdrawnByClientLoanForm: FormGroup<{
+ withdrawnOnDate: FormControl<Date | null>;
+ note: FormControl<string | null>;
+ }>;
...
- this.loanService.loanActionButtons(this.loanId, 'withdrawnByApplicant', data).subscribe((response: any) => {
+ this.loanService.loanActionButtons(this.loanId, 'withdrawnByApplicant', data).subscribe(() => {
this.gotoLoanDefaultView();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.ts`
around lines 10 - 11, The component is still importing and using
UntypedFormBuilder/UntypedFormGroup and an any-typed submit callback; replace
those with typed reactive forms: import FormBuilder and FormGroup (from
`@angular/forms`), define an interface for the form value (e.g.,
WithdrawnByClientFormModel) and change the form property to
FormGroup<WithdrawnByClientFormModel>, update any usages that build the form to
use FormBuilder (inject(FormBuilder) or constructor injection) and pass the
generic type when calling group(...), and change the submit subscription
callback parameter from any to the concrete WithdrawnByClientFormModel (or its
specific field types). Update references to the form value (e.g.,
this.form.value) to be strongly typed as needed and remove the any type from the
submit handler subscription.
Comment on lines
+127
to
148
| this.loanDatatables = this.loanProductService.isLoanProduct ? data.loanDatatables : []; | ||
| this.loanStatus = this.loanDetailsData.status; | ||
| this.loanSubStatus = this.loanDetailsData.subStatus === undefined ? null : this.loanDetailsData.subStatus; | ||
| this.currency = this.loanDetailsData.currency; | ||
| loansService.saveLoanDisbursementDetailsData(this.loanDetailsData.disbursementDetails); | ||
| if (this.loanStatus.active) { | ||
| this.loanDetailsData.transactions.forEach((lt: LoanTransaction) => { | ||
| if (!lt.manuallyReversed) { | ||
| if (lt.type.reAge) { | ||
| this.loanReAged = true; | ||
| } else if (lt.type.reAmortize) { | ||
| this.loanReAmortized = true; | ||
| if (this.loanProductService.isLoanProduct) { | ||
| this.loanDisplayArrearsDelinquency = data.loanArrearsDelinquencyConfig.value || 0; | ||
| this.loanSubStatus = this.loanDetailsData.subStatus === undefined ? null : this.loanDetailsData.subStatus; | ||
| loansService.saveLoanDisbursementDetailsData(this.loanDetailsData.disbursementDetails); | ||
| if (this.loanStatus.active) { | ||
| this.loanDetailsData.transactions.forEach((lt: LoanTransaction) => { | ||
| if (!lt.manuallyReversed) { | ||
| if (lt.type.reAge) { | ||
| this.loanReAged = true; | ||
| } else if (lt.type.reAmortize) { | ||
| this.loanReAmortized = true; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
| // Filter datatables based on entity datatable checks | ||
| this.filterDatatablesByProduct(); | ||
| } | ||
| this.setConditionalButtons(); |
There was a problem hiding this comment.
Finish the non-loan branch before reusing the shared loan view state.
In Lines 127-147 the non-loan path clears loanDatatables but never flips datatablesReady, and Line 148 still calls setConditionalButtons() even though the loan-only transaction scan was skipped. That leaves the view half-initialized and forces the default Re-Age/Re-Amortize path for non-loan products.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/loans-view.component.ts` around lines 127 - 148,
When loanProductService.isLoanProduct is false the code clears loanDatatables
but doesn't finish initialization: set datatablesReady = true in the non-loan
branch, explicitly reset loanReAged = false and loanReAmortized = false (so
prior loan state doesn't leak into non-loan views), and ensure
setConditionalButtons() is only invoked after these non-loan initializations
(mirror the end-of-loan branch ordering). Update code paths around
loanDatatables, datatablesReady, loanReAged, loanReAmortized, and the call to
setConditionalButtons() to guarantee the non-loan branch fully initializes view
state before reuse.
71786a6 to
e8eb698
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (23)
src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss (1)
78-89:⚠️ Potential issue | 🟠 MajorRemove
::ng-deepoverlay styling and migrate this panel style to globalpanelClassstyling.
Line 78triggers stylelint (selector-pseudo-element-no-unknown) andLine 79forces a 600px minimum that can overflow narrow screens. Move the panel styles to a global stylesheet and bind viapanelClasson the select.Suggested fix
-::ng-deep .loan-product-select-panel { - min-width: max(600px, 100%) !important; - - .mat-mdc-option { - height: auto !important; - padding: 10px 16px !important; - } - - .mdc-list-item__primary-text { - width: 100%; - } -} +/* moved to global stylesheet (e.g., src/theme/mifosx-theme.scss) via panelClass */+/* src/theme/mifosx-theme.scss */ +.loan-product-select-panel { + width: clamp(280px, calc(100vw - 32px), 600px); + max-width: calc(100vw - 32px); + + .mat-mdc-option { + height: auto; + padding: 8px 16px; + } + + .mdc-list-item__primary-text { + width: 100%; + } +}+<!-- loans-account-details-step.component.html --> +<mat-select panelClass="loan-product-select-panel" ...>#!/bin/bash # Verify lint failure and expected migration points (read-only checks) set -euo pipefail npx stylelint src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss || true rg -n 'panelClass\s*=\s*["'"'"']loan-product-select-panel["'"'"']' src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.html rg -n 'loan-product-select-panel' src/main.scss src/theme/mifosx-theme.scss🤖 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-details-step/loans-account-details-step.component.scss` around lines 78 - 89, Remove the ::ng-deep block from the component SCSS and migrate those selectors into a global stylesheet (e.g., main/theme SCSS) under the class name loan-product-select-panel, then bind that class to the select via panelClass="loan-product-select-panel" in the template; in the migrated styles replace min-width: max(600px, 100%) !important with responsive constraints (e.g., width: 100% and max-width: 600px) to avoid overflow, keep the .mat-mdc-option and .mdc-list-item__primary-text rules but remove !important flags where possible, and ensure the component template uses panelClass to apply the global loan-product-select-panel styles.src/assets/translations/en-US.json (1)
3583-3583:⚠️ Potential issue | 🟡 MinorUse product-neutral empty-state text.
Line 3583 still says
"No Loan Product was found", which is too narrow for a selector that now includes multiple product types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/en-US.json` at line 3583, The empty-state string currently keyed as "No Loan Product was found" is too specific; update the translation value for "No Loan Product was found" to a product-neutral phrase such as "No products were found" (or "No products available") so the selector correctly reflects multiple product types; keep the key name as-is to avoid breaking lookups and only change the value in en-US.json.src/app/loans/common-resolvers/loan-details.resolver.ts (1)
33-46:⚠️ Potential issue | 🟠 MajorResolver can still return
undefinedfor invalidloanId.Line 46 falls through without returning an
Observable, which can break route resolution flow.🛠️ Minimal fix
-import { Observable } from 'rxjs'; +import { EMPTY, Observable } from 'rxjs'; ... if (!isNaN(+loanId)) { if (resolvedProductType === LOAN_PRODUCT_TYPE.LOAN) { return this.loansService.getLoanAccountAssociationDetails(loanId); } else { return this.loansService.getWorkingCapitalLoannDetails(loanId); } } + return EMPTY;Based on learnings: Applies to
src/app/**/*.{service.ts,component.ts,resolver.ts}: “Use RxJS Observables and route resolvers to fetch and pass data rather than relying on a unified predictable state container”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/common-resolvers/loan-details.resolver.ts` around lines 33 - 46, The resolver resolve(...) can fall through and return undefined when loanId is invalid; update resolve in loan-details.resolver.ts to always return an Observable by handling the invalid loanId case (e.g., import and return rxjs EMPTY or of(null) or return throwError(...) after optional logging/navigation) instead of falling through; ensure the change is applied alongside existing logic that calls loanProductService.initialize(resolvedProductType) and the branches that call loansService.getLoanAccountAssociationDetails(loanId) or loansService.getWorkingCapitalLoannDetails(loanId) so resolve consistently returns an Observable<any> in all code paths.src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts (1)
80-80:⚠️ Potential issue | 🟠 MajorGuard
loanProductsBasicDetailsbefore calling.find().If the input is missing, Line 139 can throw before optional chaining helps.
🛠️ Hardening patch
- `@Input`() loanProductsBasicDetails: LoanProductBasicDetails[]; + `@Input`() loanProductsBasicDetails: LoanProductBasicDetails[] = []; ... - const product = this.loanProductsBasicDetails.find((p) => p.productType === productType && p.id === id); + const product = (this.loanProductsBasicDetails ?? []).find((p) => p.productType === productType && p.id === id);Also applies to: 137-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts` at line 80, The code calls loanProductsBasicDetails.find(...) without ensuring loanProductsBasicDetails is defined, which can throw if the `@Input` is missing; fix by guarding all usages (around the .find calls at the locations currently using loanProductsBasicDetails, e.g., the expressions at lines ~137-140) — either short-circuit early (if (!loanProductsBasicDetails) return or set a sensible fallback) or replace each call with a safe expression like (loanProductsBasicDetails || []).find(...) (or use optional chaining with a fallback) so .find is never invoked on undefined.src/app/clients/clients-view/general-tab/general-tab.component.ts (2)
280-283:⚠️ Potential issue | 🟠 MajorPerformance history still excludes the merged working-capital list.
Line 293 computes history from
data.clientAccountsDatainstead of the unifiedthis.loanAccounts, so loan-cycle/active-loan cards can remain incorrect.🔧 Proposed fix
- this.computePerformanceHistory(data.clientAccountsData ?? { loanAccounts: [], savingsAccounts: [] }); + this.computePerformanceHistory({ + ...(data.clientAccountsData ?? {}), + loanAccounts: this.loanAccounts, + savingsAccounts: this.savingAccounts + });Also applies to: 293-293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around lines 280 - 283, The performance history is being computed from data.clientAccountsData instead of the merged list stored on this.loanAccounts, so the merged working-capital entries get excluded; update the logic that computes history (the code that currently reads from data.clientAccountsData to build loan-cycle/active-loan cards) to use this.loanAccounts (which is populated via processLoanAccounts) as the source of truth, ensuring any references to workingCapitalLoanAccounts are either merged into or derived from this.loanAccounts so the history and cards reflect the unified list.
432-438:⚠️ Potential issue | 🟠 MajorAvoid
map()side effects and ensure explicitproductTypewins.Line 433 uses
map()without returning values (lint failure), and Lines 435-436 allowaccount.productTypeto overwrite the intended explicit type.🔧 Proposed fix
private processLoanAccounts(accounts: any[], productType: string): void { - accounts.map((account: any) => { - this.loanAccounts.push({ - productType: productType, - ...account - }); - }); + this.loanAccounts.push( + ...accounts.map((account: any) => ({ + ...account, + productType + })) + ); }#!/bin/bash set -e npx biome check src/app/clients/clients-view/general-tab/general-tab.component.ts rg -n -C2 'processLoanAccounts|accounts\.map\(|productType' src/app/clients/clients-view/general-tab/general-tab.component.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around lines 432 - 438, The processLoanAccounts function is using Array.map for side effects and spreading account last which allows account.productType to overwrite the intended productType; change accounts.map(...) to accounts.forEach(...) (or a for...of loop) to avoid the lint error and push into this.loanAccounts with the explicit productType placed after the spread so it wins (i.e., push({ ...account, productType })) in the processLoanAccounts method so productType is always the provided value.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (2)
315-319:⚠️ Potential issue | 🟠 MajorRepatch working-capital term fields on product change.
This branch updates
loansAccountTermsDatametadata only. When product changes after init, term values can stay stale in the form.🔧 Proposed fix
} else if (this.loanProductService.isWorkingCapital && this.loansAccountProductTemplate) { this.loansAccountTermsData = this.loansAccountProductTemplate; this.currency = this.loansAccountTermsData.currency; this.termFrequencyTypeData = this.loansAccountTermsData.periodFrequencyTypeOptions; + this.loansAccountTermsForm.patchValue({ + principalAmount: this.loansAccountTermsData.principal, + periodPaymentRate: this.loansAccountTermsData.periodPaymentRate, + repaymentEvery: this.loansAccountTermsData.repaymentEvery, + repaymentFrequencyType: this.loansAccountTermsData.repaymentFrequencyType?.id + }); }🤖 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 315 - 319, The current branch only replaces the metadata fields (loansAccountTermsData, currency, termFrequencyTypeData) when switching to a working-capital product, leaving the reactive form controls stale; update the actual form values after assigning loansAccountTermsData by patching the LoansAccountTerms form/group controls with the corresponding values from loansAccountTermsData (e.g., term, termAmount, currency, periodFrequencyType, and any other term-related controls) so the UI and model stay in sync — locate the branch in loans-account-terms-step.component.ts where loansAccountProductTemplate is assigned and call the form's patchValue/ setValue (or individual control updates) to mirror loansAccountTermsData into the form.
341-349:⚠️ Potential issue | 🔴 CriticalPrevent duplicate
addControlfor delinquency toggle.
enableInstallmentLevelDelinquencyis added in multiple lifecycle paths; this can throw “control already exists” at runtime.🔧 Proposed fix
- if (this.isDelinquencyEnabled()) { + if ( + this.isDelinquencyEnabled() && + !this.loansAccountTermsForm.contains('enableInstallmentLevelDelinquency') + ) { this.loansAccountTermsForm.addControl( 'enableInstallmentLevelDelinquency', new UntypedFormControl( this.loansAccountTermsData.enableInstallmentLevelDelinquency || this.loanProduct.enableInstallmentLevelDelinquency ) ); }🤖 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 341 - 349, The delinquency toggle control enableInstallmentLevelDelinquency is being added multiple times and can cause "control already exists" errors; update the logic in the component (around isDelinquencyEnabled(), loansAccountTermsForm, loansAccountTermsData, and loanProduct usage) to guard against duplicates by checking loansAccountTermsForm.hasControl('enableInstallmentLevelDelinquency') before calling addControl, or use setControl/replace logic to safely replace an existing control instead of blindly adding it.src/app/clients/clients-view/general-tab/general-tab.component.html (1)
329-337:⚠️ Potential issue | 🟠 MajorGuard the closed-loans Type icon for non-loan products.
Line 334 still dereferences
element.loanType.valueunconditionally. With mixed product rows, this can break rendering for working-capital entries.🔧 Proposed fix
<ng-container matColumnDef="Type"> <th mat-header-cell class="center" *matHeaderCellDef>{{ 'labels.inputs.Type' | translate }}</th> <td mat-cell class="center" *matCellDef="let element"> - <i - class="fa fa-large" - [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" - matTooltip=" {{ element.loanType.value }}" - matTooltipPosition="above" - ></i> + `@if` (element.productType === 'loan') { + <i + class="fa fa-large" + [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" + matTooltip="{{ element.loanType.value }}" + matTooltipPosition="above" + ></i> + } </td> </ng-container>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.html` around lines 329 - 337, The template dereferences element.loanType.value unconditionally in the ngClass and matTooltip bindings, which breaks for non-loan products; update those bindings to guard against missing loanType (e.g., use the safe navigation operator or a ternary check) so ngClass becomes conditional on element.loanType?.value (or element.loanType ? element.loanType.value : default) and matTooltip only displays when element.loanType exists (fallback to empty string or a default label); locate the bindings referenced as element.loanType.value in the <td mat-cell> block (the ngClass and matTooltip attributes) and apply the null-safe checks consistently.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html (2)
742-744:⚠️ Potential issue | 🟡 MinorAdd
type="button"and accessible label to the "Add Collateral" button.This button defaults to
type="submit", which can cause unintended form submissions. Additionally, as an icon-accompanied button, it should have an accessible label for screen readers.♿ Proposed accessibility fix
- <button mat-raised-button color="primary" (click)="addCollateral()"> + <button type="button" mat-raised-button color="primary" (click)="addCollateral()" aria-label="{{ 'labels.buttons.Add' | translate }}"> <fa-icon icon="plus" class="m-r-10"></fa-icon>{{ 'labels.buttons.Add' | translate }} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 742 - 744, The "Add Collateral" button that calls addCollateral() should be changed to avoid implicit submit and to be accessible: set the button attribute type="button" and add an accessible label (e.g. aria-label bound to a translation key like 'labels.buttons.AddCollateral' or a descriptive string) so screen readers can announce its purpose while keeping the visible icon+text intact.
772-774:⚠️ Potential issue | 🟡 MinorAdd
type="button"and accessible label to the delete collateral button.This icon-only button defaults to
type="submit"and lacks an accessible name for screen reader users.♿ Proposed accessibility fix
- <button mat-icon-button color="warn" (click)="deleteCollateral(collateralIndex)"> + <button type="button" mat-icon-button color="warn" (click)="deleteCollateral(collateralIndex)" aria-label="{{ 'tooltips.Delete' | translate }}" matTooltip="{{ 'tooltips.Delete' | translate }}"> <fa-icon icon="trash"></fa-icon> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 772 - 774, The delete icon button in loans-account-terms-step.component.html (the button that calls deleteCollateral(collateralIndex)) needs an explicit type and an accessible name: change the element to include type="button" to prevent it acting as a submit, and add an accessible label such as aria-label="Delete collateral" (or matTooltip with aria-label) so screen readers can announce its purpose.src/app/loans/loans-view/general-tab/general-tab.component.ts (2)
95-98:⚠️ Potential issue | 🟠 MajorInitialize the product context unconditionally.
The
loanProductService.initialize()is only called whenproductTypeis truthy. On direct navigation or bookmarked URLs without theproductTypequery parameter, the service remains uninitialized, causingisLoanProductchecks later in the component to rely on stale or default state.🔧 Suggested fix
constructor() { super(); const productType = this.route.snapshot.queryParamMap.get('productType') || null; - if (productType) { - this.loanProductService.initialize(productType); - } + this.loanProductService.initialize(productType);Ensure
LoanProductService.initialize()handlesnullgracefully, or provide a default derived fromloanDetails.loanTypeafter the route data subscription resolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/general-tab/general-tab.component.ts` around lines 95 - 98, The component currently only calls loanProductService.initialize(productType) when productType is truthy, so initialize must be invoked unconditionally; change the logic to always call loanProductService.initialize(...) with either the query param or a fallback derived from loanDetails.loanType after loanDetails is available (i.e., in the route/loanDetails subscription) and ensure loanProductService.initialize accepts null/undefined safely — locate the call to this.route.snapshot.queryParamMap.get('productType') and the route data/loanDetails subscription in this component, always call loanProductService.initialize(productType || derivedLoanType) there, or modify LoanProductService.initialize to handle a null argument gracefully and update references to isLoanProduct accordingly.
290-294:⚠️ Potential issue | 🟡 MinorUse the initialized product context as the source of truth.
The
loanProductType()method derives the label fromloanDetails.loanType, creating a second source of truth that can drift from the route context. The component already seedsLoanProductServicefrom the route, so use that as the authoritative source.♻️ Proposed fix
loanProductType(): string { - return this.loanDetails.loanType - ? LoanProductService.productTypeLabel('loan') - : LoanProductService.productTypeLabel('working-capital'); + return LoanProductService.productTypeLabel( + this.loanProductService.isLoanProduct ? 'loan' : 'working-capital' + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/general-tab/general-tab.component.ts` around lines 290 - 294, The loanProductType() method should use the initialized product context on LoanProductService rather than loanDetails.loanType; update loanProductType() to read the product type from LoanProductService (e.g. via LoanProductService.getProductContext() or LoanProductService.productContext) and then call LoanProductService.productTypeLabel(...) based on that context, removing reliance on loanDetails.loanType to avoid divergence from the route-seeded source of truth.src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.html (1)
34-42:⚠️ Potential issue | 🔴 CriticalKeep
productIdbound to the actual product id, notshortName.The
mat-optionvalue is set toproduct.shortName, but the form control is namedproductId. This changes the payload shape from a numeric id to a string short name, which will break downstream logic (preview step, submission) that expectsproduct.id.🔧 Suggested fix
`@for` (product of productData | async; track product) { - <mat-option [value]="product.shortName" class="product-option"> + <mat-option [value]="product.id" class="product-option">If
productChange()needs the short name for display or other logic, derive it from the selected product object within the handler instead of overloading theproductIdcontrol.🤖 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-details-step/loans-account-details-step.component.html` around lines 34 - 42, The mat-option is binding product.shortName to the productId form control, which sends a string shortName instead of the numeric product.id; change the option value to the actual id (use product.id as the value for the mat-option) so the form control productId receives the correct numeric id, and if productChange() needs the shortName derive it from the selected product (from productData or by looking up the product by id) rather than overloading productId.src/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.ts (1)
87-91:⚠️ Potential issue | 🟠 MajorHandle non-loan rejection explicitly; current branch still no-ops.
For working-capital accounts, submit currently performs no request and no feedback. Either invoke the appropriate working-capital reject action (or shared reject action if backend supports both) or block access to this action for non-loan products so users cannot submit a dead flow.
Possible fix (if reject endpoint is shared)
- if (this.isLoanProduct) { - this.loanService.loanActionButtons(this.loanId, 'reject', data).subscribe((response: any) => { - this.gotoLoanDefaultView(); - }); - } + this.loanService.loanActionButtons(this.loanId, 'reject', data).subscribe((response: any) => { + this.gotoLoanDefaultView(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.ts` around lines 87 - 91, The branch only handles loan products (isLoanProduct) and silently does nothing for non-loan accounts; add an explicit else branch to handle working-capital accounts or block the action: if the backend exposes a shared reject endpoint, call loanService.loanActionButtons(this.loanId, 'reject', data) for non-loan as well (or call the working-capital specific method, e.g., workingCapitalService.rejectAccount(this.loanId, data)); otherwise disable/return an error for non-loan products so the submit cannot no-op. Ensure the else branch either invokes the appropriate service method and subscribes to navigate via gotoLoanDefaultView() on success, or emits a user-facing error/disable flow for non-loan cases.src/app/loans/loans-view/loans-view.component.ts (1)
127-148:⚠️ Potential issue | 🟠 MajorComplete non-loan initialization before using shared view state.
The non-loan path clears datatables but does not finalize datatable readiness and does not reset re-age/re-amortize flags before conditional-button setup. This can leave the view in an incomplete/stale state.
Suggested patch
- if (this.loanProductService.isLoanProduct) { + if (this.loanProductService.isLoanProduct) { this.loanDisplayArrearsDelinquency = data.loanArrearsDelinquencyConfig.value || 0; this.loanSubStatus = this.loanDetailsData.subStatus === undefined ? null : this.loanDetailsData.subStatus; loansService.saveLoanDisbursementDetailsData(this.loanDetailsData.disbursementDetails); if (this.loanStatus.active) { this.loanDetailsData.transactions.forEach((lt: LoanTransaction) => { if (!lt.manuallyReversed) { if (lt.type.reAge) { this.loanReAged = true; } else if (lt.type.reAmortize) { this.loanReAmortized = true; } } }); } // Filter datatables based on entity datatable checks this.filterDatatablesByProduct(); + } else { + this.loanReAged = false; + this.loanReAmortized = false; + this.datatablesReady = true; } this.setConditionalButtons();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loans-view.component.ts` around lines 127 - 148, When loanProductService.isLoanProduct is false the code currently clears this.loanDatatables but doesn't finish shared view initialization: ensure the non-loan branch also sets defaults and readiness before calling setConditionalButtons by resetting this.loanReAged = false and this.loanReAmortized = false, setting this.loanDisplayArrearsDelinquency = 0 (or a safe default) and this.loanSubStatus = null, and then call this.filterDatatablesByProduct() so datatable state is finalized; keep existing loan-product logic intact and perform these resets/filters whenever loanProductService.isLoanProduct is false, before invoking this.setConditionalButtons().src/app/loans/loans-view/loans-view.component.html (1)
55-61:⚠️ Potential issue | 🟠 MajorGuard the working-capital fallback before dereferencing
client.Group-owned working-capital loans can arrive without a
client, so this branch throws before the||fallback reachesgroup.Suggested patch
- {{ loanDetailsData.client.displayName || loanDetailsData.group?.name }} - ({{ loanDetailsData.client.accountNo || loanDetailsData.group?.accountNo }}) + {{ loanDetailsData.client?.displayName || loanDetailsData.group?.name }} + ({{ loanDetailsData.client?.accountNo || loanDetailsData.group?.accountNo }})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loans-view.component.html` around lines 55 - 61, The working-capital branch dereferences loanDetailsData.client (loanDetailsData.client.displayName and .accountNo) without guarding for null, causing exceptions for group-owned loans; update the loans-view.component.html branch keyed by loanProductService.isWorkingCapital to first check for client existence (e.g. use a conditional or optional chaining) and fall back to loanDetailsData.group?.name and loanDetailsData.group?.accountNo for both displayName and accountNo so the group fallback is reached safely when client is absent.src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html (2)
13-16:⚠️ Potential issue | 🔴 CriticalGuard client display access for non-client applicant paths.
At Line [15],
loansAccountProductTemplate.client.displayNameis accessed without safe navigation. This can break whenclientis absent.💡 Minimal template-safe fallback
- {{ loansAccountProductTemplate.clientName || loansAccountProductTemplate.client.displayName }} + {{ + loansAccountProductTemplate.clientName || + loansAccountProductTemplate.client?.displayName || + (loansAccountProductTemplate.applicants?.map(a => a.displayName).join(', ')) + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html` around lines 13 - 16, The template accesses loansAccountProductTemplate.client.displayName without a null-safe check which can throw when client is missing; update the binding for the client label (the expression using loansAccountProductTemplate.clientName and loansAccountProductTemplate.client.displayName) to use the Angular safe navigation operator and/or an explicit fallback so it reads the clientName or, if absent, loansAccountProductTemplate.client?.displayName (or a final empty/default string) to prevent runtime errors when client is undefined.
396-406:⚠️ Potential issue | 🟡 MinorMove
Unassignedfallback text behind translation keys.
'Unassigned'is user-visible and currently hardcoded, which bypasses localization.As per coding guidelines:
src/app/**/*.{html,ts}must use i18n variables from@ngx-translate/corefor user-facing strings, and runnpm run translations:extractwhen adding new strings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html` around lines 396 - 406, Replace the hardcoded "Unassigned" fallback in the loans-account-preview-step template with a translation key and ngx-translate pipe (e.g., change {{ (charge.dueDate | dateFormat) || 'Unassigned' }} to use a translation key like {{ (charge.dueDate | dateFormat) || ('COMMON.UNASSIGNED' | translate) }}, and do the same for feeOnMonthDay), add the new translation key(s) to your i18n JSON files, and then run npm run translations:extract to include the new strings; update any existing keys to follow project naming (e.g., COMMON.UNASSIGNED) so they are discoverable by the extractor.src/app/loans/create-loans-account/create-loans-account.component.ts (2)
181-183:⚠️ Potential issue | 🟠 MajorRead the current principal control in
loanPrincipalgetter.The getter still reads
principal, which can returnundefinedif the active control isprincipalAmount.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around lines 181 - 183, The loanPrincipal getter currently always reads the "principal" control and can return undefined when the active control is "principalAmount"; update the loanPrincipal getter to first check for the "principalAmount" control on loansAccountTermsStep.loansAccountTermsForm and return its value if present, otherwise fall back to the existing "principal" control (use loansAccountTermsStep.loansAccountTermsForm.get('principalAmount')?.value ?? loansAccountTermsStep.loansAccountTermsForm.get('principal')?.value); keep the null/undefined-safe checks around loansAccountTermsStep and the form.
120-141:⚠️ Potential issue | 🟠 MajorReset loan-only state before product-type branching in
setTemplate().When switching from a loan product to working capital, stale loan-specific state can remain (e.g., collateral/datatable/multi-disburse flags) because nothing is reset before the
isLoanProductbranch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around lines 120 - 141, Before branching on loanProductService.isLoanProduct inside setTemplate(), clear any loan-specific state to avoid stale data when switching products: reset this.collateralOptions (e.g., to [] or null), reset this.multiDisburseLoan (to false), and clear any datatable state (call an existing helper like clearDatatables() or explicitly reset the datatable properties) before running the existing loan branch that calls clientService.getCollateralTemplate(...), sets multiDisburseLoan and calls setDatatables(); keep the existing clientId/call logic but ensure the clears happen first so working-capital products don't inherit loan state.src/app/loans/edit-loans-account/edit-loans-account.component.ts (2)
123-135:⚠️ Potential issue | 🟠 MajorWorking-capital modify flow is still a dead end.
For working capital, validation returns
falseandsubmit()routes to an emptysubmitWorkingCapitalProduct()implementation, so users can’t complete a modify flow.Also applies to: 156-160, 247-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/edit-loans-account/edit-loans-account.component.ts` around lines 123 - 135, The working-capital modify flow is blocked because loansAccountFormValidAndNotPristine returns false for loanProductService.isWorkingCapital and submit() calls a no-op submitWorkingCapitalProduct(); update the getter loansAccountFormValidAndNotPristine to include the appropriate validation/pristine checks for working capital (mirror the loan product logic or the specific forms/steps used for working capital) and implement submitWorkingCapitalProduct() to perform the real submit path (validate forms, mark controls touched, call the existing save/submit service, and navigate/close on success). Ensure references: loansAccountFormValidAndNotPristine, loanProductService.isWorkingCapital, submit(), and submitWorkingCapitalProduct() are updated so the working-capital branch performs the same validation and submission flow as other product types.
139-154:⚠️ Potential issue | 🟠 MajorEnsure
loansAccountgetter always returns a value.This getter has execution paths with no return, so it can yield
undefinedand trip lint/runtime expectations.💡 Minimal fix
get loansAccount() { if (this.loanProductService.isLoanProduct) { return { ...this.loansAccountDetailsStep.loansAccountDetails, ...this.loansAccountTermsStep.loansAccountTerms, ...this.loansAccountChargesStep?.loansAccountCharges, ...this.loansAccountTermsStep.loanCollateral, ...this.loansAccountTermsStep.disbursementData }; } else if (this.loanProductService.isWorkingCapital) { return { ...this.loansAccountDetailsStep.loansAccountDetails, ...this.loansAccountTermsStep.loansAccountTerms }; } + return {}; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/edit-loans-account/edit-loans-account.component.ts` around lines 139 - 154, The loansAccount getter currently only returns for isLoanProduct and isWorkingCapital branches, leaving other execution paths undefined; update the getter (loansAccount) so it always returns an object—e.g., add a final fallback return (empty object or a composed default) and ensure optional spreads like loansAccountChargesStep?.loansAccountCharges are null-safe (use a default {} when spreading) so the getter never yields undefined; reference the existing symbols loanProductService.isLoanProduct, loanProductService.isWorkingCapital, loansAccountDetailsStep, loansAccountTermsStep, loansAccountChargesStep, loanCollateral, and disbursementData when implementing the fallback.
🧹 Nitpick comments (11)
src/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scss (1)
42-44: Align spacing values to the 8px grid and avoid hardcoded one-off pixel spacing.
Line 42,Line 44, andLine 62use4px;Line 56uses12px;Line 83uses10px. Prefer 8px-grid increments and shared theme spacing tokens for consistency.Suggested spacing normalization
.product-option-row { display: flex; flex-direction: column; align-items: flex-start; - gap: 4px; + gap: 8px; width: 100%; - padding: 4px 0; + padding: 8px 0; } .product-option-meta { display: flex; - gap: 12px; + gap: 16px; } .meta-item { display: flex; flex-direction: column; - gap: 4px; + gap: 8px; } .mat-mdc-option { height: auto !important; - padding: 10px 16px !important; + padding: 8px 16px !important; }As per coding guidelines: "Stick to the 8px grid system for visual design and spacing" and "Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values".Also applies to: 56-57, 62-63, 83-83
🤖 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-details-step/loans-account-details-step.component.scss` around lines 42 - 44, Update the hardcoded pixel spacing in loans-account-details-step.component.scss to the 8px grid and replace explicit values with the shared SCSS spacing variables from src/main.scss or src/theme/mifosx-theme.scss: change 4px → 8px, 12px → 16px, and 10px → 8px (or the nearest theme token), and replace occurrences of gap, padding and margin (the rules currently using gap: 4px; padding: 4px 0; any 12px and 10px usages) with the appropriate theme variables (e.g., $spacing-1 / $spacing-2 or the project’s equivalent) to ensure consistent spacing across components.src/assets/translations/ne-NE.json (1)
1-4155: Run Prettier before merge to satisfy repo policy.Please run
npx prettier --write .as the final step before closing the branch.As per coding guidelines,
src/**/*.{ts,tsx,js,jsx,html,scss,css,json}: Enforce Prettier formatting by runningnpx prettier --write .before closing any branch.🤖 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 1 - 4155, The JSON file src/assets/translations/ne-NE.json is not formatted per the repo Prettier policy; run the formatter and commit the result. Fix by running npx prettier --write . (or npx prettier --write src/assets/translations/ne-NE.json), review the changed formatting for this file, add the formatted file to git, and commit/push the update before merging; ensure CI passes and no other lint/format issues remain.src/app/loans/loans-view/account-details/account-details.component.html (1)
128-149: Consolidate repeated feature-flag blocks to reduce template duplication.
enableIncomeCapitalizationandenableBuyDownFeeare each checked multiple times; wrapping each section once will improve readability and maintenance.Also applies to: 156-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/account-details/account-details.component.html` around lines 128 - 149, The template repeats the same `@if` check (loanDetails.enableIncomeCapitalization and similarly enableBuyDownFee) across consecutive blocks; refactor by wrapping all related rows for each feature flag inside a single `@if` block (e.g., a single `@if` (loanDetails.enableIncomeCapitalization) { ... } that contains the three div.layout-row entries referencing capitalizedIncomeCalculationType, capitalizedIncomeStrategy and capitalizedIncomeType), and do the same for the enableBuyDownFee section covering lines 156-185 so each feature-flag is evaluated once and duplicated checks are removed.src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts (1)
84-94: Consider tracking API response typing as a separate enhancement.The
Observable<any>pattern forgetLoanDelinquencyDataForTemplateis part of a broader, project-wide typing pattern in this codebase. While introducing specific types for the delinquency response would improve type safety, this should be tracked as a cross-cutting refactoring effort across all API response layers (affecting multiple loan action components), rather than addressed in individual bug fix PRs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts` around lines 84 - 94, The code currently subscribes to getLoanDelinquencyDataForTemplate which returns Observable<any> (in view-guarantors via loanService) which reduces type safety; rather than changing it in this bugfix PR, create a tracked refactor: open an issue to introduce a DelinquencyResponse interface and update getLoanDelinquencyDataForTemplate and all callers (including viewGuarantorsComponent and other loan action components) to return Observable<DelinquencyResponse>, and add a short TODO comment above the subscription in view-guarantors.component.ts referencing that issue and the getLoanDelinquencyDataForTemplate method so the future type-refactor is discoverable.src/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.ts (1)
39-41: Member variable typing improvements are valid, but Observable response typing should follow project-wide patterns.The
templatemember variable can be safely typed asstring | nullsince it holds the sanitized HTML output. However, typing thegetTemplateDataObservable return type or the subscription response should not be addressed in this PR—the codebase usesObservable<any>as a project-wide pattern, and comprehensive API response typing is tracked as a separate cross-cutting refactoring effort rather than included in individual bug fixes.Consider typing only the component member variables for immediate clarity:
template: string | null = null;(holds the sanitized HTML string)templatesDatashould reflect the type of the resolver'sdataObject🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.ts` around lines 39 - 41, Change the component member typings only: set the template property to a nullable string (template: string | null = null) since it stores sanitized HTML, and narrow templatesData to the resolver dataObject type (use the appropriate interface or type alias for dataObject) instead of any; do not modify the Observable/getTemplateData return typing or subscription types in this PR (keep Observable<any> as-is).src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
31-33: Remove redundantloanIdfield declaration.The base class
LoanAccountActionsBaseComponentalready providesloanId. This local declaration shadows the inherited field and could cause confusion or unexpected behavior if not initialized consistently.♻️ Proposed fix
export class AdjustLoanChargeComponent extends LoanAccountActionsBaseComponent implements OnInit { private formBuilder = inject(UntypedFormBuilder); private organizationService = inject(OrganizationService); - /** Loan Id */ - loanId: string; chargeId: string;The inherited
loanIdfrom the base component should be used instead, ensuring consistency with other refactored action components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` around lines 31 - 33, Remove the redundant local loanId declaration in AdjustLoanChargeComponent so it uses the inherited loanId from LoanAccountActionsBaseComponent; locate the class definition for AdjustLoanChargeComponent in adjust-loan-charge.component.ts, delete the line declaring loanId: string (leave chargeId), and ensure no local references or initializations rely on the removed field so all code uses the base class's loanId property.src/app/loans/loans-view/loans-view.component.html (1)
510-519: Keep the datatable tab URLs product-aware too.Every other tab link now preserves
productType, but this block still drops it. Adding the same query param here keeps the URL shape consistent when users switch to a datatable tab.Suggested patch
<a mat-tab-link *mifosxHasPermission="'READ_' + loanDatatable.registeredTableName" [routerLink]="['./datatables', loanDatatable.registeredTableName]" + [queryParams]="{ + productType: loanProductService.productType.value + }" routerLinkActive `#datatable`="routerLinkActive" [active]="datatable.isActive" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loans-view.component.html` around lines 510 - 519, The datatable tab links inside the loan product block (check loanProductService.isLoanProduct, datatablesReady and the loanDatatables loop) are not preserving the productType query param; update the mat-tab-link/routerLink for each loanDatatable (use loanDatatable.registeredTableName) to include the current productType as queryParams so the link becomes product-aware (e.g., add queryParams binding that passes productType obtained from the component/state) ensuring URL shape matches other tabs.src/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.ts (1)
48-54: Reuse the shared product-type param constant in redirects.This helper is now the common navigation path for many action components, but it hardcodes
'productType'again. Reusing the shared product-type routing constant keeps these redirects aligned with the rest of the product-aware routing if that param name changes later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.ts` around lines 48 - 54, The navigation helper gotoLoanViewTab currently hardcodes the 'productType' query param; update it to import and use the shared product-type routing constant (the project’s shared product-type param constant) instead of the string literal, and pass loanProductService.productType.value as its value; adjust the import at the top of the file and replace the literal 'productType' in the queryParams object so redirects stay consistent with the rest of product-aware routing.src/app/loans/create-loans-account/create-loans-account.component.html (1)
123-126: Replace the native<i>icon with an Angular Material/icon component.Line [125] uses a native icon element. Please use a Material-based icon pattern for consistency with UI guidelines.
As per coding guidelines:
src/app/**/*.{html,ts}must use Angular Material elements instead of native HTML where possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.html` around lines 123 - 126, Replace the native <i class="fa fa-exclamation-circle alert-check"></i> in the create-loans-account component template with the Angular Material icon component (mat-icon) using an equivalent material icon (e.g., "error" or "warning"), preserve the "alert-check" class for styling, and ensure MatIconModule is imported in the module that declares CreateLoansAccountComponent so the <mat-icon> renders correctly; also remove or stop using the Font Awesome <i> here to keep UI consistent with material guidelines.src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts (1)
158-174: Use the base navigation helper in repayment submit for consistency.
submitContractTermination()now usesgotoLoanDefaultView(), butsubmitRepayment()still navigates directly at Line [173]. Using the same helper in both flows avoids divergent behavior and keeps product-type-aware navigation centralized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts` around lines 158 - 174, The repayment submit currently navigates directly via this.router.navigate in submitRepayment; replace that direct navigation with the centralized navigation helper used by other flows: call the existing gotoLoanDefaultView(...) helper (the same one used in submitContractTermination) instead of this.router.navigate to ensure product-type-aware behavior and consistent routing after the loanService.submitLoanActionButton response.src/app/loans/create-loans-account/create-loans-account.component.ts (1)
86-92: Tighten typing on new product fields/events.New members and handler parameters (
productDetails,productType,$event) currently use loose typing; please prefer concrete interfaces/unions in this new surface.As per coding guidelines:
src/app/**/*.tsshould follow strict typing conventions.Also applies to: 143-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/create-loans-account/create-loans-account.component.ts` around lines 86 - 92, The new fields and event parameters use any/null or loose typing—tighten them: replace productDetails: any with a concrete interface (e.g., use or create a LoanProductDetails interface that mirrors the API shape and assign productDetails: LoanProductDetails | null), change productType: string | null to a string union or enum type (e.g., ProductType = 'loan'|'savings'|... and productType: ProductType | null), and update any handler signatures that accept $event (the methods handling product selection) to use the appropriate typed parameter (e.g., (event: LoanProductBasicDetails) or (event: ProductType) instead of any); reuse existing LoanProductBasicDetails where applicable and update related members (loanProductsBasicDetails, productDetails, productType, and the selection/change handler methods) to these concrete types.
Comment on lines
203
to
+212
| get loansAccountDetails() { | ||
| return this.loansAccountDetailsForm.getRawValue(); | ||
| if (this.productSelected) { | ||
| const loanAccountDetails = { | ||
| ...this.loansAccountDetailsForm.getRawValue(), | ||
| productId: this.productSelected.id | ||
| }; | ||
| return loanAccountDetails; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Returning null from loansAccountDetails getter may cause downstream issues.
If productSelected is falsy, this getter returns null. Callers expecting an object (e.g., spread operations or property access) will fail. Consider returning a partial object or throwing an error to enforce valid state.
🛡️ Defensive alternative
get loansAccountDetails() {
- if (this.productSelected) {
- const loanAccountDetails = {
- ...this.loansAccountDetailsForm.getRawValue(),
- productId: this.productSelected.id
- };
- return loanAccountDetails;
- }
- return null;
+ if (!this.productSelected) {
+ throw new Error('Product must be selected before accessing loan account details');
+ }
+ return {
+ ...this.loansAccountDetailsForm.getRawValue(),
+ productId: this.productSelected.id
+ };
}🤖 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-details-step/loans-account-details-step.component.ts`
around lines 203 - 212, The loansAccountDetails getter currently returns null
when productSelected is falsy which breaks callers that expect an object; change
it to always return an object by spreading
this.loansAccountDetailsForm.getRawValue() and setting productId to
this.productSelected?.id (or undefined) so callers can safely access properties,
or alternatively throw a clear error from loansAccountDetails if you want to
enforce state — update the getter (loansAccountDetails) to return a partial
object instead of null and use this.loansAccountDetailsForm.getRawValue() and
this.productSelected?.id to populate fields.
| export class LoansAccountPreviewStepComponent extends LoanProductBaseComponent implements OnChanges { | ||
| /** Loans Account Template */ | ||
| @Input() loansAccountTemplate: any = []; | ||
| @Input() loansAccountTemplate: any; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify candidate interfaces/types for loans account template payloads
fd "loan*.model.ts" src/app | xargs -r rg -n "export interface .*Loan|Template"
rg -n "loansAccountTemplate" src/app/loans -C3Repository: openMF/web-app
Length of output: 44694
Use Record<string, unknown> instead of any for the input property.
The component currently violates strict typing conventions by using any. A precedent exists in the sibling component loans-account-schedule-step.component.ts which uses Record<string, unknown> for the same property. Apply this pattern to maintain type safety across the loans stepper components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.ts`
at line 73, The `@Input` property loansAccountTemplate on the
LoansAccountPreviewStep component is typed as any; change its type to
Record<string, unknown> to match the sibling LoansAccountScheduleStep pattern
and satisfy strict typing. Locate the property declaration " `@Input`()
loansAccountTemplate: any;" in loans-account-preview-step.component.ts and
update the type to Record<string, unknown>, keeping the `@Input` decorator and
property name unchanged so other references stay valid.
| import { Component, OnInit, Input, inject } from '@angular/core'; | ||
| import { UntypedFormGroup, UntypedFormBuilder, Validators, ReactiveFormsModule } from '@angular/forms'; | ||
| import { ActivatedRoute, Router, RouterLink } from '@angular/router'; | ||
| import { UntypedFormGroup, UntypedFormBuilder, Validators } from '@angular/forms'; |
There was a problem hiding this comment.
Avoid untyped forms and any in this component.
UntypedFormGroup/UntypedFormBuilder and response: any weaken compile-time safety in a newly touched path. Please migrate this form to typed reactive forms and remove the any callback type (e.g., subscribe(() => ...) if response is unused).
As per coding guidelines, src/app/**/*.ts: "Use TypeScript for all application code with strict typing conventions".
Also applies to: 42-43, 90-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.ts`
at line 11, Change the untyped reactive form and any-typed callback to strictly
typed equivalents: replace UntypedFormGroup and UntypedFormBuilder with
FormGroup and FormBuilder using an explicit interface for the form controls
(e.g., define an IWriteOffForm { amount: FormControl<number>, reason:
FormControl<string>, ... } and type the component form as
FormGroup<IWriteOffForm> and the builder as FormBuilder), update all places that
call this.form.get(...) to use typed accessors (or strongly-typed getters) and
correct control types, and remove any uses of response: any in subscribe
callbacks (use subscribe(() => ...) if the response is unused or declare the
proper response type interface and use it in subscribe((res: IWriteOffResponse)
=> ...)); apply these changes to the write-off component class
(WriteOffPageComponent), its form creation method (where FormBuilder is used),
and any subscribe handlers referenced in the component.
Comment on lines
49
to
55
| showValue() { | ||
| if (this.printChars == 30) { | ||
| if (this.printChars < 1000) { | ||
| this.printChars = 1000; | ||
| } else { | ||
| this.printChars = 30; | ||
| this.printChars = this.chars; | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded threshold may break toggle when chars >= 1000.
The toggle logic uses a hardcoded value of 1000 as both the comparison threshold and expansion target. If a consumer sets chars to 1000 or higher, the condition this.printChars < 1000 will be false on init (since ngOnInit sets printChars = this.chars), causing the toggle to immediately collapse back instead of expanding.
Consider using a computed expansion value relative to text length or a configurable max:
Suggested fix
+ private readonly EXPANDED_CHARS = 10000;
+
showValue() {
- if (this.printChars < 1000) {
- this.printChars = 1000;
+ if (this.printChars === this.chars) {
+ this.printChars = this.EXPANDED_CHARS;
} else {
this.printChars = this.chars;
}
}📝 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
| showValue() { | |
| if (this.printChars == 30) { | |
| if (this.printChars < 1000) { | |
| this.printChars = 1000; | |
| } else { | |
| this.printChars = 30; | |
| this.printChars = this.chars; | |
| } | |
| } | |
| private readonly EXPANDED_CHARS = 10000; | |
| showValue() { | |
| if (this.printChars === this.chars) { | |
| this.printChars = this.EXPANDED_CHARS; | |
| } else { | |
| this.printChars = this.chars; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/shared/long-text/long-text.component.ts` around lines 49 - 55, The
toggle breaks when chars >= 1000 because a hardcoded 1000 is used for both
initial state and toggle; add a configurable collapseThreshold (default 1000)
and initialize printChars to Math.min(this.chars, collapseThreshold) in
ngOnInit, then update showValue to toggle between this.printChars = this.chars
(expand) and this.printChars = collapseThreshold (collapse) using the
collapseThreshold variable and the chars property (refer to showValue,
printChars, chars, and ngOnInit).
| "List of Users with their details": "Orodha ya Watumiaji na maelezo yao", | ||
| "Loan Account OverView": "Muhtasari wa Akaunti ya Mkopo", | ||
| "Loan Accounts": "Hesabu za Mikopo", | ||
| "Loan Amounts": "Kiasi za Mikopo", |
There was a problem hiding this comment.
Fix Swahili grammar in "Loan Amounts" translation.
At Line [1156], "Kiasi za Mikopo" should be "Kiasi cha Mikopo" for correct grammar.
💬 Suggested fix
- "Loan Amounts": "Kiasi za Mikopo",
+ "Loan Amounts": "Kiasi cha Mikopo",📝 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
| "Loan Amounts": "Kiasi za Mikopo", | |
| "Loan Amounts": "Kiasi cha Mikopo", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/sw-SW.json` at line 1156, Update the Swahili
translation value for the "Loan Amounts" key in the JSON translations (the entry
currently set to "Kiasi za Mikopo"): replace it with the correct phrase "Kiasi
cha Mikopo" so the key "Loan Amounts" maps to the grammatically correct Swahili
translation.
e8eb698 to
fd8c296
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 (9)
src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts (2)
158-175:⚠️ Potential issue | 🟠 MajorInconsistent navigation pattern:
submitRepaymentshould usegotoLoanDefaultView().
submitContractTermination()was updated to usethis.gotoLoanDefaultView()(line 182), butsubmitRepayment()still uses directrouter.navigate. For consistency with the base class refactoring and product-type-aware navigation, this should also use the base class method.🔧 Proposed fix
submitRepayment() { const prepayLoanFormData = this.prepayLoanForm.value; const locale = this.settingsService.language.code; const dateFormat = this.settingsService.dateFormat; const prevTransactionDate: Date = this.prepayLoanForm.value.transactionDate; if (prepayLoanFormData.transactionDate instanceof Date) { prepayLoanFormData.transactionDate = this.dateUtils.formatDate(prevTransactionDate, dateFormat); } const data = { ...prepayLoanFormData, dateFormat, locale }; data['transactionAmount'] = data['transactionAmount'] * 1; this.loanService.submitLoanActionButton(this.loanId, data, 'repayment').subscribe((response: any) => { - this.router.navigate(['../../general'], { relativeTo: this.route }); + this.gotoLoanDefaultView(); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts` around lines 158 - 175, The submitRepayment method currently navigates directly with this.router.navigate(...) which is inconsistent with submitContractTermination; update submitRepayment to call the base-class navigation helper instead: replace the router.navigate call inside the subscription callback with this.gotoLoanDefaultView() so the repayment action uses the same product-aware navigation implemented by gotoLoanDefaultView (ensure submitRepayment still subscribes to loanService.submitLoanActionButton and calls this.gotoLoanDefaultView() on success).
118-133:⚠️ Potential issue | 🟡 MinorPotential memory leak:
valueChangessubscription is never unsubscribed.The subscription on line 123 persists for the component's lifetime but is not cleaned up on destroy. This can cause memory leaks and unintended side effects if the component is destroyed while the subscription is still active.
🛡️ Proposed fix using takeUntilDestroyed
+import { DestroyRef, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; export class PrepayLoanComponent extends LoanAccountActionsBaseComponent implements OnInit { private formBuilder = inject(UntypedFormBuilder); private dateUtils = inject(Dates); + private destroyRef = inject(DestroyRef); // ... existing code ... setPrepayLoanDetails() { this.paymentTypes = this.dataObject.paymentTypeOptions; this.prepayLoanForm.patchValue({ transactionAmount: this.dataObject.amount }); - this.prepayLoanForm.get('transactionDate').valueChanges.subscribe((transactionDate: string) => { + this.prepayLoanForm.get('transactionDate').valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((transactionDate: string) => { const prepayDate = this.dateUtils.formatDate(transactionDate, this.settingsService.dateFormat); this.loanService.getLoanPrepayLoanActionTemplate(this.loanId, prepayDate).subscribe((response: any) => { this.prepayData = response; this.prepayLoanForm.patchValue({ transactionAmount: this.prepayData.amount }); }); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts` around lines 118 - 133, The valueChanges subscription in setPrepayLoanDetails on this.prepayLoanForm.get('transactionDate') causes a potential memory leak because it is never unsubscribed; update setPrepayLoanDetails to terminate the observable (e.g., pipe the transactionDate.valueChanges through takeUntilDestroyed() or a shared destroy$ and then subscribe) so that the loanService.getLoanPrepayLoanActionTemplate(...) call is only active while the component is alive, and ensure any existing pattern in this component (ngOnDestroy/destroy$) is reused instead of leaving the subscription open.src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
31-33:⚠️ Potential issue | 🔴 CriticalRemove the local
loanIddeclaration—it shadows the base class property and remains uninitialized.The base class
LoanAccountActionsBaseComponentalready providesprotected loanId: string, which is initialized from route params in its constructor. Redeclaring it locally (line 32) shadows the base class property and creates an uninitialized public property. Whenthis.loanIdis used on line 122 in the API call, it will beundefined, breaking the request. Remove the declaration and rely on the inherited, properly-initialized base class property (as all sibling components do).Diff
export class AdjustLoanChargeComponent extends LoanAccountActionsBaseComponent implements OnInit { private formBuilder = inject(UntypedFormBuilder); private organizationService = inject(OrganizationService); - /** Loan Id */ - loanId: string; chargeId: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` around lines 31 - 33, The local public declaration "loanId: string" in adjust-loan-charge.component.ts shadows the inherited protected loanId from LoanAccountActionsBaseComponent and remains uninitialized; remove the local declaration and keep only chargeId so the component uses the base class's initialized protected loanId (used in the API call where this.loanId is referenced) instead of an undefined local property.src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.ts (1)
54-62:⚠️ Potential issue | 🟡 MinorDate fallback logic may not work as intended.
The expression
new Date(this.dataObject.date) || new Date()will not fall back tonew Date()whenthis.dataObject.datecontains an invalid date string, becausenew Date('invalid')returns anInvalid Dateobject, which is truthy.🐛 Proposed fix
createCloseForm() { + const parsedDate = new Date(this.dataObject.date); + const transactionDate = isNaN(parsedDate.getTime()) ? new Date() : parsedDate; this.closeLoanForm = this.formBuilder.group({ transactionDate: [ - new Date(this.dataObject.date) || new Date(), + transactionDate, Validators.required ], note: [] }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.ts` around lines 54 - 62, The transactionDate initializer in createCloseForm uses new Date(this.dataObject.date) || new Date(), which doesn't handle invalid dates; change createCloseForm to first construct a Date (e.g., const d = new Date(this.dataObject.date)), check its validity with isNaN(d.getTime()) (or Number.isNaN(d.getTime())), and pass either the valid date or new Date() into this.closeLoanForm (field transactionDate) so invalid input falls back correctly; update only createCloseForm/transactionDate initialization logic.src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.html (2)
82-82:⚠️ Potential issue | 🟡 MinorTypo in translation key: "Reciept" should be "Receipt".
Suggested fix
- <mat-label>{{ 'labels.inputs.Reciept' | translate }} #</mat-label> + <mat-label>{{ 'labels.inputs.Receipt' | translate }} #</mat-label>Ensure the corresponding translation key is also corrected in the i18n files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.html` at line 82, In the recovery-repayment template (recovery-repayment.component.html) fix the typo in the mat-label translation key by replacing 'labels.inputs.Reciept' with 'labels.inputs.Receipt' in the <mat-label> that renders the receipt number, and then update all corresponding i18n translation files to add or rename the key 'labels.inputs.Receipt' (copying the existing translations from the misspelled key) and remove the old 'labels.inputs.Reciept' entry to keep keys consistent.
58-66:⚠️ Potential issue | 🟡 MinorBind
mat-slide-toggletoshowPaymentDetailsfor proper visual state.The toggle's visual state isn't bound to
showPaymentDetails. The parent<span>handles the click, but the toggle won't reflect the actual state correctly. This can cause visual inconsistency where the toggle appears off but payment details are shown (or vice versa).Suggested fix
<span class="expandcollapsebutton m-l-10 m-t-40 flex-75" (click)="addPaymentDetails()"> - <mat-slide-toggle> + <mat-slide-toggle [checked]="showPaymentDetails"> <div [className]="showPaymentDetails ? 'enabled' : 'disabled'"> <span class="m-l-10">{{ 'labels.inputs.Show Payment Details' | translate }}</span> </div> </mat-slide-toggle> </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.html` around lines 58 - 66, The mat-slide-toggle needs to reflect the component state: bind its checked state to the showPaymentDetails property and move the toggle interaction to the slide-toggle change event instead of the parent span; for example, replace the current parent (click)="addPaymentDetails()" pattern by applying [checked]="showPaymentDetails" or [(ngModel)]="showPaymentDetails" on the mat-slide-toggle and call addPaymentDetails() (or set showPaymentDetails directly) from the toggle's (change) handler so the toggle UI always matches the showPaymentDetails value.src/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.ts (1)
41-48:⚠️ Potential issue | 🟠 MajorGuard base-provided
dataObjectbefore iterating to avoid runtime crashes.Line 44 directly calls
this.dataObject.forEach(...). SincedataObjectis an@Input()with no initialization in the base class, this throws if the parent component fails to pass it or initialization is delayed. Add a safe fallback before filtering active originators.💡 Proposed hardening
ngOnInit() { this.setAttachLoanOriginatorForm(); - this.loanOriginators = []; - this.dataObject.forEach((loanOriginator: LoanOriginator) => { - if (loanOriginator.status === 'ACTIVE') { - this.loanOriginators.push(loanOriginator); - } - }); + const originators = Array.isArray(this.dataObject) ? this.dataObject : []; + this.loanOriginators = originators.filter((loanOriginator: LoanOriginator) => loanOriginator.status === 'ACTIVE'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.ts` around lines 41 - 48, ngOnInit currently calls this.dataObject.forEach(...) without guarding against dataObject being undefined which can crash when the `@Input`() dataObject isn't provided yet; update ngOnInit to first ensure dataObject is an array (e.g., default to [] or check Array.isArray) before iterating, then filter for ACTIVE items into this.loanOriginators; locate the logic in ngOnInit around setAttachLoanOriginatorForm(), this.loanOriginators and this.dataObject and add the safeguard/fallback so the component never calls forEach on an undefined value.src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
280-294:⚠️ Potential issue | 🟠 MajorPerformance history may not include working-capital loans.
processLoanAccountscorrectly merges regular and working-capital loans intothis.loanAccounts(lines 281-283), butcomputePerformanceHistoryon line 293 still receivesdata.clientAccountsDatawhich only containsloanAccounts, not the merged array includingworkingCapitalLoanAccounts. This means the performance cards may show incomplete data for clients with working-capital loans.🔧 Proposed fix
this.loanAccounts = []; this.processLoanAccounts(data.clientAccountsData?.loanAccounts ?? [], 'loan'); this.processLoanAccounts(data.clientAccountsData?.workingCapitalLoanAccounts ?? [], 'working-capital'); this.workingCapitalLoanAccounts = data.clientAccountsData?.workingCapitalLoanAccounts ?? []; this.shareAccounts = data.clientAccountsData?.shareAccounts ?? []; this.upcomingCharges = data.clientChargesData?.pageItems ?? []; this.collaterals = data.clientCollateralData ?? []; this.clientid = this.route.parent.snapshot.params['clientId']; // Compute performance history from accounts data - this.computePerformanceHistory(data.clientAccountsData ?? { loanAccounts: [], savingsAccounts: [] }); + this.computePerformanceHistory({ + ...(data.clientAccountsData ?? {}), + loanAccounts: this.loanAccounts, + savingsAccounts: data.clientAccountsData?.savingsAccounts ?? [] + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.ts` around lines 280 - 294, The performance history call uses the original data.clientAccountsData (which lacks merged working-capital loans) so computePerformanceHistory misses working-capital loans; update the call in general-tab.component.ts to pass the merged accounts (this.loanAccounts and this.shareAccounts / savings if needed) instead of data.clientAccountsData — specifically, after processLoanAccounts runs, call computePerformanceHistory with an object containing loanAccounts: this.loanAccounts (and existing savingsAccounts from data.clientAccountsData if required) so computePerformanceHistory receives the combined loan list.src/app/clients/clients-view/general-tab/general-tab.component.html (1)
329-338:⚠️ Potential issue | 🟠 MajorAdd guard to the closed-loans Type cell to prevent undefined reference for non-loan products.
Lines 331-337 unconditionally access
element.loanType.value, which will fail for working-capital rows whereloanTypeis undefined. The open-loans table correctly guards this with@if (element.productType === 'loan')at line 189, but the closed-loans table is missing this guard.🐛 Proposed fix
<ng-container matColumnDef="Type"> <th mat-header-cell class="center" *matHeaderCellDef>{{ 'labels.inputs.Type' | translate }}</th> <td mat-cell class="center" *matCellDef="let element"> + `@if` (element.productType === 'loan') { <i class="fa fa-large" [ngClass]="element.loanType.value === 'Individual' ? 'fa-user' : 'fa-group'" matTooltip=" {{ element.loanType.value }}" matTooltipPosition="above" ></i> + } </td> </ng-container>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/general-tab/general-tab.component.html` around lines 329 - 338, The Type column cell unconditionally references element.loanType.value causing errors for non-loan rows; add the same guard used in the open-loans table so the icon/tooltip only render for loan rows (e.g., wrap or conditionally render the <i> element or the <td mat-cell> with *ngIf="element.productType === 'loan'") inside the matColumnDef="Type" block to avoid accessing undefined loanType.
🧹 Nitpick comments (17)
src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts (1)
61-70: Stale JSDoc: constructor documentation references removed dependencies.The constructor now calls
super()and no longer directly injects FormBuilder, LoansService, ActivatedRoute, Router, or SettingsService. The JSDoc comments should be removed or updated to reflect the base class inheritance.📝 Proposed fix
- /** - * `@param` {FormBuilder} formBuilder Form Builder. - * `@param` {LoansService} loanService Loan Service. - * `@param` {ActivatedRoute} route Activated Route. - * `@param` {Router} router Router for navigation. - * `@param` {SettingsService} settingsService Settings Service - */ constructor() { super(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.ts` around lines 61 - 70, The JSDoc above the constructor is stale because the constructor only calls super() and no longer injects FormBuilder, LoansService, ActivatedRoute, Router, or SettingsService; remove or update the JSDoc to match the current implementation by deleting the parameter annotations or replacing them with a short note that the class uses base-class injection (reference: the constructor method in prepay-loan.component.ts and the class that extends the base component), ensuring comments reflect that only super() is called.src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.ts (4)
98-129: Subscription lacks cleanup—potential memory leak.The subscription to
existingClientvalue changes (line 99) is never unsubscribed. If the component is destroyed and recreated, old subscriptions will remain active. Consider using Angular'sDestroyRefwithtakeUntilDestroyed()or aSubject-basedtakeUntilpattern.♻️ Proposed fix using DestroyRef
Add import and injection:
+import { DestroyRef } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop';+ private destroyRef = inject(DestroyRef);Then update the subscription:
buildDependencies() { - this.newGuarantorForm.get('existingClient').valueChanges.subscribe(() => { + this.newGuarantorForm.get('existingClient').valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(() => { this.showClientDetailsForm = !this.showClientDetailsForm; // ... }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.ts` around lines 98 - 129, The subscription inside buildDependencies() to newGuarantorForm.get('existingClient').valueChanges is never torn down; update buildDependencies() to pipe the Observable through a teardown operator (e.g., inject DestroyRef and use takeUntilDestroyed() or create a private Subject and use takeUntil(this.destroy$)) so the subscription is automatically unsubscribed on component destroy, keeping the existing toggle logic for showClientDetailsForm and the add/removeControl calls unchanged.
146-151: Subscription lacks cleanup.The subscription in
clientSelected(line 148) is not cleaned up. While this method is event-driven, repeated calls will accumulate subscriptions if the previous request hasn't completed.♻️ Proposed fix
clientSelected(clientDetails: any) { this.accountOptions = []; - this.loanService.guarantorAccountResource(this.loanId, clientDetails.id).subscribe((response: any) => { - this.accountOptions = response.accountLinkingOptions; - }); + this.loanService.guarantorAccountResource(this.loanId, clientDetails.id) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((response: any) => { + this.accountOptions = response.accountLinkingOptions; + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.ts` around lines 146 - 151, The subscription in clientSelected is not cleaned up and can accumulate on repeated calls; modify the call to loanService.guarantorAccountResource(this.loanId, clientDetails.id) so the Observable is auto-unsubscribed (e.g., pipe with take(1) or first()) before calling subscribe, or replace the manual subscribe with an operator that maps the single response into setting this.accountOptions; ensure you reference clientSelected, guarantorAccountResource and this.accountOptions when making the change so the request completes and the subscription is automatically disposed.
55-64: Stale JSDoc comments reference removed dependencies.The constructor JSDoc (lines 56-60) still documents
LoansService,ActivatedRoute,Router, andSettingsServiceas parameters, but these are now provided by the base class. Either remove the JSDoc block entirely or update it to reflect the new pattern.📝 Proposed fix
- /** - * `@param` {FormBuilder} formBuilder Form Builder. - * `@param` {LoansService} loanService Loan Service. - * `@param` {ActivatedRoute} route Activated Route. - * `@param` {Router} router Router for navigation. - * `@param` {SettingsService} settingsService Settings Service - */ constructor() { super(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.ts` around lines 55 - 64, The JSDoc above the constructor in CreateGuarantorComponent is stale and lists dependencies (LoansService, ActivatedRoute, Router, SettingsService, FormBuilder) that are now provided by the base class; update the file by either removing this constructor JSDoc block entirely or replacing it with a short class-level docstring that reflects the new pattern (e.g., describing the component responsibility only), and ensure the constructor() { super(); } comment aligns with the new doc; look for the constructor in create-guarantor.component.ts and update or delete the JSDoc accordingly.
134-144: Nested subscription and missing cleanup.The subscription at line 136 is never cleaned up and contains a nested subscription at line 138. This is both an anti-pattern and a memory leak risk. Use
switchMapormergeMapto flatten the nested observable and apply cleanup.♻️ Proposed fix using switchMap and DestroyRef
+import { filter, switchMap } from 'rxjs/operators'; ngAfterViewInit() { if (this.newGuarantorForm.value.existingClient) { - this.newGuarantorForm.get('name').valueChanges.subscribe((value: string) => { - if (value.length >= 2) { - this.clientsService.getFilteredClients('displayName', 'ASC', true, value).subscribe((data: any) => { - this.clientsData = data.pageItems; - }); - } - }); + this.newGuarantorForm.get('name').valueChanges.pipe( + filter((value: string) => value.length >= 2), + switchMap((value: string) => this.clientsService.getFilteredClients('displayName', 'ASC', true, value)), + takeUntilDestroyed(this.destroyRef) + ).subscribe((data: any) => { + this.clientsData = data.pageItems; + }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.ts` around lines 134 - 144, The current ngAfterViewInit subscribes to name.valueChanges and nests a subscribe to clientsService.getFilteredClients, leaking subscriptions; replace it with a flattened pipeline: subscribe to this.newGuarantorForm.get('name').valueChanges.pipe(filter(v => v?.length >= 2), debounceTime(200), distinctUntilChanged(), switchMap(value => this.clientsService.getFilteredClients('displayName','ASC',true,value)), takeUntilDestroyed(this.destroyRef)) and assign this.clientsData from the resulting observable; inject DestroyRef as destroyRef in the component constructor (or use takeUntil with a Subject ngOnDestroy) and import needed RxJS operators (filter, debounceTime, distinctUntilChanged, switchMap, takeUntilDestroyed) so there are no nested subscriptions and cleanup happens automatically.src/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.ts (1)
29-32: Unused variables can be removed.
paymentTypesandshowPaymentDetailsare declared but never referenced in this component. These appear to be leftover artifacts that can be safely removed to improve code clarity.🧹 Proposed cleanup
- /** Payment Type Options */ - paymentTypes: any; - /** Show payment details */ - showPaymentDetails = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.ts` around lines 29 - 32, Remove the unused component properties paymentTypes and showPaymentDetails from the AddInterestPauseComponent (delete their declarations: "paymentTypes: any;" and "showPaymentDetails = false;"); ensure there are no remaining references to these symbols elsewhere in the component or its template and remove any now-unused imports or tests that only existed to support them.src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts (1)
36-38: Consider using specific types instead ofany.These properties use
anytype, which reduces type safety. Per project conventions, introduce specific interfaces for API responses.♻️ Suggested typing improvement
- /** Payment Type Options */ - paymentTypes: any = []; - chargeData: any = []; - loanDetailsData: any = []; + /** Payment Type Options */ + paymentTypes: PaymentType[] = []; + chargeData: LoanCharge; + loanDetailsData: LoanDetails;Define or import the appropriate interfaces (
PaymentType,LoanCharge,LoanDetails) to match the actual API response shapes.Based on learnings: "Avoid using Observable 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/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.ts` around lines 36 - 38, Replace the three any-typed properties with concrete types: define or import interfaces (e.g., PaymentType, LoanCharge, LoanDetails) that match the API shapes and change paymentTypes: any = []; chargeData: any = []; loanDetailsData: any = []; to use those types (e.g., paymentTypes: PaymentType[] = []; chargeData: LoanCharge[] = []; loanDetailsData: LoanDetails | null = null). Also update any related method signatures/observables that populate these fields to return/emit the new typed interfaces (Observable<PaymentType[]>, Observable<LoanCharge[]>, Observable<LoanDetails>) so the assignments are type-safe and consistent with the new types.src/assets/translations/es-MX.json (2)
2652-2652: Consider title-case consistency for this label.Line [2652] uses
Pago total, while adjacent labels in this section generally use title case. ConsiderPago Totalfor UI consistency.💡 Suggested tweak
- "Total Payment": "Pago total", + "Total Payment": "Pago Total",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 2652, The Spanish translation value for the key "Total Payment" currently uses sentence case ("Pago total"); update the value to title case ("Pago Total") to match surrounding labels — locate the "Total Payment" entry in the translations file and replace "Pago total" with "Pago Total".
1-4174: Please run Prettier once before merge.This file is under the JSON formatting policy; run
npx prettier --write .before closing the branch.As per coding guidelines
src/**/*.{ts,tsx,js,jsx,html,scss,css,json}: Enforce Prettier formatting by runningnpx prettier --write .before closing any branch.🤖 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 1 - 4174, The JSON file src/assets/translations/es-MX.json is not prettified; run the project-wide Prettier formatter (npx prettier --write .) and commit the resulting changes so this file and all files matching src/**/*.{ts,tsx,js,jsx,html,scss,css,json} comply with the repository's Prettier formatting policy before merging.src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.html (1)
50-50: Usetrack paymentType.idfor better change detection.Tracking by object reference is less efficient than tracking by a unique identifier. This can cause unnecessary DOM re-renders when the array is replaced.
Suggested fix
- `@for` (paymentType of paymentTypes; track paymentType) { + `@for` (paymentType of paymentTypes; track paymentType.id) {As per coding guidelines: "verify component separation, trackBy on *ngFor".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.html` at line 50, The ngFor currently tracks by object reference ("track paymentType") causing extra re-renders; change the tracking to use the unique identifier by replacing the tracking clause to "track paymentType.id" for the loop over paymentTypes (reference symbols: paymentTypes and paymentType in recovery-repayment.component.html) so change detection uses paymentType.id as the track key.src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts (1)
111-117: Duplicate code: both branches are identical.The
if/elsebranches execute exactly the same code. This appears to be either leftover from a planned differentiation or an oversight. Consider simplifying:♻️ Suggested simplification
- if (this.isCapitalizedIncome()) { - this.repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])); - this.updateTransactionAmountValidators(false); - } else { - this.repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])); - this.updateTransactionAmountValidators(false); - } + this.repaymentLoanForm.addControl('transactionAmount', new UntypedFormControl('', [])); + this.updateTransactionAmountValidators(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts` around lines 111 - 117, The if/else in make-repayment.component.ts around isCapitalizedIncome() contains duplicate code; remove the redundant branches and place the common logic once: add the 'transactionAmount' control to repaymentLoanForm (using addControl) and call updateTransactionAmountValidators(false) outside the conditional, or if different behavior was intended, implement the differing logic inside either the if (isCapitalizedIncome()) or the else branch for repaymentLoanForm.addControl(...) and updateTransactionAmountValidators(...).src/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.ts (1)
47-47: Avoidanyin the submit callback.Line 47 uses
response: any, butresponseis not used. Drop the parameter (or type it explicitly) to keep strict typing intact.Proposed minimal fix
- .subscribe((response: any) => { + .subscribe(() => { this.gotoLoanDefaultView(); });Based on learnings: "avoid using Observable as a project-wide pattern for API responses ... use proper typing instead of any."
As per coding guidelines: "src/app/**/*.ts: Use TypeScript for all application code with strict typing conventions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.ts` at line 47, The subscribe callback in UndoApprovalComponent currently declares an unused parameter as "response: any"; remove the unused parameter to preserve strict typing (change ".subscribe((response: any) => { ... })" to ".subscribe(() => { ... })"), or if the API does return data you intend to use, replace "any" with a concrete interface and update the Observable's generic type accordingly so the callback signature becomes ".subscribe((response: CorrectType) => { ... })". Ensure this change is made in the subscribe call inside the undo-approval.component.ts component.src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.ts (1)
11-11: Strict typing is a cross-cutting enhancement outside this PR's scope.While
UntypedFormGroupandactionButtonData: anydo weaken compile-time safety, this pattern is consistent across the loan action components. Comprehensive typing of form controls and route data is tracked as a separate enhancement rather than addressed within individual refactors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.ts` at line 11, The current use of UntypedFormGroup/UntypedFormBuilder/UntypedFormControl and the loose typing of actionButtonData in AddLoanChargeComponent intentionally remain untyped per project decision; do not convert these to strongly-typed FormGroup or change actionButtonData's type in this PR. Revert any edits that tightened these types, keep the existing imports (UntypedFormGroup, UntypedFormBuilder, Validators, UntypedFormControl) and the actionButtonData:any usage, and add a single-line TODO in the AddLoanChargeComponent class pointing to the central strict-typing enhancement task so future refactors can address it in scope.src/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.ts (1)
10-10: Consider migrating to typed forms as part of broader refactoring.All components in the loan-account-actions folder currently use
UntypedFormBuilderandUntypedFormGroup, including this component. While this migration touches the form setup during the base class refactoring, moving to typed forms (FormBuilder/FormGroup) would align with the coding guideline requiring strict typing conventions. This should ideally be a systematic effort across the entire feature area rather than a single-component change for consistency.Example typed form pattern
-import { UntypedFormBuilder, UntypedFormGroup, Validators } from '@angular/forms'; +import { FormBuilder, FormControl, FormGroup, Validators } from '@angular/forms'; ... -private formBuilder = inject(UntypedFormBuilder); +private formBuilder = inject(FormBuilder); ... -chargeoffLoanForm: UntypedFormGroup; +chargeoffLoanForm!: FormGroup<{ + transactionDate: FormControl<Date | null>; + externalId: FormControl<string | null>; + chargeOffReasonId: FormControl<string | null>; + note: FormControl<string | null>; +}>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.ts` at line 10, The component is using UntypedFormBuilder and UntypedFormGroup; replace these with the typed equivalents (FormBuilder and FormGroup) and update any form declarations and usages in ChargeOffComponent (and its form initialization method, e.g., wherever a form is created or referenced) to use a strongly-typed form model interface (declare an interface for the form shape) so controls are typed; ensure imports change from UntypedFormBuilder/UntypedFormGroup to FormBuilder/FormGroup and update any Validators usages and value accessors to respect the new generic types so the component compiles with strict typing.src/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.ts (1)
11-11: Use typed reactive forms instead ofUntyped*APIs.At line 11, importing
UntypedFormGroupandUntypedFormBuilderbypasses form type safety. The project coding guidelines require strict TypeScript typing, and thetsconfig.jsonenforcesnoImplicitAny: true.♻️ Proposed typed-form refactor
-import { UntypedFormGroup, UntypedFormBuilder } from '@angular/forms'; +import { FormBuilder, FormControl, FormGroup } from '@angular/forms'; @@ - private formBuilder = inject(UntypedFormBuilder); + private formBuilder = inject(FormBuilder); @@ - undoWriteOffLoanForm: UntypedFormGroup; + undoWriteOffLoanForm: FormGroup<{ + note: FormControl<string | null>; + }>; @@ - this.undoWriteOffLoanForm = this.formBuilder.group({ - note: [''] - }); + this.undoWriteOffLoanForm = this.formBuilder.group({ + note: this.formBuilder.control<string | null>('') + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.ts` at line 11, Replace the untyped form APIs with Angular's typed reactive form types: change the imports from UntypedFormGroup/UntypedFormBuilder to FormGroup and FormBuilder from '@angular/forms'; update the component's form property and constructor parameter (e.g., the symbol representing the form instance and the fb parameter) to use FormGroup<TFormValue> and FormBuilder respectively, providing an appropriate interface/type for the form controls (TFormValue) and add generics when calling fb.group(...) to keep control types strict; ensure any references to methods like patchValue/setValue or control getters are updated to the typed signatures.src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts (1)
84-84: Typing the delinquencyData response is part of a broader, project-tracked API refactoring effort.While strict typing is desired per guidelines, the service returns
Observable<any>as part of the codebase's established pattern. A DelinquentData interface exists but would require service-layer changes and coordination across multiple components (approve-loan, disburse, disburse-to-savings-account, view-guarantors). This cross-cutting refactoring should be tracked as a separate enhancement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts` at line 84, The review notes that stricter typing for the delinquency response is a larger, cross-cutting refactor, so do not change the current Observable<any> usage; leave the call this.loanService.getLoanDelinquencyDataForTemplate(this.loanId).subscribe((delinquencyData: any) => { ... }) unchanged and add a short TODO comment referencing the tracked refactor (e.g., "TODO: replace any with DelinquencyData interface as part of API refactor #<tracking-id>") above the subscription so the needed follow-up is discoverable; keep references to loanService and getLoanDelinquencyDataForTemplate intact.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
174-175: Prefer a typed interface overanyforallowAttributeOverrides.Using
anybypasses TypeScript's type checking. Consider defining an interface for the attribute overrides structure to catch typos and missing properties at compile time.♻️ Example typed interface
+interface AllowAttributeOverrides { + periodPaymentFrequency?: boolean; + periodPaymentFrequencyType?: boolean; + discountDefault?: boolean; + repaymentEvery?: boolean; + // add other properties as needed +} + - allowAttributeOverrides: any | null = null; + allowAttributeOverrides: AllowAttributeOverrides | null = null;Based on learnings: "In TypeScript files, 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/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 174 - 175, Replace the untyped field allowAttributeOverrides: any | null = null with a properly defined interface and typed field (e.g., define interface AttributeOverrides { /* properties like attributeName?: string; overrideValue?: string; ... */ } and change the field to allowAttributeOverrides: AttributeOverrides | null = null); update the LoansAccountTermsStepComponent usages (constructors, methods, and any assignments or Observable returns) to use AttributeOverrides instead of any so the compiler enforces shape and you can refine properties rather than rely on any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 494985ad-c6b5-403e-9121-c8c6ccfd5ae8
📒 Files selected for processing (114)
src/app/clients/clients-view/general-tab/general-tab.component.htmlsrc/app/clients/clients-view/general-tab/general-tab.component.tssrc/app/core/shell/breadcrumb/breadcrumb.component.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/common-resolvers/loan-products.resolver.tssrc/app/loans/create-loans-account/create-loans-account.component.htmlsrc/app/loans/create-loans-account/create-loans-account.component.tssrc/app/loans/edit-loans-account/edit-loans-account.component.htmlsrc/app/loans/edit-loans-account/edit-loans-account.component.tssrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-details-step/loans-account-details-step.component.tssrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/account-details/account-details.component.htmlsrc/app/loans/loans-view/account-details/account-details.component.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-collateral/add-collateral.component.tssrc/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.tssrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.htmlsrc/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.tssrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.tssrc/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/asset-transfer-loan/asset-transfer-loan.component.tssrc/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.htmlsrc/app/loans/loans-view/loan-account-actions/assign-loan-officer/assign-loan-officer.component.tssrc/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.htmlsrc/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.tssrc/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.htmlsrc/app/loans/loans-view/loan-account-actions/charge-off/charge-off.component.tssrc/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.htmlsrc/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.tssrc/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.htmlsrc/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.tssrc/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.tssrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.tssrc/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.tssrc/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.htmlsrc/app/loans/loans-view/loan-account-actions/foreclosure/foreclosure.component.tssrc/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.tssrc/app/loans/loans-view/loan-account-actions/loan-account-actions.component.tssrc/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tssrc/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.tssrc/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.htmlsrc/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.tssrc/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.tssrc/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.htmlsrc/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.tssrc/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-approval/undo-approval.component.tssrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.tssrc/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.htmlsrc/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.tssrc/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.tssrc/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.htmlsrc/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.tssrc/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.htmlsrc/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.tssrc/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.htmlsrc/app/loans/loans-view/loan-account-actions/write-off-page/write-off-page.component.tssrc/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scsssrc/app/loans/loans-view/loan-term-variations-tab/loan-term-variations-tab.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans.service.tssrc/app/loans/models/loan-product.model.tssrc/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/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/services/loan-product.service.tssrc/app/products/products.service.tssrc/app/shared/footer/footer.component.tssrc/app/shared/long-text/long-text.component.htmlsrc/app/shared/long-text/long-text.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
💤 Files with no reviewable changes (5)
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
- src/app/loans/loans-view/loan-account-dashboard/loan-account-dashboard.component.scss
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
- src/app/shared/long-text/long-text.component.html
🚧 Files skipped from review as they are similar to previous changes (40)
- src/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.html
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html
- src/app/loans/loans-view/loan-account-actions/attach-originator/attach-originator.component.html
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html
- src/app/loans/loans-view/loan-account-actions/reject-loan/reject-loan.component.html
- src/app/loans/loans-view/loan-account-actions/adjust-loan-charge/adjust-loan-charge.component.html
- src/app/loans/loans-view/loan-account-actions/loan-credit-balance-refund/loan-credit-balance-refund.component.html
- src/app/loans/loans-view/loan-account-actions/edit-repayment-schedule/edit-repayment-schedule.component.html
- src/assets/translations/fr-FR.json
- src/app/loans/loans-view/loan-account-actions/loan-reaging/loan-reaging.component.html
- src/app/products/loan-products/common/loan-product-base.component.ts
- src/app/loans/loans-view/loan-account-actions/prepay-loan/prepay-loan.component.html
- src/app/loans/loans-view/loan-account-actions/disburse-to-savings-account/disburse-to-savings-account.component.html
- src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.html
- src/app/loans/loans-view/loan-account-actions/waive-interest/waive-interest.component.ts
- src/app/loans/loans-view/loan-account-actions/loan-account-actions.component.ts
- src/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.ts
- src/assets/translations/it-IT.json
- src/assets/translations/en-US.json
- src/app/loans/models/loan-product.model.ts
- src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html
- src/app/core/shell/breadcrumb/breadcrumb.component.ts
- src/app/shared/long-text/long-text.component.ts
- src/app/loans/loans-view/loan-account-actions/create-guarantor/create-guarantor.component.html
- src/app/loans/loans-view/loan-account-actions/loan-screen-reports/loan-screen-reports.component.html
- src/app/loans/loans-view/loan-account-actions/close-as-rescheduled/close-as-rescheduled.component.ts
- src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.ts
- src/app/loans/loans-view/loan-account-actions/withdrawn-by-client/withdrawn-by-client.component.html
- src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
- src/assets/translations/ne-NE.json
- src/assets/translations/ko-KO.json
- src/app/loans/loans-view/loan-account-actions/undo-write-off/undo-write-off.component.html
- src/app/shared/footer/footer.component.ts
- src/app/loans/loans-view/loan-account-actions/add-loan-charge/add-loan-charge.component.html
- src/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.html
- src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.html
- src/app/loans/loans-view/loan-account-actions/loan-account-actions-base.component.ts
- src/app/loans/loans-view/account-details/account-details.component.ts
- src/assets/translations/cs-CS.json
- src/app/loans/loans-routing.module.ts
Comment on lines
140
to
+155
| get loansAccount() { | ||
| return { | ||
| ...this.loansAccountDetailsStep.loansAccountDetails, | ||
| ...this.loansAccountTermsStep.loansAccountTerms, | ||
| ...this.loansAccountChargesStep.loansAccountCharges, | ||
| ...this.loansAccountTermsStep.loanCollateral, | ||
| ...this.loansAccountTermsStep.disbursementData | ||
| }; | ||
| if (this.loanProductService.isLoanProduct) { | ||
| return { | ||
| ...this.loansAccountDetailsStep.loansAccountDetails, | ||
| ...this.loansAccountTermsStep.loansAccountTerms, | ||
| ...this.loansAccountChargesStep?.loansAccountCharges, | ||
| ...this.loansAccountTermsStep.loanCollateral, | ||
| ...this.loansAccountTermsStep.disbursementData | ||
| }; | ||
| } else if (this.loanProductService.isWorkingCapital) { | ||
| return { | ||
| ...this.loansAccountDetailsStep.loansAccountDetails, | ||
| ...this.loansAccountTermsStep.loansAccountTerms | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure loansAccount getter has an explicit fallback return.
When product type is neither loan nor working-capital, the getter returns undefined, which can break payload assembly downstream.
🧩 Explicit fallback (or throw) to avoid implicit undefined
get loansAccount() {
if (this.loanProductService.isLoanProduct) {
return {
...this.loansAccountDetailsStep.loansAccountDetails,
...this.loansAccountTermsStep.loansAccountTerms,
...this.loansAccountChargesStep?.loansAccountCharges,
...this.loansAccountTermsStep.loanCollateral,
...this.loansAccountTermsStep.disbursementData
};
} else if (this.loanProductService.isWorkingCapital) {
return {
...this.loansAccountDetailsStep.loansAccountDetails,
...this.loansAccountTermsStep.loansAccountTerms
};
}
+ throw new Error('Unsupported product type in EditLoansAccountComponent.loansAccount');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| get loansAccount() { | |
| return { | |
| ...this.loansAccountDetailsStep.loansAccountDetails, | |
| ...this.loansAccountTermsStep.loansAccountTerms, | |
| ...this.loansAccountChargesStep.loansAccountCharges, | |
| ...this.loansAccountTermsStep.loanCollateral, | |
| ...this.loansAccountTermsStep.disbursementData | |
| }; | |
| if (this.loanProductService.isLoanProduct) { | |
| return { | |
| ...this.loansAccountDetailsStep.loansAccountDetails, | |
| ...this.loansAccountTermsStep.loansAccountTerms, | |
| ...this.loansAccountChargesStep?.loansAccountCharges, | |
| ...this.loansAccountTermsStep.loanCollateral, | |
| ...this.loansAccountTermsStep.disbursementData | |
| }; | |
| } else if (this.loanProductService.isWorkingCapital) { | |
| return { | |
| ...this.loansAccountDetailsStep.loansAccountDetails, | |
| ...this.loansAccountTermsStep.loansAccountTerms | |
| }; | |
| } | |
| } | |
| get loansAccount() { | |
| if (this.loanProductService.isLoanProduct) { | |
| return { | |
| ...this.loansAccountDetailsStep.loansAccountDetails, | |
| ...this.loansAccountTermsStep.loansAccountTerms, | |
| ...this.loansAccountChargesStep?.loansAccountCharges, | |
| ...this.loansAccountTermsStep.loanCollateral, | |
| ...this.loansAccountTermsStep.disbursementData | |
| }; | |
| } else if (this.loanProductService.isWorkingCapital) { | |
| return { | |
| ...this.loansAccountDetailsStep.loansAccountDetails, | |
| ...this.loansAccountTermsStep.loansAccountTerms | |
| }; | |
| } | |
| throw new Error('Unsupported product type in EditLoansAccountComponent.loansAccount'); | |
| } |
🧰 Tools
🪛 Biome (2.4.6)
[error] 140-155: This getter should return a value.
(lint/suspicious/useGetterReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/edit-loans-account/edit-loans-account.component.ts` around
lines 140 - 155, The loansAccount getter currently returns undefined when
neither loanProductService.isLoanProduct nor loanProductService.isWorkingCapital
is true; update the getter to include an explicit fallback (e.g., a final else
or default return) that returns a safe value such as an empty object (or throws
a clear error) so downstream payload assembly won't receive undefined; locate
the loansAccount getter and add the explicit return at the end while keeping the
existing branches (refer to loanProductService.isLoanProduct,
loanProductService.isWorkingCapital, loansAccountDetailsStep,
loansAccountTermsStep, loansAccountChargesStep, loanCollateral,
disbursementData).
| </div> | ||
| } | ||
|
|
||
| @if (loansAccountProductTemplate.overdueCharges.length) { |
There was a problem hiding this comment.
Guard overdue-charges access with optional chaining.
Directly reading .length can throw when overdueCharges is missing.
🛡️ Null-safe condition
-@if (loansAccountProductTemplate.overdueCharges.length) {
+@if (loansAccountProductTemplate.overdueCharges?.length) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @if (loansAccountProductTemplate.overdueCharges.length) { | |
| `@if` (loansAccountProductTemplate.overdueCharges?.length) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html`
at line 426, The template condition directly accesses
loansAccountProductTemplate.overdueCharges.length which can throw if
overdueCharges is undefined; update the guard to be null-safe by checking
loansAccountProductTemplate.overdueCharges?.length (or explicitly verify
Array.isArray(loansAccountProductTemplate.overdueCharges) &&
loansAccountProductTemplate.overdueCharges.length) so the template only
evaluates length when overdueCharges exists.
...p/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
Show resolved
Hide resolved
Comment on lines
+162
to
+164
| @if (ele.key === 'Product Name') { | ||
| <span>{{ loanDetails.loanProductName || loanDetails.product.name }}</span> | ||
| } |
There was a problem hiding this comment.
Add null-safe fallback for product name rendering.
The fallback path reads loanDetails.product.name directly. If product is missing, this can break template rendering.
🐛 Minimal safe fallback
-<span>{{ loanDetails.loanProductName || loanDetails.product.name }}</span>
+<span>{{
+ loanDetails.loanProductName ||
+ loanDetails.product?.name ||
+ ('labels.inputs.Not Available' | translate)
+}}</span>Also applies to: 228-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/general-tab/general-tab.component.html` around lines
162 - 164, The template uses loanDetails.loanProductName ||
loanDetails.product.name which will error if loanDetails.product is
null/undefined; change the fallback to a null-safe expression (use the safe
navigation operator or an explicit guard) when accessing
loanDetails.product.name in the GeneralTab template so it reads the product name
only if product exists (e.g., use loanDetails.product?.name or a conditional
check), and apply the same fix to the other occurrence around lines 228-230 to
prevent runtime template errors.
Comment on lines
98
to
100
| this.loanService.addInterestPauseToLoan(this.loanId, data).subscribe((response: any) => { | ||
| this.router.navigate(['../../term-variations'], { relativeTo: this.route }); | ||
| this.gotoLoanView('term-variations'); | ||
| }); |
There was a problem hiding this comment.
Add error handling for the API call.
The subscribe callback lacks an error handler. If the API call fails, the user will receive no feedback and the navigation won't occur. Additionally, since response is unused, consider simplifying the callback.
🛡️ Proposed fix with error handling
- this.loanService.addInterestPauseToLoan(this.loanId, data).subscribe((response: any) => {
- this.gotoLoanView('term-variations');
- });
+ this.loanService.addInterestPauseToLoan(this.loanId, data).subscribe({
+ next: () => this.gotoLoanView('term-variations'),
+ error: (error) => console.error('Failed to add interest pause:', error)
+ });Based on learnings: avoid using any for API responses; introduce specific interfaces/types for response shapes as a separate enhancement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-account-actions/add-interest-pause/add-interest-pause.component.ts`
around lines 98 - 100, The subscribe on
loanService.addInterestPauseToLoan(this.loanId, data) has no error handler and
uses an unused any response; update the call in add-interest-pause.component to
pass both success and error callbacks to subscribe (e.g., success -> call
this.gotoLoanView('term-variations'), error -> show user feedback via the
existing UI notification/toast/logger and do not navigate), and replace the
loose any with an appropriate response type/interface (or omit the unused
response parameter) so the success handler is minimal and failures are surfaced
to the user.
Comment on lines
81
to
83
| this.loanService.submitLoanActionButton(this.loanId, data, 'close').subscribe((response: any) => { | ||
| this.router.navigate(['../../general'], { relativeTo: this.route }); | ||
| this.gotoLoanDefaultView(); | ||
| }); |
There was a problem hiding this comment.
Observable subscription lacks error handling and cleanup.
The subscription to submitLoanActionButton does not handle errors, which will cause silent failures. Additionally, there is no unsubscription mechanism, which could lead to memory leaks if the component is destroyed before the observable completes.
🛡️ Proposed fix with error handling and cleanup
Add DestroyRef and takeUntilDestroyed for automatic cleanup:
-import { Component, OnInit, inject } from '@angular/core';
+import { Component, DestroyRef, OnInit, inject } from '@angular/core';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';Then inject and use it:
export class LoansAccountCloseComponent extends LoanAccountActionsBaseComponent implements OnInit {
private formBuilder = inject(UntypedFormBuilder);
private dateUtils = inject(Dates);
+ private destroyRef = inject(DestroyRef);And update the subscription:
- this.loanService.submitLoanActionButton(this.loanId, data, 'close').subscribe((response: any) => {
- this.gotoLoanDefaultView();
- });
+ this.loanService.submitLoanActionButton(this.loanId, data, 'close')
+ .pipe(takeUntilDestroyed(this.destroyRef))
+ .subscribe({
+ next: () => this.gotoLoanDefaultView(),
+ error: (err) => console.error('Failed to close loan account', err)
+ });Based on learnings: avoid using Observable<any> as a project-wide pattern; introduce specific interfaces/types for the response shapes.
📝 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
| this.loanService.submitLoanActionButton(this.loanId, data, 'close').subscribe((response: any) => { | |
| this.router.navigate(['../../general'], { relativeTo: this.route }); | |
| this.gotoLoanDefaultView(); | |
| }); | |
| import { Component, DestroyRef, OnInit, inject } from '@angular/core'; | |
| import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; | |
| // ... other imports | |
| export class LoansAccountCloseComponent extends LoanAccountActionsBaseComponent implements OnInit { | |
| private formBuilder = inject(UntypedFormBuilder); | |
| private dateUtils = inject(Dates); | |
| private destroyRef = inject(DestroyRef); | |
| // ... rest of component code | |
| // At lines 81-83, update the subscription: | |
| this.loanService.submitLoanActionButton(this.loanId, data, 'close') | |
| .pipe(takeUntilDestroyed(this.destroyRef)) | |
| .subscribe({ | |
| next: () => this.gotoLoanDefaultView(), | |
| error: (err) => console.error('Failed to close loan account', err) | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-account-actions/loans-account-close/loans-account-close.component.ts`
around lines 81 - 83, The subscription to
loanService.submitLoanActionButton(this.loanId, data, 'close') in
loans-account-close.component.ts needs proper error handling and automatic
cleanup: inject DestroyRef into the component and use
takeUntilDestroyed(this.destroyRef) when subscribing to submitLoanActionButton
to ensure unsubscription on destroy, add an error callback to the subscribe call
to log/display errors and handle failure paths (do not only call
gotoLoanDefaultView on success), and replace the loose Observable<any> response
typing with a specific interface for the submit response to avoid using any.
Comment on lines
139
to
141
| this.loanService.submitLoanActionButton(this.loanId, data, 'recoverypayment').subscribe((response: any) => { | ||
| this.router.navigate(['../../general'], { relativeTo: this.route }); | ||
| this.gotoLoanDefaultView(); | ||
| }); |
There was a problem hiding this comment.
Add error handling to the HTTP subscription.
The subscribe() call lacks an error handler. If the API call fails, the user receives no feedback and the component silently fails. Consider adding error handling or using a shared toast/notification service.
Suggested fix
this.loanService.submitLoanActionButton(this.loanId, data, 'recoverypayment').subscribe({
- (response: any) => {
+ next: (response: any) => {
this.gotoLoanDefaultView();
- });
+ },
+ error: (error) => {
+ // Handle error - show notification to user
+ }
+ });📝 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
| this.loanService.submitLoanActionButton(this.loanId, data, 'recoverypayment').subscribe((response: any) => { | |
| this.router.navigate(['../../general'], { relativeTo: this.route }); | |
| this.gotoLoanDefaultView(); | |
| }); | |
| this.loanService.submitLoanActionButton(this.loanId, data, 'recoverypayment').subscribe({ | |
| next: (response: any) => { | |
| this.gotoLoanDefaultView(); | |
| }, | |
| error: (error) => { | |
| // Handle error - show notification to user | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-account-actions/recovery-repayment/recovery-repayment.component.ts`
around lines 139 - 141, The subscribe call to
loanService.submitLoanActionButton(this.loanId, data, 'recoverypayment') in
recovery-repayment.component.ts has no error handler; update the Observable
subscription on submitLoanActionButton to provide next and error callbacks (or
use .subscribe({ next: ..., error: ... })), call gotoLoanDefaultView() only on
success, and in the error callback log the error and show user feedback via the
existing toast/notification service (or a generic error handler) and restore any
UI state (e.g., disable/enable buttons) as needed; reference loanService,
submitLoanActionButton, gotoLoanDefaultView and the component class to locate
where to add the error handling.
src/app/loans/loans-view/loan-account-actions/view-guarantors/view-guarantors.component.ts
Show resolved
Hide resolved
Comment on lines
+198
to
+201
| getWorkingCapitalLoannDetails(loanId: string) { | ||
| const httpParams = new HttpParams().set('associations', 'all'); | ||
| return this.http.get(`/working-capital-loans/${loanId}`, { params: httpParams }); | ||
| } |
There was a problem hiding this comment.
Typo in method name: getWorkingCapitalLoannDetails.
The method name has a double 'n' in "Loann". This should be getWorkingCapitalLoanDetails for consistency and searchability.
🔧 Proposed fix
- getWorkingCapitalLoannDetails(loanId: string) {
+ getWorkingCapitalLoanDetails(loanId: string) {
const httpParams = new HttpParams().set('associations', 'all');
return this.http.get(`/working-capital-loans/${loanId}`, { params: httpParams });
}Note: Update all call sites accordingly (likely in loan-details.resolver.ts).
📝 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
| getWorkingCapitalLoannDetails(loanId: string) { | |
| const httpParams = new HttpParams().set('associations', 'all'); | |
| return this.http.get(`/working-capital-loans/${loanId}`, { params: httpParams }); | |
| } | |
| getWorkingCapitalLoanDetails(loanId: string) { | |
| const httpParams = new HttpParams().set('associations', 'all'); | |
| return this.http.get(`/working-capital-loans/${loanId}`, { params: httpParams }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans.service.ts` around lines 198 - 201, Rename the
incorrectly spelled method getWorkingCapitalLoannDetails to
getWorkingCapitalLoanDetails in LoansService (the function currently defined as
getWorkingCapitalLoannDetails) and update every call site to use the corrected
name (e.g., in loan-details.resolver.ts and any tests or components that call
it); ensure the method signature (params and return) remains the same and update
any imports/types if they reference the old symbol so compilation passes.
| "NoDocuments": "Nav pieejamu dokumentu.", | ||
| "NoFileSelected": "Nav izvēlēts fails", | ||
| "NoNotesAvailable": "Nav pieejamas piezīmes", | ||
| "No Loan Product was found": "Nav atrasts neviens aizdevuma produkts", |
There was a problem hiding this comment.
Update the empty-state text to include Working Capital products.
Line 3464 says only “No Loan Product was found”, which is misleading now that the selection includes Working Capital products too.
💡 Proposed text update
- "No Loan Product was found": "Nav atrasts neviens aizdevuma produkts",
+ "No Loan Product was found": "Nav atrasts neviens aizdevuma vai apgrozāmā kapitāla produkts",📝 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
| "No Loan Product was found": "Nav atrasts neviens aizdevuma produkts", | |
| "No Loan Product was found": "Nav atrasts neviens aizdevuma vai apgrozāmā kapitāla produkts", |
🤖 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 3464, Update the empty-state
translation for the key "No Loan Product was found" so it mentions both Loan and
Working Capital products; locate the string "No Loan Product was found" in
src/assets/translations/lv-LV.json and replace its value with a Latvian phrasing
that includes Working Capital (for example: "Nav atrasts neviens aizdevuma vai
darba kapitāla produkts").
72a920c to
1e77750
Compare
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-account-creation
branch
from
1e77750 to
1df0d50
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
User should be able to create a new Working Capital Loan account of this Working Capital loan product
Related issues and discussion
WEB-813
Screenshots
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Bug Fixes
Refactor