feat: quick actions #6902
feat: quick actions #6902
Conversation
…eactNative into feat/quickactions_native
WalkthroughThis PR introduces comprehensive quick actions support for iOS and Android, enabling users to perform common tasks directly from the home screen. The implementation includes native platform handlers, Redux state management, saga-based coordination, and UI integration for actions like adding servers, searching rooms, navigating to recently visited rooms, and sending feedback. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Home Screen)
participant iOS as iOS App
participant AppDelegate
participant QAStore as QAStore (Native)
participant JS as JS Runtime
participant Redux as Redux
participant Saga as quickActions Saga
participant Nav as Navigation
User->>iOS: Tap Quick Action (recent)
iOS->>AppDelegate: performActionFor shortcutItem
AppDelegate->>QAStore: RCSetPendingQuickActionType("recent")
AppDelegate->>iOS: App launches
JS->>QAStore: getInitialQuickAction()
QAStore->>JS: Returns "recent"
JS->>Redux: dispatch quickActionHandle({action: "recent"})
Redux->>Saga: QUICK_ACTIONS.QUICK_ACTION_HANDLE
Saga->>Redux: select lastVisitedRid
Saga->>Saga: waitForNavigationReady()
Saga->>Nav: navigate to RoomView
Saga->>Redux: dispatch QUICK_ACTION_HANDLED
sequenceDiagram
participant User as User
participant App as React App
participant Emitter as Emitter
participant Saga as roomVisited Saga
participant Redux as Redux
participant UserPrefs as UserPreferences
User->>App: Navigate to room
App->>App: goRoom() called
App->>Emitter: emit("roomVisited", {rid, name})
Emitter->>Saga: eventChannel receives roomVisited
Saga->>Redux: dispatch roomsStoreLastVisited(rid, name)
Saga->>UserPrefs: LAST_VISITED_ROOM_ID_KEY = rid
Saga->>UserPrefs: LAST_VISITED_ROOM_NAME_KEY = name
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
…eactNative into feat/quickactions_native
… instead of store.dispatch
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/views/RoomsListView/hooks/useSearch.ts (1)
62-83: "use memo" directive is valid; however, wrap the debounced callback inuseCallbackto prevent unnecessary effect re-runs.The
'use memo'directive is correct React Compiler syntax and correctly placed at line 62. However, the debouncedsearchcallback (lines 77–83) is recreated every render because it's not memoized. Sincesearchis listed as a dependency in the effect at line 104, the effect runs every render, even whenstate.searchEnabledhasn't changed. Wrap the callback inuseCallbackwith[state.searchEnabled]as dependencies:Suggested fix
const search = useDebounce( useCallback(async (text: string) => { if (!state.searchEnabled) return; dispatch({ type: 'SET_SEARCHING' }); const result = await searchLib({ text }); dispatch({ type: 'SEARCH_SUCCESS', payload: result as IRoomItem[] }); announceSearchResultsForAccessibility(result.length); }, [state.searchEnabled, announceSearchResultsForAccessibility]), 500 );app/AppContainer.tsx (1)
40-57: Guard against empty-string room name producing a blank quick-action title.
IflastVisitedRoomNamecan ever be'',recentRoomName ?? ...will keep it and show an empty title.Proposed tweak
useEffect(() => { - updateQuickActions({ recentRoomName: lastVisitedRoomName }); + updateQuickActions({ recentRoomName: lastVisitedRoomName || undefined }); }, [lastVisitedRoomName]);ios/RocketChatRN.xcodeproj/project.pbxproj (1)
1284-1298: Remove miswired build phases from RocketChatRN target.The RocketChatRN target contains two
[Expo] Configure projectphases (IDs:589729E8381BA997CD19EF19and9C104B12BEE385F7555E641F). The second one (line 1295) is miswired—it executes the NotificationService-specific expo-configure script and should not be in this target. Additionally, the unnamed ShellScript at line 1289 (ID:407D3EDE3DABEE15D27BD87D) outputs to a NotificationService-specific manifest file and appears misplaced. Move these phases to their appropriate targets.
🤖 Fix all issues with AI agents
In @android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt:
- Around line 34-49: onCreate currently ends up invoking
NotificationIntentHandler.handleIntent twice on cold start because
handleVideoConfIntent already delegates to
NotificationIntentHandler.handleIntent and onCreate also calls
NotificationIntentHandler.handleIntent directly; remove the redundant direct
call in onCreate (the intent?.let { NotificationIntentHandler.handleIntent(this,
it) } line) so notification intents are only processed once, ensure
handleVideoConfIntent still calls NotificationIntentHandler.handleIntent for
video-conf flows, and add equivalent logic to onNewIntent to call
NotificationIntentHandler.handleIntent(this, intent) for warm starts so all
intent handling flows go through a single place.
- Around line 58-97: Remove or gate the sensitive debug logs in
MainActivity.handleVideoConfIntent so identifiers (rid, callerName, callId,
host) are not printed in production: wrap the Log.d(...) calls that output these
fields in a debug-only check (e.g., if (BuildConfig.DEBUG) { ... }) or remove
them entirely, and apply the same change for any other Log.d/Log.* lines in the
same method range (the other occurrence referenced in lines ~104-170) so
VideoConfModule.storePendingAction still receives full data but no sensitive
identifiers are emitted to production logs.
In @android/app/src/main/res/drawable/ic_quickaction_add.xml:
- Around line 1-5: Remove the redundant android:tint attribute from the root
<vector> element in ic_quickaction_add.xml so the icon matches the other
quick-action drawables; keep the existing path fillColor="@android:color/black"
unchanged and ensure only the android:tint attribute is deleted from the
<vector> tag.
In @app/index.tsx:
- Around line 64-77: In parseDeepLinking, when handling quick-action URLs (the
url.startsWith('rocketchat://quick-action/') branch), avoid using replace(...)
directly; instead extract the raw action by removing the
'rocketchat://quick-action/' prefix, strip any querystring or fragment by
splitting on /[?#]/ and taking the first segment, then URI-decode it with
decodeURIComponent (wrap in try/catch to fall back to the raw value on decode
errors) before returning { type: 'quick-action', action } so action contains the
clean, decoded value.
In @app/lib/methods/helpers/goRoom.ts:
- Around line 141-147: The new getRoom(item.rid) call in goRoom can throw and
will abort navigation; wrap the getRoom and emitter.emit('roomVisited', ...)
sequence in a try-catch so failures are swallowed (or logged) and do not reject
goRoom. Specifically, in goRoom around the getRoom/item.rid section, catch
errors from getRoom, optionally log via the existing logger/emitter, and only
emit roomVisited when getRoom succeeds so navigation continues even if room
lookup fails.
In @app/lib/quickActions/index.ts:
- Around line 1-3: The fix: make updateQuickActions async and await
QuickActions.setItems(...) so the returned Promise<void> is handled (change
function signature of updateQuickActions and await the setItems call inside it);
for addListener(), capture the returned Subscription object (const sub =
QuickActions.addListener(...)) and ensure you call sub.remove() during cleanup
(e.g., when replacing/tearing down listeners or component unmount) to avoid
leaks and duplicate registrations. Ensure references use the exact symbols
updateQuickActions, QuickActions.setItems, QuickActions.addListener, and
Subscription.remove.
In @app/sagas/init.js:
- Line 5: Rename the constant LAST_VISITED_ROOM_Name_KEY to
LAST_VISITED_ROOM_NAME_KEY in the constants definition, update its export, and
change all imports/usages that reference LAST_VISITED_ROOM_Name_KEY (e.g., in
the sagas that reference the last visited room key) to use
LAST_VISITED_ROOM_NAME_KEY instead; ensure import statements and any
destructuring that referenced the old name are updated so the symbol name is
consistent and all references compile.
In @app/sagas/quickActions.ts:
- Around line 82-104: In the 'recent' case of the quickActions saga, don't exit
the saga early with "if (!rid) return;" because that bypasses the final yield
put({ type: QUICK_ACTIONS.QUICK_ACTION_HANDLED });; instead change the
early-return to a switch-case-safe exit (e.g., "if (!rid) { break; }") or wrap
the rest of the case logic in "if (rid) { ... }" so the case completes normally
and the QUICK_ACTION_HANDLED dispatch always runs; update the code around the
rid check in the 'recent' case that uses waitForAppReady/select (variable rid)
to ensure control flows to the end of the saga.
- Around line 61-78: The switch case for 'search' in quickActions uses a lexical
declaration (const currentRoute) without block scoping; wrap the entire 'search'
case body in braces to scope declarations properly (i.e., change case 'search':
... to case 'search': { ... } ), keeping the existing calls to
waitForAppReady(), Navigation.getCurrentRoute(),
Navigation.navigate('RoomsListView') and the yield put({ type: UI.TRIGGER_SEARCH
}) intact so the behavior of quickAction handling remains unchanged.
- Around line 36-44: waitForRoomInDB can hang because take(APP.START) waits
indefinitely for the next APP.START; replace the current try/catch with a
bounded retry loop that calls getRoom up to a fixed maxAttempts and uses a short
delay/race to avoid deadlock: inside waitForRoomInDB attempt yield call(getRoom,
rid) and return on success; on failure yield a race between take(APP.START) and
call(delay, retryDelayMs) (or just call(delay, ms)) then loop until maxAttempts
exhausted; after maxAttempts either throw/return null to fail fast. Keep
references to waitForRoomInDB, getRoom and APP.START so the retry logic replaces
the current try/catch/return flow.
In @app/sagas/roomVisited.ts:
- Line 8: The constant name uses mixed case; rename LAST_VISITED_ROOM_Name_KEY
to LAST_VISITED_ROOM_NAME_KEY in the constant definition
(app/lib/constants/keys.ts) and update all imports/usages (e.g., the import in
app/sagas/roomVisited.ts and any other files) to the new identifier; ensure you
update export(s) and run type-check/grep to catch and fix all references so
builds pass.
In @app/views/SettingsView/index.tsx:
- Around line 36-50: Replace encodeURI with encodeURIComponent for the mailto
query parameters and use the helper values directly: change subject =
encodeURIComponent('Rocket.Chat Mobile App Support') and description =
encodeURIComponent(`version: ${getReadableVersion}\ndevice: ${getDeviceModel}`)
(do not call getReadableVersion/getDeviceModel as functions); keep the
Linking.openURL call using the template string
`mailto:${email}?subject=${subject}&body=${description}` and preserve the
try/catch and logging in sendEmail.
In @ios/QuickActionsConnector.mm:
- Around line 8-16: The getInitialQuickAction method reads and clears the global
RCPendingQuickActionType without synchronization, causing a race; fix by
serializing access to that variable (e.g., create a dedicated serial
dispatch_queue or guard with @synchronized on a shared object) and perform the
read-and-clear on that queue or within the lock so getInitialQuickAction
atomically captures the value and sets RCPendingQuickActionType = nil; update
any other writers/readers of RCPendingQuickActionType to use the same
synchronization primitive.
In @ios/QuickActionsStore.h:
- Around line 1-5: The header QuickActionsStore.h is missing include guards; add
a guard to prevent multiple inclusion (use either #pragma once at the top or
traditional #ifndef/#define/#endif) so the declarations for
RCPendingQuickActionType and RCSetPendingQuickActionType are only processed once
when included via RocketChatRN-Bridging-Header.h; place the guard around the
existing FOUNDATION_EXPORT declarations to enclose the whole header content.
In @ios/RocketChatRN.xcodeproj/project.pbxproj:
- Around line 1567-1583: Remove the entire PBXShellScriptBuildPhase entry with
identifier 1E1EA8082326CCE300E22452 from the project file and delete any
references to that ID from target buildPhases arrays so the shell script is
fully removed; ensure you also remove its block from PBXBuildPhase lists (and
any duplicate occurrences) to avoid dangling references that would break the
project.
In @package.json:
- Line 73: The dependency version for expo-quick-actions in package.json is
incompatible with Expo SDK 53; change the version string for
"expo-quick-actions" from "^6.0.0" to "5.0.0" (or another 5.x release) so it
targets SDK 53, then run your package manager install to update lockfile and
ensure tests/build succeed.
🧹 Nitpick comments (16)
android/app/src/main/res/drawable/ic_quickaction_contact.xml (1)
1-5: Consider standardizing viewport dimensions.The vector drawable is correctly formatted. However, note that
ic_quickaction_contact.xmluses a 24x24 viewport whileic_quickaction_recent.xmluses 512x512. While both will render correctly, standardizing viewports across related icon resources can improve maintainability.♻️ Optional: standardize to 512x512 viewport
If you want consistency with the recent icon, you could rescale the path data to a 512x512 viewport. However, this is purely cosmetic and not required for functionality.
app/actions/actionsTypes.ts (1)
15-15: New action namespaces look consistent; consider tighteningcreateRequestTypestyping.Lines 15 and 63-65 align with the new quick-actions/UI flows. Small TS maintainability improvement:
createRequestTypes(base = {}, ...)should likely takebase: string(default'') to avoid accidental${base}becoming[object Object].Also applies to: 63-65
app/actions/quickActions.ts (1)
5-15: Makeparamsoptional (and avoidparams.actionambiguity).
Right nowIQuickAction.paramsis required (Line 9) andquickActionHandlerequires an argument (Line 11), which makes “no-param” dispatch awkward, andparams.actionis easy to confuse with Redux “action”.Proposed diff
interface IQuickActionParams { - action: string; + action: string; } interface IQuickAction extends Action { - params: Partial<IQuickActionParams>; + params?: Partial<IQuickActionParams>; } -export function quickActionHandle(params: Partial<IQuickActionParams>): IQuickAction { +export function quickActionHandle(params: Partial<IQuickActionParams> = {}): IQuickAction { return { type: QUICK_ACTIONS.QUICK_ACTION_HANDLE, params }; }app/views/RoomsListView/hooks/useSearch.ts (1)
65-104: Type your Redux dispatch and avoid clearing triggers with untyped objects.
SinceAppDispatchnow exists, preferuseDispatch<AppDispatch>()so these dispatches are type-checked (Line 66, Line 96).Proposed diff
-import { useDispatch } from 'react-redux'; +import { useDispatch } from 'react-redux'; +import type { AppDispatch } from '../../../lib/store'; ... - const storeDispatch = useDispatch(); + const storeDispatch = useDispatch<AppDispatch>();app/lib/store/index.ts (1)
29-33: Good addition; watch out forstorename collisions withauxStore.ts.
AppDispatchis a solid typing hook foruseDispatch, but it’s easy to accidentally import the wrongstoregivenapp/lib/store/auxStore.tsalso exportsstore.app/reducers/rooms.ts (1)
9-10: Inconsistent naming: mix of abbreviation and full word.The state fields use inconsistent naming styles:
lastVisitedRid— abbreviation ("Rid" for room ID)lastVisitedName— full word ("Name")The action payload uses full words (
lastVisitedRoomId,lastVisitedRoomName), which is clearer. Consider renaming the state fields to match for consistency:- lastVisitedRid: string; - lastVisitedName: string; + lastVisitedRoomId: string; + lastVisitedRoomName: string;You'll also need to update the initial state (lines 18-19) and the reducer case (lines 54-55) accordingly.
app/AppContainer.tsx (1)
4-25: Consider avoidingconnect(...)+ hooks in the same component.
Not a blocker, but it makes the data-flow harder to follow (props from HOC + hooks from store). Either approach alone would be simpler.app/i18n/locales/en.json (1)
331-331: Same i18n key hygiene note (Something_Wrong?) + optional copy polish.
If you keep the\ninError_finding_room, consider sentence punctuation for UI consistency (non-blocking).Also applies to: 457-457, 698-699, 846-846, 984-984
app/index.tsx (1)
166-184: Replace non-null assertion and add quick-action ID validation.The
deepLinking!non-null assertion is unsafe sinceLinking.getInitialURL()can returnnullorundefined. Additionally, validation here prevents accidental dispatches of unknown quick-action IDs. The handler gracefully ignores them today, but explicit validation at the entry point is clearer and prevents unnecessary Redux actions.Replace the non-null assertion:
const parsedDeepLinkingURL = deepLinking ? parseDeepLinking(deepLinking) : null;And add ID validation before dispatch in the quick-action handler to match the known cases defined in
app/lib/quickActions/index.ts('search', 'add-server', 'recent', 'contact').app/lib/quickActions/getRecentQuickAction.ts (1)
14-19: Consider simplifying the error handling return.The try-catch appropriately returns
nullon any error. The?? nullon line 16 is defensive but potentially redundant, sincegetInitialQuickActionshould return a string or null/undefined. If the native contract guarantees this, you can simplify toreturn action || null.♻️ Optional simplification
try { const action = await QuickActionsConnector.getInitialQuickAction(); - return action ?? null; + return action || null; } catch { return null; }app/reducers/ui.ts (1)
13-28: Strengthen type safety with a discriminated union for UI actions.The reducer uses a generic
Actiontype, preventing TypeScript from verifying thatUI.TRIGGER_SEARCHandUI.CLEAR_TRIGGERED_SEARCHare valid action types. Define a union of specific UI action types for better compile-time safety.♻️ Proposed type-safe action union
+type UIAction = + | { type: typeof UI.TRIGGER_SEARCH } + | { type: typeof UI.CLEAR_TRIGGERED_SEARCH }; + -export default function ui(state = initialState, action: Action): IUIState { +export default function ui(state = initialState, action: UIAction | Action): IUIState { switch (action.type) { case UI.TRIGGER_SEARCH: return { ...state, triggerSearch: true }; case UI.CLEAR_TRIGGERED_SEARCH: return { ...state, triggerSearch: false }; default: return state; } }This approach allows UI actions to be type-checked while still accepting generic actions for Redux compatibility.
app/sagas/roomVisited.ts (1)
21-31: Add error handling for UserPreferences persistence.The
UserPreferences.setStringcalls (lines 28-29) lack error handling. If persistence fails, the saga will crash and stop processing future room visitation events. Consider wrapping these calls in a try-catch to ensure the saga remains resilient.♻️ Proposed error handling
function* watchRoomVisited(): SagaIterator { const channel = yield call(createRoomVisitedChannel); while (true) { const { rid, name } = yield take(channel); yield put(roomsStoreLastVisited(rid, name)); - UserPreferences.setString(LAST_VISITED_ROOM_ID_KEY, rid); - UserPreferences.setString(LAST_VISITED_ROOM_Name_KEY, name); + try { + UserPreferences.setString(LAST_VISITED_ROOM_ID_KEY, rid); + UserPreferences.setString(LAST_VISITED_ROOM_Name_KEY, name); + } catch (error) { + // Log but don't crash the saga + console.warn('Failed to persist last visited room:', error); + } } }app/lib/methods/helpers/goRoom.ts (2)
115-116: Silent catch block may hide unexpected errors.The empty catch block with "do nothing" comment intentionally swallows all errors from the room visitation tracking. While this prevents blocking navigation, it also hides potential bugs (e.g., emitter failures, getRoom issues). Consider logging the error for debugging.
📋 Optional logging addition
} catch { - // do nothing + // Ignore errors in room visit tracking to avoid blocking navigation }Or add minimal logging:
- } catch { - // do nothing + } catch (error) { + // Log but don't block navigation + console.debug('Failed to track room visit:', error); }
141-149: Sequential database queries may impact performance.Lines 141 and 149 make two separate database calls (
getRoomandgetSubscriptionByRoomId) for the sameitem.rid. If these queries are frequently executed, consider optimizing by fetching both in parallel or combining into a single query.Example parallel fetch:
const [room, sub] = await Promise.all([ getRoom(item.rid).catch(() => null), getSubscriptionByRoomId(item.rid) ]); if (room) { emitter.emit('roomVisited', { rid: room.rid, name: room.name }); } if (sub) { _item = sub; }This reduces the total latency when both queries are independent.
app/reducers/quickActions.ts (1)
25-33: Consider refining the type narrowing.The type guard on line 26 correctly checks for the payload, but line 31 still uses a type assertion. Since
QuickActionsActionis a union including the bareActiontype, TypeScript cannot automatically narrow the type after the guard. While the current implementation is functionally correct, you could improve type safety by using a more explicit type predicate or restructuring the action types to avoid the manual cast.♻️ Alternative approach using type predicate
+function isQuickActionPayload(action: QuickActionsAction): action is IQuickActionPayloadAction { + return 'payload' in action && !!action.payload?.action; +} + export default function quickActions(state = initialState, action: QuickActionsAction): IQuickActionsState { switch (action.type) { case QUICK_ACTIONS.QUICK_ACTION_HANDLE: - if (!('payload' in action) || !action.payload?.action) { + if (!isQuickActionPayload(action)) { return state; } return { ...state, - lastAction: (action as IQuickActionPayloadAction).payload.action, + lastAction: action.payload.action, handled: false };ios/AppDelegate.swift (1)
7-7: Optional: Remove redundant Foundation import.The
Foundationimport is typically unnecessary in iOS applications as it's already available throughUIKitand other frameworks imported in this file. While not harmful, removing it can reduce clutter.
📜 Review details
Configuration used: Organization 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.
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (41)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.ktandroid/app/src/main/res/drawable/ic_quickaction_add.xmlandroid/app/src/main/res/drawable/ic_quickaction_contact.xmlandroid/app/src/main/res/drawable/ic_quickaction_find.xmlandroid/app/src/main/res/drawable/ic_quickaction_recent.xmlapp/AppContainer.tsxapp/actions/actionsTypes.tsapp/actions/quickActions.tsapp/actions/rooms.tsapp/definitions/redux/index.tsapp/i18n/locales/en.jsonapp/i18n/locales/pt-BR.jsonapp/index.tsxapp/lib/constants/keys.tsapp/lib/methods/helpers/emitter.tsapp/lib/methods/helpers/goRoom.tsapp/lib/navigation/addServer/index.tsapp/lib/quickActions/getRecentQuickAction.tsapp/lib/quickActions/index.tsapp/lib/store/index.tsapp/reducers/index.jsapp/reducers/quickActions.tsapp/reducers/rooms.tsapp/reducers/ui.tsapp/sagas/index.jsapp/sagas/init.jsapp/sagas/quickActions.tsapp/sagas/roomVisited.tsapp/sagas/rooms.jsapp/views/RoomsListView/hooks/useSearch.tsapp/views/SettingsView/index.tsxios/AppDelegate.swiftios/Podfileios/QuickActionsConnector.hios/QuickActionsConnector.mmios/QuickActionsStore.hios/QuickActionsStore.mmios/RocketChatRN-Bridging-Header.hios/RocketChatRN.xcodeproj/project.pbxprojios/RocketChatRN/Info.plistpackage.json
🧰 Additional context used
🧬 Code graph analysis (18)
app/lib/store/index.ts (1)
app/lib/store/auxStore.ts (1)
store(6-6)
app/definitions/redux/index.ts (1)
app/reducers/ui.ts (1)
IUIState(5-7)
app/lib/quickActions/index.ts (3)
app/lib/quickActions/getRecentQuickAction.ts (1)
getRecentQuickAction(9-20)app/lib/store/auxStore.ts (1)
store(6-6)app/actions/quickActions.ts (1)
quickActionHandle(11-16)
app/sagas/roomVisited.ts (4)
app/lib/methods/helpers/events.ts (1)
emit(49-59)app/lib/methods/helpers/emitter.ts (1)
emitter(28-28)app/actions/rooms.ts (1)
roomsStoreLastVisited(54-60)app/lib/constants/keys.ts (2)
LAST_VISITED_ROOM_ID_KEY(28-28)LAST_VISITED_ROOM_Name_KEY(29-29)
app/actions/rooms.ts (2)
app/sagas/deepLinking.js (1)
name(62-62)app/actions/actionsTypes.ts (1)
ROOMS(15-15)
app/lib/methods/helpers/goRoom.ts (2)
app/lib/methods/getRoom.ts (1)
getRoom(4-12)app/lib/methods/helpers/emitter.ts (1)
emitter(28-28)
app/actions/quickActions.ts (1)
app/actions/actionsTypes.ts (1)
QUICK_ACTIONS(63-63)
app/reducers/ui.ts (2)
app/reducers/quickActions.ts (1)
initialState(10-13)app/actions/actionsTypes.ts (1)
UI(65-65)
app/lib/navigation/addServer/index.ts (4)
app/lib/store/index.ts (1)
AppDispatch(30-30)app/lib/methods/helpers/log/index.ts (2)
logEvent(35-42)events(7-7)app/actions/app.ts (1)
appStart(31-37)app/actions/server.ts (1)
serverInitAdd(104-109)
app/AppContainer.tsx (5)
app/sagas/init.js (1)
lastVisitedRoomName(56-56)app/lib/hooks/useAppSelector.ts (1)
useAppSelector(6-6)app/definitions/redux/index.ts (1)
IApplicationState(53-81)app/lib/quickActions/index.ts (2)
registerQuickActions(55-69)updateQuickActions(22-53)app/actions/actionsTypes.ts (1)
NAVIGATION(64-64)
app/reducers/rooms.ts (1)
app/actions/actionsTypes.ts (1)
ROOMS(15-15)
app/reducers/quickActions.ts (2)
app/reducers/ui.ts (1)
initialState(9-11)app/actions/actionsTypes.ts (1)
QUICK_ACTIONS(63-63)
app/views/SettingsView/index.tsx (2)
app/lib/methods/helpers/log/index.ts (1)
logEvent(35-42)app/lib/methods/helpers/info.ts (1)
showErrorAlert(5-6)
app/index.tsx (3)
app/lib/quickActions/getRecentQuickAction.ts (1)
getRecentQuickAction(9-20)app/lib/store/auxStore.ts (1)
store(6-6)app/actions/quickActions.ts (1)
quickActionHandle(11-16)
app/sagas/init.js (2)
app/lib/constants/keys.ts (2)
LAST_VISITED_ROOM_ID_KEY(28-28)LAST_VISITED_ROOM_Name_KEY(29-29)app/actions/rooms.ts (1)
roomsStoreLastVisited(54-60)
app/sagas/quickActions.ts (9)
app/actions/app.ts (2)
appInit(45-49)appStart(31-37)app/actions/actionsTypes.ts (4)
APP(35-43)NAVIGATION(64-64)UI(65-65)QUICK_ACTIONS(63-63)app/lib/methods/getRoom.ts (1)
getRoom(4-12)app/sagas/init.js (2)
server(27-27)root(83-87)app/lib/constants/keys.ts (1)
CURRENT_SERVER(26-26)app/actions/server.ts (1)
serverInitAdd(104-109)app/views/SettingsView/index.tsx (1)
sendEmail(36-50)app/actions/rooms.ts (1)
roomsStoreLastVisited(54-60)app/lib/methods/helpers/goRoom.ts (1)
goRoom(87-156)
ios/QuickActionsStore.h (1)
app/sagas/deepLinking.js (1)
type(61-61)
app/views/RoomsListView/hooks/useSearch.ts (3)
app/lib/hooks/useAppSelector.ts (1)
useAppSelector(6-6)app/actions/actionsTypes.ts (1)
UI(65-65)app/lib/methods/search.ts (1)
search(114-185)
🪛 Biome (2.1.2)
app/sagas/quickActions.ts
[error] 71-71: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 Clang (14.0.6)
ios/QuickActionsConnector.h
[error] 1-1: 'React/RCTBridgeModule.h' file not found
(clang-diagnostic-error)
ios/QuickActionsStore.h
[error] 1-1: 'Foundation/Foundation.h' file not found
(clang-diagnostic-error)
🔇 Additional comments (21)
ios/Podfile (1)
12-12: LGTM!The ExpoQuickActions pod declaration is correctly configured with the standard local path to node_modules, aligning with the npm dependency addition in package.json.
android/app/src/main/res/drawable/ic_quickaction_recent.xml (1)
1-9: LGTM!The vector drawable is correctly formatted with standard Android vector attributes and provides a clear recent/clock icon for the quick action.
app/sagas/rooms.js (2)
20-24: Stylistic-only callback param change looks safe.Line 21 changes
record => { ... }formatting only; behavior remains the same.
76-128: Bulk formatting changes in record preparation look safe.Lines 78-127: only arrow-param formatting updates (
sub =>,message =>, etc.). No logic changes spotted.app/sagas/index.js (1)
18-40: No race condition exists here;quickActionsandroomVisitedhave defensive guards or no dependencies oninit()completion.The root saga's
yield all([...])pattern is safe:
- The
init(),quickActions(), androomVisited()generators are watcher sagas that register listeners and return immediately; they don't execute logic on startup.quickActionsincludes defensivewaitForAppReady()checks (lines 70, 80, 83) that block untilAPP.READYis dispatched before handling actions.roomVisitedis purely event-driven fromemitter.on('roomVisited')with no app state dependency.- The
initsaga only responds to anAPP.INITaction dispatched from the React component; actual initialization logic doesn't run until that action arrives.The parallel wiring is intentional and correct.
Likely an incorrect or invalid review comment.
ios/RocketChatRN-Bridging-Header.h (1)
13-13: The import ofQuickActionsStore.his correctly configured. The header exists, is properly exported viaFOUNDATION_EXPORT, and is already referenced multiple times in the project configuration. The import syntax matches existing working imports in the same bridging header.ios/QuickActionsConnector.h (1)
1-4: LGTM! Standard React Native bridge module header.The interface declaration follows React Native conventions for native module bridging. The static analysis error about the missing React header is a false positive—the React Native headers will be available during the Xcode build process.
app/sagas/init.js (1)
55-60: LGTM! Last visited room restoration logic is correct.The logic appropriately:
- Reads both room ID and name from UserPreferences
- Guards against partial data by checking both values exist
- Dispatches the action to restore state
app/lib/navigation/addServer/index.ts (1)
9-16: LGTM! Clean navigation helper with proper batching.The implementation correctly:
- Logs analytics events before navigation
- Uses
batch()to dispatch multiple actions atomically, preventing intermediate re-renders- Provides clear type annotations
app/reducers/rooms.ts (1)
51-56: LGTM! Reducer logic correctly stores last visited room.The reducer case properly:
- Spreads existing state
- Maps action payload to state fields
- Maintains immutability
app/definitions/redux/index.ts (1)
51-51: No review comment was provided for verification and rewriting. Please provide the review comment within<review_comment>tags so I can proceed with the verification and rewriting process.app/AppContainer.tsx (1)
69-72: NAVIGATION_READY dispatch on container ready looks good.
Keeps Redux in sync with navigation readiness and complements the existingemitter.emit('navigationReady').android/app/src/main/res/drawable/ic_quickaction_find.xml (1)
1-5: LGTM for a shortcut icon vector resource.app/i18n/locales/pt-BR.json (1)
322-322: This review comment identifies the wrong lines. Only line 827 contains punctuation in an i18n key (Something_Wrong?), but lines 447, 686-687, and 966 do not. Additionally, keys with?are already present throughout the codebase (22 keys in pt-BR.json alone, and similar patterns across all 25 locales), and the tooling (i18n-js 3.9.2) handles them without issue.Likely an incorrect or invalid review comment.
app/lib/methods/helpers/emitter.ts (1)
20-23: LGTM! Well-typed event addition.The new
roomVisitedevent is properly typed withridandnamefields, consistent with the existing event patterns in the emitter.ios/AppDelegate.swift (1)
46-53: LGTM! Correct iOS quick actions implementation.The quick action handling follows iOS best practices:
- Extracting the shortcut item from launch options and storing it via
RCSetPendingQuickActionType- Returning
falseindidFinishLaunchingWithOptionsto prevent iOS from invokingperformActionForagain- Properly implementing the
performActionForcallback with completion handlerAlso applies to: 90-97
app/actions/rooms.ts (1)
17-20: LGTM! Well-structured action additions.The new
IRoomsLastVisitedinterface androomsStoreLastVisitedaction creator are properly typed and consistent with existing action patterns in the file.Also applies to: 54-60
app/reducers/index.js (1)
29-30: LGTM! Clean reducer integration.The new
quickActionsanduireducers are properly imported and integrated into the root reducer using standard Redux patterns.Also applies to: 40-40, 43-43
app/lib/quickActions/index.ts (1)
22-53: Quick action item definitions look consistent (ids, i18n, platform icons).
Theidvalues align with saga handling (search,add-server,recent,contact), and usingrecentRoomNamefor the “recent” title is a nice touch.ios/RocketChatRN.xcodeproj/project.pbxproj (1)
13-16: QuickActions native files are correctly wired into sources (both app targets).
Looks consistent:.mmfiles are present inPBXSourcesBuildPhaseand headers are referenced.Also applies to: 477-481, 739-762, 2031-2085, 2308-2362
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (1)
104-146: Verify Expo quick-action intent contract against documentation or custom Expo module configuration.The code assumes a specific Intent action (
"expo.modules.quickactions.SHORTCUT") and Intent extra ("shortcut_data"asPersistableBundle) that cannot be confirmed against publicly available Expo Quick Actions documentation. Standard Expo Quick Actions patterns on Android useAction.paramsmapped to Intent extras viaShortcutManager, with primitives and package-namespaced keys—not a custom"expo.modules.quickactions.SHORTCUT"action or unprefixed bundle keys like"id"or"shortcutId".Confirm whether this Intent contract is defined in Rocket.Chat's Expo module configuration, custom Expo patches, or undocumented Expo behavior before relying on these assumptions.
| 1E1EA8082326CCE300E22452 /* ShellScript */ = { | ||
| isa = PBXShellScriptBuildPhase; | ||
| alwaysOutOfDate = 1; | ||
| buildActionMask = 2147483647; | ||
| files = ( | ||
| ); | ||
| inputFileListPaths = ( | ||
| ); | ||
| inputPaths = ( | ||
| ); | ||
| name = "[Expo] Configure project"; | ||
| outputFileListPaths = ( | ||
| ); | ||
| outputPaths = ( | ||
| ); | ||
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "# This script configures Expo modules and generates the modules provider file.\nbash -l -c \"./Pods/Target\\ Support\\ Files/Pods-defaults-RocketChatRN/expo-configure-project.sh\"\n"; | ||
| shellScript = "echo \"Target architectures: $ARCHS\"\n\nAPP_PATH=\"${TARGET_BUILD_DIR}/${WRAPPER_NAME}\"\n\nfind \"$APP_PATH\" -name '*.framework' -type d | while read -r FRAMEWORK\ndo\nFRAMEWORK_EXECUTABLE_NAME=$(defaults read \"$FRAMEWORK/Info.plist\" CFBundleExecutable)\nFRAMEWORK_EXECUTABLE_PATH=\"$FRAMEWORK/$FRAMEWORK_EXECUTABLE_NAME\"\necho \"Executable is $FRAMEWORK_EXECUTABLE_PATH\"\necho $(lipo -info \"$FRAMEWORK_EXECUTABLE_PATH\")\n\nFRAMEWORK_TMP_PATH=\"$FRAMEWORK_EXECUTABLE_PATH-tmp\"\n\n# remove simulator's archs if location is not simulator's directory\ncase \"${TARGET_BUILD_DIR}\" in\n*\"iphonesimulator\")\necho \"No need to remove archs\"\n;;\n*)\nif $(lipo \"$FRAMEWORK_EXECUTABLE_PATH\" -verify_arch \"i386\") ; then\nlipo -output \"$FRAMEWORK_TMP_PATH\" -remove \"i386\" \"$FRAMEWORK_EXECUTABLE_PATH\"\necho \"i386 architecture removed\"\nrm \"$FRAMEWORK_EXECUTABLE_PATH\"\nmv \"$FRAMEWORK_TMP_PATH\" \"$FRAMEWORK_EXECUTABLE_PATH\"\nfi\nif $(lipo \"$FRAMEWORK_EXECUTABLE_PATH\" -verify_arch \"x86_64\") ; then\nlipo -output \"$FRAMEWORK_TMP_PATH\" -remove \"x86_64\" \"$FRAMEWORK_EXECUTABLE_PATH\"\necho \"x86_64 architecture removed\"\nrm \"$FRAMEWORK_EXECUTABLE_PATH\"\nmv \"$FRAMEWORK_TMP_PATH\" \"$FRAMEWORK_EXECUTABLE_PATH\"\nfi\n;;\nesac\n\necho \"Completed for executable $FRAMEWORK_EXECUTABLE_PATH\"\necho $\n\ndone\n"; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the script in context
find . -name "project.pbxproj" -type f | head -5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 122
🏁 Script executed:
# Search for the specific shell script in the pbxproj file
rg "1E1EA8082326CCE300E22452" --type-list | head -5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 467
🏁 Script executed:
# Look for the file structure
ls -la ios/ 2>/dev/null | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1367
🏁 Script executed:
# Check if the pbxproj file exists and examine build phases
if [ -f "ios/RocketChatRN.xcodeproj/project.pbxproj" ]; then
wc -l "ios/RocketChatRN.xcodeproj/project.pbxproj"
echo "---"
# Try to extract lines around 1567-1583
sed -n '1560,1590p' "ios/RocketChatRN.xcodeproj/project.pbxproj"
fiRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 2364
🏁 Script executed:
# Search for similar shell scripts or lipo commands in the pbxproj
rg "lipo|ShellScript" ios/RocketChatRN.xcodeproj/project.pbxproj -A 2 -B 2 | head -50Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3171
🏁 Script executed:
# Check the Podfile for xcframeworks usage
cat ios/Podfile | head -50Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1613
🏁 Script executed:
# Search for any references or comments about this ShellScript or architecture stripping
rg "architecture|lipo|1E1EA8082326CCE300E22452" ios/ --type-listRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 467
🏁 Script executed:
# Look for CI configuration
find . -name ".github" -o -name ".gitlab-ci.yml" -o -name "*.yml" -o -name "*.yaml" | grep -E "(github|gitlab|circle|travis|action)" | head -10Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 485
🏁 Script executed:
# Search for any build configuration or documentation mentioning this
rg "ShellScript|strip.*arch" . -t md -t txt | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Search for references to architecture stripping or this build phase
rg "lipo|verify_arch" ios/ -A 2 -B 2Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1789
🏁 Script executed:
# Check the CI build configuration to understand if this is actually run
cat .github/actions/build-ios/action.yml | head -100Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3506
🏁 Script executed:
# Look for any other shell scripts in the project that might indicate modern approach
rg "use_frameworks|xcframework" ios/Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 127
🏁 Script executed:
# Search for any issues or TODOs related to architecture stripping
rg "TODO|FIXME|NOTE" ios/RocketChatRN.xcodeproj/ -iRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Check if there are any mentions of this approach in recent commits or discussions
git log --oneline ios/RocketChatRN.xcodeproj/project.pbxproj 2>/dev/null | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 129
🏁 Script executed:
# Let's verify if this script is actually being executed or if it's dead code
# Check if the build phase is referenced in the main target
rg "1E1EA8082326CCE300E22452" ios/RocketChatRN.xcodeproj/project.pbxproj -B 5 -A 5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1151
🏁 Script executed:
# Check for any modern architecture handling in CocoaPods or Xcode config
rg "ARCHS|VALID_ARCHS|arm64" ios/RocketChatRN.xcodeproj/project.pbxproj | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1739
🏁 Script executed:
# Verify the exact content of that trailing echo line
python3 << 'EOF'
import re
content = open('ios/RocketChatRN.xcodeproj/project.pbxproj', 'r').read()
# Extract the shellScript value
match = re.search(r'shellScript = "([^"]*echo \$[^"]*)"', content)
if match:
script = match.group(1)
# Decode escape sequences
script = script.encode().decode('unicode_escape')
lines = script.split('\n')
for i, line in enumerate(lines[-5:], len(lines)-4):
print(f"Line {i}: {repr(line)}")
EOFRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Let's decode the exact trailing line to confirm it's malformed
python3 << 'EOF'
# Read the pbxproj file
with open('ios/RocketChatRN.xcodeproj/project.pbxproj', 'r') as f:
content = f.read()
# Find the specific script
import re
# Look for the pattern with the echo $ at the end
match = re.search(r'echo "Completed for executable.*?\necho \$\n', content)
if match:
text = match.group(0)
print("Found trailing lines:")
print(repr(text))
print("\nDecoded:")
print(text)
else:
print("Pattern not found, searching differently...")
# Just look for the last few lines of that script
match = re.search(r'Completed for executable.*?done', content, re.DOTALL)
if match:
lines = match.group(0).split('\n')
for line in lines[-5:]:
print(repr(line))
EOFRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 197
🏁 Script executed:
# Verify if EXCLUDED_ARCHS is properly configured to handle architecture filtering
# This is the modern approach that should make the shell script unnecessary
rg "EXCLUDED_ARCHS.*iphonesimulator" ios/RocketChatRN.xcodeproj/project.pbxprojRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 231
🏁 Script executed:
# Check if this build phase is actually necessary by looking for framework handling in CocoaPods
cat ios/Podfile | grep -A 10 -B 10 "frameworks\|xcframework" || echo "No xcframework config found"Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 727
Remove this redundant and broken shell script build phase.
The script is malformed (trailing echo $ is incomplete), unnecessary, and potentially unsafe. The project already handles architecture filtering properly via EXCLUDED_ARCHS[sdk=iphonesimulator*] configuration, and modern CocoaPods with static framework linking handles framework embedding automatically. Direct lipo modifications on frameworks in the build directory can interfere with code signing and framework embedding, causing build failures especially with xcframeworks. Delete the entire 1E1EA8082326CCE300E22452 ShellScript build phase.
🤖 Prompt for AI Agents
In @ios/RocketChatRN.xcodeproj/project.pbxproj around lines 1567 - 1583, Remove
the entire PBXShellScriptBuildPhase entry with identifier
1E1EA8082326CCE300E22452 from the project file and delete any references to that
ID from target buildPhases arrays so the shell script is fully removed; ensure
you also remove its block from PBXBuildPhase lists (and any duplicate
occurrences) to avoid dangling references that would break the project.
|
The specified version ^6.0.0 targets Expo SDK 54, not SDK 53. For SDK 53 👀 |
yup, |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @app/lib/constants/keys.ts:
- Around line 28-29: Add clearing of LAST_VISITED_ROOM_ID_KEY and
LAST_VISITED_ROOM_NAME_KEY wherever server-specific keys are removed: update the
removeServerKeys() function to include LAST_VISITED_ROOM_ID_KEY and
LAST_VISITED_ROOM_NAME_KEY (referencing the constants LAST_VISITED_ROOM_ID_KEY
and LAST_VISITED_ROOM_NAME_KEY) so logout() clears them, and also ensure the
server-switch flow in selectServer.ts clears these same keys during the switch
(where other server keys are removed or reset) to prevent stale room data
persisting across logout or server changes.
In @app/sagas/quickActions.ts:
- Around line 87-91: sendEmail() is being invoked directly inside the saga (case
'contact'), breaking Redux-Saga patterns and error handling; replace the direct
call with a yield call(sendEmail) so the effect is yielded and testable, and
ensure call is imported from 'redux-saga/effects' (keep the existing yield
waitForAppReady() after the call).
- Around line 36-52: The second call to getRoom in waitForRoomInDB can throw
uncaught errors; wrap the final yield call in a try-catch (inside
waitForRoomInDB) and either return a clear failure value or rethrow a new Error
with context (e.g., `Failed to get room ${rid} after APP.START`) so callers get
a descriptive error; also remove the magic 3000 by introducing a configurable
timeout (e.g., add a timeoutMs parameter to waitForRoomInDB with a default
constant like ROOM_WAIT_TIMEOUT or read from config) and use that value in the
delay call instead of the hard-coded 3000.
In @ios/QuickActionsStore.mm:
- Around line 14-18: RCSetPendingQuickActionType currently uses dispatch_async
which can let callers (e.g.,
RCConsumePendingQuickActionType/getInitialQuickAction) run before the assignment
completes; change RCSetPendingQuickActionType to perform the assignment
synchronously on rcQuickActionQueue (use dispatch_barrier_sync or dispatch_sync
on rcQuickActionQueue) so RCPendingQuickActionType is set before control
returns; ensure you still use the same rcQuickActionQueue and keep
RCConsumePendingQuickActionType’s access on that queue to avoid race conditions.
🧹 Nitpick comments (5)
app/sagas/quickActions.ts (5)
18-25: Clarify why bothparamsandpayloadare needed.The interface accepts the same
actionfield in bothparamsandpayload. This dual structure adds complexity without clear benefit. Line 63 uses a fallback (action.params?.action ?? action.payload?.action), suggesting platform or entry-point differences, but this should be documented or refactored to use a single consistent structure.♻️ Consider standardizing on a single property
interface IQuickActionOpen extends Action { - params?: { - action?: string; - }; - payload?: { + payload: { action?: string; }; }Then update line 63:
-const quickAction = action.params?.action ?? action.payload?.action; +const quickAction = action.payload?.action;If both structures are required for platform compatibility, add a comment explaining why.
54-60: Consider wrapping the navigation ref check in a callable function.Directly accessing
Navigation.navigationRef.current(line 55) makes this saga harder to test. Wrapping this check in a helper function and calling it viacall()would improve testability and follow saga best practices.Example approach:
const isNavigationReady = () => !!Navigation.navigationRef.current; function* waitForNavigationReady(): Generator { const ready = yield call(isNavigationReady); if (ready) { return; } yield take(NAVIGATION.NAVIGATION_READY); }
77-86: Wrap Navigation calls incalleffects for testability.Direct calls to
Navigation.getCurrentRoute()(line 79) andNavigation.navigate()(line 82) make this saga harder to test and don't follow saga best practices. Wrapping these incall()enables mocking and improves testability.♻️ Use call effects
case 'search': { yield waitForAppReady(); - const currentRoute = Navigation.getCurrentRoute(); + const currentRoute = yield call([Navigation, Navigation.getCurrentRoute]); if (currentRoute?.name !== 'RoomsListView') { - Navigation.navigate('RoomsListView'); + yield call([Navigation, Navigation.navigate], 'RoomsListView'); } yield put({ type: UI.TRIGGER_SEARCH }); break; }
107-110: Replaceconsole.logwith proper error logging.Using
console.log(e)(line 108) in production is not ideal for debugging or monitoring. Use a structured logging utility or at leastconsole.errorfor error cases.} catch (e) { - console.log(e); + console.error('[QuickActions] Error loading recent room:', e); showRoomNotFoundError(); }
102-107: Consider makingisMasterDetaildynamic.The
goRoomcall (line 106) hardcodesisMasterDetail: true. If this value should depend on device type or layout, consider reading it from state or a helper function instead.If the hardcoded value is intentional for quick actions, add a comment explaining why.
📜 Review details
Configuration used: Organization 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.
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
app/AppContainer.tsxapp/lib/constants/keys.tsapp/lib/methods/helpers/goRoom.tsapp/lib/quickActions/index.tsapp/sagas/init.jsapp/sagas/quickActions.tsapp/sagas/roomVisited.tsios/QuickActionsConnector.mmios/QuickActionsStore.hios/QuickActionsStore.mmpackage.json
🚧 Files skipped from review as they are similar to previous changes (5)
- package.json
- app/sagas/init.js
- app/lib/quickActions/index.ts
- app/lib/methods/helpers/goRoom.ts
- app/sagas/roomVisited.ts
🧰 Additional context used
🧬 Code graph analysis (1)
app/AppContainer.tsx (4)
app/lib/hooks/useAppSelector.ts (1)
useAppSelector(6-6)app/definitions/redux/index.ts (1)
IApplicationState(53-81)app/lib/quickActions/index.ts (1)
updateQuickActions(14-45)app/actions/actionsTypes.ts (1)
NAVIGATION(64-64)
🪛 Clang (14.0.6)
ios/QuickActionsStore.h
[error] 1-1: 'Foundation/Foundation.h' file not found
(clang-diagnostic-error)
🔇 Additional comments (11)
ios/QuickActionsStore.h (1)
1-8: LGTM! Header follows Objective-C conventions.The header is clean and properly declares the exported functions with appropriate nullability annotations. The static analysis warning about Foundation/Foundation.h not being found is a false positive—this will resolve correctly in the actual Xcode build environment.
ios/QuickActionsStore.mm (2)
6-12: LGTM! Proper queue initialization.The constructor attribute correctly initializes the serial dispatch queue at load time, ensuring thread-safe access to the pending action type.
20-27: LGTM! Consume implementation is thread-safe.The use of
dispatch_syncwith__blockcorrectly ensures atomic read-and-clear of the pending action type. The consume-once pattern is appropriate for quick action handling.ios/QuickActionsConnector.mm (1)
8-15: The method's design is correct. Therejectparameter is intentionally unused because this method is designed to always resolve. The JavaScript side inapp/lib/quickActions/getRecentQuickAction.tscorrectly handles the resolution with the nullish coalescing operator (action ?? null) and has error handling via try-catch. No changes needed.app/AppContainer.tsx (5)
4-4: LGTM! Clean import additions.All new imports (useDispatch, quick actions utilities, types, and hooks) are properly used throughout the component and follow React/Redux best practices.
Also applies to: 6-6, 10-10, 23-24
40-41: LGTM! Proper Redux integration.The dispatch and typed selector are correctly initialized and follow Redux best practices. The selector properly accesses
state.rooms.lastVisitedNamewith full type safety.
43-49: Verify error handling for async quick actions registration.
registerQuickActions()appears to be async but lacks error handling. If registration fails (e.g., permissions, API errors), the promise rejection will be unhandled.Consider adding error handling:
🛡️ Proposed error handling
useEffect(() => { - registerQuickActions(); + registerQuickActions().catch(error => { + console.warn('Failed to register quick actions:', error); + }); return () => { - unregisterQuickActions(); + unregisterQuickActions().catch(error => { + console.warn('Failed to unregister quick actions:', error); + }); }; }, []);Verify whether the quick actions library requires explicit error handling or if it handles failures gracefully internally.
60-62: Verify error handling for async quick actions updates.
updateQuickActions()is async but lacks error handling. If the update fails, the promise rejection will be unhandled.🛡️ Proposed error handling
useEffect(() => { - updateQuickActions({ recentRoomName: lastVisitedRoomName }); + updateQuickActions({ recentRoomName: lastVisitedRoomName }).catch(error => { + console.warn('Failed to update quick actions:', error); + }); }, [lastVisitedRoomName]);Verify whether the quick actions library gracefully handles update failures or if explicit error handling is necessary.
76-76: LGTM! Navigation ready action dispatched correctly.The
NAVIGATION_READYaction is properly dispatched when navigation is ready, complementing the existing event emitter pattern. This enables Redux-based handling of navigation readiness.app/sagas/quickActions.ts (2)
27-34: LGTM!The app readiness check logic is correct, properly handling both the ready state and initialization flow.
119-125: LGTM!The error alert helper and root saga are correctly implemented. The use of
takeEveryis appropriate for handling multiple quick action triggers.
| export const LAST_VISITED_ROOM_ID_KEY = 'RC_LAST_VISITED_ROOM_ID_KEY'; | ||
| export const LAST_VISITED_ROOM_NAME_KEY = 'RC_LAST_VISITED_ROOM_NAME_KEY'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify usage of LAST_VISITED_ROOM constants and cleanup handling
echo "=== Searching for LAST_VISITED_ROOM key usage ==="
rg -n 'LAST_VISITED_ROOM_(ID|NAME)_KEY' --type ts --type js -C 3
echo -e "\n=== Searching for logout/cleanup handlers ==="
rg -n '(logout|clearUser|removeUser|serverChange|switchServer|reset)' app/sagas/ -C 2 --type ts --type js
echo -e "\n=== Checking init.js for cleanup logic ==="
rg -n 'localStorage|sessionStorage|removeItem|clear' app/sagas/ --type ts --type jsRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 8675
🏁 Script executed:
#!/bin/bash
# Check logout implementation
echo "=== Checking logout function implementation ==="
cat -n app/lib/methods/logout.ts 2>/dev/null || cat -n app/lib/methods/logout.js 2>/dev/null
echo -e "\n=== Checking selectServer.ts for server switch cleanup ==="
rg -n 'LAST_VISITED_ROOM|clearUser|removeItem' app/sagas/selectServer.ts -B 2 -A 2
echo -e "\n=== Checking UserPreferences implementation ==="
fd -t f UserPreferences && cat -n app/lib/methods/userPreferences.ts 2>/dev/null | head -100
echo -e "\n=== Checking if logout clears UserPreferences ==="
rg -n 'removeItem|clear|removeString' app/lib/methods/ -C 2 --type ts --type js | head -50Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 10973
Add cleanup of LAST_VISITED_ROOM keys on logout and server switch.
The constants are well-named and follow established conventions. However, the stored data is not being cleared on logout or server switch:
logout()inapp/lib/methods/logout.tscallsremoveServerKeys()which doesn't include these keys- Server switch in
app/sagas/selectServer.tsdoesn't clear them either - This leaves stale room data persisting across logout/server switch, causing incorrect quick actions for the next user/server
Good news: UserPreferences uses MMKV with encryption (Android Keystore/iOS Keychain), so the data is stored securely.
To fix: Add LAST_VISITED_ROOM_ID_KEY and LAST_VISITED_ROOM_NAME_KEY to the cleanup in removeServerKeys() function and on server switch in selectServer.ts.
🤖 Prompt for AI Agents
In @app/lib/constants/keys.ts around lines 28 - 29, Add clearing of
LAST_VISITED_ROOM_ID_KEY and LAST_VISITED_ROOM_NAME_KEY wherever server-specific
keys are removed: update the removeServerKeys() function to include
LAST_VISITED_ROOM_ID_KEY and LAST_VISITED_ROOM_NAME_KEY (referencing the
constants LAST_VISITED_ROOM_ID_KEY and LAST_VISITED_ROOM_NAME_KEY) so logout()
clears them, and also ensure the server-switch flow in selectServer.ts clears
these same keys during the switch (where other server keys are removed or reset)
to prevent stale room data persisting across logout or server changes.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
2 participants
Proposed changes
This pull request introduces Quick Actions support to the app, including full support when the app is launched from a killed state on both iOS and Android.
It adds the complete Quick Actions flow end-to-end, enabling users to quickly access common actions such as Search, Add Server, Recent Rooms, and Contact us directly from the app icon. Platform-specific handling is implemented to ensure reliable behavior during cold starts.
Issue(s)
closes: #1705
closes: #6227
Approach for Killed state
iOS
Android
How to test or reproduce
Screenshots
Quick Actions
Search
Add Server
Recent rooms
Contact
Types of changes
Checklist
Further comments
the quick actions icons on android are temporary and should be changed later