Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Prevent re-filtering user directory results in spotlight #11290

Merged
merged 3 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 37 additions & 22 deletions src/components/views/dialogs/spotlight/SpotlightDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -331,23 +332,40 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
useDebouncedCallback(filter === Filter.People, searchProfileInfo, searchParams);

const possibleResults = useMemo<Result[]>(() => {
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<string>());
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);

userResults.push(toMemberResult(user));
}, new Map<string, IMemberResult | IRoomResult>());

function addUserResults(users: Array<Member | RoomMember>, 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 [
Expand All @@ -369,9 +387,6 @@ const SpotlightDialog: React.FC<IProps> = ({ 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]);
Expand Down Expand Up @@ -399,7 +414,7 @@ const SpotlightDialog: React.FC<IProps> = ({ 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 {
Expand Down
46 changes: 46 additions & 0 deletions test/components/views/dialogs/SpotlightDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<SpotlightDialog initialFilter={Filter.People} initialText="Alpha" onFinished={() => 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(<SpotlightDialog initialFilter={Filter.People} initialText="Beta" onFinished={() => 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(
<SpotlightDialog
Expand Down