WEB-134: Fix floating rates creation bugs#3466
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Header tooltip removal src/app/products/floating-rates/create-floating-rate/create-floating-rate.component.html, src/app/products/floating-rates/edit-floating-rate/edit-floating-rate.component.html |
Removed the question-mark help icon and its matTooltip from the "Floating Rate Periods" section headers. |
Create/Edit components src/app/products/floating-rates/create-floating-rate/create-floating-rate.component.ts, src/app/products/floating-rates/edit-floating-rate/edit-floating-rate.component.ts |
Removed minDate class fields. Create component now prevents submit when floatingRatePeriodsData.length === 0. Add-period dialog call no longer passes fromDate by default; edit call adds isNew: true. |
Floating Rate Period Dialog src/app/products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.ts |
Compute minDate as tomorrow at 00:00:00.000, hardened dialog-data access using optional chaining, and made form control defaults resilient to missing dialog data. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- WEB-720 Prevent negative values for interest rate in floating rate period form #3148 — touches floating-rate-period dialog and interestRate form control validation; likely related to dialog/form behavior changes.
- WEB-841 Update Cancel button styling in Floating Rate Periods dialog #3363 — modifies floating-rate-period dialog/template; potentially overlaps with dialog adjustments.
Suggested reviewers
- IOhacker
- gkbishnoi07
🚥 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-134: Fix floating rates creation bugs' directly and clearly summarizes the main changes—fixing multiple usability bugs in the floating rates creation 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 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.
Actionable comments posted: 2
🤖 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/products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.ts`:
- Around line 55-59: The component currently sets minDate from the client local
clock; replace that logic to derive tomorrow from the tenant business date
provided by SettingsService.businessDate: read SettingsService.businessDate
(e.g., inject SettingsService if not already), create a new Date from that
value, add one day (date.setDate(date.getDate() + 1)), zero the time with
setHours(0,0,0,0), and assign to this.minDate so the dialog uses the tenant
business date as the baseline; update the code block that currently computes
`tomorrow` and references `this.minDate` (in
floating-rate-period-dialog.component, where `minDate` is set).
- Around line 62-66: The editFloatingRatePeriod() caller opens the
FloatingRatePeriodDialogComponent without the isNew flag, causing the dialog's
row-disabled logic (this.data?.isNew in
floating-rate-period-dialog.component.ts) to behave inconsistently; update
editFloatingRatePeriod() to pass isNew: true in the dialog data payload (same
shape as createFloatingRatePeriod's call) so the component receives
this.data.isNew and overrides date-based disabling, or alternatively change the
dialog invocation to include an explicit flag (isNew) matching the create path.
🪄 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: 8f127727-007b-40fb-a554-2a8b92f4f58b
📒 Files selected for processing (5)
src/app/products/floating-rates/create-floating-rate/create-floating-rate.component.htmlsrc/app/products/floating-rates/create-floating-rate/create-floating-rate.component.tssrc/app/products/floating-rates/edit-floating-rate/edit-floating-rate.component.htmlsrc/app/products/floating-rates/edit-floating-rate/edit-floating-rate.component.tssrc/app/products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.ts
💤 Files with no reviewable changes (2)
- src/app/products/floating-rates/edit-floating-rate/edit-floating-rate.component.html
- src/app/products/floating-rates/edit-floating-rate/edit-floating-rate.component.ts
...products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.ts
Outdated
Show resolved
Hide resolved
...products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.ts
Show resolved
Hide resolved
YousufFFFF
force-pushed
the
WEB-134-fix-floating-rates-bugs
branch
from
f8394fd to
62cb2bc
Compare
|
Hi @IOhacker! |
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
Kindlyl see my comments
| */ | ||
| addFloatingRatePeriod() { | ||
| const floatingRatePeriodDialogRef = this.dialog.open(FloatingRatePeriodDialogComponent, { | ||
| data: { |
There was a problem hiding this comment.
The fromDate: this.settingsService.businessDate was removed from the dialog data because the minDate calculation is now handled inside the dialog component itself. The dialog now calculates minDate as businessDate + 1 day (tomorrow) since floating rates must always be in the future. This centralizes the date logic in one place rather than passing it from the parent component.
| <div class="layout-row-wrap gap-70percent m-b-10 layout-lt-md-column"> | ||
| <p class="mat-title flex-25"> | ||
| {{ 'labels.inputs.Floating Rate Periods' | translate }} | ||
| <i |
There was a problem hiding this comment.
The question mark icon was removed as per the Jira ticket WEB-134 requirements. The ticket states: "There is a label 'Floating Rate Periods ?' The question mark makes no sense in labels - remove it."
|
Hey @IOhacker! |
2 participants
Description
This PR fixes several usability issues in the Create Floating Rate form:
Removed unnecessary question mark icon from the "Floating Rate Periods" label - the icon served no functional purpose and was confusing in this context
Set minimum selectable date to tomorrow - Fineract requires floating rate
periods to be in the future (not today). Previously users could select today's date which would result in a backend validation error. Now the date picker prevents selecting dates earlier than tomorrow.
Disabled submit button until at least one rate period is configured - Fineract requires at least one floating rate period record. Previously users could submit without any periods and receive an unhelpful backend error.
Added null safety checks in the floating rate period dialog to prevent potential runtime errors when data is empty
No new dependencies required.
Related issues and discussion
#WEB-134
Screenshots
After: Date picker only allows tomorrow onwards, submit button disabled until rate period added
https://github.com/user-attachments/assets/15636f14-32b1-4768-8eaf-60fd496c4118
Checklist
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
UI Changes
Form Validation
Date Selection
Dialog Stability