From 3a63c88a11a87368b8c86dd4784635b3683b4739 Mon Sep 17 00:00:00 2001 From: Germain Date: Thu, 5 May 2022 12:19:11 +0100 Subject: [PATCH] Order new search dialog results by recency (#8444) * Order new search dialog results by recency * Add getLastTs tests * Add sort rooms tests --- .../views/dialogs/SpotlightDialog.tsx | 23 +++- .../algorithms/tag-sorting/RecentAlgorithm.ts | 99 +++++++------- .../algorithms/RecentAlgorithm-test.ts | 127 ++++++++++++++++++ 3 files changed, 199 insertions(+), 50 deletions(-) create mode 100644 test/stores/room-list/algorithms/RecentAlgorithm-test.ts diff --git a/src/components/views/dialogs/SpotlightDialog.tsx b/src/components/views/dialogs/SpotlightDialog.tsx index 1d6078fa10..f5efc0b8dc 100644 --- a/src/components/views/dialogs/SpotlightDialog.tsx +++ b/src/components/views/dialogs/SpotlightDialog.tsx @@ -74,6 +74,7 @@ import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { PosthogAnalytics } from "../../../PosthogAnalytics"; import { getCachedRoomIDForAlias } from "../../../RoomAliasCache"; import { roomContextDetailsText, spaceContextDetailsText } from "../../../utils/i18n-helpers"; +import { RecentAlgorithm } from "../../../stores/room-list/algorithms/tag-sorting/RecentAlgorithm"; const MAX_RECENT_SEARCHES = 10; const SECTION_LIMIT = 50; // only show 50 results per section for performance reasons @@ -210,6 +211,8 @@ type Result = IRoomResult | IResult; const isRoomResult = (result: any): result is IRoomResult => !!result?.room; +const recentAlgorithm = new RecentAlgorithm(); + export const useWebSearchMetrics = (numResults: number, queryLength: number, viaSpotlight: boolean): void => { useEffect(() => { if (!queryLength) return; @@ -280,6 +283,7 @@ const SpotlightDialog: React.FC = ({ initialText = "", onFinished }) => const results: [Result[], Result[], Result[]] = [[], [], []]; + // Group results in their respective sections possibleResults.forEach(entry => { if (isRoomResult(entry)) { if (!entry.room.normalizedName.includes(normalizedQuery) && @@ -295,8 +299,25 @@ const SpotlightDialog: React.FC = ({ initialText = "", onFinished }) => results[entry.section].push(entry); }); + // Sort results by most recent activity + + const myUserId = cli.getUserId(); + for (const resultArray of results) { + resultArray.sort((a: Result, b: Result) => { + // This is not a room result, it should appear at the bottom of + // the list + if (!(a as IRoomResult).room) return 1; + if (!(b as IRoomResult).room) return -1; + + const roomA = (a as IRoomResult).room; + const roomB = (b as IRoomResult).room; + + return recentAlgorithm.getLastTs(roomB, myUserId) - recentAlgorithm.getLastTs(roomA, myUserId); + }); + } + return results; - }, [possibleResults, trimmedQuery]); + }, [possibleResults, trimmedQuery, cli]); const numResults = trimmedQuery ? people.length + rooms.length + spaces.length : 0; useWebSearchMetrics(numResults, query.length, true); diff --git a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts index af937e92af..cf76b033eb 100644 --- a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts @@ -63,60 +63,57 @@ export const sortRooms = (rooms: Room[]): Room[] => { } const tsCache: { [roomId: string]: number } = {}; - const getLastTs = (r: Room) => { - if (tsCache[r.roomId]) { - return tsCache[r.roomId]; - } - - const ts = (() => { - // Apparently we can have rooms without timelines, at least under testing - // environments. Just return MAX_INT when this happens. - if (!r || !r.timeline) { - return Number.MAX_SAFE_INTEGER; - } - - // If the room hasn't been joined yet, it probably won't have a timeline to - // parse. We'll still fall back to the timeline if this fails, but chances - // are we'll at least have our own membership event to go off of. - const effectiveMembership = getEffectiveMembership(r.getMyMembership()); - if (effectiveMembership !== EffectiveMembership.Join) { - const membershipEvent = r.currentState.getStateEvents("m.room.member", myUserId); - if (membershipEvent && !Array.isArray(membershipEvent)) { - return membershipEvent.getTs(); - } - } - - for (let i = r.timeline.length - 1; i >= 0; --i) { - const ev = r.timeline[i]; - if (!ev.getTs()) continue; // skip events that don't have timestamps (tests only?) - - if ( - (ev.getSender() === myUserId && shouldCauseReorder(ev)) || - Unread.eventTriggersUnreadCount(ev) - ) { - return ev.getTs(); - } - } - - // we might only have events that don't trigger the unread indicator, - // in which case use the oldest event even if normally it wouldn't count. - // This is better than just assuming the last event was forever ago. - if (r.timeline.length && r.timeline[0].getTs()) { - return r.timeline[0].getTs(); - } else { - return Number.MAX_SAFE_INTEGER; - } - })(); - - tsCache[r.roomId] = ts; - return ts; - }; return rooms.sort((a, b) => { - return getLastTs(b) - getLastTs(a); + const roomALastTs = tsCache[a.roomId] ?? getLastTs(a, myUserId); + const roomBLastTs = tsCache[b.roomId] ?? getLastTs(b, myUserId); + + tsCache[a.roomId] = roomALastTs; + tsCache[b.roomId] = roomBLastTs; + + return roomBLastTs - roomALastTs; }); }; +const getLastTs = (r: Room, userId: string) => { + const ts = (() => { + // Apparently we can have rooms without timelines, at least under testing + // environments. Just return MAX_INT when this happens. + if (!r?.timeline) { + return Number.MAX_SAFE_INTEGER; + } + + // If the room hasn't been joined yet, it probably won't have a timeline to + // parse. We'll still fall back to the timeline if this fails, but chances + // are we'll at least have our own membership event to go off of. + const effectiveMembership = getEffectiveMembership(r.getMyMembership()); + if (effectiveMembership !== EffectiveMembership.Join) { + const membershipEvent = r.currentState.getStateEvents(EventType.RoomMember, userId); + if (membershipEvent && !Array.isArray(membershipEvent)) { + return membershipEvent.getTs(); + } + } + + for (let i = r.timeline.length - 1; i >= 0; --i) { + const ev = r.timeline[i]; + if (!ev.getTs()) continue; // skip events that don't have timestamps (tests only?) + + if ( + (ev.getSender() === userId && shouldCauseReorder(ev)) || + Unread.eventTriggersUnreadCount(ev) + ) { + return ev.getTs(); + } + } + + // we might only have events that don't trigger the unread indicator, + // in which case use the oldest event even if normally it wouldn't count. + // This is better than just assuming the last event was forever ago. + return r.timeline[0]?.getTs() ?? Number.MAX_SAFE_INTEGER; + })(); + return ts; +}; + /** * Sorts rooms according to the last event's timestamp in each room that seems * useful to the user. @@ -125,4 +122,8 @@ export class RecentAlgorithm implements IAlgorithm { public sortRooms(rooms: Room[], tagId: TagID): Room[] { return sortRooms(rooms); } + + public getLastTs(room: Room, userId: string): number { + return getLastTs(room, userId); + } } diff --git a/test/stores/room-list/algorithms/RecentAlgorithm-test.ts b/test/stores/room-list/algorithms/RecentAlgorithm-test.ts new file mode 100644 index 0000000000..40ce53f225 --- /dev/null +++ b/test/stores/room-list/algorithms/RecentAlgorithm-test.ts @@ -0,0 +1,127 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { Room } from "matrix-js-sdk/src/models/room"; + +import { stubClient, mkRoom, mkMessage } from "../../../test-utils"; +import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; +import "../../../../src/stores/room-list/RoomListStore"; +import { RecentAlgorithm } from "../../../../src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm"; +import { EffectiveMembership } from "../../../../src/utils/membership"; + +describe("RecentAlgorithm", () => { + let algorithm; + let cli; + beforeEach(() => { + stubClient(); + cli = MatrixClientPeg.get(); + algorithm = new RecentAlgorithm(); + }); + + describe("getLastTs", () => { + it("returns the last ts", () => { + const room = new Room("room123", cli, "@john:matrix.org"); + + const event1 = mkMessage({ + room: room.roomId, + msg: "Hello world!", + user: "@alice:matrix.org", + ts: 5, + event: true, + }); + const event2 = mkMessage({ + room: room.roomId, + msg: "Howdy!", + user: "@bob:matrix.org", + ts: 10, + event: true, + }); + + room.getMyMembership = () => "join"; + + room.addLiveEvents([event1]); + expect(algorithm.getLastTs(room, "@jane:matrix.org")).toBe(5); + expect(algorithm.getLastTs(room, "@john:matrix.org")).toBe(5); + + room.addLiveEvents([event2]); + + expect(algorithm.getLastTs(room, "@jane:matrix.org")).toBe(10); + expect(algorithm.getLastTs(room, "@john:matrix.org")).toBe(10); + }); + + it("returns a fake ts for rooms without a timeline", () => { + const room = mkRoom(cli, "!new:example.org"); + room.timeline = undefined; + expect(algorithm.getLastTs(room, "@john:matrix.org")).toBe(Number.MAX_SAFE_INTEGER); + }); + + it("works when not a member", () => { + const room = mkRoom(cli, "!new:example.org"); + room.getMyMembership.mockReturnValue(EffectiveMembership.Invite); + expect(algorithm.getLastTs(room, "@john:matrix.org")).toBe(Number.MAX_SAFE_INTEGER); + }); + }); + + describe("sortRooms", () => { + it("orders rooms per last message ts", () => { + const room1 = new Room("room1", cli, "@bob:matrix.org"); + const room2 = new Room("room2", cli, "@bob:matrix.org"); + + room1.getMyMembership = () => "join"; + room2.getMyMembership = () => "join"; + + const evt = mkMessage({ + room: room1.roomId, + msg: "Hello world!", + user: "@alice:matrix.org", + ts: 5, + event: true, + }); + const evt2 = mkMessage({ + room: room2.roomId, + msg: "Hello world!", + user: "@alice:matrix.org", + ts: 2, + event: true, + }); + + room1.addLiveEvents([evt]); + room2.addLiveEvents([evt2]); + + expect(algorithm.sortRooms([room2, room1])).toEqual([room1, room2]); + }); + + it("orders rooms without messages first", () => { + const room1 = new Room("room1", cli, "@bob:matrix.org"); + const room2 = new Room("room2", cli, "@bob:matrix.org"); + + room1.getMyMembership = () => "join"; + room2.getMyMembership = () => "join"; + + const evt = mkMessage({ + room: room1.roomId, + msg: "Hello world!", + user: "@alice:matrix.org", + ts: 5, + event: true, + }); + + room1.addLiveEvents([evt]); + + expect(algorithm.sortRooms([room2, room1])).toEqual([room2, room1]); + }); + }); +});