Skip to content
This repository has been 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 all 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
21 changes: 18 additions & 3 deletions src/components/structures/RoomSearchView.tsx
View file
Open in desktop
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;
t3chguy marked this conversation as resolved.
Show resolved Hide resolved

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
Open in desktop
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
Open in desktop
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`,
);
});
});