Fix all rooms search generating permalinks to wrong room id

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(client.getRoom(roomId), roomId);
+ permalinkCreator.start();
+ permalinkCreators.set(roomId, permalinkCreator);
+ }
+
  ret.push(
  <SearchResultTile
  key={mxEv.getId()}

src/components/structures/RoomView.tsx

@@ -2261,7 +2261,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}
  />

src/utils/permalinks/Permalinks.ts

@@ -82,7 +82,7 @@ const ANY_REGEX = /.*/;
 // the list and magically have the link work.
 
 export class RoomPermalinkCreator {
- private roomId: string;
+ public readonly roomId: string;
  private highestPlUserId: string | null = null;
  private populationMap: { [serverName: string]: number } = {};
  private bannedHostsRegexps: RegExp[] = [];

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`,
+ );
+ });
 });