WEB-931-loan-account-application-must-allow-rate-0-the-nominal-interest-rate-field-fix#3539
Merged
IOhacker merged 1 commit intoopenMF:devfrom Apr 28, 2026
Merged
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loan Account Terms 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 |
Relaxed minimum constraint for interestRatePerPeriod numeric input from 0.01 to 0 in template and form control validators across multiple form creation and update methods. |
Loan Product Terms src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html, src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts |
Relaxed minimum constraint for period payment rate fields (minPeriodPaymentRate, periodPaymentRate, maxPeriodPaymentRate) from 0.01 to 0 in template, validators, and error messages. Added non-null assertions for null-safety on form control operations and form group .get() calls. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
- WEB-874 Prevent negative values in numeric fields of Group #3412: Modifies the same
interestRatePerPeriodconstraint in loans-account-terms-step, allowing comparison of related validation changes. - WEB-467 Interest rates field allows zero and negative values in loan product creation form #2891: Directly modifies loan product term form controls' interest-rate minimum values and error message fallbacks in the same component.
- WEB-813: Working Capital loan account creation #3257: Updates loan terms UI and form controls across both loans-account-terms-step and loan-product-terms-step for interest/period payment rate validation.
Suggested reviewers
- IOhacker
- alberto-art3ch
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title directly describes the main change: allowing a 0 nominal interest rate in the loan account application form, which aligns with the primary objective across modified files. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ 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 (2)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
569-593:⚠️ Potential issue | 🔴 CriticalThe numeric listener at lines 562-563 prevents entering zero interest.
The
valueChangessubscription forces any value less than 0.01 to 0.01, which blocks the PR objective of allowing zero interest rates. TheValidators.min(0)constraints in lines 569-593 are correct, but this listener overrides them at runtime.Fix required
Change lines 562-563 to allow zero:
- if (typeof value === 'number' && value < 0.01) { - interestRateControl.setValue(0.01, { emitEvent: false }); + if (typeof value === 'number' && value < 0) { + interestRateControl.setValue(0, { emitEvent: false });🤖 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 569 - 593, The numeric listener currently clamping interest values (attached to the interestRatePerPeriod valueChanges) prevents entering 0; update the listener inside setAdvancedPaymentStrategyControls to allow zero by removing the forced minimum of 0.01 (either set the clamp to 0 or remove the clamp entirely) and rely on the existing Validators.min(0) on the interestRatePerPeriod control; ensure the subscription still sanitizes negatives (<0) to 0 but does not coerce valid 0 values to 0.01 so zero-interest inputs are accepted.src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts (1)
393-399:⚠️ Potential issue | 🔴 CriticalAdd null checks in
zeroInterestsubscription to handle removed controls.When
isLinkedToFloatingInterestRatesis true, the interest-rate controls (minInterestRatePerPeriod,interestRatePerPeriod,maxInterestRatePerPeriod) are removed (lines 396–399). IfzeroInterestchanges while in floating-rate mode, the subscription at lines 475–495 will crash because it unconditionally accesses these controls with!.The zeroInterest handler has no guard to check control existence before calling
.get()!on them. Add null checks before accessing these controls, or guard with theisLinkedToFloatingInterestRatesstate.Proposed fix
this.zeroInterest.valueChanges.subscribe((zeroInterest) => { + const minCtrl = this.loanProductTermsForm.get('minInterestRatePerPeriod'); + const rateCtrl = this.loanProductTermsForm.get('interestRatePerPeriod'); + const maxCtrl = this.loanProductTermsForm.get('maxInterestRatePerPeriod'); + + if (!minCtrl || !rateCtrl || !maxCtrl) { + this.validateAdvancedPaymentStrategyControls(); + return; + } + if (zeroInterest) { - this.loanProductTermsForm.get('minInterestRatePerPeriod')!.patchValue(0); - this.loanProductTermsForm.get('minInterestRatePerPeriod')!.disable(); - this.loanProductTermsForm.get('interestRatePerPeriod')!.patchValue(0); - this.loanProductTermsForm.get('interestRatePerPeriod')!.disable(); - this.loanProductTermsForm.get('maxInterestRatePerPeriod')!.patchValue(0); - this.loanProductTermsForm.get('maxInterestRatePerPeriod')!.disable(); + minCtrl.patchValue(0); + minCtrl.disable(); + rateCtrl.patchValue(0); + rateCtrl.disable(); + maxCtrl.patchValue(0); + maxCtrl.disable(); } else { - this.loanProductTermsForm - .get('minInterestRatePerPeriod')! - .patchValue(this.loanProductsTemplate.minInterestRatePerPeriod); - this.loanProductTermsForm.get('minInterestRatePerPeriod')!.enable(); - this.loanProductTermsForm - .get('interestRatePerPeriod')! - .patchValue(this.loanProductsTemplate.interestRatePerPeriod); - this.loanProductTermsForm.get('interestRatePerPeriod')!.enable(); - this.loanProductTermsForm - .get('maxInterestRatePerPeriod')! - .patchValue(this.loanProductsTemplate.maxInterestRatePerPeriod); - this.loanProductTermsForm.get('maxInterestRatePerPeriod')!.enable(); + minCtrl.patchValue(this.loanProductsTemplate.minInterestRatePerPeriod); + minCtrl.enable(); + rateCtrl.patchValue(this.loanProductsTemplate.interestRatePerPeriod); + rateCtrl.enable(); + maxCtrl.patchValue(this.loanProductsTemplate.maxInterestRatePerPeriod); + maxCtrl.enable(); } this.validateAdvancedPaymentStrategyControls(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts` around lines 393 - 399, The zeroInterest subscription in loan-product-terms-step.component (the handler that reads 'zeroInterest' and then unconditionally calls this.loanProductTermsForm.get('minInterestRatePerPeriod')!, .get('interestRatePerPeriod')!, and .get('maxInterestRatePerPeriod')!) must guard for controls that may have been removed by the isLinkedToFloatingInterestRates subscription; update the zeroInterest handler to first check the form control existence (e.g., via this.loanProductTermsForm.get('...') !== null or optional chaining) or check the current isLinkedToFloatingInterestRates state before accessing or patching those controls, and skip or short-circuit updates when the controls are absent to avoid runtime crashes.
🤖 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 569-593: The numeric listener currently clamping interest values
(attached to the interestRatePerPeriod valueChanges) prevents entering 0; update
the listener inside setAdvancedPaymentStrategyControls to allow zero by removing
the forced minimum of 0.01 (either set the clamp to 0 or remove the clamp
entirely) and rely on the existing Validators.min(0) on the
interestRatePerPeriod control; ensure the subscription still sanitizes negatives
(<0) to 0 but does not coerce valid 0 values to 0.01 so zero-interest inputs are
accepted.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts`:
- Around line 393-399: The zeroInterest subscription in
loan-product-terms-step.component (the handler that reads 'zeroInterest' and
then unconditionally calls
this.loanProductTermsForm.get('minInterestRatePerPeriod')!,
.get('interestRatePerPeriod')!, and .get('maxInterestRatePerPeriod')!) must
guard for controls that may have been removed by the
isLinkedToFloatingInterestRates subscription; update the zeroInterest handler to
first check the form control existence (e.g., via
this.loanProductTermsForm.get('...') !== null or optional chaining) or check the
current isLinkedToFloatingInterestRates state before accessing or patching those
controls, and skip or short-circuit updates when the controls are absent to
avoid runtime crashes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0f42f44-be46-4bcc-a952-eb2ae0aa9c27
📒 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/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
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 :-
-Update HTML min attributes to allow 0 interest rates in loan forms
WEB-931
Summary by CodeRabbit