refactor: migrate TeamChannelsView to hooks and extract list/permission logic#7446
refactor: migrate TeamChannelsView to hooks and extract list/permission logic#7446diegolmello wants to merge 4 commits into
Conversation
Convert app/views/TeamChannelsView from a class component wrapped in connect/withTheme/withActionSheet/withDimensions/withMasterDetail HOCs to a function component using hooks (useAppSelector + shallowEqual, useTheme, useActionSheet, useWindowDimensions, useMasterDetail, useAppNavigation/ useAppRoute, useReducer). Behavior preserved; drops genuinely dead code (hasDeletePermission and unused delete-permission selectors). Adds unit tests covering mount fetch, not-found handling, pagination, search, auto-join toggle, remove, delete, and permission gating. Claude-Session: https://claude.ai/code/session_01LaynJoHJn3m7Zc5Dj8d5te
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
WalkthroughThe PR refactors team channel and permission logic into hooks, adds coverage for permission resolution and list/search behavior, and rewires the team channels screen and Add Channel Team view to use the new helpers. ChangesTeam channels refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/TeamChannelsView.tsx (1)
402-426: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate the active search list after room mutations.
Both success paths write only
data; when the action is triggered from search results, FlatList renderssearch, so toggled/removed rooms stay stale on screen.Proposed fix
- const { data } = stateRef.current; + const { data, search } = stateRef.current; const result = await updateTeamRoom({ roomId: item._id, isDefault: !item.teamDefault }); if (result.success) { - const newData = data.map(i => { - if (i._id === item._id) { - i.teamDefault = !i.teamDefault; - } - return i; - }); - updateState({ data: newData }); + const updateRoom = (room: IItem): IItem => + room._id === item._id ? { ...room, teamDefault: !room.teamDefault } : room; + updateState({ data: data.map(updateRoom), search: search.map(updateRoom) }); }- const { data, team } = stateRef.current; + const { data, search, team } = stateRef.current; const result = await removeTeamRoom({ roomId: item._id, teamId: team?.teamId as string }); if (result.success) { - const newData = data.filter(room => result.room._id !== room._id); - updateState({ data: newData }); + const filterRemovedRoom = (room: IItem): boolean => result.room._id !== room._id; + updateState({ data: data.filter(filterRemovedRoom), search: search.filter(filterRemovedRoom) }); }🤖 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 `@app/views/TeamChannelsView.tsx` around lines 402 - 426, The mutation handlers in TeamChannelsView.tsx update only the main data list, so search results stay stale when the FlatList is rendering from search state. Update the success paths in the room toggle/remove flows to also apply the same change to the search list in stateRef.current (or derive both from a shared update helper), and use the existing TeamChannelsView state/updateState logic so the active search results stay in sync after updateTeamRoom and removeTeamRoom complete.
🤖 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 `@app/views/TeamChannelsView.test.tsx`:
- Around line 239-241: The test setup in TeamChannelsView.test.tsx only clears
mock call history, so per-test implementations like
mockShowActionSheet.mockImplementation can leak into later tests. Update the
beforeEach in this test suite to fully reset mock implementations/state instead
of just calling jest.clearAllMocks(), and keep the change localized to the
affected mocks used by TeamChannelsView tests.
- Around line 500-517: The TeamChannelsView tests only verify that setOptions is
called, so they don’t confirm the create button is actually gated by permission.
Update the two cases in TeamChannelsView.test.tsx to assert the resulting
options or rendered UI reflect showCreate being true when mockHasPermission
resolves true and false when it resolves false, using TeamChannelsView and
mockSetOptions as the key hooks for the check.
In `@app/views/TeamChannelsView.tsx`:
- Line 471: The delete action in TeamChannelsView is always dispatching
ERoomType.c, which can send the wrong roomType through the delete flow for
non-channel rooms. Update the onPress handler in TeamChannelsView so it passes
the actual room type for the selected item instead of hardcoding ERoomType.c,
using the item data already available at the deleteRoom dispatch call.
- Around line 241-249: `goRoomActionsView` in `TeamChannelsView` currently
relies on an optional `screen` argument, but `RoomHeader` calls `onPress`
without one so the master-detail branch never executes. Update the
`goRoomActionsView` callback (and the `RoomHeader` wiring that invokes it) so
master-detail presses are routed through the modal navigation path even when no
`screen` is provided, using the existing `ModalStackNavigator`/`RoomHeader`
symbols to locate the flow.
- Around line 480-536: showChannelActions currently awaits permission checks
without local error handling, so a rejected hasPermission call can escape when
RoomItem triggers onLongPress(item). Wrap the async body of showChannelActions
in a try/catch, keep the existing action-sheet construction logic for
editTeamChannelPermission, removeTeamChannelPermission, and
deleteCPermission/deletePPermission, and in the catch handle/log the failure
consistently with the surrounding TeamChannelsView patterns before returning
without showing the sheet.
---
Outside diff comments:
In `@app/views/TeamChannelsView.tsx`:
- Around line 402-426: The mutation handlers in TeamChannelsView.tsx update only
the main data list, so search results stay stale when the FlatList is rendering
from search state. Update the success paths in the room toggle/remove flows to
also apply the same change to the search list in stateRef.current (or derive
both from a shared update helper), and use the existing TeamChannelsView
state/updateState logic so the active search results stay in sync after
updateTeamRoom and removeTeamRoom complete.
🪄 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: 4d5b0132-465b-4430-911f-218d6330cb63
📒 Files selected for processing (2)
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
**/*.{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 numbersUse TypeScript with strict mode enabled
Files:
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
🧠 Learnings (4)
📚 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/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/views/TeamChannelsView.test.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reset per-test mock implementations, not just call history.
jest.clearAllMocks() leaves implementations from tests like mockShowActionSheet.mockImplementation(...) in place, making later tests order-dependent.
Proposed fix
beforeEach(() => {
jest.clearAllMocks();
+ mockShowActionSheet.mockReset();
+ mockFetch.mockReset();
+ mockGetTeamListRoom.mockReset();
+ mockGetRoomInfo.mockReset();
+ mockUpdateTeamRoom.mockReset();
+ mockRemoveTeamRoom.mockReset();
+ mockHasPermission.mockReset();
});
+
+ afterEach(() => {
+ jest.restoreAllMocks();
+ });📝 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.
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| mockShowActionSheet.mockReset(); | |
| mockFetch.mockReset(); | |
| mockGetTeamListRoom.mockReset(); | |
| mockGetRoomInfo.mockReset(); | |
| mockUpdateTeamRoom.mockReset(); | |
| mockRemoveTeamRoom.mockReset(); | |
| mockHasPermission.mockReset(); | |
| }); | |
| afterEach(() => { | |
| jest.restoreAllMocks(); | |
| }); |
🤖 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 `@app/views/TeamChannelsView.test.tsx` around lines 239 - 241, The test setup
in TeamChannelsView.test.tsx only clears mock call history, so per-test
implementations like mockShowActionSheet.mockImplementation can leak into later
tests. Update the beforeEach in this test suite to fully reset mock
implementations/state instead of just calling jest.clearAllMocks(), and keep the
change localized to the affected mocks used by TeamChannelsView tests.
Replace manual memoization with the React Compiler's automatic memoization via the 'use memo' directive. Removes all 11 useCallback wrappers and the stateRef latest-value pattern. To let the compiler actually engage, restructure both effects to honest deps (no exhaustive-deps suppression), replace a throw-in-try/catch with an early return, and lift the permission ternaries out of loadTeam's try block. stateRef is dropped entirely: use-debounce already invokes the latest closure behind a stable identity, so no ref or extra effect is needed. Stabilize the selector's empty-permission fallback with a shared module-level constant so absent permissions no longer produce fresh array references; this keeps the mount effect from re-firing on unrelated Redux dispatches. Claude-Session: https://claude.ai/code/session_01LaynJoHJn3m7Zc5Dj8d5te
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
There was a problem hiding this comment.
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 `@app/views/TeamChannelsView.tsx`:
- Around line 393-399: The room state updates in toggleAutoJoin and removeRoom
are mutating items in place and only updating the data list, which leaves search
stale. Update the matching room immutably in the TeamChannelsView state helpers
by returning new objects from the map/filter logic, and apply the same change to
both data and search so the two room lists stay in sync after mutations.
🪄 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: 7b5758ac-1357-4d5b-bee8-3950071a02a5
📒 Files selected for processing (2)
app/views/TeamChannelsView.test.tsxapp/views/TeamChannelsView.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/TeamChannelsView.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/views/TeamChannelsView.tsx
**/*.{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 numbersUse TypeScript with strict mode enabled
Files:
app/views/TeamChannelsView.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/views/TeamChannelsView.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/TeamChannelsView.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/TeamChannelsView.tsx
🧠 Learnings (3)
📚 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/views/TeamChannelsView.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/TeamChannelsView.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/views/TeamChannelsView.tsx
🔇 Additional comments (6)
app/views/TeamChannelsView.tsx (6)
251-270: Duplicate: master-detail header press still depends on an argumentRoomHeaderdoes not pass.This appears to be the same previously flagged issue:
isMasterDetail && screenkeeps the modal path unreachable for header presses whenRoomHeadercallsonPresswithout a screen argument.
457-457: Duplicate: delete still dispatches the channel room type unconditionally.This matches the previous finding: private team rooms still go through
deleteRoom(ERoomType.c, item)instead of using the selected item’s actual room type.
464-518: Duplicate: action-sheet permission failures still escape the long-press path.This is the same previously raised async error-handling issue:
showChannelActionsawaits permission checks without localtry/catch, whileRoomIteminvokesonLongPress(item)without awaiting it.
5-5: LGTM!Also applies to: 36-37, 110-117, 133-168
309-358: LGTM!Also applies to: 424-441, 521-572
213-223: 🎯 Functional CorrectnessNo change needed.
useDebounce/useDebouncedCallbackkeeps the latest callback, soload()will see the updatedsearchTextafter the state update; passing astateOverrideisn’t necessary here.> Likely an incorrect or invalid review comment.
…sView
Post-review polish on the React Compiler adoption:
- toggleAutoJoin now produces a new item object (`{ ...i, teamDefault }`)
instead of mutating the existing one in place. In-place mutation is a
Rules-of-React violation the compiler's memoization assumes away, and
fresh references are what FlatList + RoomItem need to re-render reliably.
- Rename a local `showCreate` in loadTeam that shadowed the state field of
the same name.
- Document why `load` may read component state directly from its render
closure: use-debounce forwards the latest committed closure via a ref.
Claude-Session: https://claude.ai/code/session_01LaynJoHJn3m7Zc5Dj8d5te
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
…into hooks
Builds on the class->function + React Compiler migration of TeamChannelsView
with three behavior-preserving extractions:
- useTeamChannels(teamId): view-local paginated browse/search list engine.
Retains separate browse and search buckets so cancel-search restores the
browse list without a refetch.
- useTeamChannelPermissions (shared): reactive create-permission hooks built
on usePermissions; AddChannelTeamView consumes them instead of defining the
logic locally. showCreate is now derived from the hook rather than useState.
- getChannelActionPermissions: pure async channel-action permission policy;
the three checks now run in parallel via Promise.all instead of serially.
Slims the mount effect so it no longer re-runs on permission-slice ref
changes (deps now [teamId, navigation]). Also fixes a latent bug in
usePermissions' useSubscriptionRoles: deps [] -> [rid] so it re-subscribes
when the room id arrives asynchronously, guarded by a {rid, roles} state to
avoid stale-role leaks across rid changes.
Claude-Session: https://claude.ai/code/session_01LaynJoHJn3m7Zc5Dj8d5te
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
changed the title
diegolmello
requested a deployment
to
android_build
GitHub Actions
Waiting
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/lib/hooks/usePermissions.ts (1)
29-49: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the async subscription against stale/unmounted resolution.
getSubscriptionByRoomId(rid)is awaited inside the effect, but the cleanup only unsubscribessubSubscription. Ifridchanges (or the component unmounts) before the promise resolves, the cleanup runs whilesubSubscriptionis stillundefined, so the subscription created afterward is never unsubscribed and can callsetRolesByRidfor a now-stalerid. The new[rid]dependency makes this path reachable on everyridtransition.The trailing return guard avoids leaking stale roles to consumers, but not the leaked subscription/late state update.
🔒 Proposed cancellation guard
useEffect(() => { if (!rid) { return; } + let cancelled = false; let subSubscription: Subscription; getSubscriptionByRoomId(rid).then(sub => { - if (!sub) { + if (cancelled || !sub) { return; } const observable = sub.observe(); subSubscription = observable.subscribe(s => { const newRoles = orderBy(s.roles); if (rolesByRidRef.current.rid !== rid || !dequal(rolesByRidRef.current.roles, newRoles)) { rolesByRidRef.current = { rid, roles: newRoles }; setRolesByRid(rolesByRidRef.current); } }); }); return () => { + cancelled = true; if (subSubscription && subSubscription?.unsubscribe) { subSubscription.unsubscribe(); } }; }, [rid]);🤖 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 `@app/lib/hooks/usePermissions.ts` around lines 29 - 49, The async subscription in usePermissions.useEffect can resolve after rid changes or unmount, leaving subSubscription undefined during cleanup and allowing a stale observer to keep calling setRolesByRid. Add a cancellation/mounted guard inside the effect around getSubscriptionByRoomId, and ensure the cleanup marks the effect as cancelled and unsubscribes any late-created subscription using the existing subSubscription and rolesByRidRef logic. Make the fix in the useEffect that depends on [rid] so late resolutions are ignored and no stale subscription survives a rid transition.app/views/TeamChannelsView/index.tsx (1)
218-224: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
e.data.errorcan throw on non-API errors.If
getRoomInforejects with an error lacking adatafield (network/runtime error),e.data.errorthrows inside the catch, masking the original failure. Consider optional chaining.🛡️ Proposed guard
- } catch (e: any) { - if (e.data.error === 'not-allowed') { + } catch (e: any) { + if (e?.data?.error === 'not-allowed') { showErrorAlert(I18n.t('error-not-allowed')); } else { - showErrorAlert(e.data.error); + showErrorAlert(e?.data?.error); } }🤖 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 `@app/views/TeamChannelsView/index.tsx` around lines 218 - 224, The catch block in TeamChannelsView’s getRoomInfo error handling assumes every error has e.data.error, which can throw again for network/runtime failures and hide the real issue. Update the catch logic to safely inspect the error using optional chaining or equivalent guards before comparing to 'not-allowed', and fall back to a generic message when the nested error field is missing. Keep the handling inside the existing catch in index.tsx and preserve the current I18n.t('error-not-allowed') path for the allowed case.
🧹 Nitpick comments (3)
app/lib/hooks/useTeamChannelPermissions.ts (1)
8-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd explicit return types to the exported hooks.
useCreateNewPermission,useAddExistingPermission, anduseCanCreateTeamChannelare exported but rely on inferred return types. Annotating them (allboolean) makes the public contract explicit.As per coding guidelines: "add explicit type annotations to function parameters and return types".
🤖 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 `@app/lib/hooks/useTeamChannelPermissions.ts` around lines 8 - 36, The exported hooks use inferred return types, but they should explicitly declare their public contract as boolean. Update useCreateNewPermission, useAddExistingPermission, and useCanCreateTeamChannel in useTeamChannelPermissions to add explicit boolean return type annotations while keeping their current logic unchanged; this should align with the coding guideline for explicit parameter and return type annotations.Source: Coding guidelines
app/views/TeamChannelsView/useTeamChannels.test.ts (1)
163-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a mutation test while searching.
updateItem/removeItemare only covered in browse mode. A case that mutates an item whileisSearchingis true would catch the search-bucket staleness flagged inuseTeamChannels.ts.🤖 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 `@app/views/TeamChannelsView/useTeamChannels.test.ts` around lines 163 - 179, Add a mutation-focused test for useTeamChannels that exercises updateItem and removeItem while isSearching is true, not just in browse mode. Extend the existing useTeamChannels test setup to cover the search bucket state, then verify the matching item in the search results is patched and removed correctly after calling updateItem and removeItem so stale search state is caught.app/views/TeamChannelsView/useTeamChannels.ts (1)
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial
_idshould be typed asstring, notERoomType.The
_idfield holds a room identifier (e.g.,'r1'), whereasERoomTypeenumerates room types ('group','channel', etc.). Typing_idasERoomTypeis incorrect and forces unnecessaryas anycasts in tests. Change the type tostringto restore type safety.Current code
export interface IItem { _id: ERoomType;🤖 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 `@app/views/TeamChannelsView/useTeamChannels.ts` around lines 12 - 13, The IItem interface currently types _id as ERoomType, but _id is a room identifier string rather than a room type. Update IItem in useTeamChannels to use string for _id, and remove any downstream casts or assumptions that depend on ERoomType so TeamChannelsView and related tests can work with real room IDs.
🤖 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 `@app/views/TeamChannelsView/channelActionPermissions.ts`:
- Around line 23-33: The permission check results from hasPermission can be
undefined, which makes the direct array reads in channelActionPermissions.ts
unsafe. Update the caller logic in showChannelActions to use guarded access when
deriving canAutoJoin, canRemove, and canDelete, so a failed hasPermission call
falls back safely instead of throwing. Keep the Promise.all flow, but make the
result reads defensive with a default false-like fallback.
In `@app/views/TeamChannelsView/useTeamChannels.ts`:
- Around line 139-145: The `updateItem` and `removeItem` helpers in
`useTeamChannels` only update `prev.data`, so the active `search` list can go
stale while `isSearching` is true. Update both the browse bucket and the search
bucket inside these helpers so the visible `list` stays in sync when a channel
is patched or removed, using the existing `updateState`, `updateItem`, and
`removeItem` logic as the entry points.
---
Outside diff comments:
In `@app/lib/hooks/usePermissions.ts`:
- Around line 29-49: The async subscription in usePermissions.useEffect can
resolve after rid changes or unmount, leaving subSubscription undefined during
cleanup and allowing a stale observer to keep calling setRolesByRid. Add a
cancellation/mounted guard inside the effect around getSubscriptionByRoomId, and
ensure the cleanup marks the effect as cancelled and unsubscribes any
late-created subscription using the existing subSubscription and rolesByRidRef
logic. Make the fix in the useEffect that depends on [rid] so late resolutions
are ignored and no stale subscription survives a rid transition.
In `@app/views/TeamChannelsView/index.tsx`:
- Around line 218-224: The catch block in TeamChannelsView’s getRoomInfo error
handling assumes every error has e.data.error, which can throw again for
network/runtime failures and hide the real issue. Update the catch logic to
safely inspect the error using optional chaining or equivalent guards before
comparing to 'not-allowed', and fall back to a generic message when the nested
error field is missing. Keep the handling inside the existing catch in index.tsx
and preserve the current I18n.t('error-not-allowed') path for the allowed case.
---
Nitpick comments:
In `@app/lib/hooks/useTeamChannelPermissions.ts`:
- Around line 8-36: The exported hooks use inferred return types, but they
should explicitly declare their public contract as boolean. Update
useCreateNewPermission, useAddExistingPermission, and useCanCreateTeamChannel in
useTeamChannelPermissions to add explicit boolean return type annotations while
keeping their current logic unchanged; this should align with the coding
guideline for explicit parameter and return type annotations.
In `@app/views/TeamChannelsView/useTeamChannels.test.ts`:
- Around line 163-179: Add a mutation-focused test for useTeamChannels that
exercises updateItem and removeItem while isSearching is true, not just in
browse mode. Extend the existing useTeamChannels test setup to cover the search
bucket state, then verify the matching item in the search results is patched and
removed correctly after calling updateItem and removeItem so stale search state
is caught.
In `@app/views/TeamChannelsView/useTeamChannels.ts`:
- Around line 12-13: The IItem interface currently types _id as ERoomType, but
_id is a room identifier string rather than a room type. Update IItem in
useTeamChannels to use string for _id, and remove any downstream casts or
assumptions that depend on ERoomType so TeamChannelsView and related tests can
work with real room IDs.
🪄 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: 1ba5e9d7-39f8-491d-80af-27ee4fe32646
📒 Files selected for processing (11)
app/lib/hooks/usePermissions.test.tsxapp/lib/hooks/usePermissions.tsapp/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/useTeamChannelPermissions.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/channelActionPermissions.test.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/views/TeamChannelsView/index.tsxapp/views/TeamChannelsView/useTeamChannels.test.tsapp/views/TeamChannelsView/useTeamChannels.ts
✅ Files skipped from review due to trivial changes (1)
- app/views/TeamChannelsView/channelActionPermissions.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/views/TeamChannelsView/useTeamChannels.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/usePermissions.test.tsxapp/lib/hooks/useTeamChannelPermissions.tsapp/views/TeamChannelsView/useTeamChannels.test.tsapp/lib/hooks/usePermissions.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
**/*.{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 numbersUse TypeScript with strict mode enabled
Files:
app/views/TeamChannelsView/useTeamChannels.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/usePermissions.test.tsxapp/lib/hooks/useTeamChannelPermissions.tsapp/views/TeamChannelsView/useTeamChannels.test.tsapp/lib/hooks/usePermissions.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/views/TeamChannelsView/useTeamChannels.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/usePermissions.test.tsxapp/lib/hooks/useTeamChannelPermissions.tsapp/views/TeamChannelsView/useTeamChannels.test.tsapp/lib/hooks/usePermissions.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/TeamChannelsView/useTeamChannels.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/usePermissions.test.tsxapp/lib/hooks/useTeamChannelPermissions.tsapp/views/TeamChannelsView/useTeamChannels.test.tsapp/lib/hooks/usePermissions.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/TeamChannelsView/useTeamChannels.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/views/TeamChannelsView/useTeamChannels.test.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
🧠 Learnings (4)
📚 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/views/TeamChannelsView/useTeamChannels.tsapp/views/TeamChannelsView/channelActionPermissions.tsapp/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/usePermissions.test.tsxapp/lib/hooks/useTeamChannelPermissions.tsapp/views/TeamChannelsView/useTeamChannels.test.tsapp/lib/hooks/usePermissions.tsapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/lib/hooks/useTeamChannelPermissions.test.tsapp/lib/hooks/usePermissions.test.tsxapp/views/TeamChannelsView/useTeamChannels.test.tsapp/views/TeamChannelsView/TeamChannelsView.test.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/lib/hooks/usePermissions.test.tsxapp/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/AddChannelTeamView.tsxapp/views/TeamChannelsView/TeamChannelsView.test.tsxapp/views/TeamChannelsView/index.tsx
🔇 Additional comments (7)
app/lib/hooks/usePermissions.test.tsx (1)
1-80: LGTM!app/lib/hooks/useTeamChannelPermissions.test.ts (1)
1-139: LGTM!app/views/AddChannelTeamView.tsx (1)
10-28: LGTM!app/views/TeamChannelsView/useTeamChannels.ts (1)
65-104: LGTM!app/views/TeamChannelsView/useTeamChannels.test.ts (1)
39-161: LGTM!app/views/TeamChannelsView/index.tsx (1)
384-409: LGTM!app/views/TeamChannelsView/TeamChannelsView.test.tsx (1)
509-538: LGTM!
| const [editResult, removeResult, deleteResult] = await Promise.all([ | ||
| hasPermission([perms.edit], team.rid), | ||
| hasPermission([perms.remove], team.rid), | ||
| hasPermission([item.t === 'c' ? perms.deleteC : perms.deleteP], item._id) | ||
| ]); | ||
|
|
||
| return { | ||
| canAutoJoin: editResult[0], | ||
| canRemove: removeResult[0], | ||
| canDelete: deleteResult[0] | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep --pattern 'export async function hasPermission($$$) { $$$ }' --lang typescript app/lib/methods/helpers/helpers.tsRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 172
🏁 Script executed:
rg -A 30 "export.*function hasPermission" app/lib/methods/helpers/helpers.ts --type tsRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 927
🏁 Script executed:
cat app/lib/methods/helpers/helpers.tsRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3689
Defensive read against hasPermission returning undefined.
Verified in app/lib/methods/helpers/helpers.ts: the hasPermission function has a catch block that logs the error but does not return a value, resulting in an implicit undefined. If this path is taken, the destructuring editResult[0] in the caller throws a runtime error since showChannelActions lacks a surrounding try/catch.
Apply a guarded read to prevent a potential crash:
Proposed fix
return {
- canAutoJoin: editResult[0],
- canRemove: removeResult[0],
- canDelete: deleteResult[0]
+ canAutoJoin: editResult?.[0] ?? false,
+ canRemove: removeResult?.[0] ?? false,
+ canDelete: deleteResult?.[0] ?? false
};📝 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.
| const [editResult, removeResult, deleteResult] = await Promise.all([ | |
| hasPermission([perms.edit], team.rid), | |
| hasPermission([perms.remove], team.rid), | |
| hasPermission([item.t === 'c' ? perms.deleteC : perms.deleteP], item._id) | |
| ]); | |
| return { | |
| canAutoJoin: editResult[0], | |
| canRemove: removeResult[0], | |
| canDelete: deleteResult[0] | |
| }; | |
| return { | |
| canAutoJoin: editResult?.[0] ?? false, | |
| canRemove: removeResult?.[0] ?? false, | |
| canDelete: deleteResult?.[0] ?? false | |
| }; |
🤖 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 `@app/views/TeamChannelsView/channelActionPermissions.ts` around lines 23 - 33,
The permission check results from hasPermission can be undefined, which makes
the direct array reads in channelActionPermissions.ts unsafe. Update the caller
logic in showChannelActions to use guarded access when deriving canAutoJoin,
canRemove, and canDelete, so a failed hasPermission call falls back safely
instead of throwing. Keep the Promise.all flow, but make the result reads
defensive with a default false-like fallback.
| const updateItem = (id: IItem['_id'], patch: Partial<IItem>) => { | ||
| updateState(prev => ({ data: prev.data.map(item => (item._id === id ? { ...item, ...patch } : item)) })); | ||
| }; | ||
|
|
||
| const removeItem = (id: string) => { | ||
| updateState(prev => ({ data: prev.data.filter(item => item._id !== id) })); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚖️ Poor tradeoff
updateItem/removeItem only mutate the browse bucket, so list mutations are lost while searching.
The returned list is isSearching ? search : data, but both updateItem and removeItem patch/filter only prev.data. When a user long-presses a row during an active search and toggles auto-join or removes the channel, the visible search list won't reflect the change until search is cancelled. The auto-join icon stays stale and a removed channel remains on screen.
Consider patching both buckets so the active list stays consistent.
♻️ Proposed fix
const updateItem = (id: IItem['_id'], patch: Partial<IItem>) => {
- updateState(prev => ({ data: prev.data.map(item => (item._id === id ? { ...item, ...patch } : item)) }));
+ updateState(prev => ({
+ data: prev.data.map(item => (item._id === id ? { ...item, ...patch } : item)),
+ search: prev.search.map(item => (item._id === id ? { ...item, ...patch } : item))
+ }));
};
const removeItem = (id: string) => {
- updateState(prev => ({ data: prev.data.filter(item => item._id !== id) }));
+ updateState(prev => ({
+ data: prev.data.filter(item => item._id !== id),
+ search: prev.search.filter(item => item._id !== id)
+ }));
};📝 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.
| const updateItem = (id: IItem['_id'], patch: Partial<IItem>) => { | |
| updateState(prev => ({ data: prev.data.map(item => (item._id === id ? { ...item, ...patch } : item)) })); | |
| }; | |
| const removeItem = (id: string) => { | |
| updateState(prev => ({ data: prev.data.filter(item => item._id !== id) })); | |
| }; | |
| const updateItem = (id: IItem['_id'], patch: Partial<IItem>) => { | |
| updateState(prev => ({ | |
| data: prev.data.map(item => (item._id === id ? { ...item, ...patch } : item)), | |
| search: prev.search.map(item => (item._id === id ? { ...item, ...patch } : item)) | |
| })); | |
| }; | |
| const removeItem = (id: string) => { | |
| updateState(prev => ({ | |
| data: prev.data.filter(item => item._id !== id), | |
| search: prev.search.filter(item => item._id !== id) | |
| })); | |
| }; |
🤖 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 `@app/views/TeamChannelsView/useTeamChannels.ts` around lines 139 - 145, The
`updateItem` and `removeItem` helpers in `useTeamChannels` only update
`prev.data`, so the active `search` list can go stale while `isSearching` is
true. Update both the browse bucket and the search bucket inside these helpers
so the visible `list` stays in sync when a channel is patched or removed, using
the existing `updateState`, `updateItem`, and `removeItem` logic as the entry
points.
1 participant
Proposed changes
Migrates
app/views/TeamChannelsViewfrom a class component to a function component, then extracts its logic into focused, testable units.Migration: the class was wrapped in five HOCs (
connect,withTheme,withActionSheet,withDimensions,withMasterDetail), now replaced with the equivalent hooks (useAppSelector+shallowEqual,useTheme,useActionSheet,useWindowDimensions,useMasterDetail) plususeAppNavigation/useAppRoute. The component opts into React Compiler auto-memoization via a'use memo'directive.Extractions (behavior-preserving):
useTeamChannels(teamId)— view-local paginated browse/search list engine. It keeps separate browse and search buckets so cancelling a search restores the browse list without re-fetching.useTeamChannelPermissions(shared,app/lib/hooks) — reactive create-permission hooks built onusePermissions.AddChannelTeamViewnow consumes these instead of defining the logic locally, and the view's "create" affordance is derived reactively rather than stored inuseState.getChannelActionPermissions— pure async channel-action permission policy. Its three checks now run in parallel viaPromise.allinstead of serially.The mount effect was slimmed so it no longer re-runs on permission-slice reference changes (its dependencies are now just the team id and navigation).
This also fixes a latent bug in
usePermissions'useSubscriptionRoles: its effect dependency was[], so it never re-subscribed when a room id arrived asynchronously. It now depends on[rid]and re-subscribes, guarded by a{rid, roles}state shape so roles from a previous room id can never leak after the id changes. All existing callers that pass a stable id are unaffected.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-28
How to test or reproduce
AddChannelTeamView, confirm "Create New" / "Add Existing" still appear according to your permissions.Unit tests cover the extracted units and the view. Run them with:
Screenshots
No visual changes — pure refactor.
Types of changes
Checklist
Further comments
The navigation header is a reactive effect mirroring local state, so handlers only update state and the effect re-applies the correct header (search vs. normal), following the existing
CannedResponsesListViewpattern.The list engine and the permission hooks were extracted from the view so each has a single responsibility and can be unit-tested in isolation. The permission hooks are reactive (they re-derive as roles change), replacing the previous one-shot async permission reads.
Summary by CodeRabbit
New Features
Bug Fixes