Fix race condition widget refactor 6966862986207730827#117
Fix race condition widget refactor 6966862986207730827#117harrydbarnes wants to merge 18 commits intomainfrom
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Implemented permission result handling in MainActivity to refresh data or show feedback. - Replaced hardcoded SimpleDateFormat with DateUtils in CalendarCard to respect user time format preference. - Added TODO in MainViewModel regarding hardcoded news category. - Replaced e.printStackTrace() with Log.e() in repositories for better error tracking. - Added TAG constants to repositories.
- Implemented permission result handling in MainActivity. - Replaced hardcoded date format with DateUtils in CalendarCard. - Re-implemented dynamic news category selection in MainViewModel. - Added isLoading state to MainViewModel. - Replaced e.printStackTrace() with Log.e() in repositories. - Updated repositories to return Boolean success/failure status. - Implemented transactional replaceAll in ArticleDao/NewsRepository to prevent data loss. - Implemented retry logic in WidgetUpdateWorker based on refresh success. - Created missing colors.xml to fix build failure. - Implemented settings navigation in HomeScreen.
- Implemented permission result handling in MainActivity. - Replaced hardcoded date format with DateUtils in CalendarCard. - Re-implemented dynamic news category selection in MainViewModel. - Added isLoading state to MainViewModel. - Fixed combine function signature in MainViewModel to handle 8 flows. - Replaced e.printStackTrace() with Log.e() in repositories. - Updated repositories to return Boolean success/failure status. - Implemented transactional replaceAll in ArticleDao/NewsRepository to prevent data loss. - Implemented retry logic in WidgetUpdateWorker based on refresh success. - Created missing colors.xml to fix build failure. - Implemented settings navigation in HomeScreen.
…Repository.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Implemented permission result handling in MainActivity. - Replaced hardcoded date format with DateUtils in CalendarCard. - Re-implemented dynamic news category selection in MainViewModel. - Added isLoading state to MainViewModel. - Fixed combine function signature in MainViewModel to handle 8 flows. - Replaced e.printStackTrace() with Log.e() in repositories. - Updated repositories to return Boolean success/failure status. - Implemented transactional replaceAll in ArticleDao/NewsRepository to prevent data loss. - Implemented retry logic in WidgetUpdateWorker based on refresh success. - Created missing colors.xml to fix build failure. - Implemented settings navigation in HomeScreen. - Fixed syntax error in CalendarRepository.
- Implemented permission result handling in MainActivity. - Replaced hardcoded date format with DateUtils in CalendarCard. - Re-implemented dynamic news category selection in MainViewModel. - Added isLoading state to MainViewModel. - Refactored MainViewModel combine logic to use chained calls for type safety. - Replaced e.printStackTrace() with Log.e() in repositories and DayAheadWidget. - Updated repositories to return Boolean success/failure status. - Implemented transactional replaceAll in ArticleDao/NewsRepository. - Implemented retry logic in WidgetUpdateWorker. - Created missing colors.xml to fix build failure. - Implemented settings navigation in HomeScreen. - Fixed CalendarRepository syntax error and extracted magic number. - Added @JvmStatic to AppUtils for Java interop. - Extracted Hilt versions in build.gradle. - Added null/blank title filtering in NewsRepository.
- Implemented permission result handling in MainActivity. - Replaced hardcoded date format with DateUtils in CalendarCard. - Re-implemented dynamic news category selection in MainViewModel. - Added isLoading state to MainViewModel. - Refactored MainViewModel combine logic to use chained calls for type safety. - Replaced e.printStackTrace() with Log.e() in repositories and DayAheadWidget. - Updated repositories to return Boolean success/failure status. - Implemented transactional replaceAll in ArticleDao/NewsRepository. - Implemented retry logic in WidgetUpdateWorker. - Created missing colors.xml to fix build failure. - Implemented settings navigation in HomeScreen. - Fixed CalendarRepository syntax error and extracted magic number. - Added @JvmStatic to AppUtils for Java interop. - Extracted Hilt versions in build.gradle. - Added null/blank title filtering in NewsRepository. - Implemented location fetching in MainActivity and updated MainViewModel. - Updated Geocoder usage for API 33+.
- Updated `fetchLocationName` in `MainViewModel` to use `suspendCancellableCoroutine` for the Tiramisu `Geocoder` API, preventing a race condition where the coroutine scope would close before the callback fired. - Refactored `DayAheadWidget` to inject `WeatherRepository` instead of `WeatherDao` and `Gson`. - Simplified `DayAheadWidget.onUpdate` to fetch parsed weather data directly from the repository flow, removing data access and parsing logic from the UI component.
- Created `UserPreferencesRepository` to encapsulate SharedPreferences access and expose flows for user preferences. - Updated `MainViewModel` to inject `UserPreferencesRepository` instead of direct SharedPreferences usage. - Fixed race condition in `MainViewModel` Geocoder logic by implementing `GeocodeListener`. - Updated `HomeScreen` to launch `SettingsActivity` on weather card click.
Summary of ChangesHello @harrydbarnes, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a major architectural overhaul, modernizing the Android application by migrating its UI to Jetpack Compose, implementing a robust data layer with Room and Kotlin Flow, and integrating Dagger Hilt for efficient dependency management. These changes aim to enhance the application's maintainability, scalability, and overall developer experience, while specifically resolving a race condition affecting widget updates through a more structured data retrieval process. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive refactoring of the application. You've successfully migrated from a Java/XML-based architecture to a modern, Kotlin/Compose-based one, incorporating best practices like Hilt for dependency injection, Room for a database-backed single source of truth, and a reactive UI with StateFlow. The new architecture is much cleaner, more maintainable, and robust. The fix for the widget race condition is well-implemented using goAsync and coroutines. I have a few suggestions to further improve consistency and performance in the new architecture.
| val summary = context.getSharedPreferences(AppConstants.PREFS_NAME, Context.MODE_PRIVATE) | ||
| .getString(AppConstants.KEY_SUMMARY_CACHE, context.getString(R.string.widget_default_summary)) |
There was a problem hiding this comment.
For architectural consistency, it's better to access preferences via the UserPreferencesRepository instead of directly using SharedPreferences. Since DayAheadWidget is a Hilt entry point, you can inject UserPreferencesRepository and use it to retrieve the summary. This would centralize your data access logic and make the code more maintainable.
| val prefs = applicationContext.getSharedPreferences(AppConstants.PREFS_NAME, Context.MODE_PRIVATE) | ||
|
|
||
| val latStr = prefs.getString(AppConstants.KEY_LATITUDE, null) | ||
| val lonStr = prefs.getString(AppConstants.KEY_LONGITUDE, null) | ||
| val unit = prefs.getString(AppConstants.KEY_TEMP_UNIT, AppConstants.DEFAULT_TEMP_UNIT) ?: AppConstants.DEFAULT_TEMP_UNIT | ||
| val userName = prefs.getString(AppConstants.KEY_USER_NAME, "User") ?: "User" |
There was a problem hiding this comment.
For better consistency with the new architecture, I suggest injecting UserPreferencesRepository here instead of accessing SharedPreferences directly. This centralizes preference management and makes the code more maintainable by avoiding duplicated logic. You can then use the repository to get user preferences and save the summary.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
No description provided.