From 2760bfc8369f1bee640d6d7a7e910783143d4c5f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 14 Jul 2023 15:48:20 +0100 Subject: [PATCH] Prevent user from accidentally double clicking user info admin actions (#11254) * Prevent user from accidentally double clicking user info admin actions * Iterate * Improve coverage * Improve coverage * Simplify * Simplify --- src/components/views/right_panel/UserInfo.tsx | 132 +++++++++++------- .../views/right_panel/UserInfo-test.tsx | 55 +++++++- 2 files changed, 136 insertions(+), 51 deletions(-) diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index 72768064ca..9a74cc6057 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -605,6 +605,7 @@ export const useRoomPowerLevels = (cli: MatrixClient, room: Room): IPowerLevelsC interface IBaseProps { member: RoomMember; + isUpdating: boolean; startUpdating(): void; stopUpdating(): void; } @@ -612,6 +613,7 @@ interface IBaseProps { export const RoomKickButton = ({ room, member, + isUpdating, startUpdating, stopUpdating, }: Omit): JSX.Element | null => { @@ -621,6 +623,9 @@ export const RoomKickButton = ({ if (member.membership !== "invite" && member.membership !== "join") return <>; const onKick = async (): Promise => { + if (isUpdating) return; // only allow one operation at a time + startUpdating(); + const commonProps = { member, action: room.isSpaceRoom() @@ -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( @@ -702,7 +708,12 @@ export const RoomKickButton = ({ : _t("Remove from room"); return ( - + {kickLabel} ); @@ -736,6 +747,7 @@ const RedactMessagesButton: React.FC = ({ member }) => { export const BanToggleButton = ({ room, member, + isUpdating, startUpdating, stopUpdating, }: Omit): JSX.Element => { @@ -743,6 +755,9 @@ export const BanToggleButton = ({ const isBanned = member.membership === "ban"; const onBanOrUnban = async (): Promise => { + if (isUpdating) return; // only allow one operation at a time + startUpdating(); + const commonProps = { member, action: room.isSpaceRoom() @@ -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 => { if (isBanned) { @@ -851,7 +867,7 @@ export const BanToggleButton = ({ }); return ( - + {label} ); @@ -863,7 +879,15 @@ interface IBaseRoomProps extends IBaseProps { children?: ReactNode; } -const MuteToggleButton: React.FC = ({ 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 = ({ + 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 @@ -871,25 +895,15 @@ const MuteToggleButton: React.FC = ({ member, room, powerLevels, const muted = isMuted(member, powerLevels); const onMuteToggle = async (): Promise => { + 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 @@ -900,27 +914,29 @@ const MuteToggleButton: React.FC = ({ 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", { @@ -929,7 +945,7 @@ const MuteToggleButton: React.FC = ({ member, room, powerLevels, const muteLabel = muted ? _t("Unmute") : _t("Mute"); return ( - + {muteLabel} ); @@ -939,6 +955,7 @@ export const RoomAdminToolsContainer: React.FC = ({ room, children, member, + isUpdating, startUpdating, stopUpdating, powerLevels, @@ -966,17 +983,34 @@ export const RoomAdminToolsContainer: React.FC = ({ if (!isMe && canAffectUser && me.powerLevel >= kickPowerLevel) { kickButton = ( - + ); } if (me.powerLevel >= redactPowerLevel && !room.isSpaceRoom()) { redactButton = ( - + ); } if (!isMe && canAffectUser && me.powerLevel >= banPowerLevel) { banButton = ( - + ); } if (!isMe && canAffectUser && me.powerLevel >= Number(editPowerLevel) && !room.isSpaceRoom()) { @@ -985,6 +1019,7 @@ export const RoomAdminToolsContainer: React.FC = ({ member={member} room={room} powerLevels={powerLevels} + isUpdating={isUpdating} startUpdating={startUpdating} stopUpdating={stopUpdating} /> @@ -1393,6 +1428,7 @@ const BasicUserInfo: React.FC<{ powerLevels={powerLevels} member={member as RoomMember} room={room} + isUpdating={pendingUpdateCount > 0} startUpdating={startUpdating} stopUpdating={stopUpdating} > diff --git a/test/components/views/right_panel/UserInfo-test.tsx b/test/components/views/right_panel/UserInfo-test.tsx index f158384ff2..ce35d3e0cc 100644 --- a/test/components/views/right_panel/UserInfo-test.tsx +++ b/test/components/views/right_panel/UserInfo-test.tsx @@ -907,7 +907,13 @@ describe("", () => { let defaultProps: Parameters[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 = {}) => { @@ -1008,7 +1014,13 @@ describe("", () => { const memberWithBanMembership = { ...defaultMember, membership: "ban" }; let defaultProps: Parameters[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 = {}) => { @@ -1136,6 +1148,7 @@ describe("", () => { defaultProps = { room: mockRoom, member: defaultMember, + isUpdating: false, startUpdating: jest.fn(), stopUpdating: jest.fn(), powerLevels: {}, @@ -1198,7 +1211,43 @@ describe("", () => { 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(); }); });