Order new search dialog results by recency (#8444)

Germain · web-flow · commit 3a63c88a11a8 · 2022-05-05T12:19:11.000+01:00
* Order new search dialog results by recency

* Add getLastTs tests

* Add sort rooms tests
diff --git 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<IProps> = ({ 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<IProps> = ({ 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
@@ -63,58 +63,55 @@ 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?.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(EventType.RoomMember, 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.
+        return r.timeline[0]?.getTs() ?? Number.MAX_SAFE_INTEGER;
+    })();
+    return ts;
 };
 
 /**
@@ -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
@@ -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]);
+        });
+    });
+});
