-
Notifications
You must be signed in to change notification settings - Fork 626
fix(feature:client): fix stuck loading & empty state for charges UI #2428
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
fix(feature:client): fix stuck loading & empty state for charges UI #2428
Conversation
Note:
|
@biplab1 can you upload the video to confirm the changes once |
d49d242
to
210cc53
Compare
@niyajali Could you please reopen this PR? It mistakenly got closed. |
085ed09
to
d32ca3f
Compare
MifosPagingAppendProgress() | ||
when (chargesPagingList.loadState.append) { | ||
is LoadState.Error -> {} | ||
|
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.
when error occurs don't we need to show anything?
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.
Thanks for pointing that out! I've added MifosSweetError
with an appropriate message.
val code: String, | ||
val name: String, | ||
val decimalPlaces: Double, | ||
val inMultiplesOf: Int, | ||
val displaySymbol: String, | ||
val nameCode: String, | ||
val displayLabel: String, | ||
val decimalPlaces: Double? = null, | ||
val inMultiplesOf: Int? = null, |
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.
its always better to provide default values for fields, especially when the data class is used for network responses, give default values to code and name as well
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.
The name
and code
fields for Currency are required and non-nullable, so a default value shouldn’t be necessary.
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.
Update:
I’ve made code
and name
nullable in the Currency
class to safely handle cases where no currency is set up in the web app, and to prevent potential crashes. Let me know if you’d like me to handle it differently.
4bba406
to
3d1a8dc
Compare
@itsPronay @revanthkumarJ I have addressed all the comments you made. Please let me know if any further changes are needed. If everything looks good, kindly approve. |
Fixes - Jira-#459
Summary of changes:
@Serializable
to all charge-related data classes to enable proper serialization/deserialization in API communication.collect
tofirst()
and handling empty DB results with a proper empty state UI1 July 2025
instead of2025-7-1
client-charges-loading-fix.webm
Charges List:
client-charges-direct-api-fetch.webm
Run the static analysis check
./gradlew check
orci-prepush.sh
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.