Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit ddd0ec4

Browse files
floriandurosflorianduros
authored
Change user permission by using a new apply button (#12346)
* Add PowerLevelSelector.tsx. It's extracting the current behavior of the privileged users and muted of `RolesRoomSettingsTab.tsx` into a dedicated component. It's also adding a new apply button. * Use `PowerLevelSelector` to render privileged and muted users in `RolesRoomSettingsTab` * Update existing tests * Add playwright test * Fix typo * Fix typo
1 parent a5ed97b commit ddd0ec4

File tree

8 files changed

+607
-63
lines changed

8 files changed

+607
-63
lines changed

‎playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
*
3+
* Copyright 2024 The Matrix.org Foundation C.I.C.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
* /
17+
*/
18+
19+
import { Locator } from "@playwright/test";
20+
21+
import { test, expect } from "../../element-web-test";
22+
23+
test.describe("Roles & Permissions room settings tab", () => {
24+
const roomName = "Test room";
25+
26+
test.use({
27+
displayName: "Alice",
28+
});
29+
30+
let settings: Locator;
31+
32+
test.beforeEach(async ({ user, app }) => {
33+
await app.client.createRoom({ name: roomName });
34+
await app.viewRoomByName(roomName);
35+
settings = await app.settings.openRoomSettings("Roles & Permissions");
36+
});
37+
38+
test("should be able to change the role of a user", async ({ page, app, user }) => {
39+
const privilegedUserSection = settings.locator(".mx_SettingsFieldset").first();
40+
const applyButton = privilegedUserSection.getByRole("button", { name: "Apply" });
41+
42+
// Alice is admin (100) and the Apply button should be disabled
43+
await expect(applyButton).toBeDisabled();
44+
let combobox = privilegedUserSection.getByRole("combobox", { name: user.userId });
45+
await expect(combobox).toHaveValue("100");
46+
47+
// Change the role of Alice to Moderator (50)
48+
await combobox.selectOption("Moderator");
49+
await expect(combobox).toHaveValue("50");
50+
await applyButton.click();
51+
52+
// Reload and check Alice is still Moderator (50)
53+
await page.reload();
54+
settings = await app.settings.openRoomSettings("Roles & Permissions");
55+
combobox = privilegedUserSection.getByRole("combobox", { name: user.userId });
56+
await expect(combobox).toHaveValue("50");
57+
});
58+
});

‎res/css/_components.pcss‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@
335335
@import "./views/settings/_NotificationSettings2.pcss";
336336
@import "./views/settings/_Notifications.pcss";
337337
@import "./views/settings/_PhoneNumbers.pcss";
338+
@import "./views/settings/_PowerLevelSelector.pcss";
338339
@import "./views/settings/_ProfileSettings.pcss";
339340
@import "./views/settings/_SecureBackupPanel.pcss";
340341
@import "./views/settings/_SetIdServer.pcss";

‎res/css/views/settings/_PowerLevelSelector.pcss‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
*
3+
* Copyright 2024 The Matrix.org Foundation C.I.C.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
* /
17+
*/
18+
19+
.mx_PowerLevelSelector_Button {
20+
align-self: flex-start;
21+
}

‎src/components/views/settings/PowerLevelSelector.tsx‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
*
3+
* Copyright 2024 The Matrix.org Foundation C.I.C.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
* /
17+
*/
18+
19+
import React, { useState, JSX, PropsWithChildren } from "react";
20+
import { Button } from "@vector-im/compound-web";
21+
import { compare } from "matrix-js-sdk/src/utils";
22+
23+
import { useMatrixClientContext } from "../../../contexts/MatrixClientContext";
24+
import PowerSelector from "../elements/PowerSelector";
25+
import { _t } from "../../../languageHandler";
26+
import SettingsFieldset from "./SettingsFieldset";
27+
28+
/**
29+
* Display in a fieldset, the power level of the users and allow to change them.
30+
* The apply button is disabled until the power level of an user is changed.
31+
* If there is no user to display, the children is displayed instead.
32+
*/
33+
interface PowerLevelSelectorProps {
34+
/**
35+
* The power levels of the users
36+
* The key is the user id and the value is the power level
37+
*/
38+
userLevels: Record<string, number>;
39+
/**
40+
* Whether the user can change the power levels of other users
41+
*/
42+
canChangeLevels: boolean;
43+
/**
44+
* The current user power level
45+
*/
46+
currentUserLevel: number;
47+
/**
48+
* The callback when the apply button is clicked
49+
* @param value - new power level for the user
50+
* @param userId - the user id
51+
*/
52+
onClick: (value: number, userId: string) => void;
53+
/**
54+
* Filter the users to display
55+
* @param user
56+
*/
57+
filter: (user: string) => boolean;
58+
/**
59+
* The title of the fieldset
60+
*/
61+
title: string;
62+
}
63+
64+
export function PowerLevelSelector({
65+
userLevels,
66+
canChangeLevels,
67+
currentUserLevel,
68+
onClick,
69+
filter,
70+
title,
71+
children,
72+
}: PropsWithChildren<PowerLevelSelectorProps>): JSX.Element | null {
73+
const matrixClient = useMatrixClientContext();
74+
const [currentPowerLevel, setCurrentPowerLevel] = useState<{ value: number; userId: string } | null>(null);
75+
76+
// If the power level has changed, we need to enable the apply button
77+
const powerLevelChanged = Boolean(
78+
currentPowerLevel && currentPowerLevel.value !== userLevels[currentPowerLevel?.userId],
79+
);
80+
81+
// We sort the users by power level, then we filter them
82+
const users = Object.keys(userLevels)
83+
.sort((userA, userB) => sortUser(userA, userB, userLevels))
84+
.filter(filter);
85+
86+
// No user to display, we return the children into fragment to convert it to JSX.Element type
87+
if (!users.length) return <>{children}</>;
88+
89+
return (
90+
<SettingsFieldset legend={title}>
91+
{users.map((userId) => {
92+
// We only want to display users with a valid power level aka an integer
93+
if (!Number.isInteger(userLevels[userId])) return;
94+
95+
const isMe = userId === matrixClient.getUserId();
96+
// If I can change levels, I can change the level of anyone with a lower level than mine
97+
const canChange = canChangeLevels && (userLevels[userId] < currentUserLevel || isMe);
98+
99+
// When the new power level is selected, the fields are rerendered and we need to keep the current value
100+
const userLevel = currentPowerLevel?.userId === userId ? currentPowerLevel?.value : userLevels[userId];
101+
102+
return (
103+
<PowerSelector
104+
value={userLevel}
105+
disabled={!canChange}
106+
label={userId}
107+
key={userId}
108+
onChange={(value) => setCurrentPowerLevel({ value, userId })}
109+
/>
110+
);
111+
})}
112+
113+
<Button
114+
size="sm"
115+
kind="primary"
116+
// mx_Dialog_nonDialogButton is necessary to avoid the Dialog CSS to override the button style
117+
className="mx_Dialog_nonDialogButton mx_PowerLevelSelector_Button"
118+
onClick={() => {
119+
if (currentPowerLevel !== null) {
120+
onClick(currentPowerLevel.value, currentPowerLevel.userId);
121+
setCurrentPowerLevel(null);
122+
}
123+
}}
124+
disabled={!powerLevelChanged}
125+
aria-label={_t("action|apply")}
126+
>
127+
{_t("action|apply")}
128+
</Button>
129+
</SettingsFieldset>
130+
);
131+
}
132+
133+
/**
134+
* Sort the users by power level, then by name
135+
* @param userA
136+
* @param userB
137+
* @param userLevels
138+
*/
139+
function sortUser(userA: string, userB: string, userLevels: PowerLevelSelectorProps["userLevels"]): number {
140+
const powerLevelDiff = userLevels[userA] - userLevels[userB];
141+
return powerLevelDiff !== 0 ? powerLevelDiff : compare(userA.toLocaleLowerCase(), userB.toLocaleLowerCase());
142+
}

‎src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx‎

Lines changed: 23 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import React from "react";
1818
import { EventType, RoomMember, RoomState, RoomStateEvent, Room, IContent } from "matrix-js-sdk/src/matrix";
1919
import { logger } from "matrix-js-sdk/src/logger";
2020
import { throttle, get } from "lodash";
21-
import { compare } from "matrix-js-sdk/src/utils";
2221

2322
import { _t, _td, TranslationKey } from "../../../../../languageHandler";
2423
import AccessibleButton from "../../../elements/AccessibleButton";
@@ -34,6 +33,7 @@ import { AddPrivilegedUsers } from "../../AddPrivilegedUsers";
3433
import SettingsTab from "../SettingsTab";
3534
import { SettingsSection } from "../../shared/SettingsSection";
3635
import MatrixClientContext from "../../../../../contexts/MatrixClientContext";
36+
import { PowerLevelSelector } from "../../PowerLevelSelector";
3737

3838
interface IEventShowOpts {
3939
isState?: boolean;
@@ -240,9 +240,6 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
240240
title: _t("room_settings|permissions|error_changing_pl_title"),
241241
description: _t("room_settings|permissions|error_changing_pl_description"),
242242
});
243-
244-
// Rethrow so that the PowerSelector can roll back
245-
throw e;
246243
}
247244
};
248245

@@ -352,65 +349,29 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
352349
let privilegedUsersSection = <div>{_t("room_settings|permissions|no_privileged_users")}</div>;
353350
let mutedUsersSection;
354351
if (Object.keys(userLevels).length) {
355-
const privilegedUsers: JSX.Element[] = [];
356-
const mutedUsers: JSX.Element[] = [];
357-
358-
Object.keys(userLevels).forEach((user) => {
359-
if (!Number.isInteger(userLevels[user])) return;
360-
const isMe = user === client.getUserId();
361-
const canChange = canChangeLevels && (userLevels[user] < currentUserLevel || isMe);
362-
if (userLevels[user] > defaultUserLevel) {
363-
// privileged
364-
privilegedUsers.push(
365-
<PowerSelector
366-
value={userLevels[user]}
367-
disabled={!canChange}
368-
label={user}
369-
key={user}
370-
powerLevelKey={user} // Will be sent as the second parameter to `onChange`
371-
onChange={this.onUserPowerLevelChanged}
372-
/>,
373-
);
374-
} else if (userLevels[user] < defaultUserLevel) {
375-
// muted
376-
mutedUsers.push(
377-
<PowerSelector
378-
value={userLevels[user]}
379-
disabled={!canChange}
380-
label={user}
381-
key={user}
382-
powerLevelKey={user} // Will be sent as the second parameter to `onChange`
383-
onChange={this.onUserPowerLevelChanged}
384-
/>,
385-
);
386-
}
387-
});
352+
privilegedUsersSection = (
353+
<PowerLevelSelector
354+
title={_t("room_settings|permissions|privileged_users_section")}
355+
userLevels={userLevels}
356+
canChangeLevels={canChangeLevels}
357+
currentUserLevel={currentUserLevel}
358+
onClick={this.onUserPowerLevelChanged}
359+
filter={(user) => userLevels[user] > defaultUserLevel}
360+
>
361+
<div>{_t("room_settings|permissions|no_privileged_users")}</div>
362+
</PowerLevelSelector>
363+
);
388364

389-
// comparator for sorting PL users lexicographically on PL descending, MXID ascending. (case-insensitive)
390-
const comparator = (a: JSX.Element, b: JSX.Element): number => {
391-
const aKey = a.key as string;
392-
const bKey = b.key as string;
393-
const plDiff = userLevels[bKey] - userLevels[aKey];
394-
return plDiff !== 0 ? plDiff : compare(aKey.toLocaleLowerCase(), bKey.toLocaleLowerCase());
395-
};
396-
397-
privilegedUsers.sort(comparator);
398-
mutedUsers.sort(comparator);
399-
400-
if (privilegedUsers.length) {
401-
privilegedUsersSection = (
402-
<SettingsFieldset legend={_t("room_settings|permissions|privileged_users_section")}>
403-
{privilegedUsers}
404-
</SettingsFieldset>
405-
);
406-
}
407-
if (mutedUsers.length) {
408-
mutedUsersSection = (
409-
<SettingsFieldset legend={_t("room_settings|permissions|muted_users_section")}>
410-
{mutedUsers}
411-
</SettingsFieldset>
412-
);
413-
}
365+
mutedUsersSection = (
366+
<PowerLevelSelector
367+
title={_t("room_settings|permissions|muted_users_section")}
368+
userLevels={userLevels}
369+
canChangeLevels={canChangeLevels}
370+
currentUserLevel={currentUserLevel}
371+
onClick={this.onUserPowerLevelChanged}
372+
filter={(user) => userLevels[user] < defaultUserLevel}
373+
/>
374+
);
414375
}
415376

416377
const banned = room.getMembersWithMembership("ban");

0 commit comments

Comments
 (0)