-
Notifications
You must be signed in to change notification settings - Fork 16
feat: implement FCM service with background integration (Phase 2) #394
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
base: main
Are you sure you want to change the base?
Conversation
Add FCMService to handle Firebase Cloud Messaging integration following MIP-05 approach: - Implement background message handler to wake app when notifications arrive - Add FCM token management with automatic refresh handling - Set up foreground and background message listeners - Integrate with existing flutter_background_service for event processing - Add platform checks to skip Firebase on unsupported platforms (Linux, web) - Include
WalkthroughFirebase Cloud Messaging integration was added: a new FCMService with background handler, token lifecycle, permission and listener wiring; main.dart now conditionally initializes FCM at startup; a Riverpod provider exposes the service; documentation marks Phase 2 complete. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (main)
participant Init as _initializeFirebaseMessaging
participant Provider as Riverpod Provider
participant FCM as FCMService
participant Firebase as Firebase/FCM
participant Prefs as SharedPreferences
participant BGService as Background Service
App->>Init: start (skip on Linux)
Init->>Provider: construct FCMService(prefs)
Provider->>FCM: call initialize()
FCM->>Firebase: Firebase.initializeApp()
FCM->>Firebase: requestNotificationPermissions()
FCM->>Firebase: getToken()
Firebase-->>FCM: token
FCM->>Prefs: store token & flags
FCM->>Firebase: setOnTokenRefreshListener()
FCM->>Firebase: setOnMessageListener(foreground)
FCM->>Firebase: register onBackgroundMessage(firebaseMessagingBackgroundHandler)
rect rgb(230, 245, 230)
Note over Firebase,BGService: Background message flow
Firebase->>BGService: deliver background RemoteMessage
BGService->>Prefs: record wake timestamp / set pending fetch
BGService->>BGService: mark pending fetch if service inactive
end
FCM-->>App: initialization complete (or logged error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
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. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/services/fcm_service.dart (2)
190-216: Empty conditional block creates dead code.Lines 204-207 contain an empty
if (isRunning)block that does nothing. If this is intentional (relying on the background service's existing subscription mechanism), remove the block and add a comment. If future action is planned, add a TODO comment.🔎 Suggested clarification
if (isRunning) { // The background service will pick up new events through its // existing subscription mechanism + // No action needed here - background service handles fetching }Or remove the empty block entirely:
if (isRunning) { - // The background service will pick up new events through its - // existing subscription mechanism } + // Background service handles fetching via its existing subscription mechanism
173-188: Token refresh listener is properly implemented.Canceling existing subscription before setup prevents duplicate listeners. The TODO for Phase 3 encrypted token registration is appropriately documented.
The TODO at line 181 indicates Phase 3 work. Do you want me to open an issue to track this?
lib/main.dart (2)
43-44: FCM initialization placement is correct, but instance is not managed.The initialization timing after background service is appropriate. However, the
FCMServiceinstance created in_initializeFirebaseMessagingis discarded after initialization. Consider either:
- Using
fcmServiceProviderwith an override in theProviderContainerfor lifecycle management- Storing the instance for proper disposal on app termination
Currently, the service's stream subscriptions won't be disposed when the app terminates.
🔎 Suggested approach using provider override
+ FCMService? fcmService; // Initialize FCM (skip on Linux) - await _initializeFirebaseMessaging(sharedPreferences); + if (!kIsWeb && !Platform.isLinux) { + fcmService = FCMService(sharedPreferences); + await fcmService.initialize(); + } final container = ProviderContainer( overrides: [ settingsProvider.overrideWith((b) => settings), backgroundServiceProvider.overrideWithValue(backgroundService), + if (fcmService != null) fcmServiceProvider.overrideWithValue(fcmService), // ... other overrides ], );
92-108: Duplicate platform check and orphaned service instance.Two concerns:
Duplicate platform check: Lines 96-100 duplicate the check already in
FCMService.initialize()(line 81). While defensive, this adds maintenance burden.Orphaned instance: The
FCMServicecreated at line 102 goes out of scope after initialization, meaningdispose()is never called and stream subscriptions leak.Consider removing the outer platform check (let FCMService handle it) and returning the instance for lifecycle management.
🔎 Suggested simplification
-/// Initialize Firebase Cloud Messaging -Future<void> _initializeFirebaseMessaging(SharedPreferencesAsync prefs) async { +/// Initialize Firebase Cloud Messaging and return the service for lifecycle management +Future<FCMService?> _initializeFirebaseMessaging(SharedPreferencesAsync prefs) async { try { - // Skip Firebase initialization on Linux (not supported) - if (!kIsWeb && Platform.isLinux) { - debugPrint( - 'Firebase not supported on Linux - skipping FCM initialization'); - return; - } - final fcmService = FCMService(prefs); await fcmService.initialize(); - debugPrint('Firebase Cloud Messaging initialized successfully'); + if (fcmService.isInitialized) { + debugPrint('Firebase Cloud Messaging initialized successfully'); + return fcmService; + } + return null; } catch (e) { - // Log error but don't crash app if FCM initialization fails debugPrint('Failed to initialize Firebase Cloud Messaging: $e'); + return null; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/FCM_IMPLEMENTATION.mdlib/main.dartlib/services/fcm_service.dartlib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dart
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/shared/providers/providers.dartlib/shared/providers/fcm_service_provider.dartlib/services/fcm_service.dartlib/main.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/shared/providers/providers.dartlib/shared/providers/fcm_service_provider.dartlib/services/fcm_service.dartlib/main.dart
lib/shared/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing feature patterns when adding new shared utilities - refer to order, chat, and auth features as implementation examples
Files:
lib/shared/providers/providers.dartlib/shared/providers/fcm_service_provider.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/fcm_service.dart
lib/main.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')
Files:
lib/main.dart
🧠 Learnings (18)
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/shared/providers/providers.dartlib/shared/providers/fcm_service_provider.dartlib/main.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/shared/providers/providers.dartlib/shared/providers/fcm_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/**/*.dart : Follow existing feature patterns when adding new shared utilities - refer to order, chat, and auth features as implementation examples
Applied to files:
lib/shared/providers/providers.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Use dual storage strategy: store Mostro/default relays in `settings.relays` and user relays in `settings.userRelays` with full JSON metadata via `toJson()`/`fromJson()`
Applied to files:
lib/shared/providers/providers.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/shared/providers/providers.dartlib/shared/providers/fcm_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Name Riverpod providers using the `<Feature>Provider` or `<Feature>Notifier` convention
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.081Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.081Z
Learning: Run `flutter test integration_test/` only for significant changes affecting core services and main user flows
Applied to files:
lib/services/fcm_service.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Persistence, APIs, and background jobs should live in `lib/data/` and `lib/background/`; generated localization output must be in `lib/generated/` and must stay untouched
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Always check `mounted` before using BuildContext after async operations to prevent errors on disposed widgets
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Use `S.of(context)!.yourKey` for all user-facing strings instead of hardcoded text
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
Applied to files:
lib/main.dart
📚 Learning: 2025-08-15T01:37:12.243Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.243Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/data/repositories/**/*.dart : Implement Repository pattern for all data access - all data operations must go through repository classes
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/main.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (12)
lib/services/fcm_service.dart (7)
1-11: LGTM - Imports and logger setup are appropriate.The imports cover all necessary dependencies for Firebase messaging, background service integration, logging, and shared preferences. Logger instance at module level is reasonable for a service file.
20-58: Background handler implementation is well-structured.The handler correctly:
- Uses
@pragma('vm:entry-point')for AOT compilation- Skips unsupported platforms
- Initializes Firebase in the isolated background context
- Sets a pending fetch flag when background service isn't running
One observation: when
isRunningis true (line 47), no action is taken. This appears intentional per the MIP-05 design (existing service handles it), but consider adding a brief comment clarifying this is expected behavior.
76-121: Initialization flow is correct with proper error handling.The sequential setup (Firebase → permissions → token → listeners → background handler) follows FCM best practices. Error handling allows graceful degradation without crashing the app.
Note:
Firebase.initializeApp()may be called multiple times (here and infirebaseMessagingBackgroundHandler), but Firebase SDK safely handles duplicate initialization.
123-146: Permission request implementation is solid.Appropriate permission options requested, and handling both
authorizedandprovisionalstatus as granted is correct iOS behavior. Error handling returnsfalsesafely.
148-171: Token retrieval with timeout is well-implemented.The 10-second timeout prevents indefinite hangs, and the fallback to
nullallows graceful degradation when token retrieval fails.
246-264: Robust token deletion with fallback cleanup.The nested try-catch ensures local storage is cleaned up even if the Firebase token deletion fails. This prevents orphaned tokens in storage.
266-272: Dispose method properly cleans up resources.Stream subscriptions are canceled and nulled, and
_isInitializedis reset, allowing potential re-initialization if needed.lib/shared/providers/providers.dart (1)
1-3: LGTM - Export follows existing barrel file pattern.The new FCM service provider export aligns with the existing pattern of centralizing provider exports.
docs/FCM_IMPLEMENTATION.md (2)
107-107: Documentation accurately reflects Phase 2 completion.Status update to "✅ COMPLETE" is consistent with the implementation delivered in this PR.
241-243: Phase 2 section appropriately updated.Branch name and status changes align with the PR completing Phase 2 FCM service implementation.
lib/main.dart (1)
1-2: Imports are appropriate for the new FCM functionality.
dart:ioforPlatformchecks andflutter/foundation.dartforkIsWebare correctly added alongside the FCM service import.Also applies to: 13-13
lib/shared/providers/fcm_service_provider.dart (1)
1-8: Remove unused provider or refactor to use it consistently.This provider is exported in
lib/shared/providers/providers.dartbut is never actually used anywhere in the codebase. Meanwhile,main.dartcreates and initializesFCMServicedirectly instead of through the provider. Either removefcmServiceProviderif it's not needed, or refactormain.dartand any components that need FCM to use the provider instead of creating instances directly. If kept, consider using aFutureProviderto handle initialization through the provider system, which aligns with the coding guideline that Riverpod should be used for all state management.Likely an incorrect or invalid review comment.
…gging - Add check to prevent duplicate Firebase initialization - Replace logger with debugPrint for better visibility during initialization - Change _messaging from final field to getter for lazy initialization - Add empty stub files for platform-specific Firebase options - Improve initialization flow logging with step-by-step debug messages
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/services/fcm_service.dart (4)
78-86: Consider setting_isInitializedwhen skipping on Linux.When skipping initialization on Linux,
_isInitializedremainsfalse. This could cause:
- Repeated log output if
initialize()is called multiple times- Callers checking
isInitializedwill seefalseindefinitelyIf the intent is to indicate FCM is unavailable on Linux, consider adding a separate
isSupportedflag. Otherwise, set_isInitialized = truebefore returning to prevent redundant calls.🔎 Proposed fix
if (!kIsWeb && Platform.isLinux) { debugPrint('FCM: Skipping initialization on Linux (not supported)'); + _isInitialized = true; // Mark as "initialized" (no-op on Linux) return; }
182-197: Token refresh listener properly manages subscription lifecycle.Good practice canceling the existing subscription before creating a new one. The TODO for Phase 3 is noted.
Would you like me to open an issue to track the Phase 3 implementation of sending updated encrypted tokens to the notification server?
199-225: Emptyifblock is dead code.Lines 213-216 check
isRunningbut take no action inside the block—the comment explains the expected behavior, but the code does nothing. Consider either removing the empty conditional or adding an explicit service invocation if one is needed.🔎 Proposed simplification
_foregroundMessageSubscription = FirebaseMessaging.onMessage.listen( (RemoteMessage message) async { - // Silent notification received while app is in foreground - // The existing background service is already running and will - // handle fetching and displaying notifications - - // Optionally trigger an immediate refresh - try { - final service = FlutterBackgroundService(); - final isRunning = await service.isRunning(); - - if (isRunning) { - // The background service will pick up new events through its - // existing subscription mechanism - } - } catch (e) { - _logger.e('Error triggering background service: $e'); - } + // Silent notification received while app is in foreground. + // The existing background service handles fetching and displaying + // notifications through its existing subscription mechanism. + debugPrint('FCM: Foreground message received'); }, onError: (error) { _logger.e('Error receiving foreground message: $error'); }, );
11-11: Inconsistent logging approach:debugPrintvs_logger.The file mixes
debugPrintfor initialization steps and_loggerfor errors/warnings. While the commit message indicates this is intentional for "clearer initialization output," consider unifying the approach for consistency. You could use_logger.d()for debug logs to keep all logging through one mechanism.Also applies to: 53-58, 126-127
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/firebase_options_platform.dartlib/firebase_options_stub.dartlib/services/fcm_service.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/services/fcm_service.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/services/fcm_service.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/fcm_service.dart
🧠 Learnings (3)
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Applied to files:
lib/services/fcm_service.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
lib/services/fcm_service.dart (9)
1-11: Imports and logger setup look good.The imports are appropriate for FCM integration, and the module-level logger follows common Dart patterns.
20-60: Background handler implementation aligns with architecture.The handler correctly:
- Uses
@pragma('vm:entry-point')for background isolate entry- Guards against unsupported platforms
- Records wake timestamp for debugging
- Sets pending fetch flag when background service isn't running
The design where no explicit action is taken when
isRunningis true aligns with the existing architecture—the background service handles fetching and processing events through its existing subscription mechanism. Based on learnings, this separation of concerns is intentional.
62-77: Good use of dependency injection and lazy initialization.The lazy getter for
_messagingand constructor injection of_prefsenable proper testing and align with the Riverpod provider pattern described in the PR.
132-155: Permission handling looks correct.The permission request uses appropriate flags for push notifications, and the authorization status check properly handles both
authorizedandprovisionalstatuses.
157-180: Token retrieval with timeout is well implemented.Good defensive coding with the timeout wrapper and graceful handling when token is null.
227-237: Pending fetch check correctly clears the flag.The background service will handle the actual fetching when it starts, aligning with the separation of concerns in the architecture.
239-253: Token retrieval with storage-first approach is efficient.Good caching pattern that avoids unnecessary Firebase calls.
255-273: Robust token deletion with fallback cleanup.Good resilience pattern ensuring local storage is cleaned up even if the Firebase deletion fails.
275-282: Proper resource cleanup in dispose.Good practice canceling subscriptions and nullifying references. Resetting
_isInitializedenables potential re-initialization if needed.
Catrya
left a comment
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.
tACK
Description
Implements Phase 2 of FCM integration - complete Firebase Cloud Messaging service with token management, permissions, and background message handling following MIP-05 privacy approach.
Changes
SharedPreferencesAsyncflutter_background_serviceFirebase.apps.isEmptycheck to prevent double initializationFirebaseMessaging.instanceHow to Test
1. View FCM Initialization Logs
Run the app and monitor logs:
Expected output:
2. Token Generation
Verify token is generated and stored:
Expected: Token string present in preferences
3. Permissions
FCM: Notification permissions grantedFCM: Notification permissions not granted4. Background Handler
Test background message handling:
Expected: No crashes, background service triggered
5. Linux Compatibility
Expected log:
App should launch normally without Firebase errors.
Success Criteria
✅ App launches without errors
✅ FCM initialization logs appear in correct order
✅ Token generated and stored in SharedPreferences
✅ Permissions requested on first launch
✅ App continues if permissions denied
✅ No crashes on background messages
✅ Linux compatibility maintained (Firebase skipped)
✅ No double initialization errors
Notes
flutter_background_service(no duplication)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.