WEB-772 Prevent negative values in savings product settings fields#3206
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Form Template Constraints src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html |
Added HTML input constraints: min="0" step="0.01" for overdraft fields (minOverdraftForInterestCalculation, nominalAnnualInterestRateOverdraft, overdraftLimit) and min="0" step="1" for dormancy fields (daysToInactive, daysToDormancy, daysToEscheat). |
Form Control Validators src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.ts |
Added form control validators when dynamic controls are initialized: overdraft controls include Validators.min(0), dormancy controls include Validators.required and Validators.min(0). Replaced bare UntypedFormControl('') declarations with validated instances. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
- alberto-art3ch
- IOhacker
🚥 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 summarizes the main change: adding min="0" validation to prevent negative values in overdraft and dormancy tracking fields in the savings product settings form. |
| 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 docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html (2)
229-275:⚠️ Potential issue | 🟡 MinorDormancy
<mat-error>blocks need ahasError('min')branch.The
<mat-error>elements for all three dormancy fields are currently unconditional — they fire for any validation error on the control. WithValidators.min(0)now added by this PR, entering a negative number triggers theminerror, but the user reads"is required", which is incorrect and misleading.💡 Proposed fix (shown for `daysToInactive`, apply same pattern to `daysToDormancy` and `daysToEscheat`)
- <mat-error> - {{ 'labels.inputs.Number of Days to Inactive sub-status' | translate }} - {{ 'labels.commons.is' | translate }} - <strong>{{ 'labels.commons.required' | translate }}</strong> - </mat-error> + <mat-error *ngIf="savingProductSettingsForm.get('daysToInactive')?.hasError('required')"> + {{ 'labels.inputs.Number of Days to Inactive sub-status' | translate }} + {{ 'labels.commons.is' | translate }} + <strong>{{ 'labels.commons.required' | translate }}</strong> + </mat-error> + <mat-error *ngIf="savingProductSettingsForm.get('daysToInactive')?.hasError('min')"> + {{ 'labels.inputs.Number of Days to Inactive sub-status' | translate }} + {{ 'labels.commons.must be' | translate }} {{ 'labels.commons.a positive number' | translate }} + </mat-error>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html` around lines 229 - 275, The mat-error blocks for the three form controls (formControlName="daysToInactive", "daysToDormancy", "daysToEscheat") are unconditional and always show the "is required" message; update each field to render conditional errors so the min validation is shown when Validators.min(0) fails and the required message is shown for required errors — e.g., replace the single <mat-error> with two <mat-error *ngIf="control.hasError('min')"> and <mat-error *ngIf="control.hasError('required')"> branches (where control is the respective FormControl for daysToInactive/daysToDormancy/daysToEscheat) showing an appropriate "must be at least 0" message for 'min' and the existing "is required" message for 'required'.
176-208:⚠️ Potential issue | 🟠 MajorAdd
<mat-error>forminvalidation on overdraft fields.All three overdraft fields —
minOverdraftForInterestCalculation,nominalAnnualInterestRateOverdraft, andoverdraftLimit— haveValidators.min(0)wired up in the TS component (added by this PR) but contain no<mat-error>element whatsoever in the template. If a user types a negative value, Angular marks the control invalid and the field turns red, but there is no message explaining why. This silently blocks form submission and leaves the user without actionable feedback, partially defeating the purpose of the added validator.💡 Proposed fix: add error messages to each overdraft field
formControlName="minOverdraftForInterestCalculation" min="0" step="0.01" /> + <mat-error *ngIf="savingProductSettingsForm.get('minOverdraftForInterestCalculation')?.hasError('min')"> + {{ 'labels.inputs.Minimum Overdraft Required for Interest Calculation' | translate }} + {{ 'labels.commons.must be' | translate }} {{ 'labels.commons.a positive number' | translate }} + </mat-error> </mat-form-field> formControlName="nominalAnnualInterestRateOverdraft" min="0" step="0.01" /> + <mat-error *ngIf="savingProductSettingsForm.get('nominalAnnualInterestRateOverdraft')?.hasError('min')"> + {{ 'labels.inputs.Nominal Annual Interest for Overdraft' | translate }} + {{ 'labels.commons.must be' | translate }} {{ 'labels.commons.a positive number' | translate }} + </mat-error> </mat-form-field> formControlName="overdraftLimit" min="0" step="0.01" /> + <mat-error *ngIf="savingProductSettingsForm.get('overdraftLimit')?.hasError('min')"> + {{ 'labels.inputs.Maximum Overdraft Amount Limit' | translate }} + {{ 'labels.commons.must be' | translate }} {{ 'labels.commons.a positive number' | translate }} + </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/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html` around lines 176 - 208, Add a <mat-error> under each overdraft <mat-form-field> to surface the Validators.min(0) failure for the controls minOverdraftForInterestCalculation, nominalAnnualInterestRateOverdraft, and overdraftLimit: for each field, render a mat-error when the control hasError('min') and has been touched/dirty (e.g. *ngIf="form.get('minOverdraftForInterestCalculation').hasError('min') && (form.get('minOverdraftForInterestCalculation').touched || form.get('minOverdraftForInterestCalculation').dirty)"), and show a short translated message like "Must be at least 0" (or the appropriate translation key); repeat the same pattern for nominalAnnualInterestRateOverdraft and overdraftLimit so negative inputs display an explanatory error and allow the form to be corrected.
🧹 Nitpick comments (1)
src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html (1)
234-235: Considermin="1"for dormancy day fields."Number of days" to inactive, dormant, and escheat statuses with
min="0"(and correspondinglyValidators.min(0)in the TS) allows a zero-day threshold, meaning an account would immediately transition. Whether0is a valid business value should be confirmed. If not,min="1"/Validators.min(1)better reflects the intent and prevents a confusing edge case.Also applies to: 251-252, 267-268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html` around lines 234 - 235, Change the dormancy day inputs in saving-product-settings-step.component.html from min="0" to min="1" and update the corresponding Validators.min(0) checks in the saving-product-settings-step component class to Validators.min(1) so the "to inactive/dormant/escheat" day thresholds cannot be zero; locate the three related input controls in saving-product-settings-step.component.html (the fields controlling inactive, dormant and escheat days) and the matching FormControl Validators.min usages in the component TS and bump them to 1.
🤖 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/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html`:
- Around line 229-275: The mat-error blocks for the three form controls
(formControlName="daysToInactive", "daysToDormancy", "daysToEscheat") are
unconditional and always show the "is required" message; update each field to
render conditional errors so the min validation is shown when Validators.min(0)
fails and the required message is shown for required errors — e.g., replace the
single <mat-error> with two <mat-error *ngIf="control.hasError('min')"> and
<mat-error *ngIf="control.hasError('required')"> branches (where control is the
respective FormControl for daysToInactive/daysToDormancy/daysToEscheat) showing
an appropriate "must be at least 0" message for 'min' and the existing "is
required" message for 'required'.
- Around line 176-208: Add a <mat-error> under each overdraft <mat-form-field>
to surface the Validators.min(0) failure for the controls
minOverdraftForInterestCalculation, nominalAnnualInterestRateOverdraft, and
overdraftLimit: for each field, render a mat-error when the control
hasError('min') and has been touched/dirty (e.g.
*ngIf="form.get('minOverdraftForInterestCalculation').hasError('min') &&
(form.get('minOverdraftForInterestCalculation').touched ||
form.get('minOverdraftForInterestCalculation').dirty)"), and show a short
translated message like "Must be at least 0" (or the appropriate translation
key); repeat the same pattern for nominalAnnualInterestRateOverdraft and
overdraftLimit so negative inputs display an explanatory error and allow the
form to be corrected.
---
Nitpick comments:
In
`@src/app/products/saving-products/saving-product-stepper/saving-product-settings-step/saving-product-settings-step.component.html`:
- Around line 234-235: Change the dormancy day inputs in
saving-product-settings-step.component.html from min="0" to min="1" and update
the corresponding Validators.min(0) checks in the saving-product-settings-step
component class to Validators.min(1) so the "to inactive/dormant/escheat" day
thresholds cannot be zero; locate the three related input controls in
saving-product-settings-step.component.html (the fields controlling inactive,
dormant and escheat days) and the matching FormControl Validators.min usages in
the component TS and bump them to 1.
|
@IOhacker Thank You for the review |
Changes Made :-
-Added min="0" validation to overdraft and dormancy tracking fields in the savings product settings form.
WEB-772
Before : -
After :-
Summary by CodeRabbit