From 0e0be08781023026a12acecb20a7a264bb1d56e6 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 3 Aug 2022 11:19:24 -0400 Subject: [PATCH] Remove "Add Space" button from RoomListHeader when user cannot create spaces (#9129) * Remove Add Space button in RoomListHeader when user cannot createSpaces * Reuse the same booleans as SpaceContextMenu * Code review fixes * Fix test for standard case * Refactor tests and add more * Test the PlusMenu, where the bug originally was * Add tests for plus menu * Refactor tests * add type in functions and use DMRoomMap#setShared * use of wrapInMatrixClientContext * Trigger CI * Ignore enzyme deprecation in RoomListHeader-test.tsx Co-authored-by: Estelle Comment Co-authored-by: mcalinghee Co-authored-by: Michael Weimann --- src/components/views/rooms/RoomListHeader.tsx | 23 +- .../views/rooms/RoomListHeader-test.tsx | 305 ++++++++++++++---- 2 files changed, 258 insertions(+), 70 deletions(-) diff --git a/src/components/views/rooms/RoomListHeader.tsx b/src/components/views/rooms/RoomListHeader.tsx index 1fa6805ed3..3f8986bf67 100644 --- a/src/components/views/rooms/RoomListHeader.tsx +++ b/src/components/views/rooms/RoomListHeader.tsx @@ -146,15 +146,19 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { } }, [onVisibilityChange]); - const canAddRooms = activeSpace?.currentState?.maySendStateEvent(EventType.SpaceChild, cli.getUserId()); - - const canCreateRooms = shouldShowComponent(UIComponent.CreateRooms); const canExploreRooms = shouldShowComponent(UIComponent.ExploreRooms); + const canCreateRooms = shouldShowComponent(UIComponent.CreateRooms); + const canCreateSpaces = shouldShowComponent(UIComponent.CreateSpaces); + + const hasPermissionToAddSpaceChild = + activeSpace?.currentState?.maySendStateEvent(EventType.SpaceChild, cli.getUserId()); + const canAddSubRooms = hasPermissionToAddSpaceChild && canCreateRooms; + const canAddSubSpaces = hasPermissionToAddSpaceChild && canCreateSpaces; // If the user can't do anything on the plus menu, don't show it. This aims to target the // plus menu shown on the Home tab primarily: the user has options to use the menu for // communities and spaces, but is at risk of no options on the Home tab. - const canShowPlusMenu = canCreateRooms || canExploreRooms || activeSpace; + const canShowPlusMenu = canCreateRooms || canExploreRooms || canCreateSpaces || activeSpace; let contextMenu: JSX.Element; if (mainMenuDisplayed && mainMenuHandle.current) { @@ -249,10 +253,10 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { showAddExistingRooms(activeSpace); closePlusMenu(); }} - disabled={!canAddRooms} - tooltip={!canAddRooms && _t("You do not have permissions to add rooms to this space")} + disabled={!canAddSubRooms} + tooltip={!canAddSubRooms && _t("You do not have permissions to add rooms to this space")} /> - { @@ -261,11 +265,12 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { showCreateNewSubspace(activeSpace); closePlusMenu(); }} - disabled={!canAddRooms} - tooltip={!canAddRooms && _t("You do not have permissions to add spaces to this space")} + disabled={!canAddSubSpaces} + tooltip={!canAddSubSpaces && _t("You do not have permissions to add spaces to this space")} > + } ; } else if (plusMenuDisplayed) { diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index 56ff7f5635..9c1dcce6b1 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -16,25 +16,108 @@ limitations under the License. import React from 'react'; // eslint-disable-next-line deprecate/import -import { mount } from 'enzyme'; +import { mount, ReactWrapper, HTMLAttributes } from 'enzyme'; import { MatrixClient } from 'matrix-js-sdk/src/client'; +import { Room } from 'matrix-js-sdk/src/matrix'; +import { EventType } from "matrix-js-sdk/src/@types/event"; import { act } from "react-dom/test-utils"; +import { mocked } from 'jest-mock'; import SpaceStore from "../../../../src/stores/spaces/SpaceStore"; import { MetaSpace } from "../../../../src/stores/spaces"; -import RoomListHeader from "../../../../src/components/views/rooms/RoomListHeader"; +import _RoomListHeader from "../../../../src/components/views/rooms/RoomListHeader"; import * as testUtils from "../../../test-utils"; -import { createTestClient, mkSpace } from "../../../test-utils"; +import { stubClient, mkSpace } from "../../../test-utils"; import DMRoomMap from "../../../../src/utils/DMRoomMap"; -import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; +import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; import SettingsStore from "../../../../src/settings/SettingsStore"; import { SettingLevel } from "../../../../src/settings/SettingLevel"; +import { shouldShowComponent } from '../../../../src/customisations/helpers/UIComponents'; +import { UIComponent } from '../../../../src/settings/UIFeature'; + +const RoomListHeader = testUtils.wrapInMatrixClientContext(_RoomListHeader); + +jest.mock('../../../../src/customisations/helpers/UIComponents', () => ({ + shouldShowComponent: jest.fn(), +})); + +const blockUIComponent = (component: UIComponent): void => { + mocked(shouldShowComponent).mockImplementation(feature => feature !== component); +}; + +const setupSpace = (client: MatrixClient): Room => { + const testSpace: Room = mkSpace(client, "!space:server"); + testSpace.name = "Test Space"; + client.getRoom = () => testSpace; + return testSpace; +}; + +const setupMainMenu = async (client: MatrixClient, testSpace: Room): Promise => { + await testUtils.setupAsyncStoreWithClient(SpaceStore.instance, client); + act(() => { + SpaceStore.instance.setActiveSpace(testSpace.roomId); + }); + + const wrapper = mount(); + + expect(wrapper.text()).toBe("Test Space"); + act(() => { + wrapper.find('[aria-label="Test Space menu"]').hostNodes().simulate("click"); + }); + wrapper.update(); + + return wrapper; +}; + +const setupPlusMenu = async (client: MatrixClient, testSpace: Room): Promise => { + await testUtils.setupAsyncStoreWithClient(SpaceStore.instance, client); + act(() => { + SpaceStore.instance.setActiveSpace(testSpace.roomId); + }); + + const wrapper = mount(); + + expect(wrapper.text()).toBe("Test Space"); + act(() => { + wrapper.find('[aria-label="Add"]').hostNodes().simulate("click"); + }); + wrapper.update(); + + return wrapper; +}; + +const checkIsDisabled = (menuItem: ReactWrapper): void => { + expect(menuItem.props().disabled).toBeTruthy(); + expect(menuItem.props()['aria-disabled']).toBeTruthy(); +}; + +const checkMenuLabels = (items: ReactWrapper, labelArray: Array) => { + expect(items).toHaveLength(labelArray.length); + + const checkLabel = (item: ReactWrapper, label: string) => { + expect(item.find(".mx_IconizedContextMenu_label").text()).toBe(label); + }; + + labelArray.forEach((label, index) => { + console.log('index', index, 'label', label); + checkLabel(items.at(index), label); + }); +}; describe("RoomListHeader", () => { let client: MatrixClient; beforeEach(() => { - client = createTestClient(); + jest.resetAllMocks(); + + const dmRoomMap = { + getUserIdForRoomId: jest.fn(), + getDMRoomsForUserId: jest.fn(), + } as unknown as DMRoomMap; + DMRoomMap.setShared(dmRoomMap); + stubClient(); + client = MatrixClientPeg.get(); + mocked(shouldShowComponent).mockReturnValue(true); // show all UIComponents }); it("renders a main menu for the home space", () => { @@ -42,9 +125,7 @@ describe("RoomListHeader", () => { SpaceStore.instance.setActiveSpace(MetaSpace.Home); }); - const wrapper = mount( - - ); + const wrapper = mount(); expect(wrapper.text()).toBe("Home"); act(() => { @@ -59,39 +140,35 @@ describe("RoomListHeader", () => { }); it("renders a main menu for spaces", async () => { - const testSpace = mkSpace(client, "!space:server"); - testSpace.name = "Test Space"; - client.getRoom = () => testSpace; - - const getUserIdForRoomId = jest.fn(); - const getDMRoomsForUserId = jest.fn(); - // @ts-ignore - DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; - - await testUtils.setupAsyncStoreWithClient(SpaceStore.instance, client); - act(() => { - SpaceStore.instance.setActiveSpace(testSpace.roomId); - }); - - const wrapper = mount( - - ); - - expect(wrapper.text()).toBe("Test Space"); - act(() => { - wrapper.find('[aria-label="Test Space menu"]').hostNodes().simulate("click"); - }); - wrapper.update(); + const testSpace = setupSpace(client); + const wrapper = await setupMainMenu(client, testSpace); const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); - expect(items).toHaveLength(6); - expect(items.at(0).text()).toBe("Space home"); - expect(items.at(1).text()).toBe("Manage & explore rooms"); - expect(items.at(2).text()).toBe("Preferences"); - expect(items.at(3).text()).toBe("Settings"); - expect(items.at(4).text()).toBe("Room"); - expect(items.at(4).text()).toBe("Room"); + + checkMenuLabels(items, [ + "Space home", + "Manage & explore rooms", + "Preferences", + "Settings", + "Room", + "Space", + ]); + }); + + it("renders a plus menu for spaces", async () => { + const testSpace = setupSpace(client); + const wrapper = await setupPlusMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + + checkMenuLabels(items, [ + "New room", + "Explore rooms", + "Add existing room", + "Add space", + ]); }); it("closes menu if space changes from under it", async () => { @@ -100,29 +177,8 @@ describe("RoomListHeader", () => { [MetaSpace.Favourites]: true, }); - const testSpace = mkSpace(client, "!space:server"); - testSpace.name = "Test Space"; - client.getRoom = () => testSpace; - - const getUserIdForRoomId = jest.fn(); - const getDMRoomsForUserId = jest.fn(); - // @ts-ignore - DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; - - await testUtils.setupAsyncStoreWithClient(SpaceStore.instance, client); - act(() => { - SpaceStore.instance.setActiveSpace(testSpace.roomId); - }); - - const wrapper = mount( - - ); - - expect(wrapper.text()).toBe("Test Space"); - act(() => { - wrapper.find('[aria-label="Test Space menu"]').hostNodes().simulate("click"); - }); - wrapper.update(); + const testSpace = setupSpace(client); + const wrapper = await setupMainMenu(client, testSpace); act(() => { SpaceStore.instance.setActiveSpace(MetaSpace.Favourites); @@ -134,4 +190,131 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); expect(menu).toHaveLength(0); }); + + describe('UIComponents', () => { + describe('Main menu', () => { + it('does not render Add Space when user does not have permission to add spaces', async () => { + // User does not have permission to add spaces, anywhere + blockUIComponent(UIComponent.CreateSpaces); + + const testSpace = setupSpace(client); + const wrapper = await setupMainMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + checkMenuLabels(items, [ + "Space home", + "Manage & explore rooms", + "Preferences", + "Settings", + "Room", + // no add space + ]); + }); + + it('does not render Add Room when user does not have permission to add rooms', async () => { + // User does not have permission to add rooms + blockUIComponent(UIComponent.CreateRooms); + + const testSpace = setupSpace(client); + const wrapper = await setupMainMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + checkMenuLabels(items, [ + "Space home", + "Explore rooms", // not Manage & explore rooms + "Preferences", + "Settings", + // no add room + "Space", + ]); + }); + }); + + describe('Plus menu', () => { + it('does not render Add Space when user does not have permission to add spaces', async () => { + // User does not have permission to add spaces, anywhere + blockUIComponent(UIComponent.CreateSpaces); + + const testSpace = setupSpace(client); + const wrapper = await setupPlusMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + + checkMenuLabels(items, [ + "New room", + "Explore rooms", + "Add existing room", + // no Add space + ]); + }); + + it('disables Add Room when user does not have permission to add rooms', async () => { + // User does not have permission to add rooms + blockUIComponent(UIComponent.CreateRooms); + + const testSpace = setupSpace(client); + const wrapper = await setupPlusMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + + checkMenuLabels(items, [ + "New room", + "Explore rooms", + "Add existing room", + "Add space", + ]); + + // "Add existing room" is disabled + checkIsDisabled(items.at(2)); + }); + }); + }); + + describe('adding children to space', () => { + it('if user cannot add children to space, MainMenu adding buttons are hidden', async () => { + const testSpace = setupSpace(client); + mocked(testSpace.currentState.maySendStateEvent).mockImplementation( + (stateEventType, userId) => stateEventType !== EventType.SpaceChild); + + const wrapper = await setupMainMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + checkMenuLabels(items, [ + "Space home", + "Explore rooms", // not Manage & explore rooms + "Preferences", + "Settings", + // no add room + // no add space + ]); + }); + + it('if user cannot add children to space, PlusMenu add buttons are disabled', async () => { + const testSpace = setupSpace(client); + mocked(testSpace.currentState.maySendStateEvent).mockImplementation( + (stateEventType, userId) => stateEventType !== EventType.SpaceChild); + + const wrapper = await setupPlusMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + + checkMenuLabels(items, [ + "New room", + "Explore rooms", + "Add existing room", + "Add space", + ]); + + // "Add existing room" is disabled + checkIsDisabled(items.at(2)); + // "Add space" is disabled + checkIsDisabled(items.at(3)); + }); + }); });