WEB-657: Working Capital Goodwill Credit transaction - #3647
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Button Configuration Refactoring for Working Capital src/app/loans/loans-view/loan-accounts-button-config.ts |
Constructor accepts isWorkingCapital flag and branches initialization. Standard and working-capital-specific methods setWorkingCapitalButtons() and setWorkingCapitalOptions() define reduced button/option sets (e.g., limited options for "Submitted and pending approval", Goodwill Credit on "Active"). Existing "Submitted and pending approval" logic refactored into private addCommonActions() helper. |
Loans View Component Product-Type Gating src/app/loans/loans-view/loans-view.component.ts |
Button config initialized with isWorkingCapital flag. Repayment schedule edit, buy-down-fee, disbursal (Disburse to Savings, Undo Last Disbursal), and charge-off/re-age/re-amortize actions are now gated by isLoanProduct check. "Closed (obligations met)" case applies only for loan products while "Overpaid" remains independent. |
Product-Specific Action Template and Details src/app/loans/common-resolvers/loan-action-button.resolver.ts, src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts |
Goodwill Credit action template selection is conditional: getLoanActionTemplate() for loan products, getWorkingCapitalLoanActionTemplate() for working capital. Make Repayment details are hidden for working capital products via early return in showDetails(). |
Transaction Routing with Product Type Prefix src/app/loans/loans-view/transactions-tab/transactions-tab.component.ts, src/app/loans/loans-view/transactions-tab/transactions-tab.component.html |
New productTypePrefix getter returns 'L' for loan products and 'WC' for working capital. View Journal Entries route uses productTypePrefix + transaction.id instead of hardcoded 'L' prefix. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
- openMF/web-app#3419: Both PRs modify
LoanActionButtonResolver.resolve()to conditionally select action templates based onloanProductService.isLoanProduct(this PR for Goodwill Credit, referenced PR for Disburse). - openMF/web-app#3622: Both PRs introduce working-capital conditionals in the repayment flow—affecting
LoanActionButtonResolverandMakeRepaymentComponentfor different action branches (this PR: Goodwill Credit; referenced PR: Make Repayment).
Suggested reviewers
- adamsaghy
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately reflects the main change: adding Working Capital Goodwill Credit transaction support across multiple components and configuration files. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/loans/loans-view/loans-view.component.ts (1)
271-290: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHide the remaining loan-only actions for working-capital accounts.
Assign/Change Loan OfficerandPrepay Loanare still added without anisLoanProductguard, but their downstream flow still resolves through loan-only templates (getLoanTemplateandgetLoanPrepayLoanActionTemplate). Working-capital accounts can still reach unsupported screens from this view.Targeted fix
if (this.status === 'Submitted and pending approval') { - this.buttonConfig.addOption({ - name: this.loanDetailsData.loanOfficerName ? 'Change Loan Officer' : 'Assign Loan Officer', - icon: 'user-tie', - taskPermissionName: 'UPDATELOANOFFICER_LOAN' - }); + if (this.loanProductService.isLoanProduct) { + this.buttonConfig.addOption({ + name: this.loanDetailsData.loanOfficerName ? 'Change Loan Officer' : 'Assign Loan Officer', + icon: 'user-tie', + taskPermissionName: 'UPDATELOANOFFICER_LOAN' + }); + } ... } else if (this.status === 'Approved') { - this.buttonConfig.addButton({ - name: this.loanDetailsData.loanOfficerName ? 'Change Loan Officer' : 'Assign Loan Officer', - icon: 'user-tie', - taskPermissionName: 'UPDATELOANOFFICER_LOAN' - }); + if (this.loanProductService.isLoanProduct) { + this.buttonConfig.addButton({ + name: this.loanDetailsData.loanOfficerName ? 'Change Loan Officer' : 'Assign Loan Officer', + icon: 'user-tie', + taskPermissionName: 'UPDATELOANOFFICER_LOAN' + }); + } ... - if (this.recalculateInterest) { + if (this.loanProductService.isLoanProduct && this.recalculateInterest) { this.buttonConfig.addButton({ name: 'Prepay Loan', icon: 'coins', taskPermissionName: 'REPAYMENT_LOAN' }); }Also applies to: 349-355
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/loans-view.component.ts` around lines 271 - 290, The code adds "Assign/Change Loan Officer" (via buttonConfig.addOption/addButton with loanDetailsData.loanOfficerName) and "Prepay Loan" unconditionally, allowing working-capital accounts to navigate into loan-only templates (getLoanTemplate, getLoanPrepayLoanActionTemplate); wrap the additions of these actions with an isLoanProduct guard (this.loanProductService.isLoanProduct) so both the "Assign/Change Loan Officer" creation in the 'Submitted and pending approval' and 'Approved' branches and the block that adds the "Prepay Loan" action are only added when isLoanProduct is true.
🧹 Nitpick comments (2)
src/app/loans/loans-view/transactions-tab/transactions-tab.component.html (1)
254-261: 📐 Maintainability & Code Quality | 💤 Low valueConsider explicit string conversion for type safety.
The string concatenation
productTypePrefix + transaction.idrelies on implicit type coercion iftransaction.idis numeric. For clearer type safety, consider:productTypePrefix + String(transaction.id)or create a component method:
getJournalEntryRoute(transaction: LoanTransaction): string { return this.productTypePrefix + transaction.id; }and use:
[routerLink]="['/', 'accounting', 'journal-entries', 'transactions', 'view', getJournalEntryRoute(transaction)]"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/transactions-tab/transactions-tab.component.html` around lines 254 - 261, The routerLink endpoint currently concatenates productTypePrefix and transaction.id which relies on implicit coercion; change this to an explicit string conversion by either using String(transaction.id) in the routerLink expression (so productTypePrefix + String(transaction.id)) or add a component helper method getJournalEntryRoute(transaction: LoanTransaction) that returns productTypePrefix + String(transaction.id) and then reference getJournalEntryRoute(transaction) in the routerLink; update the template expression that builds the route accordingly to use the explicit string result and ensure productTypePrefix and transaction.id are referenced as shown.src/app/loans/loans-view/loan-accounts-button-config.ts (1)
166-217: 📐 Maintainability & Code Quality | 🏗️ Heavy liftDecouple the new action ids from their display labels.
These new working-capital actions are hardcoded English strings. Since
nameis also used as the routing/switch key elsewhere, they still can’t go through@ngx-translatesafely without coupling behavior to localized text. Please keep a stable action id and source the visible label from translation keys instead.Possible shape
- { - name: 'Goodwill Credit', - icon: 'coins', - taskPermissionName: 'CREATE_GOODWILL_TRANSACTION' - } + { + action: 'goodwillCredit', + labelKey: 'labels.menus.GoodwillCredit', + icon: 'coins', + taskPermissionName: 'CREATE_GOODWILL_TRANSACTION' + }As per coding guidelines, "Use proper i18n variables from
@ngx-translate/corefor all user-facing strings instead of hardcoded text" and "Runnpm run translations:extractto extract i18n variables whenever new strings are added to code."Also applies to: 349-374
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/loan-accounts-button-config.ts` around lines 166 - 217, The buttons created in setWorkingCapitalButtons are using hardcoded English strings for the action "name" which is also used as a routing/switch key; change the shape of each button object in buttonsArray to include a stable action id (e.g., actionId: 'ADD_LOAN_CHARGE') and a separate translation key property for the visible label (e.g., labelKey: 'loans.actions.addLoanCharge'), update any code that switches on or routes by name to use actionId instead of name (search for uses of buttonsArray and the name property), replace visible UI references to use the labelKey via `@ngx-translate`, apply the same changes for the other actions in this file (and the other block at lines ~349-374), and then run npm run translations:extract to generate the new translation entries.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/loans/loans-view/loans-view.component.ts`:
- Around line 271-290: The code adds "Assign/Change Loan Officer" (via
buttonConfig.addOption/addButton with loanDetailsData.loanOfficerName) and
"Prepay Loan" unconditionally, allowing working-capital accounts to navigate
into loan-only templates (getLoanTemplate, getLoanPrepayLoanActionTemplate);
wrap the additions of these actions with an isLoanProduct guard
(this.loanProductService.isLoanProduct) so both the "Assign/Change Loan Officer"
creation in the 'Submitted and pending approval' and 'Approved' branches and the
block that adds the "Prepay Loan" action are only added when isLoanProduct is
true.
---
Nitpick comments:
In `@src/app/loans/loans-view/loan-accounts-button-config.ts`:
- Around line 166-217: The buttons created in setWorkingCapitalButtons are using
hardcoded English strings for the action "name" which is also used as a
routing/switch key; change the shape of each button object in buttonsArray to
include a stable action id (e.g., actionId: 'ADD_LOAN_CHARGE') and a separate
translation key property for the visible label (e.g., labelKey:
'loans.actions.addLoanCharge'), update any code that switches on or routes by
name to use actionId instead of name (search for uses of buttonsArray and the
name property), replace visible UI references to use the labelKey via
`@ngx-translate`, apply the same changes for the other actions in this file (and
the other block at lines ~349-374), and then run npm run translations:extract to
generate the new translation entries.
In `@src/app/loans/loans-view/transactions-tab/transactions-tab.component.html`:
- Around line 254-261: The routerLink endpoint currently concatenates
productTypePrefix and transaction.id which relies on implicit coercion; change
this to an explicit string conversion by either using String(transaction.id) in
the routerLink expression (so productTypePrefix + String(transaction.id)) or add
a component helper method getJournalEntryRoute(transaction: LoanTransaction)
that returns productTypePrefix + String(transaction.id) and then reference
getJournalEntryRoute(transaction) in the routerLink; update the template
expression that builds the route accordingly to use the explicit string result
and ensure productTypePrefix and transaction.id are referenced as shown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c33c7f5-70c0-4346-b0a9-fd1a39638af6
📒 Files selected for processing (6)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-accounts-button-config.tssrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.htmlsrc/app/loans/loans-view/transactions-tab/transactions-tab.component.ts
|
@alberto-art3ch Please resolve the conflicts. |
982df45 to
a85a535
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/loans/loans-view/loan-accounts-button-config.ts`:
- Around line 166-221: In the setWorkingCapitalButtons method and the other
affected locations (also at lines 349-381 and 398-430), replace all hardcoded
user-facing strings in the button name properties with i18n translation keys
from `@ngx-translate/core`. For each action name like 'Add Loan Charge',
'Disburse', 'Undo Approval', 'Make Repayment', 'Undo Disbursal', and 'Goodwill
Credit', replace the hardcoded text with a translation key variable (e.g.,
this.translate.instant('TRANSLATION_KEY')) or use the pipe syntax in templates
if applicable. Ensure all user-facing action labels follow the Angular app
coding guidelines for i18n compliance across all three affected code sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f3ec397-09f5-40df-8ba0-5e9cdca237e8
📒 Files selected for processing (6)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loan-accounts-button-config.tssrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.htmlsrc/app/loans/loans-view/transactions-tab/transactions-tab.component.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
- src/app/loans/common-resolvers/loan-action-button.resolver.ts
- src/app/loans/loans-view/loans-view.component.ts
Description
User should be able to do Goodwill Credit transaction on active and closed/charge-off loan accounts
Related issues and discussion
WEB-957
Screenshots, if any
Screen.Recording.2026-06-12.at.5.33.43.PM.mov
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit