Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements an "offline first" architecture for the Mosquito Alert mobile application, fundamentally changing how the app handles data synchronization and storage.
Key Changes:
- Implements local-first data storage using Hive database with automatic background synchronization
- Removes direct API dependencies from providers, introducing repository pattern with offline support
- Adds outbox pattern for queuing operations when offline and syncing when connection is restored
Reviewed changes
Copilot reviewed 184 out of 205 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
lib/main.dart |
Initializes Hive database, outbox service, repositories, and auto-sync on connection changes |
lib/services/api_service.dart |
Simplified to remove auth provider dependency, using static auth repository methods |
lib/hive/hive_service.dart |
New service to initialize Hive database and register type adapters |
lib/hive/hive_adapters.g.dart |
Generated Hive type adapters for offline storage models |
lib/hive/hive_registrar.g.dart |
Generated Hive adapter registration code |
lib/features/user/data/user_repository.dart |
New repository with Hive caching and stream-based user data |
lib/utils/PushNotificationsManager.dart |
Deleted - push notification logic moved to features |
lib/utils/BackgroundTracking.dart |
Deleted - tracking logic moved to features/fixes |
lib/providers/* |
Deleted - replaced with feature-based repositories and providers |
lib/screens/* |
New screen implementations replacing old page structure |
lib/main.dart
Outdated
| final apiConnection = InternetConnection.createInstance( | ||
| customCheckOptions: [ | ||
| // TODO: use /ping endpoint. | ||
| InternetCheckOption(uri: Uri.parse(apiClient.dio.options.baseUrl)), |
There was a problem hiding this comment.
The TODO comment indicates the current approach of using the base URL for connectivity checks may not be ideal. The base URL may not be optimized for quick connectivity checks and could cause unnecessary load. Consider implementing a dedicated /ping endpoint on the backend and updating this code to use it for more efficient connectivity detection.
| final apiConnection = InternetConnection.createInstance( | |
| customCheckOptions: [ | |
| // TODO: use /ping endpoint. | |
| InternetCheckOption(uri: Uri.parse(apiClient.dio.options.baseUrl)), | |
| final String _normalizedBaseUrl = | |
| apiClient.dio.options.baseUrl.replaceAll(RegExp(r'/+$'), ''); | |
| final Uri _pingUri = Uri.parse('$_normalizedBaseUrl/ping'); | |
| final apiConnection = InternetConnection.createInstance( | |
| customCheckOptions: [ | |
| InternetCheckOption(uri: _pingUri), |
| getAccessToken: () async => await AuthRepository.getAccessToken() ?? '', | ||
| getRefreshToken: () async => | ||
| await AuthRepository.getRefreshToken() ?? '', |
There was a problem hiding this comment.
The await keyword is redundant when immediately returning a Future. The async arrow function already returns a Future, so you can remove await and just return the Future directly: getAccessToken: () => AuthRepository.getAccessToken() ?? ''
| Text( | ||
| 'ID: ', | ||
| style: TextStyle( | ||
| color: Colors.black.withValues(alpha: 0.7), |
There was a problem hiding this comment.
Using the newer withValues API for color opacity is correct for recent Flutter versions. However, ensure the minimum Flutter SDK version in pubspec.yaml supports this API (Flutter 3.27+). If targeting older versions, use the deprecated withOpacity method for compatibility.
| color: Colors.black.withValues(alpha: 0.7), | |
| color: Colors.black.withOpacity(0.7), |
| unawaited( | ||
| Future(() async { | ||
| await notificationProvider.refresh(); | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Wrapping an async call in Future(() async { ... }) is unnecessary. You can directly call notificationProvider.refresh() with unawaited. Simplify to: unawaited(notificationProvider.refresh());
| unawaited( | |
| Future(() async { | |
| await notificationProvider.refresh(); | |
| }), | |
| ); | |
| unawaited(notificationProvider.refresh()); |
lib/main.dart
Outdated
| return; | ||
| } | ||
| } | ||
| await syncManager.syncAll(); |
There was a problem hiding this comment.
The stream listener for connection status changes lacks error handling for the syncAll() call. If synchronization fails, the error is not caught or logged. Consider wrapping syncManager.syncAll() in a try-catch block to handle synchronization failures gracefully and log appropriate errors.
| await syncManager.syncAll(); | |
| try { | |
| await syncManager.syncAll(); | |
| } catch (e, stackTrace) { | |
| print('Error during automatic synchronization: $e'); | |
| print(stackTrace); | |
| } |
|
See the two pull requests I have made to fix minor issues I encountered on this branch when testing on iPhone. In addition:
|
…tdata Fix ParentDataWidget error in photo selection
…flow Fix overflow on location consent page
|
Hey John, I’ve slightly adapted the PR you opened and have already merged those changes into this branch.
I can also see 3 adult mosquito reports on the dev server from an iOS device last night (I’m guessing that was you)
|
No description provided.