WEB-657: WorkingCapital Loan disbursement#3447
Merged
alberto-art3ch merged 1 commit intoMar 27, 2026
Merged
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Disburse Component src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.html, src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.ts |
Template: render discountAmount input when isWorkingCapital. TS: add isSubmitting flag, add discountAmount control when needed, coerce transactionAmount to number, build nested paymentDetails for working-capital flows, and send transformed payload to appropriate service call. |
Approve Loan Component src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.html, src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts |
Template/TS: add conditional discountAmount form control and input when isWorkingCapital, so approve payload includes discount for working-capital flows. |
Undo-Disbursal Component src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.ts |
Selects loanCommand and conditionally routes request to loanActionButtons() or applyWorkingCapitalLoanAccountCommand() based on product type; subscription handling split into next/error handlers and sets form-level submit errors on failure. |
Shared Input Amount src/app/shared/input-amount/input-amount.component.html |
Unifies DOM structure between required/non-required branches: adds input-container class and input-group wrapper to non-required branch to match required branch (DOM/styling alignment). |
Sequence Diagram(s)
sequenceDiagram
participant User as User
participant UI as DisburseComponent
participant Service as LoanService
participant Backend as Backend
User->>UI: Submit disbursement form
UI->>UI: Validate form & set isSubmitting = true
alt isWorkingCapital
UI->>UI: Add discountAmount (if present) & nest paymentDetails
UI->>Service: applyWorkingCapitalLoanAccountCommand(payload)
else
UI->>Service: loanActionButtons(loanId, 'disburse', payload)
end
Service->>Backend: POST request
alt Success
Backend-->>Service: 200 OK
Service-->>UI: Success
UI->>UI: Navigate to loan view
else Error
Backend-->>Service: Error
Service-->>UI: Error
UI->>UI: set isSubmitting = false, mark form submitFailed
end
sequenceDiagram
participant User as User
participant UI as UndoDisbursalComponent
participant Service as LoanService
participant Backend as Backend
User->>UI: Submit undo-disbursal
UI->>UI: Compute loanCommand (undodisbursal / undolastdisbursal)
alt isLoanProduct
UI->>Service: loanActionButtons(loanId, command, {note})
else isWorkingCapital
UI->>Service: applyWorkingCapitalLoanAccountCommand(command, {note})
else Neither
UI->>UI: return (no request)
end
Service->>Backend: POST request
alt Success
Backend-->>Service: 200 OK
Service-->>UI: Success
UI->>UI: Navigate to loan view
else Error
Backend-->>Service: Error
Service-->>UI: Error
UI->>UI: mark form submitFailed
end
Estimated Code Review Effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly Related PRs
- WEB-813: Working Capital loan account creation #3257: Adds working-capital-specific form fields, payload transformations, and conditional service routing used here.
- WEB-657: WorkingCapital Loan disbursement #3419: Modifies disburse flow and templates with working-capital conditionals overlapping these edits.
- WEB-446: Validation of re-Age amount during submission #2829: Changes to
input-amountcomponent align with the shared input DOM/styling updates present in this diff.
Suggested Reviewers
- adamsaghy
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and specifically describes the main objective of the PR: adding WorkingCapital Loan disbursement functionality, which aligns with the substantial changes across disburse, approve-loan, and undo-disbursal components. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.ts (1)
140-140: Consider using explicit number conversion.Using
* 1for number coercion is unconventional and less readable. PreferNumber()or unary+for clarity.♻️ Proposed fix
- payload['transactionAmount'] = payload['transactionAmount'] * 1; + payload['transactionAmount'] = Number(payload['transactionAmount']);🤖 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/disburse/disburse.component.ts` at line 140, Replace the unconventional numeric coercion payload['transactionAmount'] = payload['transactionAmount'] * 1 with an explicit conversion such as payload['transactionAmount'] = Number(payload['transactionAmount']) (or payload['transactionAmount'] = +payload['transactionAmount']) in disburse.component.ts; update the assignment where payload.transactionAmount is prepared (the transactionAmount conversion step) to use Number(...) for clarity and maintainability.src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.ts (1)
53-71: Consider adding submission guard for consistency.The
disburse.component.tsimplements anisSubmittingflag to prevent duplicate submissions, but this component lacks the same protection. Rapid clicks on the submit button could trigger multiple undo-disbursal requests.♻️ Proposed refactor to add submission guard
Add an
isSubmittingfield to the component:+ isSubmitting = false; + submit() { let loanCommand = 'undodisbursal'; if (this.actionName === 'Undo Last Disbursal') { loanCommand = 'undolastdisbursal'; } const request$ = this.isLoanProduct ? this.loanService.loanActionButtons(this.loanId, loanCommand, { note: this.note.value }) : this.isWorkingCapital ? this.loanService.applyWorkingCapitalLoanAccountCommand(this.loanId, loanCommand, { note: this.note.value }) : undefined; if (!request$) { return; } + this.isSubmitting = true; request$.subscribe({ next: () => this.gotoLoanDefaultView(), - error: () => {} + error: () => { + this.note.setErrors({ submitFailed: true }); + this.isSubmitting = false; + } }); }Then update the template's submit button to disable when
isSubmittingis true.🤖 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-disbursal/undo-disbursal.component.ts` around lines 53 - 71, The submit() handler in undo-disbursal.component.ts lacks a submission guard, allowing duplicate requests on rapid clicks; add an isSubmitting boolean field to the component, set isSubmitting = true before calling the service and set it back to false in both the next and error handlers of request$.subscribe (or use finalize) to prevent multiple submissions, and update the template's submit button to disable when isSubmitting is true; reference submit(), request$, and loanCommand to locate where to set the guard.
🤖 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/loans/loans-view/loan-account-actions/disburse/disburse.component.html`:
- Around line 49-56: Replace the hardcoded label string in the template: inside
the mifosx-input-amount for the discount field (the element bound to
disbursementLoanForm.controls.discountAmount and rendered when
isWorkingCapital), change inputLabel from the literal 'Discount' to an i18n
translation key (e.g. use the translate pipe or the component's translated
variable such as "loans.disburse.discount" via translate.instant or |
translate). Add that key to the translation source files and run npm run
translations:extract to extract/update the i18n entries.
In
`@src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.ts`:
- Around line 141-158: The code unconditionally builds paymentDetails inside the
isWorkingCapital branch, which can include undefined fields when payment
controls are hidden; update the logic in the DisburseComponent method where
this.isWorkingCapital is handled so that you only construct and attach
payload['paymentDetails'] when payment details are shown (e.g. check
this.showPaymentDetails or the payment form group/control existence) or, if you
must build it, filter out keys with undefined values before assigning to
payload; ensure you still remove the original flat keys (paymentTypeId,
accountNumber, checkNumber, routingCode, receiptNumber, bankNumber) only when
paymentDetails was actually set to avoid sending an object with undefined
properties.
In
`@src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.ts`:
- Around line 68-71: The subscription in undo-disbursal.component.ts currently
swallows errors (request$.subscribe(... error: () => {})) so the user gets no
feedback on failure; replace the empty error handler with one that surfaces the
failure (e.g., set form/group errors or call the same error-handling used in
disburse.component.ts), pass the received error into this.form.setErrors({
undoDisbursal: error }) or invoke the shared notification/toast service, and
ensure the template can display the form error so the user sees the failure
instead of a silent no-op; keep the existing next handler
(this.gotoLoanDefaultView()) unchanged.
---
Nitpick comments:
In
`@src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.ts`:
- Line 140: Replace the unconventional numeric coercion
payload['transactionAmount'] = payload['transactionAmount'] * 1 with an explicit
conversion such as payload['transactionAmount'] =
Number(payload['transactionAmount']) (or payload['transactionAmount'] =
+payload['transactionAmount']) in disburse.component.ts; update the assignment
where payload.transactionAmount is prepared (the transactionAmount conversion
step) to use Number(...) for clarity and maintainability.
In
`@src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.ts`:
- Around line 53-71: The submit() handler in undo-disbursal.component.ts lacks a
submission guard, allowing duplicate requests on rapid clicks; add an
isSubmitting boolean field to the component, set isSubmitting = true before
calling the service and set it back to false in both the next and error handlers
of request$.subscribe (or use finalize) to prevent multiple submissions, and
update the template's submit button to disable when isSubmitting is true;
reference submit(), request$, and loanCommand to locate where to set the guard.
🪄 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: a4e311ab-57b9-41f3-a3bf-ef606b283466
📒 Files selected for processing (3)
src/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/undo-disbursal/undo-disbursal.component.ts
Comment on lines
+49
to
+56
| @if (isWorkingCapital) { | ||
| <mifosx-input-amount | ||
| [currency]="currency" | ||
| [inputFormControl]="disbursementLoanForm.controls.discountAmount" | ||
| [inputLabel]="'Discount'" | ||
| > | ||
| </mifosx-input-amount> | ||
| } |
There was a problem hiding this comment.
Use i18n translation for the "Discount" label.
The inputLabel uses a hardcoded string 'Discount' instead of an i18n variable. As per coding guidelines, all user-facing strings should use proper i18n variables from @ngx-translate/core.
🌐 Proposed fix
`@if` (isWorkingCapital) {
<mifosx-input-amount
[currency]="currency"
[inputFormControl]="disbursementLoanForm.controls.discountAmount"
- [inputLabel]="'Discount'"
+ [inputLabel]="'labels.inputs.Discount' | translate"
>
</mifosx-input-amount>
}Also run npm run translations:extract to extract the new i18n variable.
📝 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 (isWorkingCapital) { | |
| <mifosx-input-amount | |
| [currency]="currency" | |
| [inputFormControl]="disbursementLoanForm.controls.discountAmount" | |
| [inputLabel]="'Discount'" | |
| > | |
| </mifosx-input-amount> | |
| } | |
| `@if` (isWorkingCapital) { | |
| <mifosx-input-amount | |
| [currency]="currency" | |
| [inputFormControl]="disbursementLoanForm.controls.discountAmount" | |
| [inputLabel]="'labels.inputs.Discount' | translate" | |
| > | |
| </mifosx-input-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/disburse/disburse.component.html`
around lines 49 - 56, Replace the hardcoded label string in the template: inside
the mifosx-input-amount for the discount field (the element bound to
disbursementLoanForm.controls.discountAmount and rendered when
isWorkingCapital), change inputLabel from the literal 'Discount' to an i18n
translation key (e.g. use the translate pipe or the component's translated
variable such as "loans.disburse.discount" via translate.instant or |
translate). Add that key to the translation source files and run npm run
translations:extract to extract/update the i18n entries.
alberto-art3ch
force-pushed
the
WEB-657/workingcapital-loan-disbursement
branch
from
bf26442 to
93556db
Compare
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/loan-account-actions/approve-loan/approve-loan.component.ts (1)
55-55:⚠️ Potential issue | 🟡 MinorRemove debug
console.logstatement.This debug statement should be removed before merging to avoid cluttering browser console in production.
🧹 Proposed fix
this.route.data.subscribe((data: { actionButtonData: any }) => { - console.log(data.actionButtonData); this.loanData = data.actionButtonData; this.currency = data.actionButtonData.currency; });🤖 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` at line 55, In the ApproveLoanComponent (approve-loan.component.ts) remove the debug console.log call that prints data.actionButtonData; locate the console.log(data.actionButtonData) inside the component (e.g., in the method handling incoming data or ngOnInit) and delete that statement so the production bundle doesn't emit debug logs.
🧹 Nitpick comments (1)
src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.ts (1)
140-140: Consider using explicit number conversion.Using
* 1for number coercion works but is less readable. Consider usingNumber()or the unary+operator for clarity.✨ Optional style improvement
- payload['transactionAmount'] = payload['transactionAmount'] * 1; + payload['transactionAmount'] = Number(payload['transactionAmount']);🤖 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/disburse/disburse.component.ts` at line 140, Replace the implicit numeric coercion using multiplication by 1 with an explicit conversion for clarity: in the disburse component where payload['transactionAmount'] is set (the line using payload['transactionAmount'] = payload['transactionAmount'] * 1), change it to use Number(payload['transactionAmount']) or the unary + (e.g., +payload['transactionAmount']) so the intent to convert to a number is explicit; ensure you update any surrounding validation if you need to handle NaN cases after conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/app/loans/loans-view/loan-account-actions/approve-loan/approve-loan.component.ts`:
- Line 55: In the ApproveLoanComponent (approve-loan.component.ts) remove the
debug console.log call that prints data.actionButtonData; locate the
console.log(data.actionButtonData) inside the component (e.g., in the method
handling incoming data or ngOnInit) and delete that statement so the production
bundle doesn't emit debug logs.
---
Nitpick comments:
In
`@src/app/loans/loans-view/loan-account-actions/disburse/disburse.component.ts`:
- Line 140: Replace the implicit numeric coercion using multiplication by 1 with
an explicit conversion for clarity: in the disburse component where
payload['transactionAmount'] is set (the line using payload['transactionAmount']
= payload['transactionAmount'] * 1), change it to use
Number(payload['transactionAmount']) or the unary + (e.g.,
+payload['transactionAmount']) so the intent to convert to a number is explicit;
ensure you update any surrounding validation if you need to handle NaN cases
after conversion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8157a94b-cfe8-439b-8972-45d6e3e2fd29
📒 Files selected for processing (6)
src/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/disburse/disburse.component.htmlsrc/app/loans/loans-view/loan-account-actions/disburse/disburse.component.tssrc/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.tssrc/app/shared/input-amount/input-amount.component.html
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/loans/loans-view/loan-account-actions/undo-disbursal/undo-disbursal.component.ts
2 tasks
This was referenced May 12, 2026
This was referenced May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Now that we have the WorkingCapital Loan account creation we need to disburse the account once time that It has being approved
WEB-657
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