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

@@ -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<ITileProps> = ({
     children,
 }) => {
     const cli = useContext(MatrixClientContext);
-    const [joinedRoom, setJoinedRoom] = useState<Room | undefined>(() => {
+    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<ITileProps> = ({
         ev.stopPropagation();
         onJoinRoomClick()
             .then(() => awaitRoomDownSync(cli, room.room_id))
-            .then(setJoinedRoom)
             .finally(() => {
                 setBusy(false);
             });
@@ -429,7 +428,7 @@ interface IHierarchyLevelProps {
     parents: Set<string>;
     selectedMap?: Map<string, Set<string>>;
     onViewRoomClick(roomId: string, roomType?: RoomType): void;
-    onJoinRoomClick(roomId: string): Promise<unknown>;
+    onJoinRoomClick(roomId: string, parents: Set<string>): Promise<unknown>;
     onToggleClick?(parentId: string, childId: string): void;
 }
 
@@ -511,7 +510,7 @@ export const HierarchyLevel: React.FC<IHierarchyLevelProps> = ({
                     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<IHierarchyLevelProps> = ({
                         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<IProps> = ({ 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);
+                                    }}
                                 />
                             </>
                         );

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("<HierarchyLevel />", () => {
+    describe("<SpaceHierarchy />", () => {
+        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 root = mkStubRoom("space-id-1", "Space 1", client);
-        const room1 = mkStubRoom("room-id-2", "Room 2", client);
+        const room1 = mkStubRoom("room-id-2", "Room 1", client);
-        const room2 = mkStubRoom("room-id-3", "Room 3", 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;
+            world_readable: true,
-        const hierarchyRoom1 = { room_id: room1.roomId, num_joined_members: 2 } as IHierarchyRoom;
+            guest_can_join: true,
-        const hierarchyRoom2 = { room_id: root.roomId, num_joined_members: 3 } as IHierarchyRoom;
+        };
+        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,
+        };
 
-        const roomHierarchy = {
+        mocked(client.getRoomHierarchy).mockResolvedValue({
-            roomMap: new Map([
+            rooms: [hierarchyRoot, hierarchyRoom1, hierarchyRoom2, hierarchySpace1, hierarchyRoom3],
-                [root.roomId, hierarchyRoot],
+        });
-                [room1.roomId, hierarchyRoom1],
-                [room2.roomId, hierarchyRoom2],
-            ]),
-            isSuggested: jest.fn(),
-        } as unknown as RoomHierarchy;
 
-        it("renders", () => {
         const defaultProps = {
-                root: hierarchyRoot,
+            space: root,
-                roomSet: new Set([hierarchyRoom1, hierarchyRoom2]),
+            showRoom: jest.fn(),
-                hierarchy: roomHierarchy,
-                parents: new Set<string>(),
-                selectedMap: new Map<string, Set<string>>(),
-                onViewRoomClick: jest.fn(),
-                onJoinRoomClick: jest.fn(),
-                onToggleClick: jest.fn(),
         };
         const getComponent = (props = {}): React.ReactElement => (
             <MatrixClientContext.Provider value={client}>
-                    <HierarchyLevel {...defaultProps} {...props} />;
+                <SpaceHierarchy {...defaultProps} {...props} />;
             </MatrixClientContext.Provider>
         );
 
-            const { container } = render(getComponent());
+        it("renders", async () => {
-            expect(container).toMatchSnapshot();
+            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));
         });
     });
 });

test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap

@@ -1,7 +1,60 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`SpaceHierarchy <HierarchyLevel /> renders 1`] = `
+exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
-<div>
+<DocumentFragment>
+  <div
+    class="mx_SearchBox mx_textinput"
+  >
+    <input
+      autocomplete="off"
+      class="mx_textinput_icon mx_textinput_search mx_SpaceHierarchy_search mx_textinput_icon mx_textinput_search"
+      data-testid="searchbox-input"
+      placeholder="Search names and descriptions"
+      type="text"
+      value=""
+    />
+    <div
+      class="mx_AccessibleButton mx_SearchBox_closeButton"
+      role="button"
+      tabindex="-1"
+    />
+  </div>
+  <div
+    class="mx_SpaceHierarchy_listHeader"
+  >
+    <h4
+      class="mx_SpaceHierarchy_listHeader_header"
+    >
+      Rooms and spaces
+    </h4>
+    <div
+      class="mx_SpaceHierarchy_listHeader_buttons"
+    >
+      <div
+        aria-disabled="true"
+        class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_danger_outline mx_AccessibleButton_disabled"
+        disabled=""
+        role="button"
+        tabindex="0"
+      >
+        Remove
+      </div>
+      <div
+        aria-disabled="true"
+        class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline mx_AccessibleButton_disabled"
+        disabled=""
+        role="button"
+        tabindex="0"
+      >
+        Mark as not suggested
+      </div>
+    </div>
+  </div>
+  <ul
+    aria-label="Space"
+    class="mx_SpaceHierarchy_list"
+    role="tree"
+  >
     <li
       class="mx_SpaceHierarchy_roomTileWrapper"
       role="treeitem"
@@ -9,7 +62,7 @@ exports[`SpaceHierarchy <HierarchyLevel /> renders 1`] = `
       <div
         class="mx_AccessibleButton mx_SpaceHierarchy_roomTile"
         role="button"
-      tabindex="-1"
+        tabindex="0"
       >
         <div
           class="mx_SpaceHierarchy_roomTile_item"
@@ -17,51 +70,55 @@ exports[`SpaceHierarchy <HierarchyLevel /> renders 1`] = `
           <div
             class="mx_SpaceHierarchy_roomTile_avatar"
           >
+            <span
+              class="mx_BaseAvatar"
+              role="presentation"
+            >
+              <span
+                aria-hidden="true"
+                class="mx_BaseAvatar_initial"
+                style="font-size: 13px; width: 20px; line-height: 20px;"
+              >
+                U
+              </span>
               <img
                 alt=""
-            class="mx_BaseAvatar mx_BaseAvatar_image"
+                aria-hidden="true"
+                class="mx_BaseAvatar_image"
                 data-testid="avatar-img"
                 loading="lazy"
-            src="http://this.is.a.url/avatar.url/room.png"
+                src="data:image/png;base64,00"
                 style="width: 20px; height: 20px;"
               />
+            </span>
           </div>
           <div
             class="mx_SpaceHierarchy_roomTile_name"
           >
             Unnamed Room
-          <div
-            class="mx_SpaceHierarchy_roomTile_joined"
-          >
-            Joined
-          </div>
           </div>
           <div
             class="mx_SpaceHierarchy_roomTile_info"
           >
             2 members
-           · 
-          <span
-            dir="auto"
-          />
           </div>
         </div>
         <div
           class="mx_SpaceHierarchy_actions"
         >
           <div
-          class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline"
+            class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
             role="button"
-          tabindex="-1"
+            tabindex="0"
           >
-          View
+            Join
           </div>
           <span
             class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid"
           >
             <input
               id="checkbox_abdefghi"
-            tabindex="-1"
+              tabindex="0"
               type="checkbox"
             />
             <label
@@ -94,44 +151,48 @@ exports[`SpaceHierarchy <HierarchyLevel /> renders 1`] = `
           <div
             class="mx_SpaceHierarchy_roomTile_avatar"
           >
+            <span
+              class="mx_BaseAvatar"
+              role="presentation"
+            >
+              <span
+                aria-hidden="true"
+                class="mx_BaseAvatar_initial"
+                style="font-size: 13px; width: 20px; line-height: 20px;"
+              >
+                U
+              </span>
               <img
                 alt=""
-            class="mx_BaseAvatar mx_BaseAvatar_image"
+                aria-hidden="true"
+                class="mx_BaseAvatar_image"
                 data-testid="avatar-img"
                 loading="lazy"
-            src="http://this.is.a.url/avatar.url/room.png"
+                src="data:image/png;base64,00"
                 style="width: 20px; height: 20px;"
               />
+            </span>
           </div>
           <div
             class="mx_SpaceHierarchy_roomTile_name"
           >
             Unnamed Room
-          <div
-            class="mx_SpaceHierarchy_roomTile_joined"
-          >
-            Joined
-          </div>
           </div>
           <div
             class="mx_SpaceHierarchy_roomTile_info"
           >
             3 members
-           · 
-          <span
-            dir="auto"
-          />
           </div>
         </div>
         <div
           class="mx_SpaceHierarchy_actions"
         >
           <div
-          class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline"
+            class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
             role="button"
             tabindex="-1"
           >
-          View
+            Join
           </div>
           <span
             class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid"
@@ -156,6 +217,184 @@ exports[`SpaceHierarchy <HierarchyLevel /> renders 1`] = `
         </div>
       </div>
     </li>
-  ;
+    <li
+      aria-expanded="true"
+      class="mx_SpaceHierarchy_roomTileWrapper"
+      role="treeitem"
+    >
+      <div
+        class="mx_AccessibleButton mx_SpaceHierarchy_roomTile mx_SpaceHierarchy_subspace"
+        role="button"
+        tabindex="-1"
+      >
+        <div
+          class="mx_SpaceHierarchy_roomTile_item"
+        >
+          <div
+            class="mx_SpaceHierarchy_roomTile_avatar"
+          >
+            <span
+              class="mx_BaseAvatar"
+              role="presentation"
+            >
+              <span
+                aria-hidden="true"
+                class="mx_BaseAvatar_initial"
+                style="font-size: 13px; width: 20px; line-height: 20px;"
+              >
+                N
+              </span>
+              <img
+                alt=""
+                aria-hidden="true"
+                class="mx_BaseAvatar_image"
+                data-testid="avatar-img"
+                loading="lazy"
+                src="data:image/png;base64,00"
+                style="width: 20px; height: 20px;"
+              />
+            </span>
           </div>
+          <div
+            class="mx_SpaceHierarchy_roomTile_name"
+          >
+            Nested space
+          </div>
+          <div
+            class="mx_SpaceHierarchy_roomTile_info"
+          >
+            1 member · 1 room
+          </div>
+        </div>
+        <div
+          class="mx_SpaceHierarchy_actions"
+        >
+          <div
+            class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+            role="button"
+            tabindex="-1"
+          >
+            Join
+          </div>
+          <span
+            class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid"
+          >
+            <input
+              id="checkbox_abdefghi"
+              tabindex="-1"
+              type="checkbox"
+            />
+            <label
+              for="checkbox_abdefghi"
+            >
+              <div
+                class="mx_Checkbox_background"
+              >
+                <div
+                  class="mx_Checkbox_checkmark"
+                />
+              </div>
+            </label>
+          </span>
+        </div>
+        <div
+          class="mx_SpaceHierarchy_subspace_toggle mx_SpaceHierarchy_subspace_toggle_shown"
+        />
+      </div>
+      <div
+        class="mx_SpaceHierarchy_subspace_children"
+        role="group"
+      />
+    </li>
+    <li
+      class="mx_SpaceHierarchy_roomTileWrapper"
+      role="treeitem"
+    >
+      <div
+        class="mx_AccessibleButton mx_SpaceHierarchy_roomTile"
+        role="button"
+        tabindex="-1"
+      >
+        <div
+          class="mx_SpaceHierarchy_roomTile_item"
+        >
+          <div
+            class="mx_SpaceHierarchy_roomTile_avatar"
+          >
+            <span
+              class="mx_BaseAvatar"
+              role="presentation"
+            >
+              <span
+                aria-hidden="true"
+                class="mx_BaseAvatar_initial"
+                style="font-size: 13px; width: 20px; line-height: 20px;"
+              >
+                N
+              </span>
+              <img
+                alt=""
+                aria-hidden="true"
+                class="mx_BaseAvatar_image"
+                data-testid="avatar-img"
+                loading="lazy"
+                src="data:image/png;base64,00"
+                style="width: 20px; height: 20px;"
+              />
+            </span>
+          </div>
+          <div
+            class="mx_SpaceHierarchy_roomTile_name"
+          >
+            Nested room
+          </div>
+          <div
+            class="mx_SpaceHierarchy_roomTile_info"
+          >
+            3 members
+          </div>
+        </div>
+        <div
+          class="mx_SpaceHierarchy_actions"
+        >
+          <div
+            class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+            role="button"
+            tabindex="-1"
+          >
+            Join
+          </div>
+          <div
+            aria-describedby="mx_TooltipTarget_abdefghi"
+            class="mx_TextWithTooltip_target"
+            tabindex="0"
+          >
+            <span
+              class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid"
+            >
+              <input
+                disabled=""
+                id="checkbox_abdefghi"
+                tabindex="-1"
+                type="checkbox"
+              />
+              <label
+                for="checkbox_abdefghi"
+              >
+                <div
+                  class="mx_Checkbox_background"
+                >
+                  <div
+                    class="mx_Checkbox_checkmark"
+                  />
+                </div>
+              </label>
+            </span>
+          </div>
+        </div>
+      </div>
+    </li>
+  </ul>
+  ;
+</DocumentFragment>
 `;

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);