From 1cfcce474dddd139ca3332417ad99e4b8ae7ad5c Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 29 Apr 2022 11:20:26 +0100 Subject: [PATCH 1/3] Order new search dialog results by recency --- .../views/dialogs/SpotlightDialog.tsx | 23 ++++- .../algorithms/tag-sorting/RecentAlgorithm.ts | 93 ++++++++++--------- 2 files changed, 71 insertions(+), 45 deletions(-) diff --git a/src/components/views/dialogs/SpotlightDialog.tsx b/src/components/views/dialogs/SpotlightDialog.tsx index f579e262875..9d5fb152225 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 af937e92af0..6ce90b45cf0 100644 --- a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts @@ -63,58 +63,59 @@ 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; - } + return rooms.sort((a, b) => { + const roomALastTs = tsCache[a.roomId] ?? getLastTs(a, myUserId); + const roomBLastTs = tsCache[b.roomId] ?? getLastTs(b, myUserId); - // 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(); - } - } + tsCache[a.roomId] = roomALastTs; + tsCache[b.roomId] = roomBLastTs; - 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?) + return roomBLastTs - roomALastTs; + }); +}; - if ( - (ev.getSender() === myUserId && shouldCauseReorder(ev)) || - Unread.eventTriggersUnreadCount(ev) - ) { - return ev.getTs(); - } - } +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 || !r.timeline) { + return Number.MAX_SAFE_INTEGER; + } - // 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; + // 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", userId); + if (membershipEvent && !Array.isArray(membershipEvent)) { + return membershipEvent.getTs(); } - })(); + } - tsCache[r.roomId] = ts; - return ts; - }; + 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?) - return rooms.sort((a, b) => { - return getLastTs(b) - getLastTs(a); - }); + 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. + if (r.timeline.length && r.timeline[0].getTs()) { + return r.timeline[0].getTs(); + } else { + return Number.MAX_SAFE_INTEGER; + } + })(); + return ts; }; /** @@ -125,4 +126,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); + } } From 772aafe08f6599d3601ff12913dbe7e392457615 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 5 May 2022 11:44:12 +0100 Subject: [PATCH 2/3] Add getLastTs tests --- .../algorithms/tag-sorting/RecentAlgorithm.ts | 10 +-- .../algorithms/RecentAlgorithm-test.ts | 77 +++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 test/stores/room-list/algorithms/RecentAlgorithm-test.ts diff --git a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts index 6ce90b45cf0..cf76b033ebe 100644 --- a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts @@ -79,7 +79,7 @@ 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 || !r.timeline) { + if (!r?.timeline) { return Number.MAX_SAFE_INTEGER; } @@ -88,7 +88,7 @@ const getLastTs = (r: Room, userId: string) => { // 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", userId); + const membershipEvent = r.currentState.getStateEvents(EventType.RoomMember, userId); if (membershipEvent && !Array.isArray(membershipEvent)) { return membershipEvent.getTs(); } @@ -109,11 +109,7 @@ const getLastTs = (r: Room, userId: string) => { // 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; - } + return r.timeline[0]?.getTs() ?? Number.MAX_SAFE_INTEGER; })(); return ts; }; 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 00000000000..bad29174d33 --- /dev/null +++ b/test/stores/room-list/algorithms/RecentAlgorithm-test.ts @@ -0,0 +1,77 @@ +/* +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); + }); + }); +}); From 92d2a48eefec3c62e31ad094aefd8dc42a0bc34c Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 5 May 2022 11:51:34 +0100 Subject: [PATCH 3/3] Add sort rooms tests --- .../algorithms/RecentAlgorithm-test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/stores/room-list/algorithms/RecentAlgorithm-test.ts b/test/stores/room-list/algorithms/RecentAlgorithm-test.ts index bad29174d33..40ce53f2251 100644 --- a/test/stores/room-list/algorithms/RecentAlgorithm-test.ts +++ b/test/stores/room-list/algorithms/RecentAlgorithm-test.ts @@ -74,4 +74,54 @@ describe("RecentAlgorithm", () => { 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]); + }); + }); });