Skip to content

chore: delete Experimental code paths and assets#7322

Open
diegolmello wants to merge 7 commits into
remove-experimental-pr2-stop-publishingfrom
remove-experimental-pr3-delete-code
Open

chore: delete Experimental code paths and assets#7322
diegolmello wants to merge 7 commits into
remove-experimental-pr2-stop-publishingfrom
remove-experimental-pr3-delete-code

Conversation

@diegolmello
Copy link
Copy Markdown
Member

@diegolmello diegolmello commented May 13, 2026

Proposed changes

PR3 of the phased Experimental → Official migration (NATIVE-1118). Stacked on PR2 (#7321) — merge PR1 (#7320) and PR2 (#7321) first, then rebase this PR onto develop before merging.

After PR1 (in-app modal nudging Experimental users to migrate) and PR2 (stopping new Experimental publishes), this PR removes the now-dormant Experimental code paths.

JS

  • Delete app/containers/DeprecationModal/ and its mount in app/index.tsx.
  • Delete app/lib/constants/environment.ts (the isOfficial switch). No call sites remain after this PR.
  • app/lib/database/index.ts: drop the -experimental DB path suffix; the path is now always ${appGroupPath}${name}.db.
  • Remove Experimental_retirement_* i18n keys from the 25 locale files that had them (kept in PR1 to feed the modal; obsolete here).

iOS (non-Xcode-project changes)

  • Remove the IS_OFFICIAL key from RocketChatRN/Info.plist, ShareRocketChatRN/Info.plist, and NotificationService/Info.plist. No consumer reads it after this PR.
  • Drop the PlistBuddy "Set IS_OFFICIAL YES" step from .github/actions/build-ios/action.yml and .github/workflows/e2e-build-ios.yml.
  • ios/Shared/RocketChat/Database.swift: stop reading IS_OFFICIAL; always use the non-suffixed DB path (mirrors the JS change).
  • ShareRocketChatRN/Info.plist: CFBundleDisplayName Rocket.Chat ExperimentalRocket.Chat.

Android

  • Remove the experimental product flavor and the IS_OFFICIAL buildConfigField from android/app/build.gradle. The single remaining official flavor is kept to preserve all existing CI gradle task names (assembleOfficialRelease, bundleOfficialRelease, */officialRelease/* artifact paths) — flattening to a no-flavor structure is handled in PR4 (NATIVE-1129).
  • Delete android/app/src/experimental/ (launcher icons, splash assets, strings.xml, manifest — 40 files).
  • android/app/src/main/java/.../notification/Encryption.java: drop the BuildConfig.IS_OFFICIAL reflection in getDatabaseName; the suffix is gone.
  • android/app/src/debug/res/values/strings.xml: [DEBUG] Rocket.Chat Experimental[DEBUG] Rocket.Chat.

Explicitly NOT in this PR (handed off to the repo owner for the Xcode-side cleanup):

  • ios/RocketChatRN.xcodeproj/project.pbxproj — Experimental main target (Rocket.Chat Experimental.app), Experimental Watch target (Rocket.Chat Experimental Watch.app), Experimental.xcassets file refs, PRODUCT_NAME = "Rocket.Chat Experimental".
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/RocketChatRN.xcscheme, RocketChatRN Watch.xcscheme, and the BuildableName = "Rocket.Chat Experimental.app" references in ShareRocketChatRN.xcscheme / NotificationService.xcscheme / RocketChat Watch.xcscheme.
  • ios/Experimental.xcassets/ directory.
  • scripts/prepare_ios_official.sh and its invocations in ios/fastlane/Fastfile (build_official and build_official_simulator) — load-bearing until the pbxproj bundle IDs are rewritten (the script seds chat.rocket.reactnative.{ShareExtension,NotificationService}chat.rocket.ios.{Rocket-Chat-ShareExtension,NotificationService} at CI time).
  • ios/Podfile line target 'RocketChatRN' # Experimental app — removed alongside the pbxproj target.

Post-merge ops (no diff) — once this PR lands:

  • Approve official_android_build and official_ios_build environment gates on the merge run.
  • App Store Connect: archive the chat.rocket.reactnative Experimental app record (TestFlight already stopped from PR2).
  • Play Console: archive the Experimental internal app listing.
  • Delete experimental GH secrets: KEYSTORE_EXPERIMENTAL_*, EXPERIMENTAL_KEYSTORE_*, GOOGLE_SERVICES_IOS_EXPERIMENTAL, and any non-_OFFICIAL legacy keys that paired with them (cross-check — two historical naming conventions coexist). The _OFFICIAL-suffixed secrets are kept permanently — see ADR 0007.
  • Verify experimental_* GH environments are gone (experimental_android_build, experimental_ios_build, any upload_experimental_*).

Issue(s)

https://rocketchat.atlassian.net/browse/NATIVE-1122

How to test or reproduce

  • grep -rn 'isOfficial\|IS_OFFICIAL' app/ android/ ios/ .github/ scripts/ → zero hits (no consumer remains).
  • grep -rn 'Experimental' app/ android/ ios/ .github/ scripts/ → only hits inside ios/RocketChatRN.xcodeproj/, *.xcscheme, Experimental.xcassets/, scripts/prepare_ios_official.sh, and the target 'RocketChatRN' # Experimental app line in ios/Podfile — all owned by the parallel Xcode cleanup.
  • TZ=UTC yarn test → 1351/1351 pass, 357 snapshots stable.
  • npx tsc --noEmit → no errors.
  • Android clean Official build (./gradlew assembleOfficialRelease) succeeds; the single-flavor official keeps all existing artifact paths intact.

Screenshots

n/a — code/assets deletion only.

Types of changes

  • Improvement (non-breaking change which improves a current function)

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) — pure deletion of in-app paths, existing snapshots cover the removed mount
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

The Xcode-project cleanup (pbxproj target removal, scheme deletion, Experimental.xcassets removal, prepare_ios_official.sh removal, and the Podfile target line removal) ships alongside this PR but is being done by the repo owner directly in Xcode to keep the pbxproj diff sane. Once those land, a final grep -rn Experimental should return only WatermelonDB internals (experimentalSubscribe, experimentalUnsafeNativeReuse) and the VoIP architecture-doc stability marker — both unrelated.

The remaining official / *_OFFICIAL naming in Gradle flavors, Fastlane lanes, GH Actions workflows/jobs/artifacts, and the iOS asset catalog is flattened in PR4 (NATIVE-1129), stacked on this branch. CI secret and GH Environment names retain the _OFFICIAL suffix permanently per ADR 0007.

Summary by CodeRabbit

  • Features Removed

    • Deprecation modal for the experimental build removed.
    • Experimental build variant support discontinued on iOS and Android.
  • Style

    • "Experimental" removed from app display names and debug labels.
    • Experimental-specific app icons, splash and branding assets removed.
  • Chores

    • Experimental retirement localization strings removed across many languages.
    • Build and packaging configuration simplified to use the official build variant.
    • Shared database naming and metadata handling unified across builds.

PR3 of the phased Experimental → Official migration (NATIVE-1118). Stacked
on PR2 (#7321). After this PR the only remaining Experimental references
live inside ios/RocketChatRN.xcodeproj/project.pbxproj, the .xcscheme files,
ios/Experimental.xcassets/, scripts/prepare_ios_official.sh, and the
Podfile target line — all owned by the Xcode-side cleanup that ships
alongside this PR (out-of-band, repo owner handles in Xcode).

JS
- Delete app/containers/DeprecationModal/ and its mount in app/index.tsx
- Delete app/lib/constants/environment.ts (isOfficial switch)
- app/lib/database/index.ts: drop -experimental suffix from DB path
- Remove Experimental_retirement_* keys from 25 locale files

iOS
- Remove IS_OFFICIAL key from RocketChatRN/ShareRocketChatRN/NotificationService
  Info.plists
- Drop PlistBuddy "Set IS_OFFICIAL YES" step from build-ios/action.yml and
  e2e-build-ios.yml — IS_OFFICIAL is no longer read by any consumer
- Simplify Database.swift: stop reading IS_OFFICIAL, always use the
  non-suffixed DB path
- ShareRocketChatRN/Info.plist CFBundleDisplayName: "Rocket.Chat Experimental"
  → "Rocket.Chat"

Android
- Remove `experimental` product flavor + the IS_OFFICIAL buildConfigField
  from android/app/build.gradle (keep `official` flavor — single-flavor
  collapse is left as follow-up to minimize CI surface churn)
- Delete android/app/src/experimental/ resource directory (40 files:
  launcher icons, splash, strings, manifest)
- Simplify Encryption.java getDatabaseName: drop BuildConfig.IS_OFFICIAL
  reflection
- android/app/src/debug/res/values/strings.xml: "[DEBUG] Rocket.Chat
  Experimental" → "[DEBUG] Rocket.Chat"
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Remove the experimental build variant and IS_OFFICIAL branch: delete experimental Android/iOS assets, flavors and schemes; stop exporting isOfficial; standardize database filenames (drop -experimental suffix); remove DeprecationModal and related i18n keys; update CI/workflows to set Bugsnag apiKey instead of writing Info.plist IS_OFFICIAL flags. (50 words)

Changes

Single-path: remove experimental variant and normalize builds

Layer / File(s) Summary
CI: set Bugsnag key, drop IS_OFFICIAL writes
.github/actions/build-ios/action.yml, .github/workflows/e2e-build-ios.yml
Composite action and E2E iOS workflow now write Bugsnag bugsnag:apiKey into RocketChatRN/Info.plist and ShareRocketChatRN/Info.plist; removed PlistBuddy updates that set IS_OFFICIAL (NotificationService plist update removed).
Android: flavor removal and build config
android/app/build.gradle, package.json, .github/workflows/e2e-build-android.yml
Removed experimental productFlavor and all IS_OFFICIAL buildConfigField declarations; only official flavor remains. npm scripts updated from experimentalDebugofficialDebug. CI gradle.properties step changed quoting for keystore/Bugsnag secrets.
Android: experimental resources deleted & debug strings
android/app/src/experimental/..., android/app/src/debug/res/values/strings.xml
Deleted experimental drawable/mipmap/colors/strings resources under src/experimental; debug strings updated to remove “Experimental” suffix.
Android: DB naming change (Java)
android/app/src/main/java/chat/rocket/reactnative/notification/Encryption.java
Removed reflection/resource lookup for IS_OFFICIAL; getDatabaseName now derives DB name from server URL and appends .db (no -experimental suffix).
iOS: Info.plists, targets, fastlane, prepare script
ios/RocketChatRN/Info.plist, ios/ShareRocketChatRN/Info.plist, ios/NotificationService/Info.plist, ios/Podfile, ios/fastlane/Fastfile, scripts/prepare_ios_official.sh
Removed IS_OFFICIAL keys from Info.plists; ShareRocketChatRN display name set to Rocket.Chat; Podfile defaults target adjusted (added Rocket.Chat, removed experimental RocketChatRN default); Fastfile lanes no longer call prepare script; prepare script sed edits removed.
iOS: experimental assets and Xcode schemes
ios/Experimental.xcassets/*, ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/*
Removed experimental asset manifests (app icons, launch images, splash color); removed or emptied several RocketChatRN shared schemes; updated NotificationService and ShareRocketChatRN schemes to reference respective app extension targets.
iOS: DB path normalization (Swift)
ios/Shared/RocketChat/Database.swift
Database.getDatabasePath(name:) now returns groupDir + "/<name>.db" unconditionally; removed IS_OFFICIAL-dependent -experimental suffix.
App JS: remove isOfficial and Deprecation modal
app/lib/constants/environment.ts, app/lib/database/index.ts, app/containers/DeprecationModal/*, app/index.tsx
Removed exported isOfficial and its imports; database path generation no longer uses isOfficial; DeprecationModal component and styles deleted and its import/usage removed from app root.
i18n: remove experimental-retirement keys across locales
app/i18n/locales/*.json (multiple locales)
Removed Experimental_retirement_* translation keys (title, description, continue, store links, manual-open) across many locale files; surrounding key ordering adjusted.
Privacy manifest reorder
ios/PrivacyInfo.xcprivacy
Reordered NSPrivacyCollectedDataTypes and NSPrivacyTracking entries; collected-data array content unchanged.
Misc: Xcode scheme updates & NotificationService plist reordering
ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/NotificationService.xcscheme, ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/ShareRocketChatRN.xcscheme, ios/NotificationService/Info.plist
Updated schemes to reference app extension buildables; removed IS_OFFICIAL from NotificationService plist and reordered some plist blocks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately reflects the main objective: removing experimental code paths, assets, and configuration from across iOS, Android, and JavaScript layers.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (2)
  • NATIVE-1118: Request failed with status code 401
  • NATIVE-1122: Request failed with status code 401

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.

Copy link
Copy Markdown
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.

Caution

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

⚠️ Outside diff range comments (1)
android/app/src/main/java/chat/rocket/reactnative/notification/Encryption.java (1)

216-224: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Database file naming mismatch between Android and JS platforms.

The Java code creates physical files with double .db extensions (*.db.db), but the JS implementation creates files with single .db extensions. This causes the Android native code to access different database files than the JS layer, resulting in a cross-platform inconsistency.

The Java getDatabaseName() method appends .db, then passes that to WMDatabase.getInstance() which appends another .db (using context.getDatabasePath(name + ".db")). Meanwhile, the JS getDatabasePath() also appends .db before passing to SQLiteAdapter, which uses the provided name as-is without further modification.

Fix: Either remove the .db extension from getDatabaseName() in Java (letting WMDatabase handle it exclusively), or verify that this intentional difference is correct for the Android implementation.

🤖 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
`@android/app/src/main/java/chat/rocket/reactnative/notification/Encryption.java`
around lines 216 - 224, The getDatabaseName method is appending ".db" which
causes WMDatabase (via WMDatabase.getInstance / context.getDatabasePath(name +
".db")) to produce a "*.db.db" file; remove the explicit ".db" suffix from
getDatabaseName so it returns the base database name (matching the JS behavior)
and let WMDatabase/context.getDatabasePath append the ".db" itself; update the
method for serverUrl normalization (serverUrl.replaceFirst(...).replace("/",
".")) but do not add ".db".
🤖 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.

Outside diff comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/notification/Encryption.java`:
- Around line 216-224: The getDatabaseName method is appending ".db" which
causes WMDatabase (via WMDatabase.getInstance / context.getDatabasePath(name +
".db")) to produce a "*.db.db" file; remove the explicit ".db" suffix from
getDatabaseName so it returns the base database name (matching the JS behavior)
and let WMDatabase/context.getDatabasePath append the ".db" itself; update the
method for serverUrl normalization (serverUrl.replaceFirst(...).replace("/",
".")) but do not add ".db".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2cf9ef8a-bd04-46aa-9ee7-5a382f584874

📥 Commits

Reviewing files that changed from the base of the PR and between 1d3e795 and cdeb7fb.

⛔ Files ignored due to path filters (16)
  • android/app/src/experimental/ic_launcher-web.png is excluded by !**/*.png
  • android/app/src/experimental/res/drawable-hdpi/bootsplash_logo.png is excluded by !**/*.png
  • android/app/src/experimental/res/drawable-mdpi/bootsplash_logo.png is excluded by !**/*.png
  • android/app/src/experimental/res/drawable-xhdpi/bootsplash_logo.png is excluded by !**/*.png
  • android/app/src/experimental/res/drawable-xxhdpi/bootsplash_logo.png is excluded by !**/*.png
  • android/app/src/experimental/res/drawable-xxxhdpi/bootsplash_logo.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-hdpi/ic_launcher.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-hdpi/ic_launcher_round.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-mdpi/ic_launcher.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-mdpi/ic_launcher_round.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-xhdpi/ic_launcher.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-xhdpi/ic_launcher_round.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-xxhdpi/ic_launcher.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-xxhdpi/ic_launcher_round.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-xxxhdpi/ic_launcher.png is excluded by !**/*.png
  • android/app/src/experimental/res/mipmap-xxxhdpi/ic_launcher_round.png is excluded by !**/*.png
📒 Files selected for processing (46)
  • .github/actions/build-ios/action.yml
  • .github/workflows/e2e-build-ios.yml
  • android/app/build.gradle
  • android/app/src/debug/res/values/strings.xml
  • android/app/src/experimental/res/drawable-v24/ic_launcher_background.xml
  • android/app/src/experimental/res/drawable/ic_launcher_foreground.xml
  • android/app/src/experimental/res/drawable/ic_launcher_monochrome.xml
  • android/app/src/experimental/res/mipmap-anydpi-v26/ic_launcher.xml
  • android/app/src/experimental/res/mipmap-anydpi-v26/ic_launcher_round.xml
  • android/app/src/experimental/res/values/colors.xml
  • android/app/src/experimental/res/values/strings.xml
  • android/app/src/main/java/chat/rocket/reactnative/notification/Encryption.java
  • app/containers/DeprecationModal/index.tsx
  • app/containers/DeprecationModal/styles.ts
  • app/i18n/locales/ar.json
  • app/i18n/locales/bn-IN.json
  • app/i18n/locales/cs.json
  • app/i18n/locales/de.json
  • app/i18n/locales/en.json
  • app/i18n/locales/es.json
  • app/i18n/locales/fi.json
  • app/i18n/locales/fr.json
  • app/i18n/locales/hi-IN.json
  • app/i18n/locales/hu.json
  • app/i18n/locales/it.json
  • app/i18n/locales/ja.json
  • app/i18n/locales/nl.json
  • app/i18n/locales/nn.json
  • app/i18n/locales/no.json
  • app/i18n/locales/pt-BR.json
  • app/i18n/locales/pt-PT.json
  • app/i18n/locales/ru.json
  • app/i18n/locales/sl-SI.json
  • app/i18n/locales/sv.json
  • app/i18n/locales/ta-IN.json
  • app/i18n/locales/te-IN.json
  • app/i18n/locales/tr.json
  • app/i18n/locales/zh-CN.json
  • app/i18n/locales/zh-TW.json
  • app/index.tsx
  • app/lib/constants/environment.ts
  • app/lib/database/index.ts
  • ios/NotificationService/Info.plist
  • ios/RocketChatRN/Info.plist
  • ios/ShareRocketChatRN/Info.plist
  • ios/Shared/RocketChat/Database.swift
💤 Files with no reviewable changes (40)
  • android/app/src/experimental/res/drawable-v24/ic_launcher_background.xml
  • android/app/src/experimental/res/drawable/ic_launcher_monochrome.xml
  • android/app/src/experimental/res/mipmap-anydpi-v26/ic_launcher.xml
  • ios/RocketChatRN/Info.plist
  • android/app/src/experimental/res/drawable/ic_launcher_foreground.xml
  • app/i18n/locales/cs.json
  • app/containers/DeprecationModal/styles.ts
  • app/i18n/locales/es.json
  • app/i18n/locales/tr.json
  • ios/NotificationService/Info.plist
  • app/lib/constants/environment.ts
  • app/i18n/locales/nn.json
  • app/i18n/locales/pt-BR.json
  • app/i18n/locales/no.json
  • app/i18n/locales/pt-PT.json
  • app/i18n/locales/ja.json
  • .github/actions/build-ios/action.yml
  • app/i18n/locales/hi-IN.json
  • android/app/src/experimental/res/mipmap-anydpi-v26/ic_launcher_round.xml
  • android/app/src/experimental/res/values/strings.xml
  • app/i18n/locales/hu.json
  • android/app/build.gradle
  • android/app/src/experimental/res/values/colors.xml
  • app/i18n/locales/zh-CN.json
  • app/i18n/locales/fr.json
  • app/i18n/locales/bn-IN.json
  • app/i18n/locales/zh-TW.json
  • app/i18n/locales/sl-SI.json
  • app/i18n/locales/en.json
  • app/i18n/locales/de.json
  • app/i18n/locales/ta-IN.json
  • app/i18n/locales/ar.json
  • app/containers/DeprecationModal/index.tsx
  • app/i18n/locales/ru.json
  • app/i18n/locales/sv.json
  • app/i18n/locales/it.json
  • app/index.tsx
  • app/i18n/locales/fi.json
  • app/i18n/locales/nl.json
  • app/i18n/locales/te-IN.json
📜 Review details
⏰ 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 context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions

Files:

  • app/lib/database/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers

Files:

  • app/lib/database/index.ts
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Prettier formatting with tabs, single quotes, 130 character width, no trailing commas, avoid arrow parens, and bracket same line

Files:

  • app/lib/database/index.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/database/index.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.

Applied to files:

  • app/lib/database/index.ts
🔇 Additional comments (5)
.github/workflows/e2e-build-ios.yml (1)

73-77: LGTM!

ios/ShareRocketChatRN/Info.plist (1)

14-15: LGTM!

android/app/src/debug/res/values/strings.xml (1)

2-3: LGTM!

ios/Shared/RocketChat/Database.swift (1)

32-35: LGTM!

app/lib/database/index.ts (1)

32-32: LGTM!

Drops the Experimental RocketChatRN/Watch app targets, their schemes,
the Experimental.xcassets bundle, the prepare_ios_official.sh rename
shim, and its fastlane invocations.
@diegolmello diegolmello temporarily deployed to approve_e2e_testing May 13, 2026 16:23 — with GitHub Actions Inactive
Copy link
Copy Markdown
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

🤖 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.

Inline comments:
In `@ios/NotificationService/Info.plist`:
- Around line 34-40: The NSExtensionAttributes dictionary (containing
IntentsSupported and INSendMessageIntent) is currently at the top level; move
that entire <key>NSExtensionAttributes</key> <dict>…</dict> block so it becomes
a child inside the existing <key>NSExtension</key> <dict>…</dict> (the same dict
that holds extension keys) ensuring IntentsSupported remains inside NSExtension,
the plist XML stays well-formed, and no duplicate NSExtensionAttributes keys
remain at top level.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f271184e-d1fb-4a8f-81db-c96a665ac142

📥 Commits

Reviewing files that changed from the base of the PR and between cdeb7fb and 031c39a.

⛔ Files ignored due to path filters (24)
  • ios/Experimental.xcassets/AppIcon.appiconset/100.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/1024 1.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/1024.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/114.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/120.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/144.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/152.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/167.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/180.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/20.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/29.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/40.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/50.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/57.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/58.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/60.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/72.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/76.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/80.png is excluded by !**/*.png
  • ios/Experimental.xcassets/AppIcon.appiconset/87.png is excluded by !**/*.png
  • ios/Experimental.xcassets/Launch Screen Icon.imageset/icon.png is excluded by !**/*.png
  • ios/Experimental.xcassets/Launch Screen Icon.imageset/icon@2x.png is excluded by !**/*.png
  • ios/Experimental.xcassets/Launch Screen Icon.imageset/icon@3x.png is excluded by !**/*.png
  • ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • ios/Experimental.xcassets/AppIcon.appiconset/Contents.json
  • ios/Experimental.xcassets/Contents.json
  • ios/Experimental.xcassets/Launch Screen Icon.imageset/Contents.json
  • ios/Experimental.xcassets/splashBackgroundColor.colorset/Contents.json
  • ios/NotificationService/Info.plist
  • ios/Podfile
  • ios/PrivacyInfo.xcprivacy
  • ios/RocketChatRN.xcodeproj/project.pbxproj
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/NotificationService.xcscheme
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/RocketChatRN Watch.xcscheme
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/RocketChatRN.xcscheme
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/ShareRocketChatRN.xcscheme
  • ios/ShareRocketChatRN/Info.plist
  • ios/fastlane/Fastfile
  • scripts/prepare_ios_official.sh
💤 Files with no reviewable changes (8)
  • ios/Experimental.xcassets/Contents.json
  • ios/Experimental.xcassets/AppIcon.appiconset/Contents.json
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/RocketChatRN.xcscheme
  • ios/Experimental.xcassets/splashBackgroundColor.colorset/Contents.json
  • ios/fastlane/Fastfile
  • scripts/prepare_ios_official.sh
  • ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/RocketChatRN Watch.xcscheme
  • ios/Experimental.xcassets/Launch Screen Icon.imageset/Contents.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • ios/ShareRocketChatRN/Info.plist
📜 Review details
⏰ 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 (4)
ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/ShareRocketChatRN.xcscheme (1)

67-77: LGTM!

ios/RocketChatRN.xcodeproj/xcshareddata/xcschemes/NotificationService.xcscheme (1)

58-68: LGTM!

ios/PrivacyInfo.xcprivacy (1)

45-85: LGTM!

ios/Podfile (1)

32-32: Target alignment is correct; no action needed.

The Xcode project defines a PBXNativeTarget named Rocket.Chat, which matches the target declaration at line 32 of the Podfile. Pod install will succeed.

Comment on lines +34 to +40
<key>NSExtensionAttributes</key>
<dict>
<key>IntentsSupported</key>
<array>
<string>INSendMessageIntent</string>
</array>
</dict>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verifies where NSExtensionAttributes is located in ios/NotificationService/Info.plist
python - <<'PY'
import plistlib
from pathlib import Path

p = Path("ios/NotificationService/Info.plist")
data = plistlib.loads(p.read_bytes())

top_level = "NSExtensionAttributes" in data
nested = isinstance(data.get("NSExtension"), dict) and "NSExtensionAttributes" in data["NSExtension"]

print(f"top_level_NSExtensionAttributes={top_level}")
print(f"nested_under_NSExtension={nested}")
PY

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 146


🏁 Script executed:

cat -n ios/NotificationService/Info.plist

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1887


🌐 Web query:

Apple UNNotificationServiceExtension Info.plist NSExtensionAttributes schema documentation

💡 Result:

For a UNNotificationServiceExtension, the Info.plist file configures the app extension under the NSExtension dictionary. The Xcode template automatically sets NSExtensionPointIdentifier to "com.apple.usernotifications.service" and NSExtensionPrincipalClass to the subclass name (e.g., $(PRODUCT_MODULE_NAME).NotificationService) [1][2]. NSExtensionAttributes is an optional dictionary child of NSExtension containing extension-specific keys [3][2][4]. Unlike UNNotificationContentExtension (which requires UNNotificationExtensionCategory and supports additional UI keys like UNNotificationExtensionDefaultContentHidden, UNNotificationExtensionInitialContentSizeRatio, UNNotificationExtensionOverridesDefaultTitle, UNNotificationExtensionUserInteractionEnabled) [5][6], the service extension has no required NSExtensionAttributes keys documented, as it performs background content modification without UI [1][7][8]. Examples occasionally include UNNotificationExtensionCategory or UNNotificationExtensionInitialContentSizeRatio, but these are primarily for content extensions [9][10][11]. Example Info.plist structure [1][11][2][12]: <key>NSExtension</key> <dict> <key>NSExtensionPointIdentifier</key> <string>com.apple.usernotifications.service</string> <key>NSExtensionPrincipalClass</key> <string>$(PRODUCT_MODULE_NAME).NotificationService</string> <key>NSExtensionAttributes</key> <dict> <!-- Optional; no standard keys required for service extension --> </dict> </dict> NSExtensionAttributes schema is general across extensions: a dictionary of type-specific keys (e.g., UNNotification* for notifications) [3][4]. Full list from Apple docs includes many extension-type-specific keys, but for service extensions, only the point identifier and principal class are essential [1][2].

Citations:


Move NSExtensionAttributes under NSExtension (currently misplaced at top level).

According to Apple's Info.plist schema, NSExtensionAttributes must be nested as a child dictionary of NSExtension, not placed at the top level. Currently at lines 34–40, it should be moved inside the NSExtension dictionary (lines 27–33) to ensure the IntentsSupported metadata is properly recognized.

Proposed fix
 	<key>NSExtension</key>
 	<dict>
 		<key>NSExtensionPointIdentifier</key>
 		<string>com.apple.usernotifications.service</string>
 		<key>NSExtensionPrincipalClass</key>
 		<string>$(PRODUCT_MODULE_NAME).NotificationService</string>
+		<key>NSExtensionAttributes</key>
+		<dict>
+			<key>IntentsSupported</key>
+			<array>
+				<string>INSendMessageIntent</string>
+			</array>
+		</dict>
 	</dict>
-	<key>NSExtensionAttributes</key>
-	<dict>
-		<key>IntentsSupported</key>
-		<array>
-			<string>INSendMessageIntent</string>
-		</array>
-	</dict>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<key>NSExtensionAttributes</key>
<dict>
<key>IntentsSupported</key>
<array>
<string>INSendMessageIntent</string>
</array>
</dict>
<key>NSExtension</key>
<dict>
<key>NSExtensionPointIdentifier</key>
<string>com.apple.usernotifications.service</string>
<key>NSExtensionPrincipalClass</key>
<string>$(PRODUCT_MODULE_NAME).NotificationService</string>
<key>NSExtensionAttributes</key>
<dict>
<key>IntentsSupported</key>
<array>
<string>INSendMessageIntent</string>
</array>
</dict>
</dict>
🤖 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 `@ios/NotificationService/Info.plist` around lines 34 - 40, The
NSExtensionAttributes dictionary (containing IntentsSupported and
INSendMessageIntent) is currently at the top level; move that entire
<key>NSExtensionAttributes</key> <dict>…</dict> block so it becomes a child
inside the existing <key>NSExtension</key> <dict>…</dict> (the same dict that
holds extension keys) ensuring IntentsSupported remains inside NSExtension, the
plist XML stays well-formed, and no duplicate NSExtensionAttributes keys remain
at top level.

@diegolmello diegolmello temporarily deployed to official_ios_build May 13, 2026 16:27 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_android_build May 13, 2026 16:27 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to approve_e2e_testing May 13, 2026 17:10 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_android_build May 13, 2026 17:13 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_ios_build May 13, 2026 17:13 — with GitHub Actions Inactive
@diegolmello diegolmello had a problem deploying to upload_official_android May 13, 2026 18:10 — with GitHub Actions Error
@github-actions
Copy link
Copy Markdown

…figs

The deleted scripts/prepare_ios_official.sh used to rewrite these IDs at
build time. Make the rename permanent now that the Experimental targets
are gone, so the App Store provisioning profiles match the bundle IDs.
@diegolmello diegolmello temporarily deployed to approve_e2e_testing May 13, 2026 18:20 — with GitHub Actions Inactive
@diegolmello diegolmello had a problem deploying to official_android_build May 13, 2026 18:24 — with GitHub Actions Error
The double-quoted echo lines let bash re-interpret `$` and other
metacharacters in the substituted secret value, corrupting the password
written to gradle.properties and producing
`KeytoolException: keystore password was incorrect` at signing time.
Mirror the single-quoted pattern already used in
.github/actions/build-android/action.yml so the password reaches gradle
intact.
@diegolmello diegolmello temporarily deployed to approve_e2e_testing May 13, 2026 18:28 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_ios_build May 13, 2026 18:31 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_android_build May 13, 2026 18:31 — with GitHub Actions Inactive
@diegolmello diegolmello had a problem deploying to upload_official_android May 13, 2026 19:15 — with GitHub Actions Error
@github-actions
Copy link
Copy Markdown

The new build_official_simulator Fastlane lane stripped codesign with
skip_codesigning + CODE_SIGNING_ALLOWED=NO, producing a linker-signed
binary with no entitlements. Without the App Group entitlement,
containerURL(forSecurityApplicationGroupIdentifier:) returned nil, the
SQLite path collapsed to /default.db, and the JSI bridge crashed on
launch — failing every E2E shard.

Restore the previous-working signing behavior and add an empty-groupDir
guard in Database.swift matching MMKV.swift's existing guard.
@diegolmello diegolmello temporarily deployed to approve_e2e_testing May 14, 2026 13:02 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_ios_build May 14, 2026 13:09 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to official_android_build May 14, 2026 13:09 — with GitHub Actions Inactive
@diegolmello diegolmello requested a deployment to upload_official_ios May 14, 2026 16:55 — with GitHub Actions Waiting
@diegolmello diegolmello requested a deployment to upload_official_android May 14, 2026 17:03 — with GitHub Actions Waiting
@github-actions
Copy link
Copy Markdown

Copy link
Copy Markdown
Member Author

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

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

Review verdict: ship-with-nits

Pure-deletion PR3 of the NATIVE-1118 phased migration. Diffed against remove-experimental-pr2-stop-publishing (the actual PR base, not develop). No genuine blockers found. All "missing" pieces I traced are intentionally deferred to PR4.

What I verified

  • Stale-reference sweeps on the PR3 head tree:
    • isOfficial / IS_OFFICIAL → zero hits anywhere.
    • Experimental_retirement_* → zero hits anywhere.
    • DeprecationModal → zero hits anywhere; both files that referenced it (app/containers/DeprecationModal/*, mount in app/index.tsx) are removed cleanly.
    • Residual experimental* matches are only WatermelonDB internals (experimentalSubscribe, experimentalUnsafeNativeReuse) and VoIP arch-doc stability markers — unrelated.
  • All 25 locale files delete the same 6 keys uniformly (verified en, ar, cs, de, zh-CN, zh-TW, hi-IN).
  • Encryption.java compiles: import java.lang.reflect.Field was the only consumer of the removed reflection block; it's removed too.
  • Tests: no test or snapshot referenced DeprecationModal pre-PR3, so no snapshot regen needed. The PR author's note that "existing snapshots cover the removed mount" is technically a no-op claim — there's literally nothing to cover — which is fine.

Migration / data-loss risk (the user's top concern)

No risk. The Official and Experimental apps have always been distinct installables with distinct package IDs (chat.rocket.android / chat.rocket.ios vs chat.rocket.reactnative), distinct App Group containers on iOS, and distinct app-private sandboxes on Android. The Official app's getDatabasePath has always produced ${name}.db (no -experimental suffix); only the Experimental flavor produced ${name}-experimental.db. The Official app never had a -experimental file to read from on disk, so removing the conditional is a pure dead-code cleanup, not a path change for any existing user. Experimental users on the now-dormant build had the PR1 modal nudging them off; their orphaned DBs sit untouched in the Experimental sandbox. No migration code needed.

Genuine issues

None blocking.

Nits / follow-ups (non-blocking)

  1. ios/PrivacyInfo.xcprivacy — the file got reordered and lost a useful inline TODO comment ("Audit privacy manifest against full App Store Connect App Privacy disclosure for pre-existing data types..."). This is unrelated to Experimental removal and was likely an Xcode auto-format side effect. The audit reminder still applies — consider re-adding the comment or filing a tracking issue.
  2. PR body is slightly stale — it says prepare_ios_official.sh, the Podfile's target 'RocketChatRN' # Experimental app line, and Experimental.xcassets/ would be "handed off to the repo owner" and NOT in this PR, but commit 031c39a51 ("chore(ios): remove Experimental Xcode targets and assets") actually does that work in-PR. The PR body should be refreshed to reflect that the Xcode-side cleanup already landed in PR3. The code is correct; the description just trails reality.
  3. CI flakes — 4 of 28 E2E shards failed (Maestro emulator step), but Build iOS Official, Build Android Official, ESLint+Test all pass. Retry the failed shards.

Existing review reconciliation

  • CodeRabbit on ios/NotificationService/Info.plist:40 (NSExtensionAttributes nested wrong): The complaint is technically valid — per Apple's schema, NSExtensionAttributes should be a child of NSExtension, not a top-level key. However, this misplacement is pre-existing (already present at the PR2 base and on develop; predates PR3 by several releases). PR3's diff on this file is purely the IS_OFFICIAL removal plus a tab/space indentation normalization on the existing NSExtensionAttributes block. Disagree with treating this as a PR3 blocker. Acknowledge the bug and file a separate follow-up — fixing it here mixes scopes and would need a real test that INSendMessageIntent still resolves for the service extension.
  • CodeRabbit outside-diff on Encryption.java:216-224 (Android .db.db double extension vs JS single .db): Also pre-existing. The explicit comment in the method body documents the intentional convention ("WMDatabase will resolve and append its own .db internally, so the physical file becomes *.db.db, matching the JS adapter"). The JS adapter passes its ${name}.db string directly to native SQLite (no extra .db append), and the Java side replicates the final physical filename via context.getDatabasePath(name + ".db") which Android's framework converts to ${name}.db.db. The convention is load-bearing for the notification-service Android side to open the same file WatermelonDB writes. Disagree with making this a PR3 change; if revisited, it needs a coordinated JS+Android test plan and likely belongs in its own PR.
  • CodeRabbit/PR1-carryover claim on app/i18n/locales/hi-IN.json:331 about Hindi translations contradicting an "English-only" claim: Does not apply to PR3. PR3's diff for hi-IN.json is purely deletion of the 6 Experimental_retirement_* keys (the values were already in Hindi script in the file; PR3 doesn't touch values, only removes keys). PR1-era noise — ignore for this PR.

Summary

Solid deletion-only PR. Scope is tight, all sweeps come back clean, the stacked-on-PR2 base means the diff stays focused on what PR3 actually does. The Xcode-side cleanup that PR body said would land separately actually landed in-PR (good — the diff is more self-consistent that way; just refresh the body). Ship after PR1+PR2 land, retry the flaky E2E shards.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant