Skip to content

savings migration to CMP #2398

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

Merged
merged 14 commits into from
Jun 9, 2025
Merged

Conversation

itsPronay
Copy link
Contributor

@itsPronay itsPronay commented Jun 5, 2025

Fixes - https://mifosforge.jira.com/browse/MIFOSAC-427
Screenshot_2025-06-09-16-41-47-272_org.mifos.client.demo.debug.jpg

Didn't create a Jira ticket, click here to create new.

Please Add Screenshots If there are any UI changes.

Before After

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the static analysis check ./gradlew check or ci-prepush.sh to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

@therajanmaurya therajanmaurya requested a review from niyajali June 6, 2025 16:52
@revanthkumarJ
Copy link

@itsPronay also upload the screenshots

@itsPronay
Copy link
Contributor Author

@itsPronay also upload the screenshots

i am currently unable to navigate as the previous screen hasnt been completed yet. I'll try a workaround ...

@niyajali
Copy link
Collaborator

niyajali commented Jun 8, 2025

@itsPronay, Resolve merge conflicts and ask team members for approval

@revanthkumarJ
Copy link

@itsPronay let me know after that changes i will approve this pr

Copy link
Contributor

@biplab1 biplab1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should address nullability before entering the LaunchedEffect block or handle the null case inside the block before calling the function. Do not use !! unless you are 100% certain the value cannot be null. I also noticed !! being used at other places also. Please review them to make sure the null cases are handled properly.

@biplab1
Copy link
Contributor

biplab1 commented Jun 9, 2025

@itsPronay Let me know once you’ve addressed the comments—I’ll approve it then. Also, I noticed several TODOs in the code.

@niyajali niyajali merged commit 05d9d43 into openMF:kmp-impl Jun 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants