From 6c7163b569f76413c77c15c21dd03002de51a83a Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Thu, 30 Jun 2022 15:28:03 +0200 Subject: [PATCH 01/12] Remove Add Space button in RoomListHeader when user cannot createSpaces --- src/components/views/rooms/RoomListHeader.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomListHeader.tsx b/src/components/views/rooms/RoomListHeader.tsx index 1fa6805ed33..e1023938764 100644 --- a/src/components/views/rooms/RoomListHeader.tsx +++ b/src/components/views/rooms/RoomListHeader.tsx @@ -150,6 +150,7 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { const canCreateRooms = shouldShowComponent(UIComponent.CreateRooms); const canExploreRooms = shouldShowComponent(UIComponent.ExploreRooms); + const canCreateSpaces = shouldShowComponent(UIComponent.CreateSpaces); // 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 @@ -252,7 +253,7 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { disabled={!canAddRooms} tooltip={!canAddRooms && _t("You do not have permissions to add rooms to this space")} /> - { @@ -266,6 +267,7 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { > + } ; } else if (plusMenuDisplayed) { From 98b2095df32f14387b5500cca479670b3be9b259 Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Wed, 6 Jul 2022 17:23:18 +0200 Subject: [PATCH 02/12] Reuse the same booleans as SpaceContextMenu --- src/components/views/rooms/RoomListHeader.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/components/views/rooms/RoomListHeader.tsx b/src/components/views/rooms/RoomListHeader.tsx index e1023938764..19ea8592aca 100644 --- a/src/components/views/rooms/RoomListHeader.tsx +++ b/src/components/views/rooms/RoomListHeader.tsx @@ -146,16 +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 || activeSpace; // todo : canCreateSpaces ? let contextMenu: JSX.Element; if (mainMenuDisplayed && mainMenuHandle.current) { @@ -250,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")} /> - { canCreateSpaces && { @@ -262,8 +265,6 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { showCreateNewSubspace(activeSpace); closePlusMenu(); }} - disabled={!canAddRooms} - tooltip={!canAddRooms && _t("You do not have permissions to add spaces to this space")} > From 6caac04bff13e15795fe5de2a632509cd595c241 Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Thu, 7 Jul 2022 11:33:19 +0200 Subject: [PATCH 03/12] Code review fixes --- src/components/views/rooms/RoomListHeader.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomListHeader.tsx b/src/components/views/rooms/RoomListHeader.tsx index 19ea8592aca..3f8986bf67e 100644 --- a/src/components/views/rooms/RoomListHeader.tsx +++ b/src/components/views/rooms/RoomListHeader.tsx @@ -158,7 +158,7 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { // 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; // todo : canCreateSpaces ? + const canShowPlusMenu = canCreateRooms || canExploreRooms || canCreateSpaces || activeSpace; let contextMenu: JSX.Element; if (mainMenuDisplayed && mainMenuHandle.current) { @@ -256,7 +256,7 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { disabled={!canAddSubRooms} tooltip={!canAddSubRooms && _t("You do not have permissions to add rooms to this space")} /> - { canAddSubSpaces && { @@ -265,6 +265,8 @@ const RoomListHeader = ({ onVisibilityChange }: IProps) => { showCreateNewSubspace(activeSpace); closePlusMenu(); }} + disabled={!canAddSubSpaces} + tooltip={!canAddSubSpaces && _t("You do not have permissions to add spaces to this space")} > From e7db4d6924dfaca67957a27b576073ac51a80d16 Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Thu, 7 Jul 2022 12:09:17 +0200 Subject: [PATCH 04/12] Fix test for standard case --- test/components/views/rooms/RoomListHeader-test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index 1baabebbb8a..912f4d1da76 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -90,7 +90,8 @@ describe("RoomListHeader", () => { 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"); + // Look for label within the item, to ignore the text of the "Beta" pill if present. + expect(items.at(5).find(".mx_IconizedContextMenu_label").text()).toBe("Space"); }); it("closes menu if space changes from under it", async () => { From 17d1b8a7d416cbaffe523b5fbec1b9f03b0f314c Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Mon, 11 Jul 2022 13:52:42 +0100 Subject: [PATCH 05/12] Refactor tests and add more --- .../views/rooms/RoomListHeader-test.tsx | 150 ++++++++++++------ 1 file changed, 104 insertions(+), 46 deletions(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index 912f4d1da76..69b83d877fb 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -17,7 +17,9 @@ limitations under the License. import React from 'react'; import { mount } from 'enzyme'; import { MatrixClient } from 'matrix-js-sdk/src/client'; +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"; @@ -28,12 +30,54 @@ import DMRoomMap from "../../../../src/utils/DMRoomMap"; import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; 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'; + +jest.mock('../../../../src/customisations/helpers/UIComponents', () => ({ + shouldShowComponent: jest.fn(), +})); + +const blockUIComponent = component => { + mocked(shouldShowComponent).mockImplementation(feature => feature !== component); +}; + +const setupSpace = (client) => { + const testSpace = mkSpace(client, "!space:server"); + testSpace.name = "Test Space"; + client.getRoom = () => testSpace; + return testSpace; +}; + +const setupMenu = async (client, 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(); + + return wrapper; +}; describe("RoomListHeader", () => { let client: MatrixClient; beforeEach(() => { client = createTestClient(); + mocked(shouldShowComponent).mockReturnValue(true); // show all UIComponents }); it("renders a main menu for the home space", () => { @@ -58,29 +102,8 @@ 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 setupMenu(client, testSpace); const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); @@ -100,29 +123,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 setupMenu(client, testSpace); act(() => { SpaceStore.instance.setActiveSpace(MetaSpace.Favourites); @@ -134,4 +136,60 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); expect(menu).toHaveLength(0); }); + + describe('UIComponents', () => { + 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 setupMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + expect(items).toHaveLength(5); + 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"); + }); + + 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 setupMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + expect(items).toHaveLength(5); + expect(items.at(0).text()).toBe("Space home"); + expect(items.at(1).text()).toBe("Explore rooms"); + expect(items.at(2).text()).toBe("Preferences"); + expect(items.at(3).text()).toBe("Settings"); + // Look for label within the item, to ignore the text of the "Beta" pill if present. + expect(items.at(4).find(".mx_IconizedContextMenu_label").text()).toBe("Space"); + }); + }); + + describe('adding children to space', () => { + it('if user cannot add children to space, buttons are disabled', async () => { + const testSpace = setupSpace(client); + // mocked(testSpace.currentState.maySendStateEvent).mockReturnValue(false); + mocked(testSpace.currentState.maySendStateEvent).mockImplementation( + (stateEventType, userId) => stateEventType !== EventType.SpaceChild); + + const wrapper = await setupMenu(client, testSpace); + + const menu = wrapper.find(".mx_IconizedContextMenu"); + const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); + expect(items).toHaveLength(4); + expect(items.at(0).text()).toBe("Space home"); + expect(items.at(1).text()).toBe("Explore rooms"); + expect(items.at(2).text()).toBe("Preferences"); + expect(items.at(3).text()).toBe("Settings"); + }); + }); }); From 4fd3e1bc14816384d63f2d0c81e2c5988b7e1f2e Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Tue, 12 Jul 2022 17:12:58 +0100 Subject: [PATCH 06/12] Test the PlusMenu, where the bug originally was --- .../views/rooms/RoomListHeader-test.tsx | 63 ++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index 69b83d877fb..db46a0b7af1 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -48,7 +48,7 @@ const setupSpace = (client) => { return testSpace; }; -const setupMenu = async (client, testSpace) => { +const setupMainMenu = async (client, testSpace) => { const getUserIdForRoomId = jest.fn(); const getDMRoomsForUserId = jest.fn(); // @ts-ignore @@ -72,10 +72,40 @@ const setupMenu = async (client, testSpace) => { return wrapper; }; +const setupPlusMenu = async (client, 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="Add"]').hostNodes().simulate("click"); + }); + wrapper.update(); + + return wrapper; +}; + +const checkIsDisabled = menuItem => { + expect(menuItem.props().disabled).toBeTruthy(); + expect(menuItem.props()['aria-disabled']).toBeTruthy(); +}; + describe("RoomListHeader", () => { let client: MatrixClient; beforeEach(() => { + jest.resetAllMocks(); client = createTestClient(); mocked(shouldShowComponent).mockReturnValue(true); // show all UIComponents }); @@ -103,7 +133,7 @@ describe("RoomListHeader", () => { it("renders a main menu for spaces", async () => { const testSpace = setupSpace(client); - const wrapper = await setupMenu(client, testSpace); + const wrapper = await setupMainMenu(client, testSpace); const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); @@ -124,7 +154,7 @@ describe("RoomListHeader", () => { }); const testSpace = setupSpace(client); - const wrapper = await setupMenu(client, testSpace); + const wrapper = await setupMainMenu(client, testSpace); act(() => { SpaceStore.instance.setActiveSpace(MetaSpace.Favourites); @@ -143,7 +173,7 @@ describe("RoomListHeader", () => { blockUIComponent(UIComponent.CreateSpaces); const testSpace = setupSpace(client); - const wrapper = await setupMenu(client, testSpace); + const wrapper = await setupMainMenu(client, testSpace); const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); @@ -160,7 +190,7 @@ describe("RoomListHeader", () => { blockUIComponent(UIComponent.CreateRooms); const testSpace = setupSpace(client); - const wrapper = await setupMenu(client, testSpace); + const wrapper = await setupMainMenu(client, testSpace); const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); @@ -175,13 +205,12 @@ describe("RoomListHeader", () => { }); describe('adding children to space', () => { - it('if user cannot add children to space, buttons are disabled', async () => { + it('if user cannot add children to space, MainMenu buttons are hidden', async () => { const testSpace = setupSpace(client); - // mocked(testSpace.currentState.maySendStateEvent).mockReturnValue(false); mocked(testSpace.currentState.maySendStateEvent).mockImplementation( (stateEventType, userId) => stateEventType !== EventType.SpaceChild); - const wrapper = await setupMenu(client, testSpace); + const wrapper = await setupMainMenu(client, testSpace); const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); @@ -191,5 +220,23 @@ describe("RoomListHeader", () => { expect(items.at(2).text()).toBe("Preferences"); expect(items.at(3).text()).toBe("Settings"); }); + + it('if user cannot add children to space, PlusMenu 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(); + expect(items).toHaveLength(4); + expect(items.at(0).text()).toBe("New room"); + expect(items.at(1).text()).toBe("Explore rooms"); + expect(items.at(2).text()).toBe("Add existing room"); + checkIsDisabled(items.at(2)); + expect(items.at(3).find(".mx_IconizedContextMenu_label").text()).toBe("Add space"); + checkIsDisabled(items.at(3)); + }); }); }); From 500424118685490026ff2cedcdbb562a878747e6 Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Wed, 13 Jul 2022 10:24:49 +0100 Subject: [PATCH 07/12] Add tests for plus menu --- .../views/rooms/RoomListHeader-test.tsx | 129 +++++++++++++----- 1 file changed, 98 insertions(+), 31 deletions(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index db46a0b7af1..039c3df2f27 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -147,6 +147,21 @@ describe("RoomListHeader", () => { expect(items.at(5).find(".mx_IconizedContextMenu_label").text()).toBe("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(); + + expect(items).toHaveLength(4); + expect(items.at(0).text()).toBe("New room"); + expect(items.at(1).text()).toBe("Explore rooms"); + expect(items.at(2).text()).toBe("Add existing room"); + // Look for label within the item, to ignore the text of the "Beta" pill if present. + expect(items.at(3).find(".mx_IconizedContextMenu_label").text()).toBe("Add space"); + }); + it("closes menu if space changes from under it", async () => { await SettingsStore.setValue("Spaces.enabledMetaSpaces", null, SettingLevel.DEVICE, { [MetaSpace.Home]: true, @@ -168,39 +183,91 @@ describe("RoomListHeader", () => { }); describe('UIComponents', () => { - 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(); - expect(items).toHaveLength(5); - 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"); + 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(); + expect(items).toHaveLength(5); + 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"); + }); + + 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(); + expect(items).toHaveLength(5); + expect(items.at(0).text()).toBe("Space home"); + expect(items.at(1).text()).toBe("Explore rooms"); + expect(items.at(2).text()).toBe("Preferences"); + expect(items.at(3).text()).toBe("Settings"); + // Look for label within the item, to ignore the text of the "Beta" pill if present. + expect(items.at(4).find(".mx_IconizedContextMenu_label").text()).toBe("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(); - expect(items).toHaveLength(5); - expect(items.at(0).text()).toBe("Space home"); - expect(items.at(1).text()).toBe("Explore rooms"); - expect(items.at(2).text()).toBe("Preferences"); - expect(items.at(3).text()).toBe("Settings"); - // Look for label within the item, to ignore the text of the "Beta" pill if present. - expect(items.at(4).find(".mx_IconizedContextMenu_label").text()).toBe("Space"); + const checkMenuLabels = (items, labelArray) => { + expect(items).toHaveLength(labelArray.length); + + const checkLabel = (item, label) => { + 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('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", + ]); + }); + + 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 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", + ]); + }); }); }); From 9f66361a117e91eedf76ed0dfe8c511ffbe51a11 Mon Sep 17 00:00:00 2001 From: Estelle Comment Date: Wed, 13 Jul 2022 10:51:46 +0100 Subject: [PATCH 08/12] Refactor tests --- .../views/rooms/RoomListHeader-test.tsx | 122 ++++++++++-------- 1 file changed, 69 insertions(+), 53 deletions(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index 039c3df2f27..e15d9b63ae8 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -101,6 +101,19 @@ const checkIsDisabled = menuItem => { expect(menuItem.props()['aria-disabled']).toBeTruthy(); }; +const checkMenuLabels = (items, labelArray) => { + expect(items).toHaveLength(labelArray.length); + + const checkLabel = (item, label) => { + 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; @@ -137,14 +150,15 @@ describe("RoomListHeader", () => { 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"); - // Look for label within the item, to ignore the text of the "Beta" pill if present. - expect(items.at(5).find(".mx_IconizedContextMenu_label").text()).toBe("Space"); + + checkMenuLabels(items, [ + "Space home", + "Manage & explore rooms", + "Preferences", + "Settings", + "Room", + "Space", + ]); }); it("renders a plus menu for spaces", async () => { @@ -154,12 +168,12 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); - expect(items).toHaveLength(4); - expect(items.at(0).text()).toBe("New room"); - expect(items.at(1).text()).toBe("Explore rooms"); - expect(items.at(2).text()).toBe("Add existing room"); - // Look for label within the item, to ignore the text of the "Beta" pill if present. - expect(items.at(3).find(".mx_IconizedContextMenu_label").text()).toBe("Add space"); + checkMenuLabels(items, [ + "New room", + "Explore rooms", + "Add existing room", + "Add space", + ]); }); it("closes menu if space changes from under it", async () => { @@ -193,12 +207,14 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); - expect(items).toHaveLength(5); - 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"); + 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 () => { @@ -210,29 +226,17 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); - expect(items).toHaveLength(5); - expect(items.at(0).text()).toBe("Space home"); - expect(items.at(1).text()).toBe("Explore rooms"); - expect(items.at(2).text()).toBe("Preferences"); - expect(items.at(3).text()).toBe("Settings"); - // Look for label within the item, to ignore the text of the "Beta" pill if present. - expect(items.at(4).find(".mx_IconizedContextMenu_label").text()).toBe("Space"); + checkMenuLabels(items, [ + "Space home", + "Explore rooms", // not Manage & explore rooms + "Preferences", + "Settings", + // no add room + "Space", + ]); }); }); - const checkMenuLabels = (items, labelArray) => { - expect(items).toHaveLength(labelArray.length); - - const checkLabel = (item, label) => { - 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('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 @@ -248,10 +252,11 @@ describe("RoomListHeader", () => { "New room", "Explore rooms", "Add existing room", + // no Add space ]); }); - it('does not render Add Room when user does not have permission to add rooms', async () => { + 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); @@ -267,12 +272,15 @@ describe("RoomListHeader", () => { "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 buttons are hidden', async () => { + 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); @@ -281,14 +289,17 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); - expect(items).toHaveLength(4); - expect(items.at(0).text()).toBe("Space home"); - expect(items.at(1).text()).toBe("Explore rooms"); - expect(items.at(2).text()).toBe("Preferences"); - expect(items.at(3).text()).toBe("Settings"); + 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 buttons are disabled', async () => { + 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); @@ -297,12 +308,17 @@ describe("RoomListHeader", () => { const menu = wrapper.find(".mx_IconizedContextMenu"); const items = menu.find(".mx_IconizedContextMenu_item").hostNodes(); - expect(items).toHaveLength(4); - expect(items.at(0).text()).toBe("New room"); - expect(items.at(1).text()).toBe("Explore rooms"); - expect(items.at(2).text()).toBe("Add existing room"); + + checkMenuLabels(items, [ + "New room", + "Explore rooms", + "Add existing room", + "Add space", + ]); + + // "Add existing room" is disabled checkIsDisabled(items.at(2)); - expect(items.at(3).find(".mx_IconizedContextMenu_label").text()).toBe("Add space"); + // "Add space" is disabled checkIsDisabled(items.at(3)); }); }); From 2d9c8b97c734f71a0f1628c081218f5f876dda44 Mon Sep 17 00:00:00 2001 From: mcalinghee Date: Fri, 29 Jul 2022 14:44:13 +0200 Subject: [PATCH 09/12] add type in functions and use DMRoomMap#setShared --- .../views/rooms/RoomListHeader-test.tsx | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index e15d9b63ae8..0a539f77cc2 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -15,8 +15,9 @@ limitations under the License. */ import React from 'react'; -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'; @@ -37,23 +38,18 @@ jest.mock('../../../../src/customisations/helpers/UIComponents', () => ({ shouldShowComponent: jest.fn(), })); -const blockUIComponent = component => { +const blockUIComponent = (component: UIComponent): void => { mocked(shouldShowComponent).mockImplementation(feature => feature !== component); }; -const setupSpace = (client) => { - const testSpace = mkSpace(client, "!space:server"); +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, testSpace) => { - const getUserIdForRoomId = jest.fn(); - const getDMRoomsForUserId = jest.fn(); - // @ts-ignore - DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; - +const setupMainMenu = async (client: MatrixClient, testSpace: Room): Promise => { await testUtils.setupAsyncStoreWithClient(SpaceStore.instance, client); act(() => { SpaceStore.instance.setActiveSpace(testSpace.roomId); @@ -72,12 +68,7 @@ const setupMainMenu = async (client, testSpace) => { return wrapper; }; -const setupPlusMenu = async (client, testSpace) => { - const getUserIdForRoomId = jest.fn(); - const getDMRoomsForUserId = jest.fn(); - // @ts-ignore - DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; - +const setupPlusMenu = async (client: MatrixClient, testSpace: Room): Promise => { await testUtils.setupAsyncStoreWithClient(SpaceStore.instance, client); act(() => { SpaceStore.instance.setActiveSpace(testSpace.roomId); @@ -96,15 +87,15 @@ const setupPlusMenu = async (client, testSpace) => { return wrapper; }; -const checkIsDisabled = menuItem => { +const checkIsDisabled = (menuItem: ReactWrapper): void => { expect(menuItem.props().disabled).toBeTruthy(); expect(menuItem.props()['aria-disabled']).toBeTruthy(); }; -const checkMenuLabels = (items, labelArray) => { +const checkMenuLabels = (items: ReactWrapper, labelArray: Array) => { expect(items).toHaveLength(labelArray.length); - const checkLabel = (item, label) => { + const checkLabel = (item: ReactWrapper, label: string) => { expect(item.find(".mx_IconizedContextMenu_label").text()).toBe(label); }; @@ -119,6 +110,12 @@ describe("RoomListHeader", () => { beforeEach(() => { jest.resetAllMocks(); + + const dmRoomMap = { + getUserIdForRoomId: jest.fn(), + getDMRoomsForUserId: jest.fn(), + } as unknown as DMRoomMap; + DMRoomMap.setShared(dmRoomMap); client = createTestClient(); mocked(shouldShowComponent).mockReturnValue(true); // show all UIComponents }); From 533fb3152e490a3961c19d0b24d06e5c0226ed67 Mon Sep 17 00:00:00 2001 From: mcalinghee Date: Fri, 29 Jul 2022 15:59:22 +0200 Subject: [PATCH 10/12] use of wrapInMatrixClientContext --- .../views/rooms/RoomListHeader-test.tsx | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index 0a539f77cc2..eeaa58fa0e7 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -24,16 +24,18 @@ 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(), })); @@ -55,9 +57,7 @@ const setupMainMenu = async (client: MatrixClient, testSpace: Room): Promise - - ); + const wrapper = mount(); expect(wrapper.text()).toBe("Test Space"); act(() => { @@ -74,9 +74,7 @@ const setupPlusMenu = async (client: MatrixClient, testSpace: Room): Promise - - ); + const wrapper = mount(); expect(wrapper.text()).toBe("Test Space"); act(() => { @@ -116,7 +114,8 @@ describe("RoomListHeader", () => { getDMRoomsForUserId: jest.fn(), } as unknown as DMRoomMap; DMRoomMap.setShared(dmRoomMap); - client = createTestClient(); + stubClient(); + client = MatrixClientPeg.get(); mocked(shouldShowComponent).mockReturnValue(true); // show all UIComponents }); @@ -125,9 +124,7 @@ describe("RoomListHeader", () => { SpaceStore.instance.setActiveSpace(MetaSpace.Home); }); - const wrapper = mount( - - ); + const wrapper = mount(); expect(wrapper.text()).toBe("Home"); act(() => { From 09bad9f4e00119ac8cd42a10720ef99e57834a30 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 1 Aug 2022 18:37:35 +0200 Subject: [PATCH 11/12] Trigger CI From 496c76638be18abf4e840ddce756dfa8e34f776c Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Wed, 3 Aug 2022 10:17:25 +0200 Subject: [PATCH 12/12] Ignore enzyme deprecation in RoomListHeader-test.tsx --- test/components/views/rooms/RoomListHeader-test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/test/components/views/rooms/RoomListHeader-test.tsx b/test/components/views/rooms/RoomListHeader-test.tsx index eeaa58fa0e7..9c1dcce6b18 100644 --- a/test/components/views/rooms/RoomListHeader-test.tsx +++ b/test/components/views/rooms/RoomListHeader-test.tsx @@ -15,6 +15,7 @@ limitations under the License. */ import React from 'react'; +// eslint-disable-next-line deprecate/import import { mount, ReactWrapper, HTMLAttributes } from 'enzyme'; import { MatrixClient } from 'matrix-js-sdk/src/client'; import { Room } from 'matrix-js-sdk/src/matrix';