RE1-T88 Trying to use FCM for APNS

[ucswift]

Summary by CodeRabbit

• Refactor
  • Updated push notification infrastructure to use Firebase messaging backend for device token management.
  • Removed test notification feature.

[coderabbitai]

Walkthrough

The push notification service was refactored to migrate from Expo-based notifications to Firebase Cloud Messaging. Expo notification handlers and listeners were disabled, and device token retrieval now uses Firebase's messaging().getToken() instead of Expo's token flow. Two internal methods were removed as part of this transition.

Changes

Cohort / File(s)	Summary
Push Notification Service Migration src/services/push-notification.ts	Disabled Expo notification handler initialization and event listeners; replaced Expo device token retrieval with Firebase messaging token via messaging().getToken(); removed sendTokenToBackend() and sendTestNotification() methods

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant PushNotificationService
    participant ExpoFlow as Expo (Old)
    participant FirebaseFlow as Firebase (New)

    rect rgb(220, 240, 255)
        Note over ExpoFlow: Previous Flow (Disabled)
        App->>PushNotificationService: initialize()
        PushNotificationService->>ExpoFlow: Configure handlers
        PushNotificationService->>ExpoFlow: Get device token
        ExpoFlow-->>PushNotificationService: token
        PushNotificationService->>PushNotificationService: sendTokenToBackend()
    end

    rect rgb(240, 255, 220)
        Note over FirebaseFlow: New Flow (Active)
        App->>PushNotificationService: initialize()
        PushNotificationService->>FirebaseFlow: messaging().getToken()
        FirebaseFlow-->>PushNotificationService: Firebase token
        Note over PushNotificationService: Direct token usage
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Focused scope: Single file modification with clear migration pattern
• Key areas requiring attention:
  • Verify Firebase messaging initialization is properly configured elsewhere in the codebase
  • Confirm that sendTokenToBackend() and sendTestNotification() are not invoked by other components
  • Validate that the removed Expo handler configuration doesn't affect other Expo-dependent features
  • Check that Firebase token is correctly transmitted to the backend through an alternative path

Poem

🐰 Firebase feathers replace Expo wings,
Notifications soar on cloud-born strings,
Token flows new, handlers rearranged,
Old methods fade—the service has changed!
Hop forward we go! 🚀

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly relates to the main change: migrating from Expo push notifications to Firebase Cloud Messaging (FCM) for Apple Push Notification service (APNS).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch develop

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

src/services/push-notification.ts (4)

128-166: Remove unused handler methods.

Both handleNotificationReceived and handleNotificationResponse are no longer used since the Expo notification listeners have been disabled. These methods are dead code and should be removed.

Apply this diff to remove the dead code:

-  private handleNotificationReceived = (notification: Notifications.Notification): void => {
-    const data = notification.request.content.data;
-
-    logger.info({
-      message: 'Notification received',
-      context: {
-        data,
-      },
-    });
-
-    // Check if the notification has an eventCode and show modal
-    // eventCode must be a string to be valid
-    if (data && data.eventCode && typeof data.eventCode === 'string') {
-      const notificationData = {
-        eventCode: data.eventCode as string,
-        title: notification.request.content.title || undefined,
-        body: notification.request.content.body || undefined,
-        data,
-      };
-
-      // Show the notification modal using the store
-      usePushNotificationModalStore.getState().showNotificationModal(notificationData);
-    }
-  };
-
-  private handleNotificationResponse = (response: Notifications.NotificationResponse): void => {
-    const data = response.notification.request.content.data;
-
-    logger.info({
-      message: 'Notification response received',
-      context: {
-        data,
-      },
-    });
-
-    // Here you can handle navigation or other actions based on notification data
-    // For example, if the notification contains a callId, you could navigate to that call
-    // This would typically involve using a navigation service or dispatching an action
-  };
-

---

240-255: Remove dead cleanup code for unused listeners.

Since notificationReceivedListener and notificationResponseListener are no longer being set (the registration is commented out), the cleanup logic for these listeners is unnecessary.

Apply this diff:

   public cleanup(): void {
-    if (this.notificationReceivedListener) {
-      this.notificationReceivedListener.remove();
-      this.notificationReceivedListener = null;
-    }
-
-    if (this.notificationResponseListener) {
-      this.notificationResponseListener.remove();
-      this.notificationResponseListener = null;
-    }
-
     if (this.fcmOnMessageUnsubscribe) {
       this.fcmOnMessageUnsubscribe();
       this.fcmOnMessageUnsubscribe = null;
     }
   }

You should also remove the corresponding member variable declarations at lines 25-26:

   private pushToken: string | null = null;
-  private notificationReceivedListener: { remove: () => void } | null = null;
-  private notificationResponseListener: { remove: () => void } | null = null;
   private fcmOnMessageUnsubscribe: (() => void) | null = null;

---

297-297: Runtime error: calling removed method.

This line calls sendTestNotification() which has been removed from the PushNotificationService class. This will cause a runtime error when the hook is used and the method is invoked.

Apply this diff to remove the broken reference:

   return {
     pushToken: pushNotificationService.getPushToken(),
-    sendTestNotification: () => pushNotificationService.sendTestNotification(),
   };

---

36-78: Replace Expo notification APIs with Notifee for channel management and FCM compatibility.

The verification confirms the concern: the code uses Expo's setNotificationChannelAsync (lines 36–78) and Notifications.getPermissionsAsync/requestPermissionsAsync (lines 176–198) while migrating to Firebase Cloud Messaging. RNFirebase does not manage channels—use Notifee or native code to create channels.

Notifee is already imported but unused for channel creation. Migrate channel setup to Notifee's createChannel API. Additionally, Android 13+ (API 33) requires the runtime POST_NOTIFICATIONS permission, which may require native PermissionsAndroid instead of Expo APIs for proper FCM integration.

Locations requiring fixes:

• Lines 36–78: Replace Notifications.setNotificationChannelAsync with Notifee channel creation
• Lines 176–198: Review and potentially replace Expo permission handling with native Android permission requests for POST_NOTIFICATIONS

🧹 Nitpick comments (3)

src/services/push-notification.ts (3)

85-93: Remove commented code.

Commented-out code should be removed rather than left in the codebase. If this configuration is no longer needed with FCM, delete it. If you're uncertain about removing it, consider using version control history to recover it later if needed.

Apply this diff to remove the commented code:

-    // Configure notifications behavior
-    //Notifications.setNotificationHandler({
-    //  handleNotification: async () => ({
-    //    shouldShowAlert: true,
-    //    shouldPlaySound: true,
-    //    shouldSetBadge: false,
-    //    shouldShowBanner: true,
-    //    shouldShowList: true,
-    //  }),
-    //});
-

---

96-97: Remove commented code.

These commented listener registrations should be removed. The corresponding handler methods (handleNotificationReceived and handleNotificationResponse) are now unused and should also be removed.

Apply this diff:

-    // Set up notification listeners and store the subscription handles
-    //this.notificationReceivedListener = Notifications.addNotificationReceivedListener(this.handleNotificationReceived);
-    //this.notificationResponseListener = Notifications.addNotificationResponseReceivedListener(this.handleNotificationResponse);
-

---

201-206: Remove commented code and clean up the implementation.

The migration to FCM token retrieval looks correct, but the commented-out Expo code should be removed.

Apply this diff:

-      // Get the token using the non-Expo push notification service method
-      //const devicePushToken = await Notifications.getDevicePushTokenAsync();
-
-      // The token format depends on the platform
-      //const token = Platform.OS === 'ios' ? devicePushToken.data : devicePushToken.data;
-
       const token = await messaging().getToken();

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a345ff5 and 9511ac7.

📒 Files selected for processing (1)

• src/services/push-notification.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests

**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests

Files:

• src/services/push-notification.ts

**/*

📄 CodeRabbit inference engine (.cursorrules)

Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)

Files:

• src/services/push-notification.ts

⏰ 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: test

[ucswift]

Approve

[github-actions]

This PR is approved.