OpenCloud
/
riot-web
mirror of https://github.com/vector-im/riot-web
1
0
Fork 0
Code Issues Releases Wiki Activity

Reset power selector on API failure to prevent state mismatch (#12319) 

* Reset power selector on API failure to prevent state mismatch

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Allow onChange to be sync or async

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Add unmounted check

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Improve coverage

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
pull/28217/head
Michael Telatynski 2024-03-08 11:58:36 +00:00 committed by GitHub
parent ddbc6439ce
commit 42ac873c55
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 96 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/components/views/elements/PowerSelector.tsx
View File

@ -39,7 +39,7 @@ interface Props<K extends undefined | string> {
// 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<void>;
// Optional key to pass as the second argument to `onChange`
powerLevelKey: K extends undefined ? void : K;
@ -60,6 +60,7 @@ export default class PowerSelector<K extends undefined | string> extends React.C
maxValue: Infinity,
usersDefault: 0,
};
private unmounted = false;
public constructor(props: Props<K>) {
super(props);
@ -84,6 +85,10 @@ export default class PowerSelector<K extends undefined | string> 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<K extends undefined | string> extends React.C
});
}
private onSelectChange = (event: React.ChangeEvent<HTMLSelectElement>): void => {
private onSelectChange = async (event: React.ChangeEvent<HTMLSelectElement>): Promise<void> => {
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<K extends undefined | string> extends React.C
this.setState({ customValue: parseInt(event.target.value) });
};
private onCustomBlur = (event: React.FocusEvent): void => {
private onCustomBlur = async (event: React.FocusEvent): Promise<void> => {
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
}

22
src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx
View File

@ -174,7 +174,7 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
}
}
private onPowerLevelsChanged = (value: number, powerLevelKey: string): void => {
private onPowerLevelsChanged = async (value: number, powerLevelKey: string): Promise<void> => {
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<IProps> {
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<void> => {
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<IProps> {
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 {

22
test/components/views/elements/PowerSelector-test.tsx
View File

@ -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("<PowerSelector />", () => {
expect(option.selected).toBeTruthy();
expect(fn).not.toHaveBeenCalled();
});
it("should reset when onChange promise rejects", async () => {
const deferred = defer<void>();
render(
<PowerSelector
value={25}
maxValue={100}
usersDefault={0}
onChange={() => 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);
});
});

38
test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx
View File

@ -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<ISendEventResponse>();
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"));
});
});