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

Order new search dialog results by recency #8444

Merged
merged 4 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion src/components/views/dialogs/SpotlightDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) &&
Expand All @@ -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);
Expand Down
89 changes: 45 additions & 44 deletions src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand All @@ -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);
}
}
127 changes: 127 additions & 0 deletions test/stores/room-list/algorithms/RecentAlgorithm-test.ts
Original file line number Diff line number Diff line change
@@ -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;
Comment on lines +26 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

types would have been great here otherwise the tests are using any

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]);
});
});
});