Skip to content

Conversation

@diegolmello
Copy link
Member

@diegolmello diegolmello commented Dec 4, 2025

Proposed changes

Issue(s)

https://rocketchat.atlassian.net/browse/SUP-859

How to test or reproduce

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

  • Chores

    • Updated notification library to a newer compatible release and aligned platform notification behavior.
  • New Features

    • Startup now detects and prioritizes video-conference notifications to avoid unintended deep-linking.
    • Initialization signals readiness earlier in certain login flows.
    • Faster, prioritized handling for video-conference notifications on iOS.
  • Bug Fixes

    • More reliable background notification processing and persistence.
    • Improved Android back-button handling when launching external call flows.
    • Refined push-open handling and action detection for consistent notification behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@diegolmello diegolmello temporarily deployed to approve_e2e_testing December 4, 2025 18:28 — with GitHub Actions Inactive
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

Updates Notifee dependency and patches; adds videoconf-aware startup early-return; restricts and persists background videoconf handling; adjusts notification dispatch and opened-handler logic; modifies init/deep-link sagas; manages BackHandler lifecycle in Jitsi view; adds immediate videoconf handling in iOS NotificationService and new NotificationType case.

Changes

Cohort / File(s) Summary
Dependency
package.json
Bump @notifee/react-native from 7.8.2 to ^9.1.8.
Notifee patches
patches/@notifee+react-native+7.8.2.patch, patches/@notifee+react-native+9.1.8.patch
Android: use literal API level 33 and literal POST_NOTIFICATIONS permission string in runtime checks. iOS: comment out fallback re-parsing of UNNotificationRequest when notifeeNotification is nil (keep _notificationOpenedAppID assignment).
Initial notification / video conference
app/index.tsx, app/lib/notifications/videoConf/getInitialNotification.ts
getInitialNotification now returns Promise<boolean>; index.tsx calls it and returns early when a videoconf initial notification was handled to skip deep-link initialization.
Background videoconf handling
app/lib/notifications/videoConf/backgroundNotificationHandler.ts
Limit processing to event types 1 or 2; await AsyncStorage.setItem before cancelling the notification.
Notifications entry & push handling
app/lib/notifications/index.ts, app/lib/notifications/push.ts
Extend IEjson with optional notificationType?: string; iOS: skip videoconf pushes early in onNotification; registerNotificationOpened accepts optional actionResponse and attaches actions to payload; handle explicit ACCEPT/DECLINE actions and ensure completion() placement.
Init / deep-linking sagas
app/sagas/init.js, app/sagas/deepLinking.js
init: remove stored notification then put parsed push notification (replace direct call). deepLinking: dispatch appReady({}) at additional points after LOGIN.SUCCESS to proceed with navigation.
Jitsi view back-handler
app/views/JitsiMeetView/index.tsx
Add backHandlerSubscription ref to manage BackHandler listener on Jitsi-app-fail path; ensure removal on cleanup; add useRef and NativeEventSubscription usage.
iOS Notification Service & model
ios/NotificationService/NotificationService.swift, ios/Shared/Models/NotificationType.swift
On ejson decode failure return original content; detect videoconf payloads and immediately set categoryIdentifier = "VIDEOCONF" then deliver; add videoconf case to NotificationType enum.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review Notifee platform patches for API/permission literal usage and iOS fallback removal.
  • Verify getInitialNotification boolean contract and early-return integration in index.tsx.
  • Check saga changes: replacing call with put for push handling and added appReady({}) dispatch points.
  • Confirm AsyncStorage awaits and cancellation ordering in background handler.
  • Validate BackHandler subscription lifecycle in Jitsi view.
  • Inspect iOS NotificationService immediate videoconf handling and enum addition.

Suggested reviewers

  • OtavioStasiak

Poem

🐰
I hopped through code at break of dawn,
Bumped Notifee, then carried on.
I guarded events and stored with care,
Cleared back-handles from the air,
Now calls may ring — I did my share! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: upgrading @notifee/react-native from 7.8.2 to 9.1.8, which is the primary dependency change in package.json.
Linked Issues check ✅ Passed The PR addresses SUP-859 by implementing video conference notification handling with proper ringing behavior on locked devices through patches, initialization logic, and iOS category identifiers.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing Jitsi call notifications on locked devices: @notifee upgrade, video conf notification handling, initialization flow, and background notification processing.
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 chore.notifee-new-arch

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

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

@diegolmello diegolmello deployed to official_ios_build December 4, 2025 18:32 — with GitHub Actions Active
@diegolmello diegolmello temporarily deployed to experimental_android_build December 4, 2025 18:32 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_ios_build December 4, 2025 18:32 — with GitHub Actions Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1bd62a2 and 1c8bd43.

⛔ Files ignored due to path filters (2)
  • ios/Podfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • package.json (1 hunks)
  • patches/@notifee+react-native+7.8.2.patch (0 hunks)
  • patches/@notifee+react-native+9.1.8.patch (1 hunks)
💤 Files with no reviewable changes (1)
  • patches/@notifee+react-native+7.8.2.patch
⏰ 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). (2)
  • GitHub Check: ESLint and Test / run-eslint-and-test
  • GitHub Check: format
🔇 Additional comments (1)
patches/@notifee+react-native+9.1.8.patch (1)

9-30: This is an intentional architectural change, not a breaking bug.

The commented-out code handles fallback for non-Notifee notifications, but analysis of the codebase reveals this is a deliberate design decision coordinated with the related Firebase messaging patch (@react-native-firebase+messaging+21.12.2.patch). That patch disables the ReactNativeFirebaseMessagingService on Android—effectively removing Firebase Cloud Messaging as a primary notification handler across both platforms.

The app's notification architecture uses react-native-notifications as the primary handler, with Notifee specifically for video conference notifications. The patches together represent a migration away from Firebase messaging to this new stack. Removing the fallback to non-Notifee handlers prevents conflicts and duplicate notification processing.

The change is safe for this codebase's use case.

"@expo/vector-icons": "^14.1.0",
"@hookform/resolvers": "^2.9.10",
"@notifee/react-native": "7.8.2",
"@notifee/react-native": "^9.1.8",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

@notifee/react-native v9 v8 breaking changes changelog release notes

💡 Result:

Here are the main breaking / upgrade notes when moving from v8 → v9 of @notifee/react-native:

  • Minimum Android compileSdk requirement increased to 34 (you must build with compileSdk 34+). [1]
  • Notifee removed uses of Google Play Services (GMS) so it no longer depends on GMS libs (improves compatibility with F‑Droid / open distributions). [1]
  • Android FOREGROUND_SERVICE changes:
    • USE_FULL_SCREEN_INTENT permission was removed from the package manifest (apps that need it must now add it themselves). [1]
    • Foreground service types support changed — you must add required foregroundServiceTypes in your AndroidManifest and may need to replace the service definition; new API parameter available to set types dynamically. [1]
  • New Android 15 FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING flag added (if relevant). [1]
  • v8.0.0 had an alpha publish and the project moved versioning to v9 (note about version skip). [1]
  • Misc: several Android/iOS bugfixes and compatibility fixes for RN new‑architecture; some API/type tweaks (see changelog for full list). [1][2]

Sources:

  1. Notifee release notes (v9.x changelog). [docs.page / Notifee release notes]. [1]
  2. Notifee GitHub issues / releases (examples of v9.x fixes and RN compatibility notes). [2]

If you want, I can copy the exact changelog entries for v9.x (v9.0.0 → latest) or produce a migration checklist tailored to your app (AndroidManifest, Gradle, RN version, Expo), which files to edit, and sample manifest snippets.


🏁 Script executed:

#!/bin/bash
# Search for notification-related tests and @notifee imports
echo "=== Test files mentioning notification/notifee ==="
fd \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.test.js" -o -name "*.test.jsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" -o -name "*.spec.js" \) | head -20

echo -e "\n=== Searching for @notifee imports in test files ==="
rg "@notifee" --type ts --type tsx --type js --type jsx -l | head -20

echo -e "\n=== Searching for notification-related tests ==="
rg -i "notif" --type ts --type tsx --type js --type jsx -l | head -20

echo -e "\n=== Check package.json location and context ==="
cat -n package.json | grep -A 5 -B 5 "@notifee"

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1150


🏁 Script executed:

#!/bin/bash
echo "=== Checking AndroidManifest.xml for FOREGROUND_SERVICE configuration ==="
fd -name "AndroidManifest.xml" -exec cat {} \; 2>/dev/null | head -100

echo -e "\n=== Checking build.gradle for compileSdk version ==="
fd -name "build.gradle" | head -5 | xargs cat 2>/dev/null | grep -i "compileSdk\|targetSdk\|minSdk" | head -20

echo -e "\n=== Checking if GMS dependencies are still present ==="
rg "google.*play.*services|gms" package.json gradle.properties

echo -e "\n=== Checking for notifee initialization/usage in codebase ==="
rg "@notifee" --type js --type jsx -A 2 -B 2 | head -50

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 658


🏁 Script executed:

#!/bin/bash
echo "=== Finding all directories and key Android files ==="
find . -maxdepth 3 -type d -name "android" 2>/dev/null

echo -e "\n=== Locating AndroidManifest.xml ==="
find . -name "AndroidManifest.xml" 2>/dev/null

echo -e "\n=== Locating build.gradle files ==="
find . -name "build.gradle" 2>/dev/null | head -10

echo -e "\n=== Checking for notifee imports in entire codebase ==="
rg "notifee|@notifee" -i | head -30

echo -e "\n=== Checking directory structure ==="
ls -la | head -20

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 4616


🏁 Script executed:

#!/bin/bash
echo "=== Main AndroidManifest.xml ==="
cat ./android/app/src/main/AndroidManifest.xml

echo -e "\n=== build.gradle (app level) ==="
cat ./android/app/build.gradle | head -100

echo -e "\n=== Check root build.gradle ==="
cat ./android/build.gradle | head -50

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 10422


🏁 Script executed:

#!/bin/bash
echo "=== Check backgroundNotificationHandler for foreground service usage ==="
cat ./app/lib/notifications/videoConf/backgroundNotificationHandler.ts

echo -e "\n=== Search for foreground service references ==="
rg -i "foreground|FOREGROUND" --type ts --type tsx | head -30

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 3828


Add FOREGROUND_SERVICE configuration to AndroidManifest.xml and verify manifest compatibility with v9 breaking changes.

This major version upgrade (7.8.2 → ^9.1.8) includes breaking changes requiring Android manifest updates:

  • Foreground service types: v9 requires foregroundServiceTypes attribute in AndroidManifest.xml. Your notification code uses ongoing: true, which may trigger foreground service behavior. Add the required attribute to your service definition.
  • USE_FULL_SCREEN_INTENT permission: v9 removed this from the package manifest. If your app needs full-screen intent notifications, add this permission explicitly to AndroidManifest.xml.
  • Google Play Services removal: v9 no longer depends on GMS libraries (improves F-Droid compatibility).

While your build configuration is correct (compileSdkVersion = 35 meets the v9 requirement of 34+), the Android manifest has not been updated to reflect these breaking changes. This can cause runtime crashes on Android 12+ when foreground services are triggered.

Additionally, the caret range (^9.1.8) allows automatic minor and patch updates. Consider pinning to "9.1.8" until manifest migration is fully tested.

🤖 Prompt for AI Agents
In package.json around line 31, the dependency upgrade to
"@notifee/react-native": "^9.1.8" requires AndroidManifest.xml changes and a
pinned version; update AndroidManifest.xml to add the required
foregroundServiceTypes attribute on your service declaration (matching any
ongoing/foreground notifications used), explicitly add the
USE_FULL_SCREEN_INTENT permission if your app uses full-screen intents, confirm
no GMS libraries are referenced/needed, ensure your AndroidManifest and service
entries are compatible with Android 12+ (compileSdkVersion 35 is fine), and pin
the dependency to "9.1.8" (remove the caret) until you finish the manifest
migration and testing.

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

iOS Build Available

Rocket.Chat Experimental 4.68.0.107804

@diegolmello diegolmello had a problem deploying to upload_experimental_android December 4, 2025 19:11 — with GitHub Actions Error
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Android Build Available

Rocket.Chat Experimental 4.68.0.107803

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTwc2pWo72RbgScwI_jMb-C_9EttezeJBvYGB_6d1pgtmY4YiPPJYXgQYB7NBsjXlXjIbiCEULZBJLFg_mb

@diegolmello diegolmello deployed to upload_official_ios December 4, 2025 20:53 — with GitHub Actions Active
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

iOS Build Available

Rocket.Chat 4.68.0.107805

- Added support for video conference notifications in the notification service.
- Updated notification processing to skip certain actions on iOS during cold starts.
- Improved handling of notification actions and payloads in the notification index.
@diegolmello diegolmello force-pushed the chore.notifee-new-arch branch from 1f03d7b to 46d1e85 Compare December 8, 2025 14:44
@diegolmello diegolmello deployed to approve_e2e_testing December 8, 2025 14:44 — with GitHub Actions Active
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
ios/NotificationService/NotificationService.swift (2)

16-19: Graceful fallback on EJSON decode failure

Delivering the original bestAttemptContent when decoding fails avoids dropping notifications entirely and is a safe behavior change. If needed later, you could add lightweight logging/metrics here for decode failures.


59-63: Videoconf handling in processPayload

Adding a lightweight notificationType == .videoconf branch here to set categoryIdentifier covers the code path where payload content is fetched/merged, without altering existing E2E handling. The behavior is clear and low-risk.

app/lib/notifications/index.ts (2)

23-28: ACCEPT/DECLINE refactor is clearer and behavior‑equivalent

Extracting the event mapping and dispatching deepLinkingClickCallPush({ ...notification, event }) keeps the logic simple and makes the two actions symmetric without changing behavior.


32-43: Confirm behavior for videoconf notifications opened without explicit actions on iOS

The early return for isIOS && ejson.notificationType === 'videoconf' ensures that:

  • Initial “cold start” videoconf pushes are ignored here so that the registerNotificationOpened path with explicit ACCEPT/DECLINE actions can drive the call flow (as per the comment).
  • However, when a videoconf notification is opened by tapping the body (no ACCEPT_ACTION / DECLINE_ACTION), onNotification will now no‑op for iOS.

Please double‑check the following flows on real devices:

  • Locked device: Accept and Decline actions correctly dispatch deepLinkingClickCallPush with event.
  • Notification Center / background app: tapping the notification body (no action) either behaves as desired (no-op or some default behavior) or needs additional handling.

If tapping the body should also open/join the call, you may need a separate path here (distinct from the “cold start” suppression).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1f03d7b and 46d1e85.

📒 Files selected for processing (4)
  • app/lib/notifications/index.ts (2 hunks)
  • app/lib/notifications/push.ts (1 hunks)
  • ios/NotificationService/NotificationService.swift (2 hunks)
  • ios/Shared/Models/NotificationType.swift (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/lib/notifications/index.ts (3)
app/definitions/INotification.ts (1)
  • INotification (1-16)
app/lib/store/auxStore.ts (1)
  • store (6-6)
app/actions/deepLinking.ts (1)
  • deepLinkingClickCallPush (26-31)
app/lib/notifications/push.ts (1)
app/lib/notifications/index.ts (1)
  • onNotification (20-75)
⏰ 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). (2)
  • GitHub Check: ESLint and Test / run-eslint-and-test
  • GitHub Check: format
🔇 Additional comments (3)
ios/Shared/Models/NotificationType.swift (1)

11-15: New videoconf notification type looks consistent

Enum extension is straightforward and matches the string used on the JS side. Just ensure the server payloads also use "videoconf" so all platforms stay in sync.

ios/NotificationService/NotificationService.swift (1)

21-26: Early exit for videoconf notifications from the service extension

Setting categoryIdentifier = "VIDEOCONF" and returning immediately ensures video-conference pushes always get the correct category/actions without going through the rest of the extension logic. This is aligned with the JS side category registration and looks correct.

app/lib/notifications/push.ts (1)

81-99: API signature is correct; fix type annotations and test iOS foreground behavior

The three-parameter callback matches react-native-notifications v5.1.0 API, and the handler cleanly:

  • Injects actionResponse into notification.payload.action so onNotification can branch on ACCEPT/DECLINE.
  • Limits iOS onNotification invocations to background or explicit actions, preventing unintended navigation when foreground.
  • Always calls completion().

Two minor improvements:

  1. Type annotation – Remove the optional marking on actionResponse (the library passes it as a required third parameter). Also import the official type from react-native-notifications instead of inline-defining it, and add explicit NotificationCompletion type to the completion parameter for consistency with registerNotificationReceivedForeground on line 76.
  2. Device testing – Verify on iOS that tapping a notification body (no explicit action) from foreground, background, and killed states produces the expected behavior per product requirements, given the new gating logic.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
app/sagas/deepLinking.js (1)

260-276: Call appReady without a payload to match its definition

appReady is defined to take no arguments and return only { type: APP.READY } (no payload). Supplying {} here is unnecessary and slightly misleading, since the argument is ignored.

To keep usage consistent with the action creator and improve clarity, you can drop the payload in these new calls (and optionally align other call sites in this file in a follow‑up):

-			yield take(types.LOGIN.SUCCESS);
-			yield put(appReady({}));
+			yield take(types.LOGIN.SUCCESS);
+			yield put(appReady());
...
-			yield take(types.LOGIN.SUCCESS);
-			yield put(appReady({}));
+			yield take(types.LOGIN.SUCCESS);
+			yield put(appReady());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 46d1e85 and 4638dcc.

📒 Files selected for processing (1)
  • app/sagas/deepLinking.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/sagas/deepLinking.js (1)
app/actions/app.ts (1)
  • appReady (39-43)

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

Android Build Available

Rocket.Chat Experimental 4.68.0.107806

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTSsRDtGt8mAj3VfK6m2GQNjVp5acSnyP5gX1-DhTNYoNC9Eo9Z0tl768Hg03nijEosU7gKM-KgKrfYE4Or

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

iOS Build Available

Rocket.Chat Experimental 4.68.0.107807

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.

2 participants