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
2023-07-14 15:48:20 +01:00 · 2023-07-14 15:48:20 +01:00 · 2760bfc836
parent cdffd1ca1f
commit 2760bfc836
2 changed files with 136 additions and 51 deletions
--- 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<IBaseRoomProps, "powerLevels">): JSX.Element | null => {
@ -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()
@ -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 (
-        <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>
    );
@ -736,6 +747,7 @@ const RedactMessagesButton: React.FC<IBaseProps> = ({ member }) => {
 export const BanToggleButton = ({
    room,
    member,
+    isUpdating,
    startUpdating,
    stopUpdating,
 }: Omit<IBaseRoomProps, "powerLevels">): JSX.Element => {
@ -743,6 +755,9 @@ export const BanToggleButton = ({

    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()
@ -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) {
@ -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>
    );
@ -863,7 +879,15 @@ 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
@ -871,25 +895,15 @@ const MuteToggleButton: React.FC<IBaseRoomProps> = ({ member, room, powerLevels,

    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
@ -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", {
@ -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>
    );
@ -939,6 +955,7 @@ export const RoomAdminToolsContainer: React.FC<IBaseRoomProps> = ({
    room,
    children,
    member,
+    isUpdating,
    startUpdating,
    stopUpdating,
    powerLevels,
@ -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()) {
@ -985,6 +1019,7 @@ export const RoomAdminToolsContainer: React.FC<IBaseRoomProps> = ({
                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}
            >
--- a/test/components/views/right_panel/UserInfo-test.tsx
+++ b/test/components/views/right_panel/UserInfo-test.tsx
@ -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 = {}) => {
@ -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 = {}) => {
@ -1136,6 +1148,7 @@ describe("<RoomAdminToolsContainer />", () => {
        defaultProps = {
            room: mockRoom,
            member: defaultMember,
+            isUpdating: false,
            startUpdating: jest.fn(),
            stopUpdating: jest.fn(),
            powerLevels: {},
@ -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();
    });
 });