WEB-874 Prevent negative values in numeric fields of Group#3412
Open
JaySoni1 wants to merge 1 commit intoopenMF:devfrom
Open
WEB-874 Prevent negative values in numeric fields of Group#3412JaySoni1 wants to merge 1 commit intoopenMF:devfrom
JaySoni1 wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loans — Account Terms Step src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html, src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts |
Added min (and step for interest) attributes and conditional mat-error messages in the template; added Validators.min(0) to multiple numeric controls and Validators.min(0.01) for interestRatePerPeriod; added valueChanges subscribers that clamp negative values to 0 (and clamp interestRatePerPeriod to >= 0.01). |
Savings — Account Terms Step src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html, src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts |
Added min/step attributes and conditional mat-error cases in template; added Validators.min(0) to various numeric controls (including overdraft-related controls when present); added valueChanges subscriber to clamp nominalAnnualInterestRate (and other numeric fields) to >= 0 at runtime. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- WEB-772 Prevent negative values in savings product settings fields #3206: Applies
min="0"/Validators.min(0)and runtime clamping to overdraft-related savings controls (overlapping savings changes). - WEB-567 Negative Values Allowed in Loan Product Setting Fields #2982: Adds non-negativity enforcement for loan grace/delinquency numeric fields similar to the loan-side changes here.
- WEB-568 Negative values allowed for arrears ageing overdue days for npa and principal threshold fields in loan product settings #2983: Introduces
min="0"/Validators.min(0)forgraceOnArrearsAgeing, overlapping related loan-field validation updates.
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 PR title clearly summarizes the main objective: preventing negative values in numeric fields across Group account forms (Savings, Loans, and GLIM accounts). |
| 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
452-468:⚠️ Potential issue | 🟠 MajorBug:
Validators.min(0)is not applied wheninterestRatePerPeriodis recreated.The
setAdvancedPaymentStrategyControls()method removes and recreates theinterestRatePerPeriodcontrol dynamically, but only appliesValidators.required. This overwrites theValidators.min(0)defined increateloansAccountTermsForm(), making the min validation ineffective.🐛 Proposed fix to include Validators.min(0)
if (this.loansAccountTermsData.product.fixedLength) { this.loansAccountTermsForm.addControl( 'interestRatePerPeriod', - new UntypedFormControl({ value: 0, disabled: true }, Validators.required) + new UntypedFormControl({ value: 0, disabled: true }, [Validators.required, Validators.min(0)]) ); this.loansAccountTermsForm.addControl( 'fixedLength', new UntypedFormControl(this.loansAccountTermsData.product.fixedLength) ); } else { this.loansAccountTermsForm.addControl( 'interestRatePerPeriod', - new UntypedFormControl(this.loansAccountTermsData.interestRatePerPeriod, Validators.required) + new UntypedFormControl(this.loansAccountTermsData.interestRatePerPeriod, [Validators.required, Validators.min(0)]) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 452 - 468, The interestRatePerPeriod control is being removed and re-added in setAdvancedPaymentStrategyControls (inside loans-account-terms-step.component) without reapplying Validators.min(0), which breaks the minimum-value validation defined in createloansAccountTermsForm(); when recreating interestRatePerPeriod (both the disabled and enabled branches) add Validators.min(0) alongside Validators.required so the minimum validation is preserved on the new UntypedFormControl instances on loansAccountTermsForm.src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html (2)
129-145:⚠️ Potential issue | 🟡 MinorChange
min="0"tomin="1"and update validator to match loan product pattern.The
repaymentEveryfield currently allows 0 as a valid value (both in HTMLmin="0"and TypeScriptValidators.min(0)), which is invalid business logic—a loan cannot be repaid every 0 months/days. The loan product stepper correctly enforcesmin="1"in both the HTML attribute andValidators.min(1)in the form control. Align this component with that pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 129 - 145, The template allows 0 for repayment interval; change the input min attribute from min="0" to min="1" and then update the form control validator in the loansAccountTermsForm initialization (the control named repaymentEvery) to use Validators.min(1) and the same pattern used by the loan product stepper so the form-level validation matches the HTML constraint and business rule.
69-84:⚠️ Potential issue | 🟡 MinorUpdate "Number of repayments" validation to require a minimum of 1 repayment.
The field currently allows a minimum of 0 via
min="0"andValidators.min(0), but a loan with 0 repayments is invalid. SincecalculateLoanTerm()multipliesnumberOfRepayments × repaymentEveryto set the loan term, allowing 0 would result in a 0 loan term. Change both the HTML attribute and TypeScript validator tomin="1"andValidators.min(1)to enforce at least one repayment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 69 - 84, The Number of repayments input and its validator allow 0 which permits a 0-length loan term; update both the template input attribute min and the form validator to require at least 1. Change the HTML input's min="0" to min="1" for the field bound by formControlName="numberOfRepayments", and update the TypeScript form control where Validators.min(0) is used (on loansAccountTermsForm.controls.numberOfRepayments or where the FormControl is constructed) to Validators.min(1); ensure calculateLoanTerm() continues to use loansAccountTermsForm.controls.numberOfRepayments.value safely after this change.
🧹 Nitpick comments (1)
src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html (1)
93-96: Consider adding<mat-error>for min validation feedback.Fields like
minRequiredOpeningBalance,lockinPeriodFrequency, andminRequiredBalancenow havemin="0"constraints but lack corresponding<mat-error>elements to display validation feedback when users attempt to enter negative values. While the HTML5minattribute prevents negative input in most browsers, adding error messages improves accessibility and provides clearer feedback.💡 Example: Add mat-error for minRequiredOpeningBalance
<mat-form-field class="flex-48"> <mat-label>{{ 'labels.inputs.Minimum Opening Balance' | translate }}</mat-label> <input type="number" matInput formControlName="minRequiredOpeningBalance" min="0" /> + `@if` (savingsAccountTermsForm.controls.minRequiredOpeningBalance.hasError('min')) { + <mat-error> + {{ 'labels.inputs.Minimum Opening Balance' | translate }} + {{ 'labels.commons.must be' | translate }} + <strong>{{ 'labels.commons.non-negative' | translate }}</strong> + </mat-error> + } </mat-form-field>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html` around lines 93 - 96, Add validation error UI for the numeric min constraints in the template: for each mat-form-field that uses formControlName="minRequiredOpeningBalance", "lockinPeriodFrequency", and "minRequiredBalance" inside savings-account-terms-step.component.html, add a <mat-error> that displays when the control has a min validation error (e.g., control.hasError('min') or control.errors?.min) and show a translated or clear message like "Minimum value is 0"; ensure the <mat-error> is bound to the same FormControl used by the input so users get accessible feedback when they enter values below 0.
🤖 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-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`:
- Around line 129-145: The template allows 0 for repayment interval; change the
input min attribute from min="0" to min="1" and then update the form control
validator in the loansAccountTermsForm initialization (the control named
repaymentEvery) to use Validators.min(1) and the same pattern used by the loan
product stepper so the form-level validation matches the HTML constraint and
business rule.
- Around line 69-84: The Number of repayments input and its validator allow 0
which permits a 0-length loan term; update both the template input attribute min
and the form validator to require at least 1. Change the HTML input's min="0" to
min="1" for the field bound by formControlName="numberOfRepayments", and update
the TypeScript form control where Validators.min(0) is used (on
loansAccountTermsForm.controls.numberOfRepayments or where the FormControl is
constructed) to Validators.min(1); ensure calculateLoanTerm() continues to use
loansAccountTermsForm.controls.numberOfRepayments.value safely after this
change.
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 452-468: The interestRatePerPeriod control is being removed and
re-added in setAdvancedPaymentStrategyControls (inside
loans-account-terms-step.component) without reapplying Validators.min(0), which
breaks the minimum-value validation defined in createloansAccountTermsForm();
when recreating interestRatePerPeriod (both the disabled and enabled branches)
add Validators.min(0) alongside Validators.required so the minimum validation is
preserved on the new UntypedFormControl instances on loansAccountTermsForm.
---
Nitpick comments:
In
`@src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html`:
- Around line 93-96: Add validation error UI for the numeric min constraints in
the template: for each mat-form-field that uses
formControlName="minRequiredOpeningBalance", "lockinPeriodFrequency", and
"minRequiredBalance" inside savings-account-terms-step.component.html, add a
<mat-error> that displays when the control has a min validation error (e.g.,
control.hasError('min') or control.errors?.min) and show a translated or clear
message like "Minimum value is 0"; ensure the <mat-error> is bound to the same
FormControl used by the input so users get accessible feedback when they enter
values below 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f62da7e5-cbab-4c93-8772-794502e5fe78
📒 Files selected for processing (4)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.htmlsrc/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts
Contributor
Author
|
@IOhacker I will resolve the conflicts and will update the PR soon |
JaySoni1
force-pushed
the
WEB-874-prevent-negative-values-in-numeric-fields-of-group
branch
from
c841f82 to
7e69d4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (3)
593-596: Initial validator may be overwritten.The
Validators.min(0.01)added here will be replaced whensetAdvancedPaymentStrategyControls()recreates this control (lines 521-537). The recreated control uses onlyValidators.requiredwithout the minimum constraint.If the minimum constraint should persist, apply it in
setAdvancedPaymentStrategyControls()as well:Proposed fix in setAdvancedPaymentStrategyControls
} else { this.loansAccountTermsForm.addControl( 'interestRatePerPeriod', - new UntypedFormControl(this.loansAccountTermsData.interestRatePerPeriod, Validators.required) + new UntypedFormControl(this.loansAccountTermsData.interestRatePerPeriod, [Validators.required, Validators.min(0)]) ); }Note: Use
min(0)here to match the clamping behavior, ormin(0.01)if zero-interest rates should be disallowed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 593 - 596, The interestRatePerPeriod control's Validators.min(0.01) is being overwritten because setAdvancedPaymentStrategyControls() recreates the control with only Validators.required; update setAdvancedPaymentStrategyControls() so the recreated FormControl for interestRatePerPeriod includes the same minimum validator (use Validators.min(0) to match clamping or Validators.min(0.01) to forbid zero) in addition to Validators.required, ensuring interestRatePerPeriod preserves the min constraint after recreation.
468-488: Subscriptions are not cleaned up and may become stale.Two concerns:
Memory leak: The
valueChangessubscriptions (lines 471, 481) are never unsubscribed. Consider using atakeUntilpattern with a destroy subject or storing subscriptions for cleanup inngOnDestroy.Stale subscription for
interestRatePerPeriod: This control is removed and recreated insetAdvancedPaymentStrategyControls()(lines 521-537), which is called inngOnChanges(). If the product template changes, the subscription created here will reference the old control instance.Suggested pattern for cleanup
+import { Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; export class LoansAccountTermsStepComponent extends LoanProductBaseComponent implements OnInit, OnChanges { + private destroy$ = new Subject<void>(); // ... setCustomValidators() { // ... numericFieldsWithMinZero.forEach((fieldName) => { const control = this.loansAccountTermsForm.get(fieldName); if (control) { - control.valueChanges.subscribe((value) => { + control.valueChanges.pipe(takeUntil(this.destroy$)).subscribe((value) => { if (typeof value === 'number' && value < 0) { control.setValue(0, { emitEvent: false }); } }); } }); // Similar for interestRateControl... } + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } }
571-584: Consider usingValidators.min(1)fornumberOfRepaymentsandrepaymentEvery.The current
min(0)validators allow zero values for these fields. However, from a business perspective:
numberOfRepayments: 0means no repayments scheduled, which is invalid for a loanrepaymentEvery: 0results in a calculatedloanTermFrequencyof 0, which is also invalidWhile
loanTermFrequencyreasonably allowsmin(0)since it's a calculated and disabled field, the input fields should enforcemin(1)to prevent invalid loan configurations. This aligns with the pattern used in the working capital section (lines 656, 663) where similar frequency fields usemin(1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 571 - 584, Update the form validators in loans-account-terms-step.component.ts so the input fields enforce a minimum of 1: change the Validators.min(0) to Validators.min(1) for the FormControl definitions "numberOfRepayments" and "repaymentEvery" in the LoansAccountTermsStep component; leave any calculated/disabled control like "loanTermFrequency" unchanged (keep its min(0) if present). Ensure you update the validator arrays where these control names appear so the form prevents zero values for repayments and frequency.
🤖 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-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 479-488: The value-change handler for interestRatePerPeriod
(interestRateControl) currently only clamps negative numbers and misses values
>0 and <0.01; update the subscription on
this.loansAccountTermsForm.get('interestRatePerPeriod') so that any numeric
value less than 0.01 (including negatives and values between 0 and 0.01) is set
to 0.01 using interestRateControl.setValue(0.01, { emitEvent: false }); keep the
existing type check for typeof value === 'number' and preserve emitEvent: false
to avoid feedback loops.
---
Nitpick comments:
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 593-596: The interestRatePerPeriod control's Validators.min(0.01)
is being overwritten because setAdvancedPaymentStrategyControls() recreates the
control with only Validators.required; update
setAdvancedPaymentStrategyControls() so the recreated FormControl for
interestRatePerPeriod includes the same minimum validator (use Validators.min(0)
to match clamping or Validators.min(0.01) to forbid zero) in addition to
Validators.required, ensuring interestRatePerPeriod preserves the min constraint
after recreation.
- Around line 571-584: Update the form validators in
loans-account-terms-step.component.ts so the input fields enforce a minimum of
1: change the Validators.min(0) to Validators.min(1) for the FormControl
definitions "numberOfRepayments" and "repaymentEvery" in the
LoansAccountTermsStep component; leave any calculated/disabled control like
"loanTermFrequency" unchanged (keep its min(0) if present). Ensure you update
the validator arrays where these control names appear so the form prevents zero
values for repayments and frequency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 57362898-127a-49d2-aa98-3b49dd324847
📒 Files selected for processing (4)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.htmlsrc/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts
...p/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
Show resolved
Hide resolved
JaySoni1
force-pushed
the
WEB-874-prevent-negative-values-in-numeric-fields-of-group
branch
from
7e69d4d to
607d944
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-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
516-537:⚠️ Potential issue | 🟡 Minor
interestRatePerPeriodadded viasetAdvancedPaymentStrategyControlslacksValidators.min(0.01).When
interestRatePerPeriodis dynamically added at lines 523-524 or 531-534, it only hasValidators.requiredbut notValidators.min(0.01). This bypasses the minimum validation defined increateloansAccountTermsForm()since the control is removed and re-added.🐛 Proposed fix
if (this.loansAccountTermsData.product.fixedLength) { this.loansAccountTermsForm.addControl( 'interestRatePerPeriod', - new UntypedFormControl({ value: 0, disabled: true }, Validators.required) + new UntypedFormControl({ value: 0, disabled: true }, [Validators.required, Validators.min(0.01)]) ); this.loansAccountTermsForm.addControl( 'fixedLength', new UntypedFormControl(this.loansAccountTermsData.product.fixedLength) ); } else { this.loansAccountTermsForm.addControl( 'interestRatePerPeriod', - new UntypedFormControl(this.loansAccountTermsData.interestRatePerPeriod, Validators.required) + new UntypedFormControl(this.loansAccountTermsData.interestRatePerPeriod, [Validators.required, Validators.min(0.01)]) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 516 - 537, setAdvancedPaymentStrategyControls currently removes and re-adds the interestRatePerPeriod control without the Validators.min(0.01), which bypasses the minimum validation established in createloansAccountTermsForm; update setAdvancedPaymentStrategyControls so whenever it adds interestRatePerPeriod to loansAccountTermsForm (both the disabled branch and the enabled branch) it applies Validators.required and Validators.min(0.01) (compose them together) so the control keeps the same minimum validation as defined originally.
🧹 Nitpick comments (2)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html (1)
59-61: Hardcoded error text should use i18n translation.The error message
must be at least <strong>0</strong>is hardcoded in English. Per coding guidelines, all user-facing strings should use@ngx-translate/corefor internationalization.This applies to all similar
mat-errorblocks added in this file (lines 59-61, 86-88, 137-141, 203-205, 268-272, 456-460, 472-476, 487-491, 497-501, 507-511).♻️ Example refactor using translation keys
`@if` (loansAccountTermsForm.controls.repaymentEvery.hasError('min')) { - <mat-error> {{ 'labels.inputs.Repaid every' | translate }} must be at least <strong>0</strong> </mat-error> + <mat-error> + {{ 'labels.inputs.Repaid every' | translate }} {{ 'labels.validations.must be at least' | translate }} + <strong>0</strong> + </mat-error> }After adding new translation keys, run
npm run translations:extractto update the i18n files. As per coding guidelines: "Use proper i18n variables from@ngx-translate/corefor all user-facing strings instead of hardcoded text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 59 - 61, Replace the hardcoded English error messages inside the mat-error blocks in loans-account-terms-step.component.html (e.g., the block checking loansAccountTermsForm.controls.repaymentEvery.hasError('min')) with translated strings using the `@ngx-translate/core` pipe (e.g., "{{ 'errors.minValue' | translate:{ field: ('labels.inputs.Repaid every' | translate), min: 0 } }}") and add corresponding translation keys for each distinct message used in the other mat-error blocks in this template (the ones you flagged). Ensure you keep existing bindings/HTML (like <strong> styling) by using translation parameters or markup-aware translations, update all similar occurrences (repaymentEvery and the other error checks listed), and then run npm run translations:extract to update i18n files.src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts (1)
188-195: Consider extending clamping to other numeric fields for consistency.Currently, only
nominalAnnualInterestRatehas runtime clamping viavalueChanges. The other fields (minRequiredOpeningBalance,lockinPeriodFrequency,minRequiredBalance) haveValidators.min(0)but won't auto-correct negative input. The loans component applies clamping to multiple fields.If auto-correction is desired for consistency, you could extend the pattern:
♻️ Optional refactor to add clamping for all numeric fields
buildDependencies() { - const nominalInterestControl = this.savingsAccountTermsForm.get('nominalAnnualInterestRate'); - if (nominalInterestControl) { - nominalInterestControl.valueChanges.subscribe((value) => { - if (typeof value === 'number' && value < 0) { - nominalInterestControl.setValue(0, { emitEvent: false }); - } - }); - } + const numericFieldsWithMinZero = [ + 'nominalAnnualInterestRate', + 'minRequiredOpeningBalance', + 'lockinPeriodFrequency', + 'minRequiredBalance' + ]; + + numericFieldsWithMinZero.forEach((fieldName) => { + const control = this.savingsAccountTermsForm.get(fieldName); + if (control) { + control.valueChanges.subscribe((value) => { + if (typeof value === 'number' && value < 0) { + control.setValue(0, { emitEvent: false }); + } + }); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts` around lines 188 - 195, Only nominalAnnualInterestRate has runtime clamping; add identical valueChanges-based clamping for the other numeric controls on savingsAccountTermsForm (minRequiredOpeningBalance, lockinPeriodFrequency, minRequiredBalance) so negative inputs are auto-corrected to 0. For each control (use this.savingsAccountTermsForm.get('<controlName>')), if the control exists subscribe to valueChanges and if typeof value === 'number' && value < 0 call control.setValue(0, { emitEvent: false }); place these subscriptions alongside the existing nominalAnnualInterestRate subscription (e.g., in the same init block) to keep behavior consistent.
🤖 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-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 516-537: setAdvancedPaymentStrategyControls currently removes and
re-adds the interestRatePerPeriod control without the Validators.min(0.01),
which bypasses the minimum validation established in
createloansAccountTermsForm; update setAdvancedPaymentStrategyControls so
whenever it adds interestRatePerPeriod to loansAccountTermsForm (both the
disabled branch and the enabled branch) it applies Validators.required and
Validators.min(0.01) (compose them together) so the control keeps the same
minimum validation as defined originally.
---
Nitpick comments:
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`:
- Around line 59-61: Replace the hardcoded English error messages inside the
mat-error blocks in loans-account-terms-step.component.html (e.g., the block
checking loansAccountTermsForm.controls.repaymentEvery.hasError('min')) with
translated strings using the `@ngx-translate/core` pipe (e.g., "{{
'errors.minValue' | translate:{ field: ('labels.inputs.Repaid every' |
translate), min: 0 } }}") and add corresponding translation keys for each
distinct message used in the other mat-error blocks in this template (the ones
you flagged). Ensure you keep existing bindings/HTML (like <strong> styling) by
using translation parameters or markup-aware translations, update all similar
occurrences (repaymentEvery and the other error checks listed), and then run npm
run translations:extract to update i18n files.
In
`@src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts`:
- Around line 188-195: Only nominalAnnualInterestRate has runtime clamping; add
identical valueChanges-based clamping for the other numeric controls on
savingsAccountTermsForm (minRequiredOpeningBalance, lockinPeriodFrequency,
minRequiredBalance) so negative inputs are auto-corrected to 0. For each control
(use this.savingsAccountTermsForm.get('<controlName>')), if the control exists
subscribe to valueChanges and if typeof value === 'number' && value < 0 call
control.setValue(0, { emitEvent: false }); place these subscriptions alongside
the existing nominalAnnualInterestRate subscription (e.g., in the same init
block) to keep behavior consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fd8ba23f-834a-4bcf-9e50-ce8ebd0d2833
📒 Files selected for processing (4)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.htmlsrc/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts
✅ Files skipped from review due to trivial changes (1)
- src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html
Contributor
Author
|
@IOhacker @alberto-art3ch I have resolve the conflicts can you please review |
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
3 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.
Changes Made :-
-Add min(0) validation to numeric fields in Savings Account Terms, Loans Account Terms, and GLIM account in GROUP .
WEB-874
Before :-
After :-
Summary by CodeRabbit