Skip to content

fix: room member search to use server data instead of local data#6938

Open
Rohit3523 wants to merge 28 commits intodevelopfrom
search-filter-fix
Open

fix: room member search to use server data instead of local data#6938
Rohit3523 wants to merge 28 commits intodevelopfrom
search-filter-fix

Conversation

@Rohit3523
Copy link
Contributor

@Rohit3523 Rohit3523 commented Jan 21, 2026

Proposed changes

Search functionality in the room/group members list fails to return results for users who haven’t been loaded into the local state. Only users loaded initially or after scrolling to the bottom are searchable.

Issue(s)

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

How to test or reproduce

  1. Go to the member search in any room with a large number of members.
  2. Type the name of a user who is a member of the room but not currently visible in the list.
  3. The app fails to display the member in the search results.

Screenshots

iOS E2E
Screenshot 2026-01-23 at 11 40 54 PM

Android E2E
Screenshot 2026-01-23 at 11 23 05 PM

Types of changes

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

Checklist

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

Further comments

Android E2E Passed: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/21682322880/job/62533139246?pr=6938#step:9:283
iOS E2E Passed: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/21682322880/job/62535258698?pr=6938#step:12:23

Summary by CodeRabbit

  • Improvements

    • Debounced member search for smoother typing and fewer requests.
    • Server-filtered member list as single source of truth with paginated loading, deduplication, and reliable end/load state.
    • Stale search responses ignored to prevent flicker; header focus, Online/All toggle, and role-aware loading refresh the view.
    • Search input integrated into the list header.
  • Tests

    • Added an automated end-to-end test for the "Search Member" flow.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

Introduces debounced, server-side member search with pagination and requestId-based stale-response guarding in RoomMembersView; moves filtering off the client, adds conditional role fetching when permitted, revises header/filter UI semantics, and adds a Maestro test for the search flow.

Changes

Cohort / File(s) Summary
Room members view
app/views/RoomMembersView/index.tsx
Reworked member loading/filtering: added useRef, useCallback, useDebounce; flipped initial allUsers; set header on mount; removed client-side status filtering; SearchBox wired to debounced server search.
Fetch, pagination & deduplication
app/views/RoomMembersView/index.tsx
Introduced fetchMembers with requestId-based dedupe (latestSearchRequest), PAGE_SIZE pagination, merge/dedupe behavior, page-0 replacement, loading/end flags, and guarded error handling.
Roles & debounced search wiring
app/views/RoomMembersView/index.tsx
Added fetchRoles (conditional on permissions/room type); added debounceFilterChange to trim input, reset pagination/state, trigger debounced fetch; header action sheet semantics (Online/All) adjusted; onEndReached triggers fetchMembers.
Automated test
.maestro/tests/room/search-member.yaml
New Maestro test "Search Member": sets up environment, creates a user, logs in, opens room members, searches rohit.bansal, and asserts the member appears.

Sequence Diagram(s)

sequenceDiagram
  participant User as User
  participant SB as SearchBox
  participant RMV as RoomMembersView
  participant API as ServerAPI
  participant Roles as RolesAPI

  User->>SB: type query
  SB->>RMV: onChangeText (debounced)
  rect rgba(200,200,255,0.5)
    RMV->>RMV: increment latestSearchRequest\nreset page/members/end, set loading
  end
  RMV->>API: fetchMembers(filter, page=0, requestId)
  alt need roles && hasPermission
    RMV->>Roles: fetchRoles(roomId)
    Roles-->>RMV: roles
  end
  API-->>RMV: response (members, page, requestId)
  alt response.requestId == latestSearchRequest
    rect rgba(200,255,200,0.5)
      RMV->>RMV: dedupe & merge members\nupdate page & end, clear loading
    end
  else stale response
    rect rgba(255,200,200,0.5)
      RMV-->>RMV: ignore response
    end
  end
  RMV->>SB: update displayed members
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • diegolmello

Poem

🐇 I twitch my whiskers, ears alert and spry,
I chase each query as it hops on by.
Pages stitched in order, stale echoes dismissed,
Fresh faces arrive — a tidy, bustling list. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'fix: room member search to use server data instead of local data' directly and clearly describes the primary change: migrating from local data filtering to server-side search.
Linked Issues check ✅ Passed The code changes implement server-side member search with deduplication and pagination, fully addressing SUP-973's requirement for discovering room members not in the locally loaded list.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the room member search functionality: server-side fetch logic, debounced filtering, pagination, and test automation for the search flow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch search-filter-fix

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • D356-4208: Request failed with status code 404

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (2)
app/views/RoomMembersView/index.tsx (2)

44-44: Refetch on cleared search + use sanitizeLikeString (fixes lint).
Line 44 currently fails lint, and when the filter is cleared the list is reset but never reloaded. Normalize the filter and always fetch so the list repopulates and the unused import is resolved.

✅ Proposed fix
- const handleFilterChange = (text: string) => {
- 	const trimmedFilter = text.trim();
+ const handleFilterChange = (text: string) => {
+ 	const normalizedFilter = sanitizeLikeString(text.trim());
 
 	updateState({
-		filter: trimmedFilter,
+		filter: normalizedFilter,
 		page: 0,
 		members: [],
 		end: false
 	});
 
-	if (trimmedFilter.length > 0) {
-		fetchMembersWithNewFilter(trimmedFilter);
-	}
+	fetchMembersWithNewFilter(normalizedFilter);
 };

Also applies to: 425-437


440-468: Use stop() instead of cancel() for debounce cleanup, and remove the redundant filteredMembers variable.

The debounce helper exposes a stop() method (not cancel()), and filteredMembers is redundant—it always equals state.members because the condition state.members.length > 0 ? state.members : null followed by filteredMembers || state.members always evaluates to state.members.

The memoization pattern is correct to prevent recreating the debounced function per render and avoid orphaned callbacks after unmount.

♻️ Corrected refactor
-import React, { useEffect, useReducer } from 'react';
+import React, { useEffect, useReducer, useCallback, useMemo } from 'react';

-const handleFilterChange = (text: string) => {
+const handleFilterChange = useCallback((text: string) => {
 	// existing body
-};
+}, [fetchMembersWithNewFilter]);

-const debounceFilterChange = debounce(handleFilterChange, 500);
-const filteredMembers = state.members.length > 0 ? state.members : null;
+const debounceFilterChange = useMemo(() => debounce(handleFilterChange, 500), [handleFilterChange]);
+useEffect(() => () => debounceFilterChange.stop?.(), [debounceFilterChange]);

And update the FlatList data prop from filteredMembers || state.members to simply state.members.

🤖 Fix all issues with AI agents
In `@app/views/RoomMembersView/index.tsx`:
- Around line 371-381: When filter is active the code assigns newMembers =
membersResult which replaces prior pages and breaks scrolling; update the logic
in the block that references filter, membersResult, newMembers and members so
that when filter is truthy you either (A) append membersResult to existing
members with de-duplication by _id (e.g. merge members + membersResult and
remove duplicates by _id) to preserve pagination, or (B) explicitly disable
pagination while filtered (set end = true or prevent page increments) so
filtered requests don't replace earlier pages—apply the chosen approach
consistently where PAGE_SIZE, membersResult, newMembers and members are used.
- Around line 395-418: The fetchMembersWithNewFilter function can have
out-of-order debounced responses overwrite newer results; add a request-tracking
guard (e.g., increment a requestId counter or store the latestFilter in
component state) before calling getRoomMembers and capture the current
id/filter, then when the response returns only call updateState(members...) if
the captured id/filter still matches the latest one in state; ensure
requestId/latestFilter is updated at the start of fetchMembersWithNewFilter and
used to ignore stale responses so members/end/page/isLoading updates come only
from the most recent request.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@app/views/RoomMembersView/index.tsx`:
- Line 447: debounceFilterChange is recreated every render which breaks
debouncing; memoize the debounced function instead of creating it in the
component body by wrapping the debounce(handleFilterChange, 500) call in useMemo
(or store it in a useRef) so the same debounced instance persists across
renders; if handleFilterChange needs stable identity or fresh state, make
handleFilterChange a useCallback with appropriate deps or keep a ref to the
latest handler and call that from the debounced function.
- Around line 432-445: handleFilterChange currently clears members and resets
state but only calls fetchMembersWithNewFilter when trimmedFilter.length > 0,
leaving the list empty when the user clears the search; modify
handleFilterChange so that when trimmedFilter is empty it also triggers the
unfiltered fetch (e.g., call fetchMembersWithNewFilter('') or a dedicated
fetchUnfilteredMembers function) after updateState, keeping the page reset and
end flag behavior intact; reference the handleFilterChange function,
trimmedFilter variable, updateState call, and fetchMembersWithNewFilter to
locate and change the logic.
♻️ Duplicate comments (1)
app/views/RoomMembersView/index.tsx (1)

374-380: Filtered pagination still drops earlier pages.

When a filter is active and the user scrolls to load more, fetchMembers is triggered via onEndReached. The current logic replaces members with just the new page instead of appending. Consider either appending with de-duplication for filtered paging, or disabling onEndReached while a filter is active.

🐛 Proposed fix: Append with de-dupe for all cases
-		let newMembers: TUserModel[];
-		if (filter) {
-			newMembers = membersResult || [];
-		} else {
-			const membersResultFiltered = membersResult?.filter((member: TUserModel) => !members.some(m => m._id === member._id));
-			newMembers = [...members, ...membersResultFiltered];
-		}
+		const membersResultFiltered = membersResult?.filter((member: TUserModel) => !members.some(m => m._id === member._id));
+		const newMembers = [...members, ...(membersResultFiltered || [])];
🧹 Nitpick comments (1)
app/views/RoomMembersView/index.tsx (1)

448-448: Redundant filteredMembers variable.

The expression filteredMembers || state.members always evaluates to state.members:

  • If state.members.length > 0: filteredMembers = state.members
  • If state.members.length === 0: filteredMembers = null, fallback = state.members (empty array)
♻️ Simplify by using state.members directly
-const filteredMembers = state.members.length > 0 ? state.members : null;
 ...
-data={filteredMembers || state.members}
+data={state.members}

Also applies to: 453-453

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
app/views/RoomMembersView/index.tsx (1)

352-386: Prevent stale pagination responses from mixing with a new search.
If a page fetch started before a filter/status change returns late, it can append unfiltered members into the filtered list because fetchMembers has no staleness guard. Add a shared request token (or query signature ref) so only the latest request updates state.

🐛 Proposed fix (shared request guard)
@@
-const latestSearchRequest = useRef(0);
+const latestSearchRequest = useRef(0);
+const latestMembersRequest = useRef(0);
+const latestQueryRef = useRef({ filter: '', allUsers: false });
+
+useEffect(() => {
+	latestQueryRef.current = { filter: state.filter, allUsers: state.allUsers };
+}, [state.filter, state.allUsers]);
@@
 const fetchMembers = async (status: boolean) => {
+	const requestId = ++latestMembersRequest.current;
+	const filterAtCall = state.filter;
+	const allUsersAtCall = status;
 	const { members, isLoading, end, room, filter, page } = state;
@@
 	try {
 		const membersResult = await getRoomMembers({
@@
 		const end = membersResult?.length < PAGE_SIZE;
 
 		const membersResultFiltered = membersResult?.filter((member: TUserModel) => !members.some(m => m._id === member._id));
 		const newMembers = [...members, ...(membersResultFiltered || [])];
 
+		if (
+			requestId !== latestMembersRequest.current ||
+			filterAtCall !== latestQueryRef.current.filter ||
+			allUsersAtCall !== latestQueryRef.current.allUsers
+		) {
+			return;
+		}
+
 		updateState({
 			members: newMembers,
 			isLoading: false,
 			end,
 			page: page + 1
 		});
 	} catch (e) {
 		log(e);
-		updateState({ isLoading: false });
+		if (requestId === latestMembersRequest.current) {
+			updateState({ isLoading: false });
+		}
 	}
 };
♻️ Duplicate comments (1)
app/views/RoomMembersView/index.tsx (1)

442-470: Debounced handler is recreated every render (debounce breaks + stale timers).
Line 442 creates a new debounce instance each render, so timeouts from prior renders can still fire and the debounce no longer coalesces rapid input. Memoize the debounced function and clean it up on unmount.

🐛 Proposed fix (stable debounce + cleanup)
-import React, { useEffect, useReducer, useRef } from 'react';
+import React, { useEffect, useMemo, useReducer, useRef } from 'react';
@@
-const debounceFilterChange = debounce(handleFilterChange, 500);
+const handleFilterChangeRef = useRef(handleFilterChange);
+useEffect(() => {
+	handleFilterChangeRef.current = handleFilterChange;
+}, [handleFilterChange]);
+
+const debounceFilterChange = useMemo(
+	() => debounce((text: string) => handleFilterChangeRef.current(text), 500),
+	[]
+);
+useEffect(() => () => debounceFilterChange.stop(), [debounceFilterChange]);

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@app/views/RoomMembersView/index.tsx`:
- Around line 171-184: The filter debounce handler (debounceFilterChange) and
fetchMembersWithNewFilter call flow can be clobbered by in-flight fetchMembers
responses because there’s no shared request-id guard; add a single shared
request identifier (e.g., currentRequestId) used by both fetchMembers and
fetchMembersWithNewFilter so every fetch captures the id before dispatch and
each response checks it and ignores stale ids, and when the filter is cleared
(trimmedFilter.length === 0) increment/invalidate the currentRequestId so any
pending filtered responses are ignored; updateState calls remain the same but
ensure all code paths that call fetchMembers or fetchMembersWithNewFilter use
that guard to prevent stale overwrites.
🧹 Nitpick comments (1)
app/views/RoomMembersView/index.tsx (1)

442-447: Simplify redundant filteredMembers logic.

filteredMembers is either state.members or null, and data={filteredMembers || state.members} always resolves to state.members. You can remove the intermediate variable.

♻️ Suggested simplification
-const filteredMembers = state.members.length > 0 ? state.members : null;
...
-	data={filteredMembers || state.members}
+	data={state.members}

@Rohit3523
Copy link
Contributor Author

@coderabbitai review

@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing February 5, 2026 14:24 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build February 5, 2026 14:27 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build February 5, 2026 14:27 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build February 5, 2026 14:27 — with GitHub Actions Error
Copy link
Contributor

@OtavioStasiak OtavioStasiak left a comment

Choose a reason for hiding this comment

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

Just wait E2E tests run and you can merge it!

@Rohit3523 Rohit3523 had a problem deploying to official_android_build February 5, 2026 22:02 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build February 5, 2026 22:02 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build February 5, 2026 22:02 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build February 6, 2026 07:22 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build February 6, 2026 07:22 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build February 6, 2026 07:22 — with GitHub Actions Error
@Rohit3523 Rohit3523 temporarily deployed to approve_e2e_testing February 6, 2026 14:20 — with GitHub Actions Inactive
@Rohit3523 Rohit3523 requested a deployment to experimental_android_build February 6, 2026 14:23 — with GitHub Actions Waiting
@Rohit3523 Rohit3523 requested a deployment to experimental_ios_build February 6, 2026 14:23 — with GitHub Actions Waiting
@Rohit3523 Rohit3523 requested a deployment to official_android_build February 6, 2026 14:23 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants