Skip to content

fix(notifications): handle FCM background/terminated notification tap navigation#5127

Open
mdmohsin7 wants to merge 2 commits intomainfrom
fix/notification-tap-navigation
Open

fix(notifications): handle FCM background/terminated notification tap navigation#5127
mdmohsin7 wants to merge 2 commits intomainfrom
fix/notification-tap-navigation

Conversation

@mdmohsin7
Copy link
Member

Problem

Fixes #5126

When a user taps an Omi response push notification:

  • From background state: app opens to the last screen instead of the conversation
  • From terminated state: same issue — app opens but doesn't navigate

Root Cause

The Flutter app was missing two FCM handlers in notification_service_fcm.dart:

  1. FirebaseMessaging.onMessageOpenedApp — fires when user taps a notification while app is in the background
  2. FirebaseMessaging.instance.getInitialMessage() — fires when app is launched from a killed state via notification tap

The backend already sends navigate_to (e.g. /chat/omi) in the FCM data payload — the app just never read it on tap.

Changes

  • notification_service_fcm.dart: Added onMessageOpenedApp listener and getInitialMessage() call in listenForMessages()
  • notifications.dart: Exposed NotificationUtil.handleNavigateTo(String route) as a public static method (wraps existing _handleAppLinkOrDeepLink)

Testing

  1. Send yourself a chat message via the Omi app
  2. Put app in background → tap the notification → should navigate to the conversation
  3. Kill the app → tap the notification → app should open directly to the conversation

… navigation

- Add FirebaseMessaging.onMessageOpenedApp listener to navigate when user
  taps a notification while app is in the background
- Add FirebaseMessaging.getInitialMessage() call to handle notification taps
  that launch the app from terminated state
- Expose NotificationUtil.handleNavigateTo() as a public static method to
  support programmatic deep-link navigation from FCM handlers

Fixes: tapping Omi response notifications opens app to last screen instead
of navigating to the relevant conversation

Closes #5126
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 correctly addresses an issue with notification tap navigation from background and terminated states by adding the necessary FCM handlers. The changes are logical and follow the PR's objective. I've added one suggestion to refactor the new handlers to reduce code duplication and improve type safety, aligning with the preference for helper functions to centralize complex logic, which will enhance the code's maintainability and robustness.

…plicate logic

Addresses Gemini review: consolidates duplicated background/terminated
notification tap handling into a single local helper and uses `is String`
type check instead of `.toString()` for safer payload handling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
beastoin
beastoin previously approved these changes Mar 10, 2026
Copy link
Collaborator

@beastoin beastoin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by @ryo

Clean fix. Small change (+20/-0), correctly handles both FCM notification tap paths:

  • BackgroundonMessageOpenedApp.listen() — fires when app is backgrounded and user taps notification
  • TerminatedgetInitialMessage() — fires when app was killed and reopened via notification

The public handleNavigateTo() method properly delegates to the existing _handleAppLinkOrDeepLink() navigation system. No side effects on existing notification flows.

Recommendation: Safe to merge.

@beastoin beastoin dismissed their stale review March 10, 2026 01:50

Withdrawing review — re-reviewing after internal discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Tapping Omi response notifications doesn't navigate to the correct conversation

2 participants