Fix all rooms search generating permalinks to wrong room id (#10625)

* Fix all rooms search generating permalinks to wrong room id

* Iterate

* Add comment

* Iterate

* Add coverage

* Iterate

* Add comment

* Restore src/utils/permalinks/Permalinks.ts

* Update src/components/structures/RoomSearchView.tsx

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

src/components/structures/RoomSearchView.tsx

@@ -50,7 +50,6 @@ interface Props {
     promise: Promise<ISearchResults>;
     abortController?: AbortController;
     resizeNotifier: ResizeNotifier;
-    permalinkCreator: RoomPermalinkCreator;
     className: string;
     onUpdate(inProgress: boolean, results: ISearchResults | null): void;
 }
@@ -59,7 +58,7 @@ interface Props {
 // XXX: why doesn't searching on name work?
 export const RoomSearchView = forwardRef<ScrollPanel, Props>(
     (
-        { term, scope, promise, abortController, resizeNotifier, permalinkCreator, className, onUpdate }: Props,
+        { term, scope, promise, abortController, resizeNotifier, className, onUpdate }: Props,
         ref: RefObject<ScrollPanel>,
     ) => {
         const client = useContext(MatrixClientContext);
@@ -68,6 +67,15 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
         const [highlights, setHighlights] = useState<string[] | null>(null);
         const [results, setResults] = useState<ISearchResults | null>(null);
         const aborted = useRef(false);
+        // A map from room ID to permalink creator
+        const permalinkCreators = useRef(new Map<string, RoomPermalinkCreator>()).current;
+
+        useEffect(() => {
+            return () => {
+                permalinkCreators.forEach((pc) => pc.stop());
+                permalinkCreators.clear();
+            };
+        }, [permalinkCreators]);
 
         const handleSearchResult = useCallback(
             (searchPromise: Promise<ISearchResults>): Promise<boolean> => {
@@ -217,7 +225,7 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
             const result = results.results[i];
 
             const mxEv = result.context.getEvent();
-            const roomId = mxEv.getRoomId();
+            const roomId = mxEv.getRoomId()!;
             const room = client.getRoom(roomId);
             if (!room) {
                 // if we do not have the room in js-sdk stores then hide it as we cannot easily show it
@@ -283,6 +291,13 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
                 ourEventsIndexes.push(result.context.getOurEventIndex());
             }
 
+            let permalinkCreator = permalinkCreators.get(roomId);
+            if (!permalinkCreator) {
+                permalinkCreator = new RoomPermalinkCreator(room);
+                permalinkCreator.start();
+                permalinkCreators.set(roomId, permalinkCreator);
+            }
+
             ret.push(
                 <SearchResultTile
                     key={mxEv.getId()}

src/components/structures/RoomView.tsx

@@ -2265,7 +2265,6 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
                     promise={this.state.search.promise}
                     abortController={this.state.search.abortController}
                     resizeNotifier={this.props.resizeNotifier}
-                    permalinkCreator={this.permalinkCreator}
                     className={this.messagePanelClassNames}
                     onUpdate={this.onSearchUpdate}
                 />

test/components/structures/RoomSearchView-test.tsx

@@ -28,7 +28,6 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix";
 import { RoomSearchView } from "../../../src/components/structures/RoomSearchView";
 import { SearchScope } from "../../../src/components/views/rooms/SearchBar";
 import ResizeNotifier from "../../../src/utils/ResizeNotifier";
-import { RoomPermalinkCreator } from "../../../src/utils/permalinks/Permalinks";
 import { stubClient } from "../../test-utils";
 import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
 import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
@@ -43,15 +42,13 @@ describe("<RoomSearchView/>", () => {
     const resizeNotifier = new ResizeNotifier();
     let client: MatrixClient;
     let room: Room;
-    let permalinkCreator: RoomPermalinkCreator;
 
     beforeEach(async () => {
         stubClient();
         client = MatrixClientPeg.get();
         client.supportsThreads = jest.fn().mockReturnValue(true);
-        room = new Room("!room:server", client, client.getUserId()!);
+        room = new Room("!room:server", client, client.getSafeUserId());
         mocked(client.getRoom).mockReturnValue(room);
-        permalinkCreator = new RoomPermalinkCreator(room, room.roomId);
 
         jest.spyOn(Element.prototype, "clientHeight", "get").mockReturnValue(100);
     });
@@ -69,7 +66,6 @@ describe("<RoomSearchView/>", () => {
                 scope={SearchScope.All}
                 promise={deferred.promise}
                 resizeNotifier={resizeNotifier}
-                permalinkCreator={permalinkCreator}
                 className="someClass"
                 onUpdate={jest.fn()}
             />,
@@ -92,7 +88,7 @@ describe("<RoomSearchView/>", () => {
                                     result: {
                                         room_id: room.roomId,
                                         event_id: "$2",
-                                        sender: client.getUserId()!,
+                                        sender: client.getSafeUserId(),
                                         origin_server_ts: 1,
                                         content: { body: "Foo Test Bar", msgtype: "m.text" },
                                         type: EventType.RoomMessage,
@@ -103,7 +99,7 @@ describe("<RoomSearchView/>", () => {
                                             {
                                                 room_id: room.roomId,
                                                 event_id: "$1",
-                                                sender: client.getUserId()!,
+                                                sender: client.getSafeUserId(),
                                                 origin_server_ts: 1,
                                                 content: { body: "Before", msgtype: "m.text" },
                                                 type: EventType.RoomMessage,
@@ -113,7 +109,7 @@ describe("<RoomSearchView/>", () => {
                                             {
                                                 room_id: room.roomId,
                                                 event_id: "$3",
-                                                sender: client.getUserId()!,
+                                                sender: client.getSafeUserId(),
                                                 origin_server_ts: 1,
                                                 content: { body: "After", msgtype: "m.text" },
                                                 type: EventType.RoomMessage,
@@ -128,7 +124,6 @@ describe("<RoomSearchView/>", () => {
                         count: 1,
                     })}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -154,7 +149,7 @@ describe("<RoomSearchView/>", () => {
                                     result: {
                                         room_id: room.roomId,
                                         event_id: "$2",
-                                        sender: client.getUserId()!,
+                                        sender: client.getSafeUserId(),
                                         origin_server_ts: 1,
                                         content: { body: "Foo Test Bar", msgtype: "m.text" },
                                         type: EventType.RoomMessage,
@@ -172,7 +167,6 @@ describe("<RoomSearchView/>", () => {
                         count: 1,
                     })}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -192,7 +186,7 @@ describe("<RoomSearchView/>", () => {
                         result: {
                             room_id: room.roomId,
                             event_id: "$2",
-                            sender: client.getUserId()!,
+                            sender: client.getSafeUserId(),
                             origin_server_ts: 1,
                             content: { body: "Foo Test Bar", msgtype: "m.text" },
                             type: EventType.RoomMessage,
@@ -221,7 +215,7 @@ describe("<RoomSearchView/>", () => {
                         result: {
                             room_id: room.roomId,
                             event_id: "$4",
-                            sender: client.getUserId()!,
+                            sender: client.getSafeUserId(),
                             origin_server_ts: 4,
                             content: { body: "Potato", msgtype: "m.text" },
                             type: EventType.RoomMessage,
@@ -245,7 +239,6 @@ describe("<RoomSearchView/>", () => {
                     scope={SearchScope.All}
                     promise={Promise.resolve(searchResults)}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -267,7 +260,6 @@ describe("<RoomSearchView/>", () => {
                     scope={SearchScope.All}
                     promise={deferred.promise}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -291,7 +283,6 @@ describe("<RoomSearchView/>", () => {
                     scope={SearchScope.All}
                     promise={deferred.promise}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -315,7 +306,6 @@ describe("<RoomSearchView/>", () => {
                     scope={SearchScope.All}
                     promise={deferred.promise}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -417,7 +407,6 @@ describe("<RoomSearchView/>", () => {
                     scope={SearchScope.All}
                     promise={Promise.resolve(searchResults)}
                     resizeNotifier={resizeNotifier}
-                    permalinkCreator={permalinkCreator}
                     className="someClass"
                     onUpdate={jest.fn()}
                 />
@@ -437,4 +426,130 @@ describe("<RoomSearchView/>", () => {
         expect(betweenNode.compareDocumentPosition(foo2Node) == Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
         expect(foo2Node.compareDocumentPosition(afterNode) == Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
     });
+
+    it("should pass appropriate permalink creator for all rooms search", async () => {
+        const room2 = new Room("!room2:server", client, client.getSafeUserId());
+        const room3 = new Room("!room3:server", client, client.getSafeUserId());
+        mocked(client.getRoom).mockImplementation(
+            (roomId) => [room, room2, room3].find((r) => r.roomId === roomId) ?? null,
+        );
+
+        render(
+            <MatrixClientContext.Provider value={client}>
+                <RoomSearchView
+                    term="search term"
+                    scope={SearchScope.All}
+                    promise={Promise.resolve<ISearchResults>({
+                        results: [
+                            SearchResult.fromJson(
+                                {
+                                    rank: 1,
+                                    result: {
+                                        room_id: room.roomId,
+                                        event_id: "$2",
+                                        sender: client.getSafeUserId(),
+                                        origin_server_ts: 1,
+                                        content: { body: "Room 1", msgtype: "m.text" },
+                                        type: EventType.RoomMessage,
+                                    },
+                                    context: {
+                                        profile_info: {},
+                                        events_before: [],
+                                        events_after: [],
+                                    },
+                                },
+                                eventMapper,
+                            ),
+                            SearchResult.fromJson(
+                                {
+                                    rank: 2,
+                                    result: {
+                                        room_id: room2.roomId,
+                                        event_id: "$22",
+                                        sender: client.getSafeUserId(),
+                                        origin_server_ts: 1,
+                                        content: { body: "Room 2", msgtype: "m.text" },
+                                        type: EventType.RoomMessage,
+                                    },
+                                    context: {
+                                        profile_info: {},
+                                        events_before: [],
+                                        events_after: [],
+                                    },
+                                },
+                                eventMapper,
+                            ),
+                            SearchResult.fromJson(
+                                {
+                                    rank: 2,
+                                    result: {
+                                        room_id: room2.roomId,
+                                        event_id: "$23",
+                                        sender: client.getSafeUserId(),
+                                        origin_server_ts: 2,
+                                        content: { body: "Room 2 message 2", msgtype: "m.text" },
+                                        type: EventType.RoomMessage,
+                                    },
+                                    context: {
+                                        profile_info: {},
+                                        events_before: [],
+                                        events_after: [],
+                                    },
+                                },
+                                eventMapper,
+                            ),
+                            SearchResult.fromJson(
+                                {
+                                    rank: 3,
+                                    result: {
+                                        room_id: room3.roomId,
+                                        event_id: "$32",
+                                        sender: client.getSafeUserId(),
+                                        origin_server_ts: 1,
+                                        content: { body: "Room 3", msgtype: "m.text" },
+                                        type: EventType.RoomMessage,
+                                    },
+                                    context: {
+                                        profile_info: {},
+                                        events_before: [],
+                                        events_after: [],
+                                    },
+                                },
+                                eventMapper,
+                            ),
+                        ],
+                        highlights: [],
+                        count: 1,
+                    })}
+                    resizeNotifier={resizeNotifier}
+                    className="someClass"
+                    onUpdate={jest.fn()}
+                />
+            </MatrixClientContext.Provider>,
+        );
+
+        const event1 = await screen.findByText("Room 1");
+        expect(event1.closest(".mx_EventTile_line").querySelector("a")).toHaveAttribute(
+            "href",
+            `https://matrix.to/#/${room.roomId}/$2`,
+        );
+
+        const event2 = await screen.findByText("Room 2");
+        expect(event2.closest(".mx_EventTile_line").querySelector("a")).toHaveAttribute(
+            "href",
+            `https://matrix.to/#/${room2.roomId}/$22`,
+        );
+
+        const event2Message2 = await screen.findByText("Room 2 message 2");
+        expect(event2Message2.closest(".mx_EventTile_line").querySelector("a")).toHaveAttribute(
+            "href",
+            `https://matrix.to/#/${room2.roomId}/$23`,
+        );
+
+        const event3 = await screen.findByText("Room 3");
+        expect(event3.closest(".mx_EventTile_line").querySelector("a")).toHaveAttribute(
+            "href",
+            `https://matrix.to/#/${room3.roomId}/$32`,
+        );
+    });
 });