WEB-849: Replace reschedule date picker with installment dropdown and fix spacing#3379
Merged
IOhacker merged 1 commit intoopenMF:devfrom Mar 14, 2026
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loan Reschedule Component src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.html, src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss, src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts |
Replaced date input/datepicker with a populated mat-select of unpaid installments. Added unpaidInstallments: RepaymentSchedulePeriod[], loadRepaymentSchedule() invoked on init, getInstallmentDueDate() and selectedInstallment accessors. Added select-trigger markup and theme-aware styles; removed datepicker-specific bindings. |
Translations (i18n) src/assets/translations/cs-CS.json, de-DE.json, en-US.json, es-CL.json, es-MX.json, fr-FR.json, it-IT.json, ko-KO.json, lt-LT.json, lv-LV.json, ne-NE.json, pt-PT.json, sw-SW.json |
Added Installment translation key across many locales and standardized/emphasized dash/em-dash characters in platform description strings; minor formatting adjustments (trailing commas, empty-line removals). |
Sequence Diagram(s)
mermaid
sequenceDiagram
participant Component
participant LoanService
participant API
Component->>LoanService: loadRepaymentSchedule(loanId)
LoanService->>API: GET /loanAccounts/{id}?associations=repaymentSchedule
API-->>LoanService: repaymentSchedule.periods
LoanService-->>Component: periods
Component->>Component: filter unpaidInstallments & update form/select
Component-->>User: render mat-select options & select-trigger
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
- IOhacker
- alberto-art3ch
🚥 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 accurately reflects the main changes: replacing a date picker with an installment dropdown and fixing spacing issues in the loan reschedule component. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📝 Coding Plan
- Generate coding plan for human review comments
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: 9
🧹 Nitpick comments (2)
src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts (1)
136-144: Getter creates Date objects on each access during change detectionThe
selectedInstallmentgetter iterates throughunpaidInstallmentsand callsgetInstallmentDueDate()for each element on every access. Since getters are invoked during each change detection cycle, this creates unnecessary Date objects repeatedly.Consider computing the selected installment reactively when the form value changes:
♻️ Suggested approach
selectedInstallment: RepaymentSchedulePeriod | null = null; // In ngOnInit or after form setup: this.rescheduleLoanForm.get('rescheduleFromDate')?.valueChanges .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe((selectedDate: Date | null) => { if (!selectedDate || !(selectedDate instanceof Date)) { this.selectedInstallment = null; return; } const selectedTime = selectedDate.getTime(); this.selectedInstallment = this.unpaidInstallments.find( (inst) => this.getInstallmentDueDate(inst).getTime() === selectedTime ) ?? 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/loan-reschedule/loan-reschedule.component.ts` around lines 136 - 144, The getter selectedInstallment currently recalculates dates on every change-detection pass; replace it with a class property (e.g., selectedInstallment: RepaymentSchedulePeriod | null) and compute/update that property reactively by subscribing to this.rescheduleLoanForm.get('rescheduleFromDate')?.valueChanges (in ngOnInit or right after form setup) using takeUntilDestroyed(this.destroyRef) or equivalent cleanup; in the subscription, validate the Date, compute selectedTime = selectedDate.getTime(), and set selectedInstallment = this.unpaidInstallments.find(inst => this.getInstallmentDueDate(inst).getTime() === selectedTime) ?? null; remove the expensive logic from the getter so change detection no longer creates Date objects repeatedly.src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss (1)
18-52: Consider using SCSS theme variables for colorsHardcoded color values like
rgb(0 0 0 / 70%),#3a3a3a, and#4a4a4ashould ideally leverage SCSS variables fromsrc/theme/mifosx-theme.scssfor theme consistency and maintainability. This applies to both the new installment styles and the dark-theme overrides.🤖 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.scss` around lines 18 - 52, Replace hardcoded color literals in the shown selectors with the SCSS theme variables: import your theme variables and use the appropriate tokens instead of rgb(0 0 0 / 70%), rgb(255 255 255 / 70%), `#3a3a3a` and `#4a4a4a`; specifically update .installment-option-details and .trigger-installment-details to use the theme's muted/text color variables and update .penalty-item-container (and its dark-theme override under :host-context(.dark-theme)) to use the theme surface/background and border color variables so colors come from the central src/theme/mifosx-theme.scss variables instead of hardcoded values.
🤖 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/loan-reschedule/loan-reschedule.component.html`:
- Around line 16-50: The mat-select is comparing Date objects by reference
because getInstallmentDueDate(installment) returns a new Date each render, so
implement a compare function on the component (e.g. compareDates(date1: Date,
date2: Date): boolean that returns true when both are non-null and
date1.getTime() === date2.getTime()) and bind it to the mat-select using
compareWith="compareDates" so the selected value is matched by timestamp rather
than object reference; ensure the compare function safely handles null/undefined
values.
In
`@src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts`:
- Around line 96-106: In loadRepaymentSchedule(), replace the loose any on the
subscription callback by using the concrete response type returned by
loanService.getLoanAccountAssociationDetails (e.g., a
LoanAccountAssociationDetails or similar interface that includes
repaymentSchedule and periods) and use RepaymentSchedulePeriod for periods; keep
assigning this.unpaidInstallments from periods.filter(...) as before. Add
takeUntilDestroyed(this.destroyRef) to the observable chain
(loanService.getLoanAccountAssociationDetails(...).pipe(...)) to ensure the
subscription is cleaned up. In the error handler, do not silently swallow
failures: log the error (or call the component’s toast/notification method) with
context so users are informed and still set this.unpaidInstallments = [] when
appropriate. Ensure you import/use the correct types and keep method name
loadRepaymentSchedule and property unpaidInstallments unchanged.
In `@src/assets/translations/de-DE.json`:
- Line 312: The "Platform Description" value in de-DE.json contains
double-escaped quotes (\\\") and an em dash (—); update the string for key
"Platform Description" to use single-escaped quotes (\"...\" in the JSON value)
and replace the em dash with a standard hyphen/minus (-) so the UI doesn't show
backslashes and matches the PR's dash normalization.
In `@src/assets/translations/en-US.json`:
- Line 313: The translation value for the "Platform Description" key uses an em
dash (—); update the string to replace the em dash with a standard ASCII dash ("
- ") so it matches the project's dash normalization and existing copy; edit the
value for "Platform Description" to use " - " in place of the em dash while
preserving the surrounding HTML and escape sequences.
In `@src/assets/translations/es-CL.json`:
- Line 312: The string value for the "Platform Description" key contains an em
dash (—); replace that em dash with a standard hyphen (-) in the JSON value for
"Platform Description" so the translation uses a consistent dash character;
update the value text that currently includes "Plataforma Mifos X—el sistema
bancario central..." to "Plataforma Mifos X - el sistema bancario central..."
ensuring escaping/quoting in the JSON remains valid.
In `@src/assets/translations/es-MX.json`:
- Line 312: The "Platform Description" translation string still uses an em dash
(—); update the value for the "Platform Description" key in
src/assets/translations/es-MX.json to replace the em dash with a standard hyphen
(-) so it matches the dash normalization across translations, keeping the rest
of the text and HTML unchanged.
In `@src/assets/translations/lt-LT.json`:
- Line 312: In the lt-LT.json translation for the "Platform Description" key
replace the Unicode em dash (—) after the closing </a> with a standard ASCII
hyphen/minus (-) so the string uses the consistent dash character; update the
value associated with "Platform Description" accordingly and ensure the JSON
string remains properly escaped (keep the existing escaped quotes like
\\\"paskutinę mylią\\\").
In `@src/assets/translations/lv-LV.json`:
- Line 312: The "Platform Description" value still contains an em dash (—);
replace that character with a standard hyphen-minus (-) in the "Platform
Description" string so the JSON uses a regular dash; ensure the replacement is
done inside the value for the "Platform Description" key and that the JSON
remains properly escaped and valid after the change.
In `@src/assets/translations/ne-NE.json`:
- Around line 2826-2827: The translation for the "Installment" key uses "किस्त"
but the file uses "किस्ता" elsewhere; update the value for the "Installment" key
to "किस्ता" to keep terminology consistent (find the "Installment" JSON key in
src/assets/translations/ne-NE.json and replace its value), ensuring the rest of
the file's installment-related strings remain unchanged.
---
Nitpick comments:
In
`@src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss`:
- Around line 18-52: Replace hardcoded color literals in the shown selectors
with the SCSS theme variables: import your theme variables and use the
appropriate tokens instead of rgb(0 0 0 / 70%), rgb(255 255 255 / 70%), `#3a3a3a`
and `#4a4a4a`; specifically update .installment-option-details and
.trigger-installment-details to use the theme's muted/text color variables and
update .penalty-item-container (and its dark-theme override under
:host-context(.dark-theme)) to use the theme surface/background and border color
variables so colors come from the central src/theme/mifosx-theme.scss variables
instead of hardcoded values.
In
`@src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts`:
- Around line 136-144: The getter selectedInstallment currently recalculates
dates on every change-detection pass; replace it with a class property (e.g.,
selectedInstallment: RepaymentSchedulePeriod | null) and compute/update that
property reactively by subscribing to
this.rescheduleLoanForm.get('rescheduleFromDate')?.valueChanges (in ngOnInit or
right after form setup) using takeUntilDestroyed(this.destroyRef) or equivalent
cleanup; in the subscription, validate the Date, compute selectedTime =
selectedDate.getTime(), and set selectedInstallment =
this.unpaidInstallments.find(inst => this.getInstallmentDueDate(inst).getTime()
=== selectedTime) ?? null; remove the expensive logic from the getter so change
detection no longer creates Date objects repeatedly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f100cab8-9e8c-4f25-873e-8f8ed21a8d10
📒 Files selected for processing (16)
src/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.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.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
Comment on lines
+16
to
+50
| <mat-select required formControlName="rescheduleFromDate"> | ||
| @if (selectedInstallment) { | ||
| <mat-select-trigger> | ||
| <span class="trigger-installment-title" | ||
| >{{ 'labels.inputs.Due Date' | translate }}: {{ selectedInstallment.dueDate | dateFormat }}</span | ||
| > | ||
| <span class="trigger-installment-details"> | ||
| <span> | ||
| · {{ 'labels.inputs.Principal Due' | translate }}: | ||
| {{ selectedInstallment.principalDue | formatNumber }}</span | ||
| > | ||
| <span> | ||
| · {{ 'labels.inputs.Interest' | translate }}: | ||
| {{ selectedInstallment.interestDue | formatNumber }}</span | ||
| > | ||
| </span> | ||
| </mat-select-trigger> | ||
| } | ||
| @for (installment of unpaidInstallments; track installment.period) { | ||
| <mat-option [value]="getInstallmentDueDate(installment)"> | ||
| <div class="installment-option-title"> | ||
| {{ 'labels.inputs.Due Date' | translate }}: {{ installment.dueDate | dateFormat }} | ||
| </div> | ||
| <div class="installment-option-details"> | ||
| <span> | ||
| · {{ 'labels.inputs.Principal Due' | translate }}: | ||
| {{ installment.principalDue | formatNumber }}</span | ||
| > | ||
| <span> | ||
| · {{ 'labels.inputs.Interest' | translate }}: {{ installment.interestDue | formatNumber }}</span | ||
| > | ||
| </div> | ||
| </mat-option> | ||
| } | ||
| </mat-select> |
There was a problem hiding this comment.
Date object reference comparison will cause selection display issues
The mat-select compares option values by reference. Since getInstallmentDueDate(installment) creates a new Date object on each call, the form control's stored Date won't match option values by reference after re-renders, causing the selected option to not display correctly.
Add a compareWith function to compare dates by timestamp:
🐛 Proposed fix
In the template:
-<mat-select required formControlName="rescheduleFromDate">
+<mat-select required formControlName="rescheduleFromDate" [compareWith]="compareDates">In the component TypeScript:
compareDates = (date1: Date, date2: Date): boolean => {
if (!date1 || !date2) return false;
return date1.getTime() === date2.getTime();
};🤖 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.html`
around lines 16 - 50, The mat-select is comparing Date objects by reference
because getInstallmentDueDate(installment) returns a new Date each render, so
implement a compare function on the component (e.g. compareDates(date1: Date,
date2: Date): boolean that returns true when both are non-null and
date1.getTime() === date2.getTime()) and bind it to the mat-select using
compareWith="compareDates" so the selected value is matched by timestamp rather
than object reference; ensure the compare function safely handles null/undefined
values.
Comment on lines
+96
to
106
| loadRepaymentSchedule() { | ||
| this.loanService.getLoanAccountAssociationDetails(this.loanId).subscribe({ | ||
| next: (loanDetails: any) => { | ||
| const periods: RepaymentSchedulePeriod[] = loanDetails?.repaymentSchedule?.periods || []; | ||
| this.unpaidInstallments = periods.filter((period) => period.period != null && !period.complete); | ||
| }, | ||
| error: () => { | ||
| this.unpaidInstallments = []; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if route resolvers already provide loan details with repayment schedule
rg -n "LoanReschedule" --type ts -A 10 | head -60
# Check the routing module for resolvers on the reschedule route
fd -e ts -p "loans.*routing" --exec rg -n "reschedule" -A 5 -B 2 {}Repository: openMF/web-app
Length of output: 7774
🏁 Script executed:
#!/bin/bash
# Check the LoanReschedulesResolver implementation
fd -e ts -p "loan-reschedules.resolver" --exec cat {}
# Find where loan-reschedule.component.ts is used in routing
rg -n "loan-reschedule.component" --type ts
# Check the full route configuration for loan-account-actions
rg -n "loan-account-actions" -A 10 --type ts | head -80Repository: openMF/web-app
Length of output: 12703
🏁 Script executed:
#!/bin/bash
# Check the service methods being called
rg -n "getLoanAccountAssociationDetails|loanRescheduleRequests" src/app/loans/loans.service.ts -A 3
# Check how LoanRescheduleComponent is used in loan-account-actions
rg -n "LoanRescheduleComponent" src/app/loans/loans-view/loan-account-actions/loan-account-actions.component.ts -B 3 -A 10Repository: openMF/web-app
Length of output: 2265
🏁 Script executed:
#!/bin/bash
# Read the full loan-reschedule.component.ts file to verify concerns
cat -n src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts | head -120
# Check the imports to understand the component better
head -50 src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.tsRepository: openMF/web-app
Length of output: 6787
🏁 Script executed:
#!/bin/bash
# Check if subscription is unmanaged (no takeUntilDestroyed)
sed -n '96,110p' src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts
# Check how other components in the same directory handle subscriptions
rg -n "takeUntilDestroyed|takeUntil" src/app/loans/loans-view/loan-account-actions/ -A 1 | head -30
# Check if there's a toast/notification pattern for errors in the codebase
rg -n "toastr|snackBar|MatSnackBar|notify" src/app/loans/loans-view/loan-account-actions/loan-reschedule/ -B 2 -A 2Repository: openMF/web-app
Length of output: 1259
Add type safety, proper error handling, and subscription cleanup to loadRepaymentSchedule()
- Replace
loanDetails: anywith a proper interface type to ensure strict type safety. - The error handler silently clears
unpaidInstallmentswithout user feedback. Consider logging or showing a toast notification so users understand why the dropdown is empty. - Add
takeUntilDestroyed(this.destroyRef)to the subscription to prevent memory leaks, consistent with the pattern already used elsewhere in the component (line 218).
🤖 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 96 - 106, In loadRepaymentSchedule(), replace the loose any on the
subscription callback by using the concrete response type returned by
loanService.getLoanAccountAssociationDetails (e.g., a
LoanAccountAssociationDetails or similar interface that includes
repaymentSchedule and periods) and use RepaymentSchedulePeriod for periods; keep
assigning this.unpaidInstallments from periods.filter(...) as before. Add
takeUntilDestroyed(this.destroyRef) to the observable chain
(loanService.getLoanAccountAssociationDetails(...).pipe(...)) to ensure the
subscription is cleaned up. In the error handler, do not silently swallow
failures: log the error (or call the component’s toast/notification method) with
context so users are informed and still set this.unpaidInstallments = [] when
appropriate. Ensure you import/use the correct types and keep method name
loadRepaymentSchedule and property unpaidInstallments unchanged.
Member
Author
|
Hey @IOhacker and @alberto-art3ch ! |
IOhacker
requested changes
Mar 14, 2026
Contributor
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
"Platfrom description" translation should me removed from the PR.
YousufFFFF
force-pushed
the
WEB-849-improve-loan-reschedule-dropdown
branch
2 times, most recently
from
94af9cc to
8593439
Compare
Member
Author
Done! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/assets/translations/es-CL.json (1)
312-312:⚠️ Potential issue | 🟡 MinorReplace the em dash with a standard hyphen for normalization consistency.
Line 312 still uses
—in"Platform Description", so the dash-standardization objective is not fully applied.Suggested fix
- "Platform Description": "Cerramos esta brecha proporcionando la <a href=\"https://mifos.org\" target=\"_blank\" rel=\"noopener noreferrer\">Plataforma Mifos X</a>—el sistema bancario central de código abierto líder en el mundo. Al hacer que la tecnología bancaria de nivel empresarial sea gratuita y accesible, reducimos las barreras de entrada para las Instituciones de Microfinanzas (IMFs), Fintechs y ONGs, permitiéndoles escalar su impacto y atender la \"última milla\" de manera eficiente.", + "Platform Description": "Cerramos esta brecha proporcionando la <a href=\"https://mifos.org\" target=\"_blank\" rel=\"noopener noreferrer\">Plataforma Mifos X</a> - el sistema bancario central de código abierto líder en el mundo. Al hacer que la tecnología bancaria de nivel empresarial sea gratuita y accesible, reducimos las barreras de entrada para las Instituciones de Microfinanzas (IMFs), Fintechs y ONGs, permitiéndoles escalar su impacto y atender la \"última milla\" de manera eficiente.",🤖 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 312, The "Platform Description" translation value still contains an em dash (—); update that string to use a standard hyphen (-) instead, i.e., replace the em dash between "Plataforma Mifos X" and "el sistema" in the value for the "Platform Description" key so the JSON uses a normal hyphen for dash normalization.src/assets/translations/lv-LV.json (1)
312-312:⚠️ Potential issue | 🟡 MinorReplace the em dash with a standard hyphen in Platform Description.
Line 312 still uses
—instead of a regular-, which conflicts with this PR’s dash-standardization change.✏️ Suggested fix
- "Platform Description": "Mēs pārvaramam šo plaisu, nodrošinot <a href=\"https://mifos.org\" target=\"_blank\" rel=\"noopener noreferrer\">Mifos X platformu</a>—pasaulē vadošo atvērtā koda pamata banku sistēmu. Padarot uzņēmuma līmeņa banku tehnoloģiju bezmaksas un pieejamu, mēs samazinām ieejas barjeras Mikrofinanšu iestādēm (MFI), Fintech un NVO, ļaujot tām paplašināt savu ietekmi un efektīvi apkalpot \"pēdējo jūdzi\".", + "Platform Description": "Mēs pārvaramam šo plaisu, nodrošinot <a href=\"https://mifos.org\" target=\"_blank\" rel=\"noopener noreferrer\">Mifos X platformu</a> - pasaulē vadošo atvērtā koda pamata banku sistēmu. Padarot uzņēmuma līmeņa banku tehnoloģiju bezmaksas un pieejamu, mēs samazinām ieejas barjeras Mikrofinanšu iestādēm (MFI), Fintech un NVO, ļaujot tām paplašināt savu ietekmi un efektīvi apkalpot \"pēdējo jūdzi\".",🤖 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 312, The JSON value for the "Platform Description" key contains an em dash (—); replace it with a standard hyphen (-) so the string uses the PR's dash-standardization; edit the value for "Platform Description" to swap the em dash for a regular hyphen and ensure the JSON string remains valid (keep existing quotes, escapes and HTML link intact).src/assets/translations/ne-NE.json (1)
2826-2827:⚠️ Potential issue | 🟡 MinorKeep “Installment” terminology consistent with existing Nepali strings.
Line 2827 uses
किस्त, but installment-related strings in this file predominantly useकिस्ता.💡 Suggested patch
- "Installment": "किस्त" + "Installment": "किस्ता"🤖 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 2826 - 2827, The translation for the "Installment" key is inconsistent with other installment-related entries; change the value of the "Installment" key (currently "किस्त") to "किस्ता" to match the existing terminology across this file and maintain consistency with other installment-related strings.
🤖 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/loan-reschedule/loan-reschedule.component.html`:
- Around line 34-50: The select currently renders no options when
unpaidInstallments is empty, blocking the required control; update the template
around the `@for` (unpaidInstallments) block to add an `@empty` fallback option
(e.g., a single disabled mat-option or a helper message) that explains there are
no unpaid installments and/or conditionally disable the mat-select when
unpaidInstallments is empty; use the existing unpaidInstallments array and
getInstallmentDueDate reference to implement the empty-state option and ensure
the required validation no longer leaves the control unusable.
In
`@src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss`:
- Around line 23-25: Replace all hardcoded color literals in
loan-reschedule.component.scss (e.g., "color: rgb(0 0 0 / 70%)" and the similar
literals around the file) with the centralized SCSS theme tokens used by the
app; locate the color declarations in loan-reschedule.component.scss and swap
them for the appropriate variables from the shared theme (for example use the
project’s $text-muted / $text-primary / equivalent tokens defined in the
main/theme SCSS) so the component uses the centralized theme variables instead
of inline rgb()/hex values.
In `@src/assets/translations/it-IT.json`:
- Line 312: The "Platform Description" value contains a Unicode em dash (—);
replace that em dash with a standard ASCII hyphen (-) inside the JSON string for
the "Platform Description" key so the file follows the dash-standardization rule
and avoids locale rendering inconsistencies, ensuring any surrounding escaping
(quotes, backslashes) remains valid for the JSON value.
In `@src/assets/translations/ne-NE.json`:
- Line 312: The translation string for the "Platform Description" key still
contains an em dash (—); replace that em dash with a standard hyphen (-) inside
the JSON value for "Platform Description" (the string that starts "हामी <a
href=\"https://mifos.org\"...>"). After replacing the character, ensure the JSON
remains valid (proper escaping of quotes) and run the project's
localization/JSON linter to verify no other non-standard dashes remain.
---
Duplicate comments:
In `@src/assets/translations/es-CL.json`:
- Line 312: The "Platform Description" translation value still contains an em
dash (—); update that string to use a standard hyphen (-) instead, i.e., replace
the em dash between "Plataforma Mifos X" and "el sistema" in the value for the
"Platform Description" key so the JSON uses a normal hyphen for dash
normalization.
In `@src/assets/translations/lv-LV.json`:
- Line 312: The JSON value for the "Platform Description" key contains an em
dash (—); replace it with a standard hyphen (-) so the string uses the PR's
dash-standardization; edit the value for "Platform Description" to swap the em
dash for a regular hyphen and ensure the JSON string remains valid (keep
existing quotes, escapes and HTML link intact).
In `@src/assets/translations/ne-NE.json`:
- Around line 2826-2827: The translation for the "Installment" key is
inconsistent with other installment-related entries; change the value of the
"Installment" key (currently "किस्त") to "किस्ता" to match the existing
terminology across this file and maintain consistency with other
installment-related strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ef406b35-a3c3-44e6-993a-799ca03f48ce
📒 Files selected for processing (16)
src/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.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.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 skipped from review as they are similar to previous changes (10)
- src/assets/translations/en-US.json
- src/assets/translations/cs-CS.json
- src/assets/translations/es-MX.json
- src/assets/translations/fr-FR.json
- src/assets/translations/ko-KO.json
- src/assets/translations/de-DE.json
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts
- src/assets/translations/lt-LT.json
- src/assets/translations/sw-SW.json
- src/assets/translations/pt-PT.json
Comment on lines
+34
to
+50
| @for (installment of unpaidInstallments; track installment.period) { | ||
| <mat-option [value]="getInstallmentDueDate(installment)"> | ||
| <div class="installment-option-title"> | ||
| {{ 'labels.inputs.Due Date' | translate }}: {{ installment.dueDate | dateFormat }} | ||
| </div> | ||
| <div class="installment-option-details"> | ||
| <span> | ||
| · {{ 'labels.inputs.Principal Due' | translate }}: | ||
| {{ installment.principalDue | formatNumber }}</span | ||
| > | ||
| <span> | ||
| · {{ 'labels.inputs.Interest' | translate }}: {{ installment.interestDue | formatNumber }}</span | ||
| > | ||
| </div> | ||
| </mat-option> | ||
| } | ||
| </mat-select> |
There was a problem hiding this comment.
Handle empty unpaidInstallments to avoid a blocked required field.
When unpaidInstallments is empty, the required control has no selectable options and no empty-state guidance. Add an @empty fallback (and optionally disable the select) so users are not stuck.
🤖 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.html`
around lines 34 - 50, The select currently renders no options when
unpaidInstallments is empty, blocking the required control; update the template
around the `@for` (unpaidInstallments) block to add an `@empty` fallback option
(e.g., a single disabled mat-option or a helper message) that explains there are
no unpaid installments and/or conditionally disable the mat-select when
unpaidInstallments is empty; use the existing unpaidInstallments array and
getInstallmentDueDate reference to implement the empty-state option and ensure
the required validation no longer leaves the control unusable.
Comment on lines
+23
to
+25
| color: rgb(0 0 0 / 70%); | ||
| line-height: 1.5; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace hardcoded colors with shared theme variables/tokens.
These new color literals bypass the centralized theme system and make dark/light maintenance harder. Please switch these declarations to the existing SCSS variables/tokens from the theme files.
As per coding guidelines, "Leverage SCSS variables defined in src/main.scss and src/theme/mifosx-theme.scss rather than generating custom classes and explicit pixel values".
Also applies to: 35-35, 40-45, 48-50
🤖 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.scss`
around lines 23 - 25, Replace all hardcoded color literals in
loan-reschedule.component.scss (e.g., "color: rgb(0 0 0 / 70%)" and the similar
literals around the file) with the centralized SCSS theme tokens used by the
app; locate the color declarations in loan-reschedule.component.scss and swap
them for the appropriate variables from the shared theme (for example use the
project’s $text-muted / $text-primary / equivalent tokens defined in the
main/theme SCSS) so the component uses the centralized theme variables instead
of inline rgb()/hex values.
Member
Author
There was a problem hiding this comment.
Project’s stylelint configuration enforces modern color-function notation and percentage alpha values!
There was a problem hiding this comment.
🧩 Analysis chain
Let me verify the stylelint configuration and check what theme variables are available.
🏁 Script executed:
#!/bin/bash
# Check stylelint config
fd -e json -e js -e cjs "stylelint*" --exec cat {}
# Also check for .stylelintrc files
fd ".stylelintrc*" --exec cat {}
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check theme files for color variables
fd "mifosx-theme.scss" --exec cat {}
fd "main.scss" --exec grep -n "color\|text-muted\|rgba\|rgb\|70%" {}
Length of output: 5167
🏁 Script executed:
#!/bin/bash
# Check package.json for stylelint config
cat package.json | python3 -c "import json,sys; d=json.load(sys.stdin); print(json.dumps(d.get('stylelint', {}), indent=2)); print(json.dumps(d.get('devDependencies', {}).get('stylelint', ''), indent=2))"
# Also check for stylelint in angular.json
grep -n "stylelint\|color-function\|modern" package.json | head -30
Length of output: 329
🏁 Script executed:
#!/bin/bash
# Search for hidden stylelint config files
find . -maxdepth 2 -name ".stylelintrc*" -o -name "stylelint.config*" 2>/dev/null | head -20
# Also check if it's embedded in package.json under a different key
cat package.json | python3 -c "import json,sys; d=json.load(sys.stdin); [print(k,':',json.dumps(d[k])[:200]) for k in d if 'style' in k.lower() or 'lint' in k.lower()]"
Length of output: 229
🏁 Script executed:
#!/bin/bash
cat .stylelintrc
Length of output: 1675
@YousufFFFF, you're absolutely right. The .stylelintrc extends stylelint-config-standard, which enforces color-function-notation: "modern" — requiring the space-separated rgb(0 0 0 / 70%) syntax over the legacy rgba(0, 0, 0, 0.7) form. Additionally, the theme files (mifosx-theme.scss / main.scss) don't define reusable SCSS variables for muted-text colors — they rely on Angular Material palette functions and CSS custom properties (var(--md-sys-color-...)). The rgb() modern notation already used in main.scss (e.g. rgb(255 255 255 / 87%)) confirms this is the project-standard pattern. My suggestion doesn't apply here — the colors are correctly written. Apologies for the noise!
( ˶ᵔ ᵕ ᵔ˶) 🐇
✏️ Learnings added
Learnt from: YousufFFFF
Repo: openMF/web-app PR: 3379
File: src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss:23-25
Timestamp: 2026-03-14T16:51:03.026Z
Learning: In openMF/web-app (Angular, SCSS), the project's `.stylelintrc` extends `stylelint-config-standard` which enforces `color-function-notation: "modern"`. This requires the modern CSS space-separated syntax, e.g. `rgb(0 0 0 / 70%)` and `rgb(255 255 255 / 87%)`, instead of legacy `rgba(0, 0, 0, 0.7)`. Do NOT flag this notation as a style issue — it is enforced by the linter configuration. Furthermore, the theme files (`src/theme/mifosx-theme.scss`, `src/main.scss`) do not define reusable SCSS variables for muted/secondary text colors; they rely on Angular Material palette functions and CSS custom properties like `var(--md-sys-color-on-surface)`. Avoid suggesting non-existent SCSS variable replacements for inline rgb() color values in component SCSS files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: openMF/web-app PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T06:23:30.388Z
Learning: Applies to src/**/*.{scss,html} : Leverage SCSS variables defined in `src/main.scss` and `src/theme/mifosx-theme.scss` rather than generating custom classes and explicit pixel values
Learnt from: DeathGun44
Repo: openMF/web-app PR: 3099
File: src/app/core/shell/sidenav/sidenav.component.scss:192-221
Timestamp: 2026-02-07T06:19:21.085Z
Learning: In Angular Material v20+ (MDC-based), prefer styling MDC DOM classes (e.g., .mat-mdc-list-item, .mdc-list-item__primary-text) in SCSS instead of legacy selectors (e.g., .mat-list-item). This SCSS rule should apply broadly to components using MDC-based Material, not just this file. Update selectors and specificity to target the MDC classes that render in templates to ensure styles apply with the new DOM structure.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/assets/translations/cs-CS.json`:
- Line 312: Revert the unrelated change to the "Platform Description" key in
cs-CS.json that came from WEB-849: restore the original translation value for
the "Platform Description" entry (undo the edit that added the Mifos X
anchor/HTML) so this file no longer contains unrelated localization churn; pull
the original string from the base branch or remove the modification to match the
pre-PR state.
In `@src/assets/translations/fr-FR.json`:
- Line 312: Replace the em dash (—) in the "Platform Description" translation
value with a standard hyphen/minus (-) to match the project's dash style; locate
the "Platform Description" key in the fr-FR.json file and normalize the
punctuation inside the string (replace the — before "le système bancaire..."
with -), keeping the rest of the text and HTML unchanged.
In `@src/assets/translations/pt-PT.json`:
- Line 312: The "Platform Description" translation string contains an em dash
(—); update the value for the "Platform Description" key to replace the em dash
with a regular hyphen/dash (" - ") so punctuation matches the project's
standardization while preserving the existing HTML anchor and escaped quotes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5e06e13-a137-4f6a-8960-f87dbb8bb6e2
📒 Files selected for processing (16)
src/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.scsssrc/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.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 skipped from review as they are similar to previous changes (10)
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.scss
- src/assets/translations/lt-LT.json
- src/assets/translations/it-IT.json
- src/assets/translations/ko-KO.json
- src/assets/translations/de-DE.json
- src/assets/translations/sw-SW.json
- src/assets/translations/en-US.json
- src/assets/translations/lv-LV.json
- src/assets/translations/ne-NE.json
- src/app/loans/loans-view/loan-account-actions/loan-reschedule/loan-reschedule.component.ts
YousufFFFF
force-pushed
the
WEB-849-improve-loan-reschedule-dropdown
branch
from
8593439 to
00bbd27
Compare
YousufFFFF
force-pushed
the
WEB-849-improve-loan-reschedule-dropdown
branch
from
00bbd27 to
8841180
Compare
Member
Author
|
I have done all the necessary changes @IOhacker! |
Member
Author
Thanks for you review @IOhacker. |
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
This pull request improves the loan rescheduling user interface by replacing the date picker with an installment-based dropdown selection. Selecting an installment provides clearer context to users by showing relevant repayment details such as due date, principal due, and interest due. This makes the rescheduling workflow more intuitive and reduces ambiguity when selecting a specific repayment period.
Additionally, the spacing and visual alignment of the selected installment display have been corrected to ensure better UI consistency.
The changes introduced include:
These updates enhance usability when rescheduling loans by providing more meaningful context for installment selection and ensuring proper localization support for the updated UI elements.
Related issues and discussion
#WEB-849
After Video:
Create.Provisioning.Entry.-.Google.Chrome.2026-03-03.21-13-21.mp4
Checklist
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Style
Chores