WEB-657: Working Capital Loan Delinquency - Reset & Undo Reset#3729
WEB-657: Working Capital Loan Delinquency - Reset & Undo Reset#3729somasorosdpc wants to merge 2 commits into
Conversation
|
👋 Hi @somasorosdpc — thank you for your pull request. This PR is currently blocked because we do not have a Contributor License Agreement (CLA) on file for your GitHub account. To get unblocked:
|
Signed-off-by: Soma Sörös <soma.soros@dpc.hu>
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Reset dialog and form src/app/loans/custom-dialog/loan-delinquency-action-reset-dialog/* |
Adds a standalone reactive-form dialog with a startNewPeriod checkbox and Cancel/Reset actions. |
Reset action orchestration src/app/loans/loans-view/loan-delinquency-tags-tab/* |
Adds permission-gated Reset and Undo Reset controls, confirmation handling, updated action call sites, and startNewPeriod in reset payloads. |
Reset action translations src/assets/translations/* |
Adds localized Undo Reset, Start new period, and undo-reset confirmation strings across supported locales. |
Service worker serving path
| Layer / File(s) | Summary |
|---|---|
Browser build serving package.json |
Points the serve:sw script to ./dist/web-app/browser. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Sequence Diagram(s)
sequenceDiagram
participant User
participant LoanDelinquencyTagsTabComponent
participant LoanDelinquencyActionResetDialogComponent
participant DelinquencyActionAPI
User->>LoanDelinquencyTagsTabComponent: choose Reset
LoanDelinquencyTagsTabComponent->>LoanDelinquencyActionResetDialogComponent: open form
LoanDelinquencyActionResetDialogComponent-->>LoanDelinquencyTagsTabComponent: return startNewPeriod
LoanDelinquencyTagsTabComponent->>DelinquencyActionAPI: submit reset payload
User->>LoanDelinquencyTagsTabComponent: choose Undo Reset
LoanDelinquencyTagsTabComponent->>DelinquencyActionAPI: submit undo-reset action
Possibly related PRs
- openMF/web-app#3695: Modifies the same delinquency pause/resume flow in
LoanDelinquencyTagsTabComponent.
Suggested reviewers: saksham869
🚥 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 clearly summarizes the main change: working capital loan delinquency reset and undo reset support. |
| 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. |
✨ 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.
cbb3d3c to
eab7e62
Compare
|
👋 Hi @somasorosdpc — thank you for your pull request. This PR is currently blocked because we do not have a Contributor License Agreement (CLA) on file for your GitHub account. To get unblocked:
|
|
👋 Hi @somasorosdpc — thank you for your pull request. This PR is currently blocked because we do not have a Contributor License Agreement (CLA) on file for your GitHub account. To get unblocked:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts (1)
423-425: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
anytyped API response.Based on learnings, avoid using
anyfor API responses. Since theresultparameter is entirely unused in the callback, remove it to improve type safety and clean up the signature.♻️ Proposed refactor
this.loansServices .createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload) - .subscribe((result: any) => { + .subscribe(() => { this.loansServices🤖 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-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts` around lines 423 - 425, Remove the unused result parameter and its any type from the subscribe callback in the createDelinquencyActions call, leaving the callback body and request behavior unchanged.Source: Learnings
🤖 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/custom-dialog/loan-delinquency-action-reset-dialog/loan-delinquency-action-reset-dialog.component.ts`:
- Around line 40-54: Replace UntypedFormBuilder and UntypedFormGroup in the
delinquencyActionForm setup with FormBuilder and a strongly typed FormGroup
containing a FormControl<boolean> for startNewPeriod. Update the imports and the
createDelinquencyActionForm method while preserving the existing default value
of false.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html`:
- Around line 166-181: Replace the m-r-10 class on the calendar icons in
createDelinquencyActionReset and createDelinquencyActionUndoReset with the
existing 8px-aligned margin utility, such as m-r-8, while preserving the
buttons’ behavior and styling.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 330-334: Prevent cancelled dialogs from causing null-pointer
errors in both afterClosed subscriptions: in the loanDelinquencyActionDialogRef
handler at
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts:330-334,
return before accessing response.data.value.startNewPeriod when response is
absent; in the confirmation handler at the same file:344-348, use optional
chaining when checking response.confirm.
In `@src/assets/translations/en-US.json`:
- Line 3393: Update the translation value for the existing “Are you sure you
want to undo last reset action” key to “Are you sure you want to undo the last
reset action?” while keeping the key unchanged.
In `@src/assets/translations/lt-LT.json`:
- Line 746: Update both “Undo Reset” translation entries and the related
confirmation text in the Lithuanian translations to use wording that clearly
conveys undoing a reset, consistently replacing the current “cancel settings
restoration” terminology.
---
Nitpick comments:
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 423-425: Remove the unused result parameter and its any type from
the subscribe callback in the createDelinquencyActions call, leaving the
callback body and request behavior unchanged.
🪄 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: 89cf43d2-0071-4abc-b4c0-7333849c6c0c
📒 Files selected for processing (19)
package.jsonsrc/app/loans/custom-dialog/loan-delinquency-action-reset-dialog/loan-delinquency-action-reset-dialog.component.htmlsrc/app/loans/custom-dialog/loan-delinquency-action-reset-dialog/loan-delinquency-action-reset-dialog.component.scsssrc/app/loans/custom-dialog/loan-delinquency-action-reset-dialog/loan-delinquency-action-reset-dialog.component.tssrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.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
| private formBuilder = inject(UntypedFormBuilder); | ||
|
|
||
| delinquencyActionForm: UntypedFormGroup; | ||
|
|
||
| constructor() { | ||
| this.createDelinquencyActionForm(); | ||
| } | ||
|
|
||
| createDelinquencyActionForm() { | ||
| this.delinquencyActionForm = this.formBuilder.group({ | ||
| startNewPeriod: [ | ||
| false | ||
| ] | ||
| }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use strongly typed forms instead of untyped forms.
As per coding guidelines, use strict typing conventions. UntypedFormBuilder and UntypedFormGroup bypass compile-time type safety. Prefer strongly typed FormBuilder and FormGroup for the reactive form.
♻️ Proposed refactor
Update your imports to replace UntypedFormBuilder and UntypedFormGroup with FormBuilder, FormGroup, and FormControl, then apply this strongly-typed pattern:
- private formBuilder = inject(UntypedFormBuilder);
+ private formBuilder = inject(FormBuilder);
- delinquencyActionForm: UntypedFormGroup;
+ delinquencyActionForm: FormGroup;
constructor() {
this.createDelinquencyActionForm();
}
createDelinquencyActionForm() {
this.delinquencyActionForm = this.formBuilder.group({
- startNewPeriod: [
- false
- ]
+ startNewPeriod: [false]
});
}🤖 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/custom-dialog/loan-delinquency-action-reset-dialog/loan-delinquency-action-reset-dialog.component.ts`
around lines 40 - 54, Replace UntypedFormBuilder and UntypedFormGroup in the
delinquencyActionForm setup with FormBuilder and a strongly typed FormGroup
containing a FormControl<boolean> for startNewPeriod. Update the imports and the
createDelinquencyActionForm method while preserving the existing default value
of false.
Source: Coding guidelines
| <button | ||
| mat-raised-button | ||
| color="primary" | ||
| (click)="createDelinquencyActionReset()" | ||
| *mifosxHasPermission="'CREATE_WC_DELINQUENCY_RESET'" | ||
| > | ||
| <fa-icon icon="calendar" class="m-r-10"></fa-icon>{{ 'labels.buttons.Reset' | translate }} | ||
| </button> | ||
| <button | ||
| mat-raised-button | ||
| color="primary" | ||
| (click)="createDelinquencyActionUndoReset()" | ||
| *mifosxHasPermission="'CREATE_WC_DELINQUENCY_UNDO_RESET'" | ||
| > | ||
| <fa-icon icon="calendar" class="m-r-10"></fa-icon>{{ 'labels.buttons.Undo Reset' | translate }} | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align margin utility classes with the 8px grid system.
As per coding guidelines, visual design and spacing must stick to the 8px grid system. The m-r-10 class introduces a 10px margin, which violates this rule. Please replace m-r-10 with an 8px-aligned utility class (e.g., m-r-8) or an appropriate SCSS variable.
♻️ Proposed fix
<button
mat-raised-button
color="primary"
(click)="createDelinquencyActionReset()"
*mifosxHasPermission="'CREATE_WC_DELINQUENCY_RESET'"
>
- <fa-icon icon="calendar" class="m-r-10"></fa-icon>{{ 'labels.buttons.Reset' | translate }}
+ <fa-icon icon="calendar" class="m-r-8"></fa-icon>{{ 'labels.buttons.Reset' | translate }}
</button>
<button
mat-raised-button
color="primary"
(click)="createDelinquencyActionUndoReset()"
*mifosxHasPermission="'CREATE_WC_DELINQUENCY_UNDO_RESET'"
>
- <fa-icon icon="calendar" class="m-r-10"></fa-icon>{{ 'labels.buttons.Undo Reset' | translate }}
+ <fa-icon icon="calendar" class="m-r-8"></fa-icon>{{ 'labels.buttons.Undo Reset' | translate }}
</button>📝 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.
| <button | |
| mat-raised-button | |
| color="primary" | |
| (click)="createDelinquencyActionReset()" | |
| *mifosxHasPermission="'CREATE_WC_DELINQUENCY_RESET'" | |
| > | |
| <fa-icon icon="calendar" class="m-r-10"></fa-icon>{{ 'labels.buttons.Reset' | translate }} | |
| </button> | |
| <button | |
| mat-raised-button | |
| color="primary" | |
| (click)="createDelinquencyActionUndoReset()" | |
| *mifosxHasPermission="'CREATE_WC_DELINQUENCY_UNDO_RESET'" | |
| > | |
| <fa-icon icon="calendar" class="m-r-10"></fa-icon>{{ 'labels.buttons.Undo Reset' | translate }} | |
| </button> | |
| <button | |
| mat-raised-button | |
| color="primary" | |
| (click)="createDelinquencyActionReset()" | |
| *mifosxHasPermission="'CREATE_WC_DELINQUENCY_RESET'" | |
| > | |
| <fa-icon icon="calendar" class="m-r-8"></fa-icon>{{ 'labels.buttons.Reset' | translate }} | |
| </button> | |
| <button | |
| mat-raised-button | |
| color="primary" | |
| (click)="createDelinquencyActionUndoReset()" | |
| *mifosxHasPermission="'CREATE_WC_DELINQUENCY_UNDO_RESET'" | |
| > | |
| <fa-icon icon="calendar" class="m-r-8"></fa-icon>{{ 'labels.buttons.Undo Reset' | translate }} | |
| </button> |
🤖 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-delinquency-tags-tab/loan-delinquency-tags-tab.component.html`
around lines 166 - 181, Replace the m-r-10 class on the calendar icons in
createDelinquencyActionReset and createDelinquencyActionUndoReset with the
existing 8px-aligned margin utility, such as m-r-8, while preserving the
buttons’ behavior and styling.
Source: Coding guidelines
| loanDelinquencyActionDialogRef.afterClosed().subscribe((response: { data: any }) => { | ||
| const startNewPeriod: boolean = response.data.value.startNewPeriod; | ||
|
|
||
| this.sendDelinquencyAction(action, null, null, null, null, null, null, startNewPeriod); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Null pointer exception on dialog cancellation.
Both afterClosed() subscriptions lack a null check for response. If the user cancels the dialog (e.g., by clicking the backdrop or pressing ESC), response will be undefined, causing a runtime crash (TypeError) when accessing its properties.
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts#L330-L334: Add a guard (e.g.,if (!response) return;) before accessingresponse.data.value.startNewPeriod.src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts#L344-L348: Use optional chaining (if (response?.confirm)) before evaluating the confirmation check.
📍 Affects 1 file
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts#L330-L334(this comment)src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts#L344-L348
🤖 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-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`
around lines 330 - 334, Prevent cancelled dialogs from causing null-pointer
errors in both afterClosed subscriptions: in the loanDelinquencyActionDialogRef
handler at
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts:330-334,
return before accessing response.data.value.startNewPeriod when response is
absent; in the confirmation handler at the same file:344-348, use optional
chaining when checking response.confirm.
| "Are you sure you want to delete checker": "Are you sure you want to delete checker", | ||
| "Are you sure you want to Approve Loan": "Are you sure you want to Approve Loan", | ||
| "Are you sure you want to Disburse Loan": "Are you sure you want to Disburse Loan", | ||
| "Are you sure you want to undo last reset action": "Are you sure you want to undo last reset action", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Polish the undo-reset confirmation text.
Use “Are you sure you want to undo the last reset action?” and keep the translation key unchanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/en-US.json` at line 3393, Update the translation
value for the existing “Are you sure you want to undo last reset action” key to
“Are you sure you want to undo the last reset action?” while keeping the key
unchanged.
| "Unassign Staff": "Atšaukti personalo priskyrimą", | ||
| "Undo": "Anuliuoti", | ||
| "Undo Rejection": "Anuliuoti atmetimą", | ||
| "Undo Reset": "Atšaukti nustatymų atkūrimą", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File size =="
wc -l src/assets/translations/lt-LT.json
echo
echo "== Context around changed lines =="
sed -n '730,760p' src/assets/translations/lt-LT.json
echo
sed -n '1550,1575p' src/assets/translations/lt-LT.json
echo
sed -n '2920,2945p' src/assets/translations/lt-LT.json
echo
sed -n '3350,3375p' src/assets/translations/lt-LT.json
echo
echo "== Search for related source keys =="
rg -n '"Undo Reset"|"Start new period"|delinquen|reset' src -g '!src/assets/translations/lt-LT.json'Repository: openMF/web-app
Length of output: 50371
Use wording that clearly means “undo reset.”
Atšaukti nustatymų atkūrimą reads like “cancel settings restoration,” which is misleading for this reset action. Update both Undo Reset entries and the confirmation text to consistent reset/undo terminology.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/lt-LT.json` at line 746, Update both “Undo Reset”
translation entries and the related confirmation text in the Lithuanian
translations to use wording that clearly conveys undoing a reset, consistently
replacing the current “cancel settings restoration” terminology.
alberto-art3ch
left a comment
There was a problem hiding this comment.
LGTM - Please only one commit
|
already fixed. #3731 |
Description
As of Fineract introduces Delinquency reset and undo reset features for working capital loans, this Pr is about to implement UI for those features.
On the Loan - Delinquency Tag tab there will be 2 new button on the top right corner.
On positive response both case will call the create delinquency action endpoint and refreshes the lists on the delinquency tab.
All Translations set for new labels.
Related issues and discussion
#{WEB-657}
Screenshots, if any
Delinquency Tags Tab

Reset Dialog

Undo Reset dialog

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