Skip to content

Commit

Permalink
Prevent user from accidentally double clicking user info admin actions (
Browse files Browse the repository at this point in the history 
#11254)

* Prevent user from accidentally double clicking user info admin actions

* Iterate

* Improve coverage

* Improve coverage

* Simplify

* Simplify
  • Loading branch information
@t3chguy
t3chguy committed Jul 14, 2023
1 parent cdffd1c commit 2760bfc
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 51 deletions.
132 changes: 84 additions & 48 deletions src/components/views/right_panel/UserInfo.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,13 +605,15 @@ export const useRoomPowerLevels = (cli: MatrixClient, room: Room): IPowerLevelsC

interface IBaseProps {
member: RoomMember;
isUpdating: boolean;
startUpdating(): void;
stopUpdating(): void;
}

export const RoomKickButton = ({
room,
member,
isUpdating,
startUpdating,
stopUpdating,
}: Omit<IBaseRoomProps, "powerLevels">): JSX.Element | null => {
Expand All @@ -621,6 +623,9 @@ export const RoomKickButton = ({
if (member.membership !== "invite" && member.membership !== "join") return <></>;

const onKick = async (): Promise<void> => {
if (isUpdating) return; // only allow one operation at a time
startUpdating();

const commonProps = {
member,
action: room.isSpaceRoom()
Expand Down Expand Up @@ -669,9 +674,10 @@ export const RoomKickButton = ({
}

const [proceed, reason, rooms = []] = await finished;
if (!proceed) return;

startUpdating();
if (!proceed) {
stopUpdating();
return;
}

bulkSpaceBehaviour(room, rooms, (room) => cli.kick(room.roomId, member.userId, reason || undefined))
.then(
Expand Down Expand Up @@ -702,7 +708,12 @@ export const RoomKickButton = ({
: _t("Remove from room");

return (
<AccessibleButton kind="link" className="mx_UserInfo_field mx_UserInfo_destructive" onClick={onKick}>
<AccessibleButton
kind="link"
className="mx_UserInfo_field mx_UserInfo_destructive"
onClick={onKick}
disabled={isUpdating}
>
{kickLabel}
</AccessibleButton>
);
Expand Down Expand Up @@ -736,13 +747,17 @@ const RedactMessagesButton: React.FC<IBaseProps> = ({ member }) => {
export const BanToggleButton = ({
room,
member,
isUpdating,
startUpdating,
stopUpdating,
}: Omit<IBaseRoomProps, "powerLevels">): JSX.Element => {
const cli = useContext(MatrixClientContext);

const isBanned = member.membership === "ban";
const onBanOrUnban = async (): Promise<void> => {
if (isUpdating) return; // only allow one operation at a time
startUpdating();

const commonProps = {
member,
action: room.isSpaceRoom()
Expand Down Expand Up @@ -809,9 +824,10 @@ export const BanToggleButton = ({
}

const [proceed, reason, rooms = []] = await finished;
if (!proceed) return;

startUpdating();
if (!proceed) {
stopUpdating();
return;
}

const fn = (roomId: string): Promise<unknown> => {
if (isBanned) {
Expand Down Expand Up @@ -851,7 +867,7 @@ export const BanToggleButton = ({
});

return (
<AccessibleButton kind="link" className={classes} onClick={onBanOrUnban}>
<AccessibleButton kind="link" className={classes} onClick={onBanOrUnban} disabled={isUpdating}>
{label}
</AccessibleButton>
);
Expand All @@ -863,33 +879,31 @@ interface IBaseRoomProps extends IBaseProps {
children?: ReactNode;
}

const MuteToggleButton: React.FC<IBaseRoomProps> = ({ member, room, powerLevels, startUpdating, stopUpdating }) => {
// We do not show a Mute button for ourselves so it doesn't need to handle warning self demotion
const MuteToggleButton: React.FC<IBaseRoomProps> = ({
member,
room,
powerLevels,
isUpdating,
startUpdating,
stopUpdating,
}) => {
const cli = useContext(MatrixClientContext);

// Don't show the mute/unmute option if the user is not in the room
if (member.membership !== "join") return null;

const muted = isMuted(member, powerLevels);
const onMuteToggle = async (): Promise<void> => {
if (isUpdating) return; // only allow one operation at a time
startUpdating();

const roomId = member.roomId;
const target = member.userId;

// if muting self, warn as it may be irreversible
if (target === cli.getUserId()) {
try {
if (!(await warnSelfDemote(room?.isSpaceRoom()))) return;
} catch (e) {
logger.error("Failed to warn about self demotion: ", e);
return;
}
}

const powerLevelEvent = room.currentState.getStateEvents("m.room.power_levels", "");
if (!powerLevelEvent) return;

const powerLevels = powerLevelEvent.getContent();
const levelToSend =
(powerLevels.events ? powerLevels.events["m.room.message"] : null) || powerLevels.events_default;
const powerLevels = powerLevelEvent?.getContent();
const levelToSend = powerLevels?.events?.["m.room.message"] ?? powerLevels?.events_default;
let level;
if (muted) {
// unmute
Expand All @@ -900,27 +914,29 @@ const MuteToggleButton: React.FC<IBaseRoomProps> = ({ member, room, powerLevels,
}
level = parseInt(level);

if (!isNaN(level)) {
startUpdating();
cli.setPowerLevel(roomId, target, level, powerLevelEvent)
.then(
() => {
// NO-OP; rely on the m.room.member event coming down else we could
// get out of sync if we force setState here!
logger.log("Mute toggle success");
},
function (err) {
logger.error("Mute error: " + err);
Modal.createDialog(ErrorDialog, {
title: _t("Error"),
description: _t("Failed to mute user"),
});
},
)
.finally(() => {
stopUpdating();
});
if (isNaN(level)) {
stopUpdating();
return;
}

cli.setPowerLevel(roomId, target, level, powerLevelEvent)
.then(
() => {
// NO-OP; rely on the m.room.member event coming down else we could
// get out of sync if we force setState here!
logger.log("Mute toggle success");
},
function (err) {
logger.error("Mute error: " + err);
Modal.createDialog(ErrorDialog, {
title: _t("Error"),
description: _t("Failed to mute user"),
});
},
)
.finally(() => {
stopUpdating();
});
};

const classes = classNames("mx_UserInfo_field", {
Expand All @@ -929,7 +945,7 @@ const MuteToggleButton: React.FC<IBaseRoomProps> = ({ member, room, powerLevels,

const muteLabel = muted ? _t("Unmute") : _t("Mute");
return (
<AccessibleButton kind="link" className={classes} onClick={onMuteToggle}>
<AccessibleButton kind="link" className={classes} onClick={onMuteToggle} disabled={isUpdating}>
{muteLabel}
</AccessibleButton>
);
Expand All @@ -939,6 +955,7 @@ export const RoomAdminToolsContainer: React.FC<IBaseRoomProps> = ({
room,
children,
member,
isUpdating,
startUpdating,
stopUpdating,
powerLevels,
Expand Down Expand Up @@ -966,17 +983,34 @@ export const RoomAdminToolsContainer: React.FC<IBaseRoomProps> = ({

if (!isMe && canAffectUser && me.powerLevel >= kickPowerLevel) {
kickButton = (
<RoomKickButton room={room} member={member} startUpdating={startUpdating} stopUpdating={stopUpdating} />
<RoomKickButton
room={room}
member={member}
isUpdating={isUpdating}
startUpdating={startUpdating}
stopUpdating={stopUpdating}
/>
);
}
if (me.powerLevel >= redactPowerLevel && !room.isSpaceRoom()) {
redactButton = (
<RedactMessagesButton member={member} startUpdating={startUpdating} stopUpdating={stopUpdating} />
<RedactMessagesButton
member={member}
isUpdating={isUpdating}
startUpdating={startUpdating}
stopUpdating={stopUpdating}
/>
);
}
if (!isMe && canAffectUser && me.powerLevel >= banPowerLevel) {
banButton = (
<BanToggleButton room={room} member={member} startUpdating={startUpdating} stopUpdating={stopUpdating} />
<BanToggleButton
room={room}
member={member}
isUpdating={isUpdating}
startUpdating={startUpdating}
stopUpdating={stopUpdating}
/>
);
}
if (!isMe && canAffectUser && me.powerLevel >= Number(editPowerLevel) && !room.isSpaceRoom()) {
Expand All @@ -985,6 +1019,7 @@ export const RoomAdminToolsContainer: React.FC<IBaseRoomProps> = ({
member={member}
room={room}
powerLevels={powerLevels}
isUpdating={isUpdating}
startUpdating={startUpdating}
stopUpdating={stopUpdating}
/>
Expand Down Expand Up @@ -1393,6 +1428,7 @@ const BasicUserInfo: React.FC<{
powerLevels={powerLevels}
member={member as RoomMember}
room={room}
isUpdating={pendingUpdateCount > 0}
startUpdating={startUpdating}
stopUpdating={stopUpdating}
>
Expand Down
55 changes: 52 additions & 3 deletions test/components/views/right_panel/UserInfo-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,13 @@ describe("<RoomKickButton />", () => {

let defaultProps: Parameters<typeof RoomKickButton>[0];
beforeEach(() => {
defaultProps = { room: mockRoom, member: defaultMember, startUpdating: jest.fn(), stopUpdating: jest.fn() };
defaultProps = {
room: mockRoom,
member: defaultMember,
startUpdating: jest.fn(),
stopUpdating: jest.fn(),
isUpdating: false,
};
});

const renderComponent = (props = {}) => {
Expand Down Expand Up @@ -1008,7 +1014,13 @@ describe("<BanToggleButton />", () => {
const memberWithBanMembership = { ...defaultMember, membership: "ban" };
let defaultProps: Parameters<typeof BanToggleButton>[0];
beforeEach(() => {
defaultProps = { room: mockRoom, member: defaultMember, startUpdating: jest.fn(), stopUpdating: jest.fn() };
defaultProps = {
room: mockRoom,
member: defaultMember,
startUpdating: jest.fn(),
stopUpdating: jest.fn(),
isUpdating: false,
};
});

const renderComponent = (props = {}) => {
Expand Down Expand Up @@ -1136,6 +1148,7 @@ describe("<RoomAdminToolsContainer />", () => {
defaultProps = {
room: mockRoom,
member: defaultMember,
isUpdating: false,
startUpdating: jest.fn(),
stopUpdating: jest.fn(),
powerLevels: {},
Expand Down Expand Up @@ -1198,7 +1211,43 @@ describe("<RoomAdminToolsContainer />", () => {
powerLevels: { events: { "m.room.power_levels": 1 } },
});

expect(screen.getByText(/mute/i)).toBeInTheDocument();
const button = screen.getByText(/mute/i);
expect(button).toBeInTheDocument();
fireEvent.click(button);
expect(defaultProps.startUpdating).toHaveBeenCalled();
});

it("should disable buttons when isUpdating=true", () => {
const mockMeMember = new RoomMember(mockRoom.roomId, "arbitraryId");
mockMeMember.powerLevel = 51; // defaults to 50
mockRoom.getMember.mockReturnValueOnce(mockMeMember);

const defaultMemberWithPowerLevelAndJoinMembership = { ...defaultMember, powerLevel: 0, membership: "join" };

renderComponent({
member: defaultMemberWithPowerLevelAndJoinMembership,
powerLevels: { events: { "m.room.power_levels": 1 } },
isUpdating: true,
});

const button = screen.getByText(/mute/i);
expect(button).toBeInTheDocument();
expect(button).toHaveAttribute("disabled");
expect(button).toHaveAttribute("aria-disabled", "true");
});

it("should not show mute button for one's own member", () => {
const mockMeMember = new RoomMember(mockRoom.roomId, mockClient.getSafeUserId());
mockMeMember.powerLevel = 51; // defaults to 50
mockRoom.getMember.mockReturnValueOnce(mockMeMember);

renderComponent({
member: mockMeMember,
powerLevels: { events: { "m.room.power_levels": 100 } },
});

const button = screen.queryByText(/mute/i);
expect(button).not.toBeInTheDocument();
});
});

Expand Down

0 comments on commit 2760bfc

Please sign in to comment.