diff --git a/src/components/views/elements/PowerSelector.tsx b/src/components/views/elements/PowerSelector.tsx index 36bb51e4bbe..7dcf3d78ecc 100644 --- a/src/components/views/elements/PowerSelector.tsx +++ b/src/components/views/elements/PowerSelector.tsx @@ -39,7 +39,7 @@ interface Props { // The name to annotate the selector with label?: string; - onChange(value: number, powerLevelKey: K extends undefined ? void : K): void; + onChange(value: number, powerLevelKey: K extends undefined ? void : K): void | Promise; // Optional key to pass as the second argument to `onChange` powerLevelKey: K extends undefined ? void : K; @@ -60,6 +60,7 @@ export default class PowerSelector extends React.C maxValue: Infinity, usersDefault: 0, }; + private unmounted = false; public constructor(props: Props) { super(props); @@ -84,6 +85,10 @@ export default class PowerSelector extends React.C } } + public componentWillUnmount(): void { + this.unmounted = true; + } + private initStateFromProps(): void { // This needs to be done now because levelRoleMap has translated strings const levelRoleMap = Roles.levelRoleMap(this.props.usersDefault); @@ -106,14 +111,20 @@ export default class PowerSelector extends React.C }); } - private onSelectChange = (event: React.ChangeEvent): void => { + private onSelectChange = async (event: React.ChangeEvent): Promise => { const isCustom = event.target.value === CUSTOM_VALUE; if (isCustom) { this.setState({ custom: true }); } else { const powerLevel = parseInt(event.target.value); - this.props.onChange(powerLevel, this.props.powerLevelKey); this.setState({ selectValue: powerLevel }); + try { + await this.props.onChange(powerLevel, this.props.powerLevelKey); + } catch { + if (this.unmounted) return; + // If the request failed, roll back the state of the selector. + this.initStateFromProps(); + } } }; @@ -121,12 +132,18 @@ export default class PowerSelector extends React.C this.setState({ customValue: parseInt(event.target.value) }); }; - private onCustomBlur = (event: React.FocusEvent): void => { + private onCustomBlur = async (event: React.FocusEvent): Promise => { event.preventDefault(); event.stopPropagation(); if (Number.isFinite(this.state.customValue)) { - this.props.onChange(this.state.customValue, this.props.powerLevelKey); + try { + await this.props.onChange(this.state.customValue, this.props.powerLevelKey); + } catch { + if (this.unmounted) return; + // If the request failed, roll back the state of the selector. + this.initStateFromProps(); + } } else { this.initStateFromProps(); // reset, invalid input } diff --git a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx index 7bce2ccb17f..5f03e7f9505 100644 --- a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx @@ -174,7 +174,7 @@ export default class RolesRoomSettingsTab extends React.Component { } } - private onPowerLevelsChanged = (value: number, powerLevelKey: string): void => { + private onPowerLevelsChanged = async (value: number, powerLevelKey: string): Promise => { const client = this.context; const room = this.props.room; const plEvent = room.currentState.getStateEvents(EventType.RoomPowerLevels, ""); @@ -203,17 +203,22 @@ export default class RolesRoomSettingsTab extends React.Component { parentObj[keyPath[keyPath.length - 1]] = value; } - client.sendStateEvent(this.props.room.roomId, EventType.RoomPowerLevels, plContent).catch((e) => { + try { + await client.sendStateEvent(this.props.room.roomId, EventType.RoomPowerLevels, plContent); + } catch (e) { logger.error(e); Modal.createDialog(ErrorDialog, { title: _t("room_settings|permissions|error_changing_pl_reqs_title"), description: _t("room_settings|permissions|error_changing_pl_reqs_description"), }); - }); + + // Rethrow so that the PowerSelector can roll back + throw e; + } }; - private onUserPowerLevelChanged = (value: number, powerLevelKey: string): void => { + private onUserPowerLevelChanged = async (value: number, powerLevelKey: string): Promise => { const client = this.context; const room = this.props.room; const plEvent = room.currentState.getStateEvents(EventType.RoomPowerLevels, ""); @@ -226,14 +231,19 @@ export default class RolesRoomSettingsTab extends React.Component { if (!plContent["users"]) plContent["users"] = {}; plContent["users"][powerLevelKey] = value; - client.sendStateEvent(this.props.room.roomId, EventType.RoomPowerLevels, plContent).catch((e) => { + try { + await client.sendStateEvent(this.props.room.roomId, EventType.RoomPowerLevels, plContent); + } catch (e) { logger.error(e); Modal.createDialog(ErrorDialog, { title: _t("room_settings|permissions|error_changing_pl_title"), description: _t("room_settings|permissions|error_changing_pl_description"), }); - }); + + // Rethrow so that the PowerSelector can roll back + throw e; + } }; public render(): React.ReactNode { diff --git a/test/components/views/elements/PowerSelector-test.tsx b/test/components/views/elements/PowerSelector-test.tsx index 4636aaafc79..bf300e197ff 100644 --- a/test/components/views/elements/PowerSelector-test.tsx +++ b/test/components/views/elements/PowerSelector-test.tsx @@ -16,6 +16,7 @@ limitations under the License. import React from "react"; import { fireEvent, render, screen } from "@testing-library/react"; +import { defer } from "matrix-js-sdk/src/utils"; import PowerSelector from "../../../../src/components/views/elements/PowerSelector"; @@ -75,4 +76,25 @@ describe("", () => { expect(option.selected).toBeTruthy(); expect(fn).not.toHaveBeenCalled(); }); + + it("should reset when onChange promise rejects", async () => { + const deferred = defer(); + render( + deferred.promise} + powerLevelKey="key" + />, + ); + + const input = screen.getByLabelText("Power level"); + fireEvent.change(input, { target: { value: 40 } }); + fireEvent.blur(input); + + await screen.findByDisplayValue(40); + deferred.reject("Some error"); + await screen.findByDisplayValue(25); + }); }); diff --git a/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx index 0a9c8406f19..f121f6f045b 100644 --- a/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx @@ -15,9 +15,10 @@ limitations under the License. */ import React from "react"; -import { fireEvent, render, RenderResult, screen } from "@testing-library/react"; -import { MatrixClient, EventType, MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix"; +import { fireEvent, render, RenderResult, screen, waitFor } from "@testing-library/react"; +import { MatrixClient, EventType, MatrixEvent, Room, RoomMember, ISendEventResponse } from "matrix-js-sdk/src/matrix"; import { mocked } from "jest-mock"; +import { defer } from "matrix-js-sdk/src/utils"; import RolesRoomSettingsTab from "../../../../../../src/components/views/settings/tabs/room/RolesRoomSettingsTab"; import { mkStubRoom, withClientContextRenderOptions, stubClient } from "../../../../../test-utils"; @@ -231,4 +232,37 @@ describe("RolesRoomSettingsTab", () => { expect(screen.getByTitle("Banned by Alice")).toBeInTheDocument(); }); }); + + it("should roll back power level change on error", async () => { + const deferred = defer(); + mocked(cli.sendStateEvent).mockReturnValue(deferred.promise); + mocked(cli.getRoom).mockReturnValue(room); + // @ts-ignore - mocked doesn't support overloads properly + mocked(room.currentState.getStateEvents).mockImplementation((type, key) => { + if (key === undefined) return [] as MatrixEvent[]; + if (type === "m.room.power_levels") { + return new MatrixEvent({ + sender: "@sender:server", + room_id: roomId, + type: "m.room.power_levels", + state_key: "", + content: { + users: { + [cli.getUserId()!]: 100, + }, + }, + }); + } + return null; + }); + mocked(room.currentState.mayClientSendStateEvent).mockReturnValue(true); + const { container } = renderTab(); + + const selector = container.querySelector(`[placeholder="${cli.getUserId()}"]`)!; + fireEvent.change(selector, { target: { value: "50" } }); + expect(selector).toHaveValue("50"); + + deferred.reject("Error"); + await waitFor(() => expect(selector).toHaveValue("100")); + }); });