From b6b9ce3c46289f5fa42be13b2ab09c290335f076 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 1 Jun 2023 13:35:47 +0100 Subject: [PATCH] When joining room in sub-space join the parents too (#11011) * When joining room in sub-space join the parents too * Fix joined state not updating on sync * Add membership check * Update tests * Improve coverage * Make TS happier * Make TS happier --- src/components/structures/SpaceHierarchy.tsx | 20 +- .../structures/SpaceHierarchy-test.tsx | 157 ++++-- .../SpaceHierarchy-test.tsx.snap | 445 ++++++++++++++---- test/test-utils/test-utils.ts | 1 + 4 files changed, 474 insertions(+), 149 deletions(-) diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index fe246f6b354..77d85bf55bb 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -32,7 +32,7 @@ import { Room, RoomEvent } from "matrix-js-sdk/src/models/room"; import { RoomHierarchy } from "matrix-js-sdk/src/room-hierarchy"; import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import { IHierarchyRelation, IHierarchyRoom } from "matrix-js-sdk/src/@types/spaces"; -import { MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix"; +import { ClientEvent, MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix"; import classNames from "classnames"; import { sortBy, uniqBy } from "lodash"; import { GuestAccess, HistoryVisibility } from "matrix-js-sdk/src/@types/partials"; @@ -101,7 +101,7 @@ const Tile: React.FC = ({ children, }) => { const cli = useContext(MatrixClientContext); - const [joinedRoom, setJoinedRoom] = useState(() => { + const joinedRoom = useTypedEventEmitterState(cli, ClientEvent.Room, () => { const cliRoom = cli?.getRoom(room.room_id); return cliRoom?.getMyMembership() === "join" ? cliRoom : undefined; }); @@ -128,7 +128,6 @@ const Tile: React.FC = ({ ev.stopPropagation(); onJoinRoomClick() .then(() => awaitRoomDownSync(cli, room.room_id)) - .then(setJoinedRoom) .finally(() => { setBusy(false); }); @@ -429,7 +428,7 @@ interface IHierarchyLevelProps { parents: Set; selectedMap?: Map>; onViewRoomClick(roomId: string, roomType?: RoomType): void; - onJoinRoomClick(roomId: string): Promise; + onJoinRoomClick(roomId: string, parents: Set): Promise; onToggleClick?(parentId: string, childId: string): void; } @@ -511,7 +510,7 @@ export const HierarchyLevel: React.FC = ({ suggested={hierarchy.isSuggested(root.room_id, room.room_id)} selected={selectedMap?.get(root.room_id)?.has(room.room_id)} onViewRoomClick={() => onViewRoomClick(room.room_id, room.room_type as RoomType)} - onJoinRoomClick={() => onJoinRoomClick(room.room_id)} + onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} hasPermissions={hasPermissions} onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} /> @@ -532,7 +531,7 @@ export const HierarchyLevel: React.FC = ({ suggested={hierarchy.isSuggested(root.room_id, space.room_id)} selected={selectedMap?.get(root.room_id)?.has(space.room_id)} onViewRoomClick={() => onViewRoomClick(space.room_id, RoomType.Space)} - onJoinRoomClick={() => onJoinRoomClick(space.room_id)} + onJoinRoomClick={() => onJoinRoomClick(space.room_id, newParents)} hasPermissions={hasPermissions} onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, space.room_id) : undefined} > @@ -839,7 +838,14 @@ const SpaceHierarchy: React.FC = ({ space, initialText = "", showRoom, a selectedMap={selected} onToggleClick={hasPermissions ? onToggleClick : undefined} onViewRoomClick={(roomId, roomType) => showRoom(cli, hierarchy, roomId, roomType)} - onJoinRoomClick={(roomId) => joinRoom(cli, hierarchy, roomId)} + onJoinRoomClick={async (roomId, parents) => { + for (const parent of parents) { + if (cli.getRoom(parent)?.getMyMembership() !== "join") { + await joinRoom(cli, hierarchy, parent); + } + } + await joinRoom(cli, hierarchy, roomId); + }} /> ); diff --git a/test/components/structures/SpaceHierarchy-test.tsx b/test/components/structures/SpaceHierarchy-test.tsx index b81a84facaa..95842df7d63 100644 --- a/test/components/structures/SpaceHierarchy-test.tsx +++ b/test/components/structures/SpaceHierarchy-test.tsx @@ -16,7 +16,7 @@ limitations under the License. import React from "react"; import { mocked } from "jest-mock"; -import { render } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from "@testing-library/react"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { Room } from "matrix-js-sdk/src/models/room"; import { RoomHierarchy } from "matrix-js-sdk/src/room-hierarchy"; @@ -25,7 +25,7 @@ import { IHierarchyRoom } from "matrix-js-sdk/src/@types/spaces"; import { MatrixClientPeg } from "../../../src/MatrixClientPeg"; import { mkStubRoom, stubClient } from "../../test-utils"; import dispatcher from "../../../src/dispatcher/dispatcher"; -import { HierarchyLevel, showRoom, toLocalRoom } from "../../../src/components/structures/SpaceHierarchy"; +import SpaceHierarchy, { showRoom, toLocalRoom } from "../../../src/components/structures/SpaceHierarchy"; import { Action } from "../../../src/dispatcher/actions"; import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; import DMRoomMap from "../../../src/utils/DMRoomMap"; @@ -158,7 +158,18 @@ describe("SpaceHierarchy", () => { }); }); - describe("", () => { + describe("", () => { + beforeEach(() => { + // IntersectionObserver isn't available in test environment + const mockIntersectionObserver = jest.fn(); + mockIntersectionObserver.mockReturnValue({ + observe: () => null, + unobserve: () => null, + disconnect: () => null, + }); + window.IntersectionObserver = mockIntersectionObserver; + }); + stubClient(); const client = MatrixClientPeg.get(); @@ -167,55 +178,123 @@ describe("SpaceHierarchy", () => { } as unknown as DMRoomMap; jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap); - const root = mkStubRoom("room-id-1", "Room 1", client); - const room1 = mkStubRoom("room-id-2", "Room 2", client); - const room2 = mkStubRoom("room-id-3", "Room 3", client); + const root = mkStubRoom("space-id-1", "Space 1", client); + const room1 = mkStubRoom("room-id-2", "Room 1", client); + const room2 = mkStubRoom("room-id-3", "Room 2", client); + const space1 = mkStubRoom("space-id-4", "Space 2", client); + const room3 = mkStubRoom("room-id-5", "Room 3", client); + mocked(client.getRooms).mockReturnValue([root]); + mocked(client.getRoom).mockImplementation( + (roomId) => client.getRooms().find((room) => room.roomId === roomId) ?? null, + ); + [room1, room2, space1, room3].forEach((r) => mocked(r.getMyMembership).mockReturnValue("leave")); - const hierarchyRoot = { + const hierarchyRoot: IHierarchyRoom = { room_id: root.roomId, num_joined_members: 1, + room_type: "m.space", children_state: [ { state_key: room1.roomId, content: { order: "1" }, + origin_server_ts: 111, + type: "m.space.child", + sender: "@other:server", }, { state_key: room2.roomId, content: { order: "2" }, + origin_server_ts: 111, + type: "m.space.child", + sender: "@other:server", + }, + { + state_key: space1.roomId, + content: { order: "3" }, + origin_server_ts: 111, + type: "m.space.child", + sender: "@other:server", }, ], - } as IHierarchyRoom; - const hierarchyRoom1 = { room_id: room1.roomId, num_joined_members: 2 } as IHierarchyRoom; - const hierarchyRoom2 = { room_id: root.roomId, num_joined_members: 3 } as IHierarchyRoom; - - const roomHierarchy = { - roomMap: new Map([ - [root.roomId, hierarchyRoot], - [room1.roomId, hierarchyRoom1], - [room2.roomId, hierarchyRoom2], - ]), - isSuggested: jest.fn(), - } as unknown as RoomHierarchy; - - it("renders", () => { - const defaultProps = { - root: hierarchyRoot, - roomSet: new Set([hierarchyRoom1, hierarchyRoom2]), - hierarchy: roomHierarchy, - parents: new Set(), - selectedMap: new Map>(), - onViewRoomClick: jest.fn(), - onJoinRoomClick: jest.fn(), - onToggleClick: jest.fn(), - }; - const getComponent = (props = {}): React.ReactElement => ( - - ; - - ); - - const { container } = render(getComponent()); - expect(container).toMatchSnapshot(); + world_readable: true, + guest_can_join: true, + }; + const hierarchyRoom1: IHierarchyRoom = { + room_id: room1.roomId, + num_joined_members: 2, + children_state: [], + world_readable: true, + guest_can_join: true, + }; + const hierarchyRoom2: IHierarchyRoom = { + room_id: room2.roomId, + num_joined_members: 3, + children_state: [], + world_readable: true, + guest_can_join: true, + }; + const hierarchyRoom3: IHierarchyRoom = { + name: "Nested room", + room_id: room3.roomId, + num_joined_members: 3, + children_state: [], + world_readable: true, + guest_can_join: true, + }; + const hierarchySpace1: IHierarchyRoom = { + room_id: space1.roomId, + name: "Nested space", + num_joined_members: 1, + room_type: "m.space", + children_state: [ + { + state_key: room3.roomId, + content: { order: "1" }, + origin_server_ts: 111, + type: "m.space.child", + sender: "@other:server", + }, + ], + world_readable: true, + guest_can_join: true, + }; + + mocked(client.getRoomHierarchy).mockResolvedValue({ + rooms: [hierarchyRoot, hierarchyRoom1, hierarchyRoom2, hierarchySpace1, hierarchyRoom3], + }); + + const defaultProps = { + space: root, + showRoom: jest.fn(), + }; + const getComponent = (props = {}): React.ReactElement => ( + + ; + + ); + + it("renders", async () => { + const { asFragment } = render(getComponent()); + // Wait for spinners to go away + await waitForElementToBeRemoved(screen.getAllByRole("progressbar")); + expect(asFragment()).toMatchSnapshot(); + }); + + it("should join subspace when joining nested room", async () => { + mocked(client.joinRoom).mockResolvedValue({} as Room); + + const { getByText } = render(getComponent()); + // Wait for spinners to go away + await waitForElementToBeRemoved(screen.getAllByRole("progressbar")); + const button = getByText("Nested room")!.closest("li")!.querySelector(".mx_AccessibleButton_kind_primary")!; + fireEvent.click(button); + + await waitFor(() => { + expect(client.joinRoom).toHaveBeenCalledTimes(2); + }); + // Joins subspace + expect(client.joinRoom).toHaveBeenCalledWith(space1.roomId, expect.any(Object)); + expect(client.joinRoom).toHaveBeenCalledWith(room3.roomId, expect.any(Object)); }); }); }); diff --git a/test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap b/test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap index 83c347affe8..ea82ff880e3 100644 --- a/test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap +++ b/test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap @@ -1,161 +1,400 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`SpaceHierarchy renders 1`] = ` -
-
  • renders 1`] = ` + +
    +
    +
    +
    +

    + Rooms and spaces +

    +
    + Remove +
    +
    + Mark as not suggested +
    +
    +
    +
      +
    • +
      - -
      -
      - Unnamed Room
      + + + + +
      +
      + Unnamed Room +
      +
      - Joined + 2 members
      - 2 members - · +
      + Join +
      + class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid" + > + +
      +
      • +
    • - View +
      + + + + +
      +
      + Unnamed Room +
      +
      + 3 members +
      - - -
      -
    -
    • -
  • -
    +
  • - -
    -
    - Unnamed Room
    + + + + +
    +
    + Nested space +
    +
    - Joined + 1 member · 1 room
    - 3 members - · +
    + Join +
    + class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid" + > + +
    • +
  • +
    - View +
    + + + + +
    +
    + Nested room +
    +
    + 3 members +
    - - -
    -
    • + + ; -
    + `; diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index c9cf352c481..9a36c0d5185 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -236,6 +236,7 @@ export function createTestClient(): MatrixClient { searchUserDirectory: jest.fn().mockResolvedValue({ limited: false, results: [] }), setDeviceVerified: jest.fn(), + joinRoom: jest.fn(), } as unknown as MatrixClient; client.reEmitter = new ReEmitter(client);