Repository navigation
Fix/refactor and loading bug - #1086
gietabhi10 wants to merge 4 commits into
Conversation
…uite (Re-applied with conflict resolution)
|
@gietabhi10 What did the refactor change about the database/entity/type that caused Room to no longer deserialize this enum correctly, since it worked before without an issue? Or is it redundant? |
|
Also, changes in the "backup" area. To be clear, I don't see the bug appear while importing an existing backup. It only seems to appear when updating the app itself, with no backups imported |
…ng and fix init hang
0632e93 to
6cf7ed8
Compare
|
@PranshulGG I've investigated the cause and confirmed that the refactor itself didn't change the entities, but it exposed a limitation in Room's default Enum handling. 1. The Issue: Room's default Enum deserialization is "strict". If an existing user has a string in their database that doesn't exactly match the current Enum (which can happen with stale data from older versions), Room crashes the entire query. This is why it worked for fresh installs but failed for existing ones. |
Interesting. What are the steps to reproduce? Everything else makes sense. |
|
@PranshulGG "Great question! Here are the steps to reproduce the 'Initialization Logic' bug in the previous version:
What happens: Because the initial weather fetch fails (offline), the code enters the WeatherResult.Error block. In the previous implementation, isInitialized was only set to true on a successful network response. As a result, the SplashScreen (in MainActivity) remains visible until it hits its hard 5-second timeout, even though the app has cached data ready to display. The Fix: I updated handleWeatherData to set isInitialized = result.cacheWeather != null in the error path. This ensures that if we have any data (even cached), we dismiss the splash screen and show the UI immediately, providing a much faster and smoother experience for existing users. Glad everything else makes sense! Let me know if you need any more details." |
|
Hey @gietabhi10 , |
|
@reveler-hub Yep, looks like ai The "Initialization Logic" is redundant. In MainActivity, it already sets isInitialized to true if it takes longer than 5 seconds, or 3, I don't remember but it does But the PR seems ai generated. @gietabhi10 Can you disclose what you used it for? |
There were no changes to the enum, so how could that be the issue? And in the BlockRepository, it is already handled IF the default blocks do not match the returned DB blocks → there are new blocks → return both saved and missing blocks The crash shouldn't happen |
… and Init logic as per review)
|
Thanks for the catch, @PranshulGG and @reveler-hub ! AI Disclosure: Yes, I'm using the Android Studio AI agent (Gemini) to help orchestrate this refactor and generate the unit tests. I'm carefully reviewing all its suggestions to ensure they align with the project's goals. Technical Fixes: |
|
@gietabhi10 I'm not sure I wanna merge it, as it will conflict with my already-made changes to using managers instead of use cases, but I appreciate that you want to contribute to the app But big changes that modify the main core code should be human reviewed so they know what's going on, as AI misses so many edge cases or doesn't understand why the change it made was the right call |
|
@PranshulGG Totally understand! I completely respect your decision to go with a Managers-based pattern and your preference for human-driven core changes. The goal was to help with the ViewModel growth, but I definitely don't want to introduce architectural conflicts with your current work. I'll close this PR for now to keep your queue clean. If you're open to it, I'd love to contribute again in the future on a smaller scale—perhaps helping with the migration to those Managers once you have the pattern established? Thanks for the feedback and for all the work you do on the app! |
|
@gietabhi10 Thanks! and yes, you're free to contribute any time after the base is ready |
|
Thanks for the encouraging words! As I'm looking to grow as an open-source contributor, I'd love your perspective: Beyond architectural preferences, what do you think are the most important things a contributor can do to make their PRs easier to review and more 'trustworthy' for a maintainer? Also, are there any general skills or habits in the field of Android development that you've seen separate great contributors from average ones? I'd appreciate any generic advice you have for someone wanting to help out more effectively in the future! |
Well, making a contribution more trustworthy is mostly about showing that you understand the code and UX. The thing with ai generated code is that it honestly looks great. Like, I thought your UseCase implementation was great, but the thing that threw me off was redundant changes or changes that weren't connected to the actual issues. It kind of makes me think, "So, does this person understand the codebase?" :P I won't say ai is bad. I think it helps with investigations, and even if the code is ai generated, it should still be understood by the person who submitted it! I think that makes it really easy to review because you know what you did
Honestly reading the Contirubtions guide that is in the repo many miss it and they sumbit a PR which might not get merged because they missed things that were in the guide |
This PR re-introduces the Clean Architecture refactor (Use Cases) with a critical fix for existing installs.
✅ Fix for Existing Installs
🏗️ Refactor Summary