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

Commit

Permalink
Fix all rooms search generating permalinks to wrong room id (#10625)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@t3chguy @richvdh
t3chguy and richvdh authored Apr 26, 2023
1 parent 7f32b42 commit 2b3e0b4
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 22 deletions.
21 changes: 18 additions & 3 deletions src/components/structures/RoomSearchView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ interface Props {
promise: Promise<ISearchResults>;
abortController?: AbortController;
resizeNotifier: ResizeNotifier;
permalinkCreator: RoomPermalinkCreator;
className: string;
onUpdate(inProgress: boolean, results: ISearchResults | null): void;
}
Expand All @@ -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);
Expand All @@ -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> => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()}
Expand Down
1 change: 0 additions & 1 deletion src/components/structures/RoomView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>
Expand Down
151 changes: 133 additions & 18 deletions test/components/structures/RoomSearchView-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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);
});
Expand All @@ -69,7 +66,6 @@ describe("<RoomSearchView/>", () => {
scope={SearchScope.All}
promise={deferred.promise}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -128,7 +124,6 @@ describe("<RoomSearchView/>", () => {
count: 1,
})}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand All @@ -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,
Expand All @@ -172,7 +167,6 @@ describe("<RoomSearchView/>", () => {
count: 1,
})}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -245,7 +239,6 @@ describe("<RoomSearchView/>", () => {
scope={SearchScope.All}
promise={Promise.resolve(searchResults)}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand All @@ -267,7 +260,6 @@ describe("<RoomSearchView/>", () => {
scope={SearchScope.All}
promise={deferred.promise}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand All @@ -291,7 +283,6 @@ describe("<RoomSearchView/>", () => {
scope={SearchScope.All}
promise={deferred.promise}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand All @@ -315,7 +306,6 @@ describe("<RoomSearchView/>", () => {
scope={SearchScope.All}
promise={deferred.promise}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand Down Expand Up @@ -417,7 +407,6 @@ describe("<RoomSearchView/>", () => {
scope={SearchScope.All}
promise={Promise.resolve(searchResults)}
resizeNotifier={resizeNotifier}
permalinkCreator={permalinkCreator}
className="someClass"
onUpdate={jest.fn()}
/>
Expand All @@ -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`,
);
});
});

0 comments on commit 2b3e0b4

Please sign in to comment.