-
Notifications
You must be signed in to change notification settings - Fork 0
Fix app status architecture #73
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
Conversation
- Add StatusPage widget to display status during critical startup sequence - Handle config fetching and error states - Implement retry functionality for failed config fetch - Update view exports to include new StatusPage
- Move maintenance and update checks from AppOpened event into _fetchAppConfig - Remove AppOpened event handler - Ensure remote config is fetched before evaluating app status - Implement a prioritized check sequence: maintenance > update > user role
- Introduce a new stable startup architecture using BlocBuilder - Add full-screen status pages for maintenance, update required, and config fetching states - Move main app UI build to a more stable point in the app lifecycle - Simplify theme and localization setup in MaterialApp - Update StatusPage to use new localization approach
- Remove unnecessary checks for app config and URI - Consolidate print statements and remove detailed debug logs - Simplify unauthenticated user redirect logic - Reduce complexity in anonymous/authenticated user handling - Remove fallback case, as all statuses should be handled
- Add theme customization parameters in app.dart - Update import statements for l10n - Adjust theme settings in StatusPage - TODO: Implement proper localization for retry button text
- Enhance comments to explain the purpose and functionality of key widgets - Reorganize comment sections for better clarity and flow - Add explanations for the use of separate MaterialApp widgets - Clarify the theme configuration process for different app states - Emphasize the importance of the stable main application UI root
- Removed the conditional check that prevented refetching remote config for the current user session - This change allows for more frequent updates to remote config without being restricted to error recovery scenarios
- Add root path as initial location to prevent complex page builds - Implement redirect logic from root to appropriate path - Add comments explaining the routing changes
- Add environment check to skip periodic app status checks in demo mode - Implement conditional app lifecycle checks based on environment - Update AppStatusService constructor to include AppEnvironment parameter - Modify _AppViewState to pass environment to AppStatusService These changes improve the demo experience by reducing unnecessary reloads and optimizing resource usage when running in demo mode.
- Introduce background check option for AppConfig fetching - Optimize state transitions and error handling - Simplify event classes for better maintainability - Enhance logging for background fetch operations
- Explain maintenance mode and forced update functionalities - Highlight the advantage of dynamic control over user experience - Emphasize the pre-implemented logic for graceful server downtime and version enforcement
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.
Summary of Changes
Hello @fulleni, 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 introduces a significant architectural overhaul to how the application manages its critical status and startup sequence. By centralizing the evaluation of remote configuration states within the AppBloc
and implementing a robust "master switch" in the root App
widget, the app now gracefully handles conditions like maintenance mode, forced updates, and initial configuration loading. This refactoring enhances the application's stability, improves the user experience during critical states, and simplifies the routing logic by offloading app-wide status management from the router.
Highlights
- Centralized App Status Management: The application's core
AppBloc
now directly evaluates critical remote configuration states (maintenance mode, forced updates) and immediately updates theAppStatus
. This ensures that the app's UI reflects these critical states as soon as the configuration is fetched, regardless of whether it's an initial startup or a background check. - Robust Startup Architecture: The main
App
widget'sBlocBuilder
has been refactored to act as a "master switch." It now conditionally renders full-screen status pages (MaintenancePage
,UpdateRequiredPage
,StatusPage
) before the mainMaterialApp.router
is built. This significantly improvesBuildContext
stability and preventsl10n
(localization) related crashes during the app's initial loading sequence. - Dedicated Status Page: A new
StatusPage
has been introduced to specifically handle theconfigFetching
andconfigFetchFailed
states. This page provides a clear loading indicator during initial configuration fetch and offers a retry mechanism for network failures, enhancing user experience during startup. - Silent Background Configuration Checks: The
AppConfigFetchRequested
event now includes anisBackgroundCheck
parameter. This allows periodic and app lifecycle-driven configuration fetches (managed byAppStatusService
) to occur silently without displaying a loading screen, preventing disruptive UI changes for the user. - Router Simplification: The
GoRouter
'sredirect
logic has been streamlined. It no longer attempts to handle critical app status states (likeconfigFetching
orunderMaintenance
), as these are now managed at a higher level by the rootApp
widget. The router's primary responsibility is now solely authentication-based route protection. - Demo Mode Optimization: The
AppStatusService
now accepts anAppEnvironment
parameter. Indemo
mode, periodic status checks and app lifecycle-based checks (e.g., on app resume) are skipped. This is particularly useful for web demos, preventing unnecessary reloads or backend calls when switching browser tabs. - Documentation Update: The
README.md
has been updated to reflect the new "Backend-Driven Behavior" capabilities, specifically detailing how Maintenance Mode and Forced Updates are handled client-side.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces a significant and beneficial architectural refactoring of the application's startup and status handling logic. By introducing a "master switch" BlocBuilder
in _AppView
, the app now robustly handles critical states like maintenance mode, forced updates, and configuration fetching before rendering the main UI. This resolves potential race conditions and BuildContext
instability. The related changes in AppBloc
, AppEvent
, AppStatusService
, and GoRouter
are well-aligned with this new, cleaner architecture.
My review focuses on a potential issue with localization within the new status pages. I've left two high-severity comments suggesting to restore the UiKitLocalizations
delegates to the status page MaterialApp
widgets and to use the correct localized string in the new StatusPage
.
Overall, this is an excellent improvement to the application's stability and maintainability.
Description
Type of Change