diff --git a/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts b/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts index 7cc4001c58..06ee25e9cb 100644 --- a/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts +++ b/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts @@ -37,6 +37,15 @@ test.describe("Roles & Permissions room settings tab", () => { // Change the role of Alice to Moderator (50) await combobox.selectOption("Moderator"); await expect(combobox).toHaveValue("50"); + + // Should display a modal to warn that we are demoting the only admin user + const modal = await page.locator(".mx_Dialog", { + hasText: "Warning", + }); + await expect(modal).toBeVisible(); + // Click on the continue button in the modal + await modal.getByRole("button", { name: "Continue" }).click(); + const respPromise = page.waitForRequest("**/state/**"); await applyButton.click(); await respPromise; diff --git a/src/components/viewmodels/roomlist/useStickyRoomList.tsx b/src/components/viewmodels/roomlist/useStickyRoomList.tsx index e8234d14ae..06feb58581 100644 --- a/src/components/viewmodels/roomlist/useStickyRoomList.tsx +++ b/src/components/viewmodels/roomlist/useStickyRoomList.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { SdkContextClass } from "../../../contexts/SDKContext"; import { useDispatcher } from "../../../hooks/useDispatcher"; @@ -13,6 +13,7 @@ import dispatcher from "../../../dispatcher/dispatcher"; import { Action } from "../../../dispatcher/actions"; import type { Room } from "matrix-js-sdk/src/matrix"; import type { Optional } from "matrix-events-sdk"; +import SpaceStore from "../../../stores/spaces/SpaceStore"; function getIndexByRoomId(rooms: Room[], roomId: Optional): number | undefined { const index = rooms.findIndex((room) => room.roomId === roomId); @@ -90,8 +91,10 @@ export function useStickyRoomList(rooms: Room[]): StickyRoomListResult { roomsWithStickyRoom: rooms, }); + const currentSpaceRef = useRef(SpaceStore.instance.activeSpace); + const updateRoomsAndIndex = useCallback( - (newRoomId?: string, isRoomChange: boolean = false) => { + (newRoomId: string | null, isRoomChange: boolean = false) => { setListState((current) => { const activeRoomId = newRoomId ?? SdkContextClass.instance.roomViewStore.getRoomId(); const newActiveIndex = getIndexByRoomId(rooms, activeRoomId); @@ -110,7 +113,21 @@ export function useStickyRoomList(rooms: Room[]): StickyRoomListResult { // Re-calculate the index when the list of rooms has changed. useEffect(() => { - updateRoomsAndIndex(); + let newRoomId: string | null = null; + let isRoomChange = false; + const newSpace = SpaceStore.instance.activeSpace; + if (currentSpaceRef.current !== newSpace) { + /* + If the space has changed, we check if we can immediately set the active + index to the last opened room in that space. Otherwise, we might see a + flicker because of the delay between the space change event and + active room change dispatch. + */ + newRoomId = SpaceStore.instance.getLastSelectedRoomIdForSpace(newSpace); + isRoomChange = true; + currentSpaceRef.current = newSpace; + } + updateRoomsAndIndex(newRoomId, isRoomChange); }, [rooms, updateRoomsAndIndex]); return { activeIndex: listState.index, rooms: listState.roomsWithStickyRoom }; diff --git a/src/components/views/settings/PowerLevelSelector.tsx b/src/components/views/settings/PowerLevelSelector.tsx index 32731715ff..0b570c1516 100644 --- a/src/components/views/settings/PowerLevelSelector.tsx +++ b/src/components/views/settings/PowerLevelSelector.tsx @@ -13,6 +13,8 @@ import { useMatrixClientContext } from "../../../contexts/MatrixClientContext"; import PowerSelector from "../elements/PowerSelector"; import { _t } from "../../../languageHandler"; import SettingsFieldset from "./SettingsFieldset"; +import Modal from "../../../Modal"; +import QuestionDialog from "../dialogs/QuestionDialog"; /** * Display in a fieldset, the power level of the users and allow to change them. @@ -77,6 +79,13 @@ export function PowerLevelSelector({ // No user to display, we return the children into fragment to convert it to JSX.Element type if (!users.length) return <>{children}; + // check at least one admin in the list + const roomHasAtLeastOneAdmin = (usersLevels: Record): boolean => { + const userLevelValues = Object.values(usersLevels); + // At least one user as the pL 100 which means he is admin + return userLevelValues.some((uL) => uL === 100); + }; + return ( {users.map((userId) => { @@ -96,7 +105,24 @@ export function PowerLevelSelector({ disabled={!canChange} label={userId} key={userId} - onChange={(value) => setCurrentPowerLevel({ value, userId })} + onChange={async (value) => { + const userLevelsTmp = Object.assign({}, userLevels); + userLevelsTmp[userId] = value; + if (!roomHasAtLeastOneAdmin(userLevelsTmp)) { + const { finished } = Modal.createDialog(QuestionDialog, { + title: _t("common|warning"), + description:
{_t("user_info|demote_self_confirm_room")}
, + button: _t("action|continue"), + }); + const [confirmed] = await finished; + if (!confirmed) { + // if cancel, we reput initial value + setCurrentPowerLevel({ value: userLevels[userId], userId }); + return; + } + } + setCurrentPowerLevel({ value, userId }); + }} /> ); })} diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 690beaa0b7..ac4ffdaf0c 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -270,7 +270,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (contextSwitch) { // view last selected room from space - const roomId = window.localStorage.getItem(getSpaceContextKey(space)); + const roomId = this.getLastSelectedRoomIdForSpace(space); // if the space being selected is an invite then always view that invite // else if the last viewed room in this space is joined then view that @@ -320,6 +320,17 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } } + /** + * Returns the room-id of the last active room in a given space. + * This is the room that would be opened when you switch to a given space. + * @param space The space you're interested in. + * @returns room-id of the room or null if there's no last active room. + */ + public getLastSelectedRoomIdForSpace(space: SpaceKey): string | null { + const roomId = window.localStorage.getItem(getSpaceContextKey(space)); + return roomId; + } + private async loadSuggestedRooms(space: Room): Promise { const suggestedRooms = await this.fetchSuggestedRooms(space); if (this._activeSpace === space.roomId) { diff --git a/test/unit-tests/components/viewmodels/roomlist/RoomListViewModel-test.tsx b/test/unit-tests/components/viewmodels/roomlist/RoomListViewModel-test.tsx index bf92272e08..309d721d37 100644 --- a/test/unit-tests/components/viewmodels/roomlist/RoomListViewModel-test.tsx +++ b/test/unit-tests/components/viewmodels/roomlist/RoomListViewModel-test.tsx @@ -355,6 +355,43 @@ describe("RoomListViewModel", () => { expect(vm.rooms[i].roomId).toEqual(roomId); } + it("active index is calculated with the last opened room in a space", () => { + // Let's say there's two spaces: !space1:matrix.org and !space2:matrix.org + // Let's also say that the current active space is !space1:matrix.org + let currentSpace = "!space1:matrix.org"; + jest.spyOn(SpaceStore.instance, "activeSpace", "get").mockImplementation(() => currentSpace); + + const rooms = range(10).map((i) => mkStubRoom(`foo${i}:matrix.org`, `Foo ${i}`, undefined)); + // Let's say all the rooms are in space1 + const roomsInSpace1 = [...rooms]; + // Let's say all rooms with even index are in space 2 + const roomsInSpace2 = [...rooms].filter((_, i) => i % 2 === 0); + jest.spyOn(RoomListStoreV3.instance, "getSortedRoomsInActiveSpace").mockImplementation(() => + currentSpace === "!space1:matrix.org" ? roomsInSpace1 : roomsInSpace2, + ); + + // Let's say that the room at index 4 is currently active + const roomId = rooms[4].roomId; + jest.spyOn(SdkContextClass.instance.roomViewStore, "getRoomId").mockImplementation(() => roomId); + + const { result: vm } = renderHook(() => useRoomListViewModel()); + expect(vm.current.activeIndex).toEqual(4); + + // Let's say that space is changed to "!space2:matrix.org" + currentSpace = "!space2:matrix.org"; + // Let's say that room[6] is active in space 2 + const activeRoomIdInSpace2 = rooms[6].roomId; + jest.spyOn(SpaceStore.instance, "getLastSelectedRoomIdForSpace").mockImplementation( + () => activeRoomIdInSpace2, + ); + act(() => { + RoomListStoreV3.instance.emit(LISTS_UPDATE_EVENT); + }); + + // Active index should be 3 even without the room change event. + expectActiveRoom(vm.current, 3, activeRoomIdInSpace2); + }); + it("active room and active index are retained on order change", () => { const { rooms } = mockAndCreateRooms(); diff --git a/test/unit-tests/components/views/settings/PowerLevelSelector-test.tsx b/test/unit-tests/components/views/settings/PowerLevelSelector-test.tsx index 7837db5f71..2b9b3e62fa 100644 --- a/test/unit-tests/components/views/settings/PowerLevelSelector-test.tsx +++ b/test/unit-tests/components/views/settings/PowerLevelSelector-test.tsx @@ -61,7 +61,12 @@ describe("PowerLevelSelector", () => { it("should be able to change the power level of the current user", async () => { const onClick = jest.fn(); - renderPLS({ onClick }); + const userLevels = { + [currentUser]: 100, + "@alice:server.org": 100, + "@bob:server.org": 0, + }; + renderPLS({ userLevels, onClick }); // Until the power level is changed, the apply button should be disabled // compound button is using aria-disabled instead of the disabled attribute, we can't toBeDisabled on it @@ -107,4 +112,58 @@ describe("PowerLevelSelector", () => { expect(screen.getByText("empty label")).toBeInTheDocument(); }); + + it("should display modal warning if user is last admin", async () => { + const onClick = jest.fn(); + + renderPLS({ onClick }); + + // Until the power level is changed, the apply button should be disabled + // compound button is using aria-disabled instead of the disabled attribute, we can't toBeDisabled on it + expect(screen.getByRole("button", { name: "Apply" })).toHaveAttribute("aria-disabled", "true"); + + const select = screen.getByRole("combobox", { name: currentUser }); + // Sanity check + expect(select).toHaveValue("100"); + + // Change current user power level to 50 + await userEvent.selectOptions(select, "50"); + + // modal should appear because only admin in the room + expect(screen.findByText("WARNING")).toBeTruthy(); + + await userEvent.click(screen.getByRole("button", { name: "Continue" })); + + expect(select).toHaveValue("50"); + // After the user level changes, the apply button should be enabled + expect(screen.getByRole("button", { name: "Apply" })).toHaveAttribute("aria-disabled", "false"); + + // Click on Apply should call onClick with the new power level + await userEvent.click(screen.getByRole("button", { name: "Apply" })); + expect(onClick).toHaveBeenCalledWith(50, currentUser); + }); + + it("should display modal warning if user is last admin and return to initial value if user cancel", async () => { + const onClick = jest.fn(); + + renderPLS({ onClick }); + + // Until the power level is changed, the apply button should be disabled + // compound button is using aria-disabled instead of the disabled attribute, we can't toBeDisabled on it + expect(screen.getByRole("button", { name: "Apply" })).toHaveAttribute("aria-disabled", "true"); + + const select = screen.getByRole("combobox", { name: currentUser }); + // Sanity check + expect(select).toHaveValue("100"); + + // Change current user power level to 50 + await userEvent.selectOptions(select, "50"); + + // modal should appear because only admin in the room + expect(screen.findByText("WARNING")).toBeTruthy(); + + await userEvent.click(screen.getByRole("button", { name: "Cancel" })); + // the power level should be back to initial value + expect(select).toHaveValue("100"); + }); }); diff --git a/test/unit-tests/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx b/test/unit-tests/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx index 9dc5860119..604fe8ae41 100644 --- a/test/unit-tests/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx +++ b/test/unit-tests/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx @@ -224,6 +224,9 @@ describe("RolesRoomSettingsTab", () => { content: { users: { [cli.getUserId()!]: 100, + // needs at least one remaning admin in the room if we want to demote our user + // otherwise another modal will be displayed + ["@admin:server"]: 100, }, }, });