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

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

Merged
merged 12 commits into from
Apr 26, 2023
Merged

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ecd2a8e
Fix all rooms search generating permalinks to wrong room id
t3chguy Apr 17, 2023
08dc09e
Iterate
t3chguy Apr 17, 2023
64bf46e
Add comment
t3chguy Apr 17, 2023
7f07110
Iterate
t3chguy Apr 17, 2023
516cf7e
Merge branch 'develop' into t3chguy/fix/25115
t3chguy Apr 18, 2023
6540bb3
Add coverage
t3chguy Apr 18, 2023
97e3743
Merge remote-tracking branch 'origin/t3chguy/fix/25115' into t3chguy/…
t3chguy Apr 18, 2023
3c02237
Iterate
t3chguy Apr 18, 2023
dadd7e6
Add comment
t3chguy Apr 18, 2023
81c1a8d
Restore src/utils/permalinks/Permalinks.ts
t3chguy Apr 18, 2023
c583f0a
Update src/components/structures/RoomSearchView.tsx
t3chguy Apr 18, 2023
be56e7a
Merge branch 'develop' into t3chguy/fix/25115
t3chguy Apr 26, 2023
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
35 changes: 33 additions & 2 deletions src/components/structures/RoomSearchView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,16 @@ 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,
permalinkCreator: _permalinkCreator,
className,
onUpdate,
}: Props,
ref: RefObject<ScrollPanel>,
) => {
const client = useContext(MatrixClientContext);
Expand All @@ -68,6 +77,17 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
const [highlights, setHighlights] = useState<string[] | null>(null);
const [results, setResults] = useState<ISearchResults | null>(null);
const aborted = useRef(false);
// The permalinkCreator prop we are passed is only for the room the user was viewing
// So we will need to create additional room permalink creators for "All rooms" mode results
// to be able to generate share permalinks for results from other rooms
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 +237,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 +303,17 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
ourEventsIndexes.push(result.context.getOurEventIndex());
}

let permalinkCreator = _permalinkCreator;
if (roomId !== permalinkCreator.roomId) {
if (permalinkCreators.has(roomId)) {
permalinkCreator = permalinkCreators.get(roomId)!;
} else {
permalinkCreator = new RoomPermalinkCreator(client.getRoom(roomId), roomId);
permalinkCreator.start();
permalinkCreators.set(roomId, permalinkCreator);
}
}

ret.push(
<SearchResultTile
key={mxEv.getId()}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/permalinks/Permalinks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down
141 changes: 134 additions & 7 deletions test/components/structures/RoomSearchView-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe("<RoomSearchView/>", () => {
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);

Expand Down Expand Up @@ -92,7 +92,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 +103,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 +113,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 Down Expand Up @@ -154,7 +154,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 @@ -192,7 +192,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 +221,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 Down Expand Up @@ -437,4 +437,131 @@ 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}
permalinkCreator={permalinkCreator}
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`,
);
});
});