RU-T50 Trying to fix build issues

[ucswift]

Summary by CodeRabbit

• Chores

  • Updated push notification infrastructure to improve delivery and handling reliability.
  • Refactored notification initialization and registration flow for enhanced stability.
  • Removed legacy notification service dependencies and configuration.

• Tests

  • Updated push notification test suite to validate notification reception, registration, and event handling workflows.

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaces @react-native-firebase/messaging with expo-notifications as the push notification transport. The PushNotificationService is rewritten to use expo-notifications listeners, permission APIs, and getDevicePushTokenAsync. Build config, Metro web shims, Expo plugins, iOS entitlements, and Android build properties are updated accordingly, and the full test suite is rewritten to match.

Changes

Firebase → expo-notifications migration

Layer / File(s)	Summary
Dependency swap and build config package.json, app.config.ts, metro.config.js, src/types/declarations.d.ts	Removes @react-native-firebase/app and @react-native-firebase/messaging from dependencies, Metro web shims, Expo plugins, and the TypeScript ambient declaration; adds expo-notifications@0.32.16. iOS aps-environment: production entitlement is added, googleServicesFile is removed, Android useFrameworks/buildReactNativeFromSource build properties are dropped, and ./plugins/withForegroundNotifications.js is removed from the plugins list.
Service: handler wiring and initialization src/services/push-notification.ts	Updates imports and stored listener fields from FCM to expo-notifications. Adds setNotificationHandler for foreground presentation, a showModalForData helper gated on eventCode, and handleNotificationReceived/handleNotificationResponse methods. Rewrites initialize() to register expo-notifications received/response listeners, Notifee PRESS/ACTION_PRESS routing, and killed-state launch via getLastNotificationResponseAsync with a delayed modal display.
Service: registration and cleanup src/services/push-notification.ts	Replaces registerForPushNotifications from messaging().hasPermission/getToken to Notifications.getPermissionsAsync/requestPermissionsAsync (with iOS critical alerts) and getDevicePushTokenAsync. Updates cleanup() to remove expo-notifications listeners and Notifee foreground unsubscribe instead of FCM callbacks.
Test suite rewrite src/services/__tests__/push-notification.test.ts	Replaces all Firebase mocks with expo-notifications mocks (permissions, token, listener registration). Rebuilds notification payload helpers to produce expo-notifications-shaped objects. Rewrites received-handler (including eventCode gating, partial payloads, extra fields), listener lifecycle, registration permission flows, and cleanup assertions.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(173, 216, 230, 0.5)
    Note over Device,PushNotificationService: Initialization
    PushNotificationService->>ExpoNotifications: addNotificationReceivedListener
    PushNotificationService->>ExpoNotifications: addNotificationResponseReceivedListener
    PushNotificationService->>ExpoNotifications: getLastNotificationResponseAsync (killed state)
  end

  rect rgba(144, 238, 144, 0.5)
    Note over Device,ModalStore: Foreground notification received
    ExpoNotifications->>PushNotificationService: handleNotificationReceived(notification)
    PushNotificationService->>PushNotificationService: showModalForData (check eventCode)
    PushNotificationService->>ModalStore: showNotificationModal(title, body, data)
  end

  rect rgba(255, 222, 173, 0.5)
    Note over Device,ModalStore: User taps notification
    ExpoNotifications->>PushNotificationService: handleNotificationResponse(response)
    PushNotificationService->>PushNotificationService: setTimeout (mount delay)
    PushNotificationService->>ModalStore: showNotificationModal(title, body, data)
  end

  rect rgba(216, 191, 216, 0.5)
    Note over Device,Backend: Push token registration
    PushNotificationService->>ExpoNotifications: getPermissionsAsync / requestPermissionsAsync
    PushNotificationService->>ExpoNotifications: getDevicePushTokenAsync
    PushNotificationService->>Backend: registerDeviceForPushNotifications(token)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Resgrid/Unit#159: Modifies the same Notifications.requestPermissionsAsync iOS critical-alerts call in push-notification.ts that this PR also rewrites.
• Resgrid/Unit#176: Makes the reverse dependency/plugin changes in package.json and app.config.ts for the same Firebase modules and expo-notifications, directly overlapping this PR's removals.
• Resgrid/Unit#183: Refactors the same push-notification implementation and test suite by swapping notification delivery and permission/token flows between Expo Notifications and Firebase Messaging.

Suggested reviewers

• github-actions

🐇 No more Firebase to juggle and spin,
Expo Notifications let the pings come in!
APNs entitlement, production mode set,
FCM credentials? We won't miss 'em yet.
The token hops straight from device to backend gate —
Hooray for the cleaner push-notification state! 🔔

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'RU-T50 Trying to fix build issues' is vague and does not clearly convey the primary change, which is a migration from Firebase Cloud Messaging to expo-notifications.	Replace with a more specific title that describes the main change, such as 'Migrate push notifications from Firebase to expo-notifications' or 'Replace Firebase Cloud Messaging with expo-notifications'.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• 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]

🧹 Nitpick comments (1)

src/services/__tests__/push-notification.test.ts (1)

156-193: 💤 Low value

Consider using typed mocks instead of any.

The test helpers use any for the notification mock objects. Per coding guidelines, prefer precise types. You could define a minimal typed interface for the mock notification shape to improve type safety.

interface MockNotificationInput {
  title?: string;
  body?: string;
  data?: Record<string, unknown> | null;
}

interface MockNotification {
  request: {
    content: {
      title: string | null;
      body: string | null;
      data: Record<string, unknown>;
    };
  };
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/__tests__/push-notification.test.ts` around lines 156 - 193,
Replace the `any` type annotations in the test helper functions with properly
typed interfaces. Define two interfaces: one for the input data to
createMockNotification (with optional title, body, and data properties) and one
for the mock notification shape itself (with the nested request.content
structure). Update the createMockNotification function to accept the input
interface and return the notification interface type, and update
simulateNotificationReceived to accept the notification interface instead of
`any`. This improves type safety throughout the test file while maintaining the
same functionality.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/services/__tests__/push-notification.test.ts`:
- Around line 156-193: Replace the `any` type annotations in the test helper
functions with properly typed interfaces. Define two interfaces: one for the
input data to createMockNotification (with optional title, body, and data
properties) and one for the mock notification shape itself (with the nested
request.content structure). Update the createMockNotification function to accept
the input interface and return the notification interface type, and update
simulateNotificationReceived to accept the notification interface instead of
`any`. This improves type safety throughout the test file while maintaining the
same functionality.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 905353ed-c789-4a58-952c-80653f91147d

📥 Commits

Reviewing files that changed from the base of the PR and between ac129f0 and 7aac8fa.

⛔ Files ignored due to path filters (1)

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (6)

• app.config.ts
• metro.config.js
• package.json
• src/services/__tests__/push-notification.test.ts
• src/services/push-notification.ts
• src/types/declarations.d.ts

💤 Files with no reviewable changes (2)

• src/types/declarations.d.ts
• metro.config.js

[ucswift]

Approve

[github-actions]

This PR is approved.