diff --git a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx index b5baa4e253..6cd5387391 100644 --- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx +++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx @@ -142,6 +142,10 @@ interface IRoomResult extends IBaseResult { interface IMemberResult extends IBaseResult { member: Member | RoomMember; + /** + * If the result is from a filtered server API then we set true here to avoid locally culling it in our own filters + */ + alreadyFiltered: boolean; } interface IResult extends IBaseResult { @@ -201,7 +205,8 @@ const toRoomResult = (room: Room): IRoomResult => { } }; -const toMemberResult = (member: Member | RoomMember): IMemberResult => ({ +const toMemberResult = (member: Member | RoomMember, alreadyFiltered: boolean): IMemberResult => ({ + alreadyFiltered, member, section: Section.Suggestions, filter: [Filter.People], @@ -240,13 +245,9 @@ const findVisibleRooms = (cli: MatrixClient, msc3946ProcessDynamicPredecessor: b }); }; -const findVisibleRoomMembers = ( - cli: MatrixClient, - msc3946ProcessDynamicPredecessor: boolean, - filterDMs = true, -): RoomMember[] => { +const findVisibleRoomMembers = (visibleRooms: Room[], cli: MatrixClient, filterDMs = true): RoomMember[] => { return Object.values( - findVisibleRooms(cli, msc3946ProcessDynamicPredecessor) + visibleRooms .filter((room) => !filterDMs || !DMRoomMap.shared().getUserIdForRoomId(room.roomId)) .reduce((members, room) => { for (const member of room.getJoinedMembers()) { @@ -331,23 +332,40 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n useDebouncedCallback(filter === Filter.People, searchProfileInfo, searchParams); const possibleResults = useMemo(() => { + const visibleRooms = findVisibleRooms(cli, msc3946ProcessDynamicPredecessor); + const roomResults = visibleRooms.map(toRoomResult); const userResults: IMemberResult[] = []; - const roomResults = findVisibleRooms(cli, msc3946ProcessDynamicPredecessor).map(toRoomResult); - // If we already have a DM with the user we're looking for, we will - // show that DM instead of the user themselves + + // If we already have a DM with the user we're looking for, we will show that DM instead of the user themselves const alreadyAddedUserIds = roomResults.reduce((userIds, result) => { const userId = DMRoomMap.shared().getUserIdForRoomId(result.room.roomId); if (!userId) return userIds; if (result.room.getJoinedMemberCount() > 2) return userIds; - userIds.add(userId); + userIds.set(userId, result); return userIds; - }, new Set()); - for (const user of [...findVisibleRoomMembers(cli, msc3946ProcessDynamicPredecessor), ...users]) { - // Make sure we don't have any user more than once - if (alreadyAddedUserIds.has(user.userId)) continue; - alreadyAddedUserIds.add(user.userId); + }, new Map()); - userResults.push(toMemberResult(user)); + function addUserResults(users: Array, alreadyFiltered: boolean): void { + for (const user of users) { + // Make sure we don't have any user more than once + if (alreadyAddedUserIds.has(user.userId)) { + const result = alreadyAddedUserIds.get(user.userId)!; + if (alreadyFiltered && isMemberResult(result) && !result.alreadyFiltered) { + // But if they were added as not yet filtered then mark them as already filtered to avoid + // culling this result based on local filtering. + result.alreadyFiltered = true; + } + continue; + } + const result = toMemberResult(user, alreadyFiltered); + alreadyAddedUserIds.set(user.userId, result); + userResults.push(result); + } + } + addUserResults(findVisibleRoomMembers(visibleRooms, cli), false); + addUserResults(users, true); + if (profile) { + addUserResults([new DirectoryMember(profile)], true); } return [ @@ -369,9 +387,6 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n })), ...roomResults, ...userResults, - ...(profile && !alreadyAddedUserIds.has(profile.user_id) ? [new DirectoryMember(profile)] : []).map( - toMemberResult, - ), ...publicRooms.map(toPublicRoomResult), ].filter((result) => filter === null || result.filter.includes(filter)); }, [cli, users, profile, publicRooms, filter, msc3946ProcessDynamicPredecessor]); @@ -399,7 +414,7 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n ) return; // bail, does not match query } else if (isMemberResult(entry)) { - if (!entry.query?.some((q) => q.includes(lcQuery))) return; // bail, does not match query + if (!entry.alreadyFiltered && !entry.query?.some((q) => q.includes(lcQuery))) return; // bail, does not match query } else if (isPublicRoomResult(entry)) { if (!entry.query?.some((q) => q.includes(lcQuery))) return; // bail, does not match query } else { diff --git a/test/components/views/dialogs/SpotlightDialog-test.tsx b/test/components/views/dialogs/SpotlightDialog-test.tsx index 2857225309..9c987e9c1c 100644 --- a/test/components/views/dialogs/SpotlightDialog-test.tsx +++ b/test/components/views/dialogs/SpotlightDialog-test.tsx @@ -338,6 +338,52 @@ describe("Spotlight Dialog", () => { }); }); + it("should not filter out users sent by the server", async () => { + mocked(mockedClient.searchUserDirectory).mockResolvedValue({ + results: [ + { user_id: "@user1:server", display_name: "User Alpha", avatar_url: "mxc://1/avatar" }, + { user_id: "@user2:server", display_name: "User Beta", avatar_url: "mxc://2/avatar" }, + ], + limited: false, + }); + + render( null} />); + // search is debounced + jest.advanceTimersByTime(200); + await flushPromisesWithFakeTimers(); + + const content = document.querySelector("#mx_SpotlightDialog_content")!; + const options = content.querySelectorAll("li.mx_SpotlightDialog_option"); + expect(options.length).toBeGreaterThanOrEqual(2); + expect(options[0]).toHaveTextContent("User Alpha"); + expect(options[1]).toHaveTextContent("User Beta"); + }); + + it("should not filter out users sent by the server even if a local suggestion gets filtered out", async () => { + const member = new RoomMember(testRoom.roomId, testPerson.user_id); + member.name = member.rawDisplayName = testPerson.display_name!; + member.getMxcAvatarUrl = jest.fn().mockReturnValue("mxc://0/avatar"); + mocked(testRoom.getJoinedMembers).mockReturnValue([member]); + mocked(mockedClient.searchUserDirectory).mockResolvedValue({ + results: [ + { user_id: "@janedoe:matrix.org", display_name: "User Alpha", avatar_url: "mxc://1/avatar" }, + { user_id: "@johndoe:matrix.org", display_name: "User Beta", avatar_url: "mxc://2/avatar" }, + ], + limited: false, + }); + + render( null} />); + // search is debounced + jest.advanceTimersByTime(200); + await flushPromisesWithFakeTimers(); + + const content = document.querySelector("#mx_SpotlightDialog_content")!; + const options = content.querySelectorAll("li.mx_SpotlightDialog_option"); + expect(options.length).toBeGreaterThanOrEqual(2); + expect(options[0]).toHaveTextContent(testPerson.display_name!); + expect(options[1]).toHaveTextContent("User Beta"); + }); + it("should start a DM when clicking a person", async () => { render(