Include non-matching DMs in Spotlight recent conversations when the DM's userId is part of the search API results (#11374)

* This addresses two issues:

     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

* Applying feedback from PR:
1. Updated comments
2. Renamed users to userDirectorySearchResults
3. Makes sure linter is happy
pull/28788/head^2
Milton Moura 2023-08-10 17:27:24 +00:00 committed by GitHub
parent c55400de18
commit d240f06810
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 80 additions and 12 deletions

View File

@ -315,7 +315,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
} = usePublicRoomDirectory(); } = usePublicRoomDirectory();
const [showRooms, setShowRooms] = useState(true); const [showRooms, setShowRooms] = useState(true);
const [showSpaces, setShowSpaces] = useState(false); const [showSpaces, setShowSpaces] = useState(false);
const { loading: peopleLoading, users, search: searchPeople } = useUserDirectory(); const { loading: peopleLoading, users: userDirectorySearchResults, search: searchPeople } = useUserDirectory();
const { loading: profileLoading, profile, search: searchProfileInfo } = useProfileInfo(); const { loading: profileLoading, profile, search: searchProfileInfo } = useProfileInfo();
const searchParams: [IDirectoryOpts] = useMemo( const searchParams: [IDirectoryOpts] = useMemo(
() => [ () => [
@ -363,7 +363,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
} }
} }
addUserResults(findVisibleRoomMembers(visibleRooms, cli), false); addUserResults(findVisibleRoomMembers(visibleRooms, cli), false);
addUserResults(users, true); addUserResults(userDirectorySearchResults, true);
if (profile) { if (profile) {
addUserResults([new DirectoryMember(profile)], true); addUserResults([new DirectoryMember(profile)], true);
} }
@ -389,7 +389,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
...userResults, ...userResults,
...publicRooms.map(toPublicRoomResult), ...publicRooms.map(toPublicRoomResult),
].filter((result) => filter === null || result.filter.includes(filter)); ].filter((result) => filter === null || result.filter.includes(filter));
}, [cli, users, profile, publicRooms, filter, msc3946ProcessDynamicPredecessor]); }, [cli, userDirectorySearchResults, profile, publicRooms, filter, msc3946ProcessDynamicPredecessor]);
const results = useMemo<Record<Section, Result[]>>(() => { const results = useMemo<Record<Section, Result[]>>(() => {
const results: Record<Section, Result[]> = { const results: Record<Section, Result[]> = {
@ -407,12 +407,18 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
possibleResults.forEach((entry) => { possibleResults.forEach((entry) => {
if (isRoomResult(entry)) { if (isRoomResult(entry)) {
if ( // If the room is a DM with a user that is part of the user directory search results,
!entry.room.normalizedName?.includes(normalizedQuery) && // we can assume the user is a relevant result, so include the DM with them too.
!entry.room.getCanonicalAlias()?.toLowerCase().includes(lcQuery) && const userId = DMRoomMap.shared().getUserIdForRoomId(entry.room.roomId);
!entry.query?.some((q) => q.includes(lcQuery)) if (!userDirectorySearchResults.some((user) => user.userId === userId)) {
) if (
return; // bail, does not match query !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)) { } else if (isMemberResult(entry)) {
if (!entry.alreadyFiltered && !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)) { } else if (isPublicRoomResult(entry)) {
@ -463,7 +469,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
} }
return results; return results;
}, [trimmedQuery, filter, cli, possibleResults, memberComparator]); }, [trimmedQuery, filter, cli, possibleResults, userDirectorySearchResults, memberComparator]);
const numResults = sum(Object.values(results).map((it) => it.length)); const numResults = sum(Object.values(results).map((it) => it.length));
useWebSearchMetrics(numResults, query.length, true); useWebSearchMetrics(numResults, query.length, true);

View File

@ -16,7 +16,6 @@ limitations under the License.
import { groupBy, mapValues, maxBy, minBy, sumBy, takeRight } from "lodash"; import { groupBy, mapValues, maxBy, minBy, sumBy, takeRight } from "lodash";
import { MatrixClient, Room, RoomMember } from "matrix-js-sdk/src/matrix"; import { MatrixClient, Room, RoomMember } from "matrix-js-sdk/src/matrix";
import { compare } from "matrix-js-sdk/src/utils";
import { Member } from "./direct-messages"; import { Member } from "./direct-messages";
import DMRoomMap from "./DMRoomMap"; import DMRoomMap from "./DMRoomMap";
@ -39,7 +38,9 @@ export const compareMembers =
if (aScore === bScore) { if (aScore === bScore) {
if (aNumRooms === bNumRooms) { 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; return bNumRooms - aNumRooms;

View File

@ -143,7 +143,11 @@ describe("Spotlight Dialog", () => {
guest_can_join: false, guest_can_join: false,
}; };
const testDMRoomId = "!testDM:example.com";
const testDMUserId = "@alice:matrix.org";
let testRoom: Room; let testRoom: Room;
let testDM: Room;
let testLocalRoom: LocalRoom; let testLocalRoom: LocalRoom;
let mockedClient: MatrixClient; let mockedClient: MatrixClient;
@ -159,6 +163,19 @@ describe("Spotlight Dialog", () => {
jest.spyOn(DMRoomMap, "shared").mockReturnValue({ jest.spyOn(DMRoomMap, "shared").mockReturnValue({
getUserIdForRoomId: jest.fn(), getUserIdForRoomId: jest.fn(),
} as unknown as DMRoomMap); } 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", () => { describe("should apply filters supplied via props", () => {
@ -391,6 +408,50 @@ describe("Spotlight Dialog", () => {
expect(options[1]).toHaveTextContent("User Beta"); 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 () => {
const serverList = [
{ 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 () => { it("should start a DM when clicking a person", async () => {
render( render(
<SpotlightDialog <SpotlightDialog