diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index 0d41e56c925..f460784dc67 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -32,7 +32,6 @@ import { RoomType, GuestAccess, HistoryVisibility, - type HierarchyRelation, type HierarchyRoom, JoinRule, } from "matrix-js-sdk/src/matrix"; @@ -71,6 +70,7 @@ import { getTopic } from "../../hooks/room/useTopic"; import { SdkContextClass } from "../../contexts/SDKContext"; import { getDisplayAliasForAliasSet } from "../../Rooms"; import SettingsStore from "../../settings/SettingsStore"; +import { filterBoolean } from "../../utils/arrays.ts"; interface IProps { space: Room; @@ -504,68 +504,67 @@ export const HierarchyLevel: React.FC = ({ const space = cli.getRoom(root.room_id); const hasPermissions = space?.currentState.maySendStateEvent(EventType.SpaceChild, cli.getSafeUserId()); - const sortedChildren = sortBy(root.children_state, (ev) => { - return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); - }); - - const [subspaces, childRooms] = sortedChildren.reduce( - (result, ev: HierarchyRelation) => { - const room = hierarchy.roomMap.get(ev.state_key); - if (room && roomSet.has(room)) { - result[room.room_type === RoomType.Space ? 0 : 1].push(toLocalRoom(cli, room, hierarchy)); - } - return result; - }, - [[] as HierarchyRoom[], [] as HierarchyRoom[]], + const sortedChildren = filterBoolean( + sortBy(root.children_state, (ev) => { + return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); + }).map((ev) => { + const hierarchyRoom = hierarchy.roomMap.get(ev.state_key); + if (!hierarchyRoom || !roomSet.has(hierarchyRoom)) return null; + // Find the most up-to-date info for this room, if it has been upgraded and we know about it. + return toLocalRoom(cli, hierarchyRoom, hierarchy); + }), ); const newParents = new Set(parents).add(root.room_id); return ( - {uniqBy(childRooms, "room_id").map((room) => ( - onViewRoomClick(room.room_id, room.room_type as RoomType)} - onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} - hasPermissions={hasPermissions} - onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} - /> - ))} - - {subspaces - .filter((room) => !newParents.has(room.room_id)) - .map((space) => ( - { - const room = hierarchy.roomMap.get(ev.state_key); - return room && roomSet.has(room) && !room.room_type; - }).length - } - 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, newParents)} - hasPermissions={hasPermissions} - onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, space.room_id) : undefined} - > - { + if (room.room_type !== RoomType.Space) { + return ( + onViewRoomClick(room.room_id, room.room_type as RoomType)} + onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} + hasPermissions={hasPermissions} + onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} /> - - ))} + ); + } else { + if (newParents.has(room.room_id)) return null; // prevent cycles + return ( + { + const child = hierarchy.roomMap.get(ev.state_key); + return child && roomSet.has(child) && !child.room_type; + }).length + } + suggested={hierarchy.isSuggested(root.room_id, room.room_id)} + selected={selectedMap?.get(root.room_id)?.has(room.room_id)} + onViewRoomClick={() => onViewRoomClick(room.room_id, RoomType.Space)} + onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} + hasPermissions={hasPermissions} + onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} + > + + + ); + } + })} ); }; diff --git a/test/unit-tests/components/structures/SpaceHierarchy-test.tsx b/test/unit-tests/components/structures/SpaceHierarchy-test.tsx index cea704b90b7..f0d6429789d 100644 --- a/test/unit-tests/components/structures/SpaceHierarchy-test.tsx +++ b/test/unit-tests/components/structures/SpaceHierarchy-test.tsx @@ -169,11 +169,14 @@ describe("SpaceHierarchy", () => { 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); + const space2 = mkStubRoom("space-id-6", "Space 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(KnownMembership.Leave)); + [room1, room2, space1, room3, space2].forEach((r) => + mocked(r.getMyMembership).mockReturnValue(KnownMembership.Leave), + ); const hierarchyRoot: HierarchyRoom = { room_id: root.roomId, @@ -324,5 +327,36 @@ describe("SpaceHierarchy", () => { undefined, ); }); + + it("should not render cycles", async () => { + const hierarchySpace2: HierarchyRoom = { + room_id: space2.roomId, + name: "Space with cycle", + num_joined_members: 1, + room_type: "m.space", + children_state: [ + { + state_key: root.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, hierarchySpace2], + }); + + const { getAllByText, queryByText, asFragment } = render(getComponent()); + // Wait for spinners to go away + await waitForElementToBeRemoved(screen.getAllByRole("progressbar")); + expect(getAllByText("Nested space")).toHaveLength(1); + expect(queryByText("Space 1")).not.toBeInTheDocument(); + expect(asFragment()).toMatchSnapshot(); + }); }); }); diff --git a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap index e97a0cf7e17..def0b2772c3 100644 --- a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap +++ b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap @@ -254,12 +254,13 @@ exports[`SpaceHierarchy renders 1`] = `
  • @@ -271,13 +272,13 @@ exports[`SpaceHierarchy renders 1`] = ` > - K + N
    renders 1`] = ` - Knock room + Nested space
    - 3 members + 1 member · 1 room
    - View + Join
    renders 1`] = `
    +
    +
  • @@ -385,13 +392,13 @@ exports[`SpaceHierarchy renders 1`] = ` - Nested space + Nested room
    - 1 member · 1 room + 3 members
    renders 1`] = ` > Join
    + +
    +
    +
    +
    + +
    + +
    +
    +
    +
    +
    + + +
    + +
    • +
  • +
    +
    +
    + + K + +
    +
    + + Knock room + +
    +
    + 3 members +
    +
    +
    +
    + View +
    @@ -417,9 +528,9 @@ exports[`SpaceHierarchy renders 1`] = ` class="_container_1hel1_10" > renders 1`] = `
    -
    +
    • + + +`; + +exports[`SpaceHierarchy should not render cycles 1`] = ` + +
    + +
    +
    +
    +

    + Rooms and spaces +

    +
    + aria-disabled="true" + aria-label="Remove" + class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_danger_outline mx_AccessibleButton_disabled" + disabled="" + role="button" + tabindex="0" + > + Remove +
    +
    + Mark as not suggested +
    +
    +
    +
      +
    • +
      +
      +
      + + U + +
      +
      + + Unnamed Room + +
      +
      + 2 members +
      +
      +
      +
      + Join +
      +
      +
      +
      +
      + +
      + +
      +
      +
      +
      +
      + +
      +
    • @@ -475,22 +738,22 @@ exports[`SpaceHierarchy renders 1`] = ` > - N + U
    - Nested room + Unnamed Room
    renders 1`] = ` > Join
    - -
    +
    - - + +
    +
    +
    +
    +
    + + + +
  • +
    +
    +
    + + N + +
    +
    + + Nested space + +
    +
    + 1 member · 0 rooms +
    +
    +
    +
    + Join +
    +
    +
    +
    + class="_container_1hel1_10" + > + +
    + +
    +
    - - +
    +
    +
    +
    +