Feature/offline connectivity login #2851
Conversation
📝 WalkthroughWalkthroughAdds connectivity awareness across auth and UI: login page listens to AuthBloc and ConnectivityBloc; auth repository enforces/handles online/offline with Hive fallback for login; connectivity bloc uses on handlers, reachability checks, and timed banner dismissal; no-internet banner becomes animated and actionable. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginPage
participant AuthBloc
participant ConnectivityBloc
participant AuthRepo
participant Nav
User->>LoginPage: Tap "Login"
LoginPage->>AuthBloc: LoginRequested(email, password)
ConnectivityBloc-->>LoginPage: Emits Online / Offline
alt Offline
LoginPage->>User: Show SnackBar "No internet"
AuthRepo-->>AuthBloc: Return cached token or throw
else Online
AuthBloc->>AuthRepo: loginWithEmailAndPassword
AuthRepo-->>AuthBloc: token or error
end
alt AuthLoaded
LoginPage->>Nav: pushAndRemoveUntil(NavPage)
else EmailUnverifiedError
LoginPage->>User: Show "Email Verification Required" dialog
User->>LoginPage: Verify Now
LoginPage->>Nav: push(EmailVerificationScreen(email))
else AuthLoadingError
LoginPage->>User: Show error
end
sequenceDiagram
participant DeviceNet as Device Network
participant Connectivity as ConnectivityBloc
participant Banner as NoInternetBanner
participant Timer as DismissTimer
DeviceNet-->>Connectivity: Network change
Connectivity->>Connectivity: _hasInternetConnection()
alt Reachable
Connectivity-->>Banner: ConnectivityOnline
else Unreachable
Connectivity-->>Banner: ConnectivityOffline(reappeared?)
end
Banner->>Connectivity: Refresh tap -> ConnectivityCheckRequested
Connectivity->>Connectivity: Recheck reachability
Connectivity-->>Banner: Online / Offline
Banner->>Connectivity: Dismiss tap -> ConnectivityBannerDismissed
Connectivity->>Timer: start(5m)
Timer-->>Connectivity: onExpire -> ConnectivityCheckRequested
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart (1)
169-179: Replace assert with runtime check for AIRQO_MOBILE_TOKEN.Asserts don’t run in release. Guard the header at runtime and surface a controlled error.
Apply this diff:
- final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"]; - assert(apiToken != null, 'AIRQO_MOBILE_TOKEN missing in .env'); + final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"]; + if (apiToken == null || apiToken.isEmpty) { + loggy.error('AuthRepository: Missing AIRQO_MOBILE_TOKEN in .env'); + throw Exception('App configuration error. Please try again later.'); + } @@ - "Authorization": apiToken!, + "Authorization": apiToken,
🧹 Nitpick comments (12)
src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart (2)
198-202: Avoid throwing raw response bodies to callers.Throwing “Invalid response: ” can leak server internals to the UI. Prefer a generic message and log details.
Apply this diff:
- loggy.error( - 'AuthRepository: Invalid response during email verification - $e'); - throw Exception("Invalid response: ${verifyResponse.body}"); + loggy.error( + 'AuthRepository: Invalid response during email verification - $e | body: ${verifyResponse.body}'); + throw Exception("Unable to verify email at the moment. Please try again.");
41-44: Trim PII in logs (emails/usernames).Multiple info/error logs include usernames/emails. Consider masking (e.g., j***@d***.com) or removing them in production logs to reduce PII exposure.
If helpful, I can add a small obfuscation helper and apply it across these logs.
Also applies to: 75-76, 100-101, 168-169, 217-218, 243-249
src/mobile-v3/lib/src/app/shared/bloc/connectivity_event.dart (1)
19-20: Make the event const to avoid unnecessary allocations.No fields → prefer a const constructor for reuse in UI and tests.
Apply this diff:
-class ConnectivityCheckRequested extends ConnectivityEvent {} +class ConnectivityCheckRequested extends ConnectivityEvent { + const ConnectivityCheckRequested(); +}src/mobile-v3/lib/src/app/auth/pages/login_page.dart (2)
158-165: Prevent stacked SnackBars when repeatedly receiving offline states.Hide any current SnackBar before showing a new one and mark the duration const.
Apply this diff:
- ScaffoldMessenger.of(context).showSnackBar( + final messenger = ScaffoldMessenger.of(context); + messenger.hideCurrentSnackBar(); + messenger.showSnackBar( SnackBar( content: Text( 'No Internet Connection. Please check your network.'), - duration: Duration(seconds: 3), + duration: const Duration(seconds: 3), ), );
222-223: Remove no-op onChanged handlers.They add noise without functionality.
Apply this diff:
- controller: emailController, - onChanged: (value) {}, + controller: emailController, @@ - controller: passwordController, - onChanged: (value) {}, + controller: passwordController,Also applies to: 255-257
src/mobile-v3/lib/src/app/shared/pages/no_internet_banner.dart (2)
53-56: Consider fixed spacing for consistent padding across devices.MediaQuery-scaled padding can feel too tight on small phones and too spacious on tall devices. Fixed values keep the banner readable.
Apply this diff:
- padding: EdgeInsets.symmetric( - horizontal: MediaQuery.of(context).size.width * 0.04, - vertical: MediaQuery.of(context).size.height * 0.01, - ), + padding: const EdgeInsets.symmetric( + horizontal: 16, + vertical: 8, + ),
61-75: Optional: Announce as a status message for screen readers.Wrap the text with Semantics(liveRegion: true) or use a SnackBar alternative when appropriate to ensure assistive tech announces connectivity loss.
I can draft the a11y adjustments if you’d like.
src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart (5)
15-16: Prevent stale emissions from async connectivity listener (race-safe).Because the listener awaits reachability, quick successive changes can cause out-of-order results to be dispatched. Guard with a monotonic check-id so only the latest result emits.
Apply this diff:
- Timer? _resetDismissTimer; + Timer? _resetDismissTimer; + // Monotonic guard for async reachability checks + int _lastReachabilityCheckId = 0; - _connectivity.onConnectivityChanged.listen((result) async { - logDebug('Connectivity changed: $result'); - final isConnected = - result != ConnectivityResult.none && await _hasInternetConnection(); - add(ConnectivityChanged(isConnected)); - }); + _connectivity.onConnectivityChanged.listen((result) async { + logDebug('Connectivity changed: $result'); + final checkId = ++_lastReachabilityCheckId; + final isConnected = + result != ConnectivityResult.none && await _hasInternetConnection(); + if (checkId == _lastReachabilityCheckId) { + add(ConnectivityChanged(isConnected)); + } else { + logDebug('Stale reachability result dropped'); + } + });Also applies to: 21-26
28-32: Cancel the dismissal timer when the app is back online.Prevents a stale timer from re-triggering a check 5 minutes later when already online.
if (event.isConnected) { logInfo('Emitting ConnectivityOnline'); + _resetDismissTimer?.cancel(); // avoid stale offline re-checks emit(ConnectivityOnline());
28-39: Reduce duplication in online/offline emission logic.Both handlers compute the same reappeared flag and emit identical states. Consider extracting a small helper to centralize this logic, or route the stream listener to add ConnectivityCheckRequested and keep all mapping in one place. Keeps behavior consistent and easier to evolve.
Also applies to: 41-56
2-2: Reachability: avoid DNS-only check to google.com and add an explicit timeout.DNS to google.com may be blocked (or slow), causing false negatives/long waits. A short TCP connect to a well-known resolver with a timeout is generally more robust on mobile.
-import 'dart:io' show InternetAddress; +import 'dart:io' show InternetAddress, Socket;Future<bool> _hasInternetConnection() async { - try { - final result = await InternetAddress.lookup('google.com'); - return result.isNotEmpty && result[0].rawAddress.isNotEmpty; - } catch (_) { - return false; - } + try { + // Fast, DNS-agnostic reachability check with explicit timeout. + final socket = + await Socket.connect('1.1.1.1', 53, timeout: const Duration(seconds: 2)); + socket.destroy(); + return true; + } catch (_) { + return false; + } }If you’d prefer a library solution, internet_connection_checker provides a battle-tested reachability stream with configurable endpoints.
Also applies to: 71-76
58-67: LGTM: Banner dismissal and timed re-check flow looks solid.Nice UX touch with 5-minute snooze and auto re-check. Emission toggles isDismissed and reappeared as expected.
If helpful, I can scaffold unit tests using fakeAsync to simulate the 5-minute timer and assert reappeared toggling. Want me to push a test draft?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/mobile-v3/lib/src/app/auth/pages/login_page.dart(11 hunks)src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart(4 hunks)src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart(2 hunks)src/mobile-v3/lib/src/app/shared/bloc/connectivity_event.dart(1 hunks)src/mobile-v3/lib/src/app/shared/bloc/connectivity_state.dart(1 hunks)src/mobile-v3/lib/src/app/shared/pages/no_internet_banner.dart(1 hunks)
🔇 Additional comments (9)
src/mobile-v3/lib/src/app/shared/bloc/connectivity_state.dart (1)
16-20: Nice addition: reappeared flag + copyWith are clean and future-proof.The new field and const/named constructor are straightforward, and copyWith keeps the state ergonomic.
Also applies to: 24-29
src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart (1)
28-39: VerifiedgetDatasignature and usages
TheHiveRepository.getData<T>(String key, String boxName)signature expects the key first, then the box name. All calls likeawait HiveRepository.getData('token', HiveBoxNames.authBox);match this order correctly. No changes needed.
src/mobile-v3/lib/src/app/auth/pages/login_page.dart (1)
68-75: LGTM: Success navigation clears back stack correctly.pushAndRemoveUntil to NavPage on AuthLoaded is the right UX here.
src/mobile-v3/lib/src/app/shared/pages/no_internet_banner.dart (1)
25-33: Solid UX upgrade: animated, accessible banner with retry/dismiss actions.The AnimatedSwitcher + SlideTransition works nicely, and the Semantics label and action hooks are on point.
Also applies to: 34-43, 77-97
src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart (5)
2-2: Confirm platform scope for dart:io usage.This bloc imports dart:io; ensure this file is excluded from web builds (or guarded via conditional imports), since dart:io is not supported on Flutter web.
83-83: LGTM: Initial connectivity diagnostics.The extra debug line will help trace startup behavior without altering logic.
95-95: Good cleanup on close.Cancelling the dismissal timer alongside the subscription prevents leaks and surprises.
61-61: Const Constructor Verified for ConnectivityOfflineThe
ConnectivityOfflinestate in src/mobile-v3/lib/src/app/shared/bloc/connectivity_state.dart already declares aconstconstructor (lines 14–18), so theemit(const ConnectivityOffline(...))call will compile as intended. The related events (ConnectivityCheckRequestedandConnectivityBannerDismissed) are also defined in connectivity_event.dart and correctly marked as part of the same library.No changes required.
7-7: Confirmed Loggy is initialized on app startup
- In core/utils/app_loggy_setup.dart, AppLoggySetup.init() invokes Loggy.initLoggy(...)
- main.dart calls AppLoggySetup.init(isDevelopment: !kReleaseMode) before runApp
- Global logDebug/logInfo will now function as expected
| final data = json.decode(registerResponse.body); | ||
|
|
||
| if (registerResponse.statusCode != 200) { | ||
| throw Exception(data['errors']['message'] ?? data['message']); | ||
| final errorMessage = data['errors']?['message'] ?? data['message']; | ||
| loggy.error('AuthRepository: Registration failed - $errorMessage'); | ||
| throw Exception(errorMessage); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Registration: parse errors defensively to avoid crashes on non-JSON bodies.
Same rationale as login: decode safely and fall back when needed.
Apply this diff:
- final data = json.decode(registerResponse.body);
+ Map<String, dynamic> data = {};
+ try {
+ final decoded = jsonDecode(registerResponse.body);
+ if (decoded is Map<String, dynamic>) data = decoded;
+ } catch (_) {
+ loggy.warning('AuthRepository: Non-JSON register response');
+ }
@@
- final errorMessage = data['errors']?['message'] ?? data['message'];
+ final errorMessage =
+ (data['errors'] is Map ? data['errors']['message'] as String? : null) ??
+ (data['message'] as String?) ??
+ 'Registration failed. Please try again.';
loggy.error('AuthRepository: Registration failed - $errorMessage');
throw Exception(errorMessage);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final data = json.decode(registerResponse.body); | |
| if (registerResponse.statusCode != 200) { | |
| throw Exception(data['errors']['message'] ?? data['message']); | |
| final errorMessage = data['errors']?['message'] ?? data['message']; | |
| loggy.error('AuthRepository: Registration failed - $errorMessage'); | |
| throw Exception(errorMessage); | |
| } | |
| Map<String, dynamic> data = {}; | |
| try { | |
| final decoded = jsonDecode(registerResponse.body); | |
| if (decoded is Map<String, dynamic>) data = decoded; | |
| } catch (_) { | |
| loggy.warning('AuthRepository: Non-JSON register response'); | |
| } | |
| if (registerResponse.statusCode != 200) { | |
| final errorMessage = | |
| (data['errors'] is Map ? data['errors']['message'] as String? : null) ?? | |
| (data['message'] as String?) ?? | |
| 'Registration failed. Please try again.'; | |
| loggy.error('AuthRepository: Registration failed - $errorMessage'); | |
| throw Exception(errorMessage); | |
| } |
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
82 to 88, the code calls json.decode(registerResponse.body) directly which can
throw if the body is not valid JSON; wrap the decode in a try/catch (or use a
safe parse) and fall back to using registerResponse.body (or a default map) when
parsing fails, then extract the error message defensively from the parsed map
using null-aware lookups (e.g. data?['errors']?['message'] ?? data?['message']
?? registerResponse.body ?? 'Registration failed') before logging and throwing
so invalid/non-JSON responses don't crash the app.
There was a problem hiding this comment.
@Tina-marion here as well
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
Improvements
Style