WEB-874 Prevent negative values in numeric fields of group #3483
Merged
IOhacker merged 1 commit intoopenMF:devfrom Apr 7, 2026
Merged
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loans Account Terms HTML src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html |
Adjusted numeric input markup: set min="1" for working-capital repaymentEvery, added/strengthened min on other numeric fields (min="0" or min="0.01" with step="0.01" for rates), and marked numberOfRepayments required. No new conditional mat-error blocks added in this diff. |
Loans Account Terms TypeScript src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts |
Initialized loansAccountTermsForm, defaulted @Input() loansAccountFormValid = false, added safe navigation for optional controls, added Validators.min(0) / Validators.min(0.01) to numeric controls, introduced setNumericFieldListeners() subscribing to valueChanges to clamp negatives (and enforce ≥0.01 for interestRatePerPeriod), and applied optional chaining when reading principalAmount. |
Savings Account Terms HTML src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html |
Added min="0" (and step="0.01" for rates) to multiple numeric inputs; replaced an always-rendered mat-error for nominalAnnualInterestRate with conditional rendering tied to control errors. |
Savings Account Terms TypeScript src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.ts |
Added Validators.min(0) to nominalAnnualInterestRate, minRequiredOpeningBalance, lockinPeriodFrequency, minRequiredBalance; overdraft-related dynamic controls now include Validators.min(0); buildDependencies() subscribes to nominalAnnualInterestRate and clamps negative values to 0. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- WEB-874 Prevent negative values in numeric fields of Group #3412: Modifies the same loans and savings account term components with similar min validators and clamping listeners.
- WEB-707 Allowing Negative Values in Loan Product Numeric Fields #3398: Adds non-negative constraints to loan numeric fields in the loans-account-terms-step component.
- WEB-813: Working Capital loan account creation #3257: Related edits to the same loans-account-terms-step component (form/control lifecycle and numeric validation changes).
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 'WEB-874 Prevent negative values in numeric fields of group' directly and clearly summarizes the main change: adding validation to prevent negative values in numeric fields across loan and savings account components. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
JaySoni1
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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.html`:
- Around line 66-68: The mat-error messages in
loans-account-terms-step.component (e.g., the min-validation message for
loansAccountTermsForm.controls.repaymentEvery and the other similar validation
messages referenced at the listed locations) use hardcoded English text; replace
each raw string and inline label (like "Repaid every" and "must be at least 0")
with ngx-translate keys (use the translate pipe or translate service and
parameterized keys where appropriate) so the UI reads values via keys (e.g.,
labels.inputs.RepaidEvery and validation.minValue with params) and update any
strong/static numbers to be passed as params; after changing the templates, run
npm run translations:extract to generate the new i18n entries.
- Around line 66-68: The template's repaymentEvery min/display is inconsistent
with the component validator: update the UI to match Validators.min(1) by
changing any input attribute/min label currently showing 0 to 1 and adjust the
mat-error for loansAccountTermsForm.controls.repaymentEvery to reflect "must be
at least 1" (use the translation pipe instead of hardcoded English), e.g.,
replace the hardcoded text with a translated key and the numeric 1 so the
message and input constraint align with the component validator
(Validators.min(1)).
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 636-639: When recreating the interestRatePerPeriod form control
inside setAdvancedPaymentStrategyControls(), restore the min validator so it
matches the initial control; update the logic that removes and re-adds the
control (references: setAdvancedPaymentStrategyControls, interestRatePerPeriod)
to add Validators.min(0.01) alongside Validators.required so the template's
hasError('min') path remains reachable and validation feedback displays
correctly.
In
`@src/app/savings/savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html`:
- Around line 30-32: The mat-error currently contains hardcoded English "must be
zero or greater"; replace that literal with a translation key (e.g.
'errors.nominalInterest.min' or your repo's existing naming convention) and use
the translate pipe just like the label: {{ 'errors.nominalInterest.min' |
translate }} so the message uses
savingsAccountTermsForm.controls.nominalAnnualInterestRate within the same
mat-error element; after adding the key, add the corresponding entry to your
i18n JSON files and run npm run translations:extract to pick up the new string.
- Around line 98-101: The min-validator failures aren't surfaced for fields like
minRequiredOpeningBalance (and the other savings inputs that now use
Validators.min(0)), so add a mat-error block for each affected input (same
pattern used for nominalAnnualInterestRate) that checks the control's
hasError('min') and displays a user-facing translated message indicating the
value must be 0 or greater; update the template for minRequiredOpeningBalance
and the other inputs with matching formControlName attributes so negative values
show an error message and the user knows which field is invalid.
🪄 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: 63311981-536c-4b07-a14d-2af5b0d7c963
📒 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
...loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
Outdated
Show resolved
Hide resolved
...p/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
Show resolved
Hide resolved
...savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html
Outdated
Show resolved
Hide resolved
...savings-account-stepper/savings-account-terms-step/savings-account-terms-step.component.html
Show resolved
Hide resolved
…tribute error and add validators
JaySoni1
force-pushed
the
WEB-874-prevent-negative-values-numeric-fields-fixed
branch
from
86f7d51 to
79d9278
Compare
Contributor
Author
|
@IOhacker Thank You for the 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
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.
Changes Made :-
-Changes Made :-
-Add min(0) validation to numeric fields in Savings Account Terms, Loans Account Terms, and GLIM account in GROUP .
WEB-874
Summary by CodeRabbit