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

Commit

Permalink
This addresses two issues:
Browse files Browse the repository at this point in the history
     1. Include non-matching DMs in Spotlight suggestions if the userId of the DM is included in the user directory search results
     2. The user directory search results order is kept when there is no relevant activity between users, instead of sorting by MXID
  • Loading branch information
mgcm committed Aug 4, 2023
1 parent e6af09e commit 4b11fa3
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 9 deletions.
20 changes: 13 additions & 7 deletions src/components/views/dialogs/spotlight/SpotlightDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import { WebSearch as WebSearchEvent } from "@matrix-org/analytics-events/types/
import classNames from "classnames";
import { capitalize, sum } from "lodash";
import { IHierarchyRoom } from "matrix-js-sdk/src/@types/spaces";
import { IPublicRoomsChunkRoom, MatrixClient, RoomMember, RoomType, Room } from "matrix-js-sdk/src/matrix";
import { IPublicRoomsChunkRoom, MatrixClient, RoomMember, RoomType } from "matrix-js-sdk/src/matrix";
import { Room } from "matrix-js-sdk/src/models/room";

Check failure on line 22 in src/components/views/dialogs/spotlight/SpotlightDialog.tsx

View workflow job for this annotation

GitHub Actions / ESLint

'matrix-js-sdk/src/models/room' import is restricted from being used. Please use matrix-js-sdk/src/matrix instead
import { normalize } from "matrix-js-sdk/src/utils";
import React, { ChangeEvent, RefObject, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react";
import sanitizeHtml from "sanitize-html";
Expand Down Expand Up @@ -406,12 +407,17 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n

possibleResults.forEach((entry) => {
if (isRoomResult(entry)) {
if (
!entry.room.normalizedName?.includes(normalizedQuery) &&
!entry.room.getCanonicalAlias()?.toLowerCase().includes(lcQuery) &&
!entry.query?.some((q) => q.includes(lcQuery))
)
return; // bail, does not match query
// Check if the DM userId is part of the user directory results, if so keep it
const userId = DMRoomMap.shared().getUserIdForRoomId(entry.room.roomId);
if (!users.some((user) => user.userId === userId)) {
if (
!entry.room.normalizedName?.includes(normalizedQuery) &&
!entry.room.getCanonicalAlias()?.toLowerCase().includes(lcQuery) &&
!entry.query?.some((q) => q.includes(lcQuery))
) {
return; // bail, does not match query
}
}
} else if (isMemberResult(entry)) {
if (!entry.alreadyFiltered && !entry.query?.some((q) => q.includes(lcQuery))) return; // bail, does not match query
} else if (isPublicRoomResult(entry)) {
Expand Down
5 changes: 3 additions & 2 deletions src/utils/SortMembers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.

import { groupBy, mapValues, maxBy, minBy, sumBy, takeRight } from "lodash";
import { MatrixClient, Room, RoomMember } from "matrix-js-sdk/src/matrix";
import { compare } from "matrix-js-sdk/src/utils";

import { Member } from "./direct-messages";
import DMRoomMap from "./DMRoomMap";
Expand All @@ -39,7 +38,9 @@ export const compareMembers =

if (aScore === bScore) {
if (aNumRooms === bNumRooms) {
return compare(a.userId, b.userId);
// If there is no activity between members,
// keep the order received from the user directory search results
return 0;
}

return bNumRooms - aNumRooms;
Expand Down
58 changes: 58 additions & 0 deletions test/components/views/dialogs/SpotlightDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ describe("Spotlight Dialog", () => {
};

let testRoom: Room;
let testDMRoomId = "!testDM:example.com";

Check failure on line 140 in test/components/views/dialogs/SpotlightDialog-test.tsx

View workflow job for this annotation

GitHub Actions / ESLint

'testDMRoomId' is never reassigned. Use 'const' instead
let testDMUserId = "@alice:matrix.org"

Check failure on line 141 in test/components/views/dialogs/SpotlightDialog-test.tsx

View workflow job for this annotation

GitHub Actions / ESLint

'testDMUserId' is never reassigned. Use 'const' instead
let testDM: Room;
let testLocalRoom: LocalRoom;

let mockedClient: MatrixClient;
Expand All @@ -152,6 +155,19 @@ describe("Spotlight Dialog", () => {
jest.spyOn(DMRoomMap, "shared").mockReturnValue({
getUserIdForRoomId: jest.fn(),
} as unknown as DMRoomMap);

testDM = mkRoom(mockedClient, testDMRoomId)
testDM.name = "Chat with Alice";
mocked(testDM.getMyMembership).mockReturnValue("join");

mocked(DMRoomMap.shared().getUserIdForRoomId).mockImplementation((roomId: string) => {
if (roomId === testDMRoomId) {
return testDMUserId;
}
return undefined;
});

mocked(mockedClient.getVisibleRooms).mockReturnValue([testRoom, testLocalRoom, testDM]);
});

describe("should apply filters supplied via props", () => {
Expand Down Expand Up @@ -384,6 +400,48 @@ describe("Spotlight Dialog", () => {
expect(options[1]).toHaveTextContent("User Beta");
});

it("show non-matching query members with DMs if they are present in the server search results", async () => {
mocked(mockedClient.searchUserDirectory).mockResolvedValue({
results: [
{ user_id: testDMUserId, display_name: "Alice Wonder", avatar_url: "mxc://1/avatar" },
{ user_id: "@bob:matrix.org", display_name: "Bob Wonder", avatar_url: "mxc://2/avatar" },
],
limited: false,
});
render(<SpotlightDialog initialFilter={Filter.People} initialText="Something Wonder" 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(testDMUserId);
expect(options[1]).toHaveTextContent("Bob Wonder");
});

it("don't sort the order of users sent by the server", async () => {
var serverList = [

Check failure on line 424 in test/components/views/dialogs/SpotlightDialog-test.tsx

View workflow job for this annotation

GitHub Actions / ESLint

Unexpected var, use let or const instead
{ user_id: "@user2:server", display_name: "User Beta", avatar_url: "mxc://2/avatar" },
{ user_id: "@user1:server", display_name: "User Alpha", avatar_url: "mxc://1/avatar" },
]
mocked(mockedClient.searchUserDirectory).mockResolvedValue({
results: serverList,
limited: false,
});

render(<SpotlightDialog initialFilter={Filter.People} initialText="User" 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 Beta");
expect(options[1]).toHaveTextContent("User Alpha");
});

it("should start a DM when clicking a person", async () => {
render(
<SpotlightDialog
Expand Down

0 comments on commit 4b11fa3

Please sign in to comment.