-
Notifications
You must be signed in to change notification settings - Fork 807
fix: Non required fields are mandatories for Savings application #3033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughConfiguration and logic adjustments to the savings application flow: maxLineLength threshold increased from 120 to 140, button enabled state binding removed, field officer selection dropdown removed, empty-amount validation bypass introduced, and the consolidated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
config/detekt/detekt.ymlfeature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationScreen.ktfeature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationViewModel.ktfeature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/savingsApplication/SavingsApplyScreen.ktfeature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/savingsApplication/SavingsApplyViewModel.kt
💤 Files with no reviewable changes (2)
- feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/savingsApplication/SavingsApplyViewModel.kt
- feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationScreen.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR Checks KMP / Static Analysis Check
🔇 Additional comments (2)
config/detekt/detekt.yml (1)
803-803: The maxLineLength increase to 140 is justified.The Savings Application includes legitimate lines that exceed 120 characters, including a conditional check with a fully-qualified generated resource reference (122 characters). Refactoring to avoid exceeding 120 characters would require introducing intermediate variables, making the code less readable without practical benefit. The increase accommodates necessary complexity in generated code and multi-line parameter formatting.
feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationViewModel.kt (1)
115-117: LGTM: Formatting improvements.The multi-line parameter formatting with trailing commas improves readability and aligns with Kotlin coding conventions.
Also applies to: 123-126, 128-130, 146-148, 254-257, 313-316, 475-478, 894-895
...lin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationViewModel.kt
Show resolved
Hide resolved
...lin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationViewModel.kt
Show resolved
Hide resolved
...kotlin/org/mifos/mobile/feature/savings/application/savingsApplication/SavingsApplyScreen.kt
Show resolved
Hide resolved
|
A new ticket has been created for the preview screen. The preview screen will be designed under that ticket. |
|
The screen seems to be OK, just to comment that the Lock In period can be 0 |
@itsPronay confirm that Lock In period can accept 0 ? |
|
@therajanmaurya yes it can |
Screen_Recording_20251227_051316_Mifos.Mobile.mp4
Fixes - https://mifosforge.jira.com/browse/MM-469
Please Add Screenshots If there are any UI changes.
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.