PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Prevent re-filtering user directory results in spotlight (#11290)

Browse source 
* Prevent re-filtering user directory results in spotlight

As they were already filtered by the server and may be fuzzier than any filtering we can do locally,
e.g. matching against email addresses or other fields not available to the client

* deduplicate work

* Improve coverage
This commit is contained in:
Michael Telatynski 2023-07-25 09:05:54 +01:00 committed by GitHub
parent d9aaed0ef6
commit 9136a581d2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 82 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
src/components/views/dialogs/spotlight/SpotlightDialog.tsx
View file

@ -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<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);
}, new Map<string, IMemberResult | IRoomResult>());
userResults.push(toMemberResult(user));
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 [
@ -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]);
@ -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 {

46
test/components/views/dialogs/SpotlightDialog-test.tsx
View file

@ -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