From b94b27db3e6dab09f4b5bf4b34114e5672948802 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 19 Apr 2023 18:00:04 +1200 Subject: [PATCH 01/13] hack to fix console noise from unfaked timers and clearAllModals --- test/test-utils/utilities.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/test-utils/utilities.ts b/test/test-utils/utilities.ts index 420e5d3bca2..6590ef52306 100644 --- a/test/test-utils/utilities.ts +++ b/test/test-utils/utilities.ts @@ -211,6 +211,17 @@ export const clearAllModals = async (): Promise => { // of removing the same modal because the promises don't flush otherwise. // // XXX: Maybe in the future with Jest 29.5.0+, we could use `runAllTimersAsync` instead. - await flushPromisesWithFakeTimers(); + + // this is called in some places where timers are not faked + // which causes a lot of noise in the console + // to make a hack even hackier check if timers are faked using a weird trick from github + // then call the appropriate promise flusher + // https://github.com/facebook/jest/issues/10555#issuecomment-1136466942 + const jestTimersFaked = setTimeout.name === "setTimeout"; + if (jestTimersFaked) { + await flushPromisesWithFakeTimers(); + } else { + await flushPromises(); + } } }; From aa579efd4ea31a77f4daad0cffd2f9aafb3f37c5 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 19 Apr 2023 18:13:04 +1200 Subject: [PATCH 02/13] remove old debug logging in AsyncWrapper --- src/AsyncWrapper.tsx | 3 --- test/components/structures/auth/ForgotPassword-test.tsx | 2 -- test/components/views/dialogs/InviteDialog-test.tsx | 1 - test/components/views/rooms/MessageComposer-test.tsx | 3 --- test/toasts/UnverifiedSessionToast-test.tsx | 2 +- .../components/molecules/VoiceBroadcastRecordingPip-test.tsx | 4 +--- test/voice-broadcast/models/VoiceBroadcastPlayback-test.tsx | 1 - 7 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/AsyncWrapper.tsx b/src/AsyncWrapper.tsx index 770f5f027b7..1173ca3fd06 100644 --- a/src/AsyncWrapper.tsx +++ b/src/AsyncWrapper.tsx @@ -45,9 +45,6 @@ export default class AsyncWrapper extends React.Component { public state: IState = {}; public componentDidMount(): void { - // XXX: temporary logging to try to diagnose - // https://github.com/vector-im/element-web/issues/3148 - logger.log("Starting load of AsyncWrapper for modal"); this.props.prom .then((result) => { if (this.unmounted) return; diff --git a/test/components/structures/auth/ForgotPassword-test.tsx b/test/components/structures/auth/ForgotPassword-test.tsx index c193342a41c..7feef7d6a1a 100644 --- a/test/components/structures/auth/ForgotPassword-test.tsx +++ b/test/components/structures/auth/ForgotPassword-test.tsx @@ -70,8 +70,6 @@ describe("", () => { filterConsole( // not implemented by js-dom https://github.com/jsdom/jsdom/issues/1937 "Not implemented: HTMLFormElement.prototype.requestSubmit", - // not of interested for this test - "Starting load of AsyncWrapper for modal", ); beforeEach(() => { diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx index 89be4f80300..9b13d9fc77f 100644 --- a/test/components/views/dialogs/InviteDialog-test.tsx +++ b/test/components/views/dialogs/InviteDialog-test.tsx @@ -104,7 +104,6 @@ describe("InviteDialog", () => { "Error retrieving profile for userId @carol:example.com", "Error retrieving profile for userId @localpart:server.tld", "Error retrieving profile for userId @localpart:server:tld", - "Starting load of AsyncWrapper for modal", "[Invite:Recents] Excluding @alice:example.org from recents", ); diff --git a/test/components/views/rooms/MessageComposer-test.tsx b/test/components/views/rooms/MessageComposer-test.tsx index d2a6e9c12ee..e076b8ae4a5 100644 --- a/test/components/views/rooms/MessageComposer-test.tsx +++ b/test/components/views/rooms/MessageComposer-test.tsx @@ -23,7 +23,6 @@ import userEvent from "@testing-library/user-event"; import { clearAllModals, createTestClient, - filterConsole, flushPromises, mkEvent, mkStubRoom, @@ -94,8 +93,6 @@ describe("MessageComposer", () => { stubClient(); const cli = createTestClient(); - filterConsole("Starting load of AsyncWrapper for modal"); - beforeEach(() => { mockPlatformPeg(); }); diff --git a/test/toasts/UnverifiedSessionToast-test.tsx b/test/toasts/UnverifiedSessionToast-test.tsx index 7a6dae4079b..a65f7dec79f 100644 --- a/test/toasts/UnverifiedSessionToast-test.tsx +++ b/test/toasts/UnverifiedSessionToast-test.tsx @@ -37,7 +37,7 @@ describe("UnverifiedSessionToast", () => { let client: Mocked; let renderResult: RenderResult; - filterConsole("Starting load of AsyncWrapper for modal", "Dismissing unverified sessions: ABC123"); + filterConsole("Dismissing unverified sessions: ABC123"); beforeAll(() => { client = mocked(stubClient()); diff --git a/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx b/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx index dddaa858272..240d5732886 100644 --- a/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx +++ b/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx @@ -28,7 +28,7 @@ import { VoiceBroadcastRecording, VoiceBroadcastRecordingPip, } from "../../../../src/voice-broadcast"; -import { filterConsole, flushPromises, stubClient } from "../../../test-utils"; +import { flushPromises, stubClient } from "../../../test-utils"; import { mkVoiceBroadcastInfoStateEvent } from "../../utils/test-utils"; import { requestMediaPermissions } from "../../../../src/utils/media/requestMediaPermissions"; import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../../src/MediaDeviceHandler"; @@ -85,8 +85,6 @@ describe("VoiceBroadcastRecordingPip", () => { }); }; - filterConsole("Starting load of AsyncWrapper for modal"); - beforeAll(() => { client = stubClient(); mocked(requestMediaPermissions).mockResolvedValue({ diff --git a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.tsx b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.tsx index 9ee165d87af..1c7e6b66c12 100644 --- a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.tsx +++ b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.tsx @@ -220,7 +220,6 @@ describe("VoiceBroadcastPlayback", () => { }; filterConsole( - "Starting load of AsyncWrapper for modal", // expected for some tests "Unable to load broadcast playback", ); From 714101cbf17aea7c84566c77901ef0a8e7bf325c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 19 Apr 2023 18:22:45 +1200 Subject: [PATCH 03/13] pass room to room settings tabs --- .../views/dialogs/RoomSettingsDialog.tsx | 48 ++++++++++++------- .../views/dialogs/SpaceSettingsDialog.tsx | 4 +- .../tabs/room/AdvancedRoomSettingsTab.tsx | 30 ++++++------ .../settings/tabs/room/BridgeSettingsTab.tsx | 7 ++- .../tabs/room/GeneralRoomSettingsTab.tsx | 22 +++++---- .../settings/tabs/room/PollHistoryTab.tsx | 11 ++--- .../tabs/room/RolesRoomSettingsTab.tsx | 35 +++++++------- .../tabs/room/SecurityRoomSettingsTab.tsx | 40 ++++++++-------- .../tabs/room/VoipRoomSettingsTab.tsx | 20 ++++---- .../room/AdvancedRoomSettingsTab-test.tsx | 2 +- .../tabs/room/RolesRoomSettingsTab-test.tsx | 2 +- .../tabs/room/VoipRoomSettingsTab-test.tsx | 4 +- 12 files changed, 120 insertions(+), 105 deletions(-) diff --git a/src/components/views/dialogs/RoomSettingsDialog.tsx b/src/components/views/dialogs/RoomSettingsDialog.tsx index 8c5de0b579e..998c369234b 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.tsx +++ b/src/components/views/dialogs/RoomSettingsDialog.tsx @@ -16,7 +16,7 @@ limitations under the License. */ import React from "react"; -import { RoomEvent } from "matrix-js-sdk/src/models/room"; +import { RoomEvent, Room } from "matrix-js-sdk/src/models/room"; import TabbedView, { Tab } from "../../structures/TabbedView"; import { _t, _td } from "../../../languageHandler"; @@ -54,6 +54,7 @@ interface IProps { interface IState { roomName: string; + room: Room; } export default class RoomSettingsDialog extends React.Component { @@ -61,7 +62,15 @@ export default class RoomSettingsDialog extends React.Component public constructor(props: IProps) { super(props); - this.state = { roomName: "" }; + + const room = MatrixClientPeg.get().getRoom(this.props.roomId)!; + + // something is really wrong if we encounter this + if (!room) { + throw new Error(`Cannot find room ${this.props.roomId}`); + } + + this.state = { roomName: "", room }; } public componentDidMount(): void { @@ -70,6 +79,18 @@ export default class RoomSettingsDialog extends React.Component this.onRoomName(); } + public componentDidUpdate(): void { + if (this.props.roomId !== this.state.room.roomId) { + const room = MatrixClientPeg.get().getRoom(this.props.roomId)!; + + // something is really wrong if we encounter this + if (!room) { + throw new Error(`Cannot find room ${this.props.roomId}`); + } + this.setState({ room }); + } + } + public componentWillUnmount(): void { if (this.dispatcherRef) { dis.unregister(this.dispatcherRef); @@ -88,7 +109,7 @@ export default class RoomSettingsDialog extends React.Component private onRoomName = (): void => { this.setState({ - roomName: MatrixClientPeg.get().getRoom(this.props.roomId)?.name ?? "", + roomName: this.state.room.name ?? "", }); }; @@ -100,7 +121,7 @@ export default class RoomSettingsDialog extends React.Component ROOM_GENERAL_TAB, _td("General"), "mx_RoomSettingsDialog_settingsIcon", - , + , "RoomSettingsGeneral", ), ); @@ -110,7 +131,7 @@ export default class RoomSettingsDialog extends React.Component ROOM_VOIP_TAB, _td("Voice & Video"), "mx_RoomSettingsDialog_voiceIcon", - , + , ), ); } @@ -119,12 +140,7 @@ export default class RoomSettingsDialog extends React.Component ROOM_SECURITY_TAB, _td("Security & Privacy"), "mx_RoomSettingsDialog_securityIcon", - ( - this.props.onFinished(true)} - /> - ), + this.props.onFinished(true)} />, "RoomSettingsSecurityPrivacy", ), ); @@ -133,7 +149,7 @@ export default class RoomSettingsDialog extends React.Component ROOM_ROLES_TAB, _td("Roles & Permissions"), "mx_RoomSettingsDialog_rolesIcon", - , + , "RoomSettingsRolesPermissions", ), ); @@ -144,7 +160,7 @@ export default class RoomSettingsDialog extends React.Component "mx_RoomSettingsDialog_notificationsIcon", ( this.props.onFinished(true)} /> ), @@ -158,7 +174,7 @@ export default class RoomSettingsDialog extends React.Component ROOM_BRIDGES_TAB, _td("Bridges"), "mx_RoomSettingsDialog_bridgesIcon", - , + , "RoomSettingsBridges", ), ); @@ -169,7 +185,7 @@ export default class RoomSettingsDialog extends React.Component ROOM_POLL_HISTORY_TAB, _td("Poll history"), "mx_RoomSettingsDialog_pollsIcon", - this.props.onFinished(true)} />, + this.props.onFinished(true)} />, ), ); @@ -181,7 +197,7 @@ export default class RoomSettingsDialog extends React.Component "mx_RoomSettingsDialog_warningIcon", ( this.props.onFinished(true)} /> ), diff --git a/src/components/views/dialogs/SpaceSettingsDialog.tsx b/src/components/views/dialogs/SpaceSettingsDialog.tsx index e67e6f21ffa..8cf8e8418b7 100644 --- a/src/components/views/dialogs/SpaceSettingsDialog.tsx +++ b/src/components/views/dialogs/SpaceSettingsDialog.tsx @@ -70,14 +70,14 @@ const SpaceSettingsDialog: React.FC = ({ matrixClient: cli, space, onFin SpaceSettingsTab.Roles, _td("Roles & Permissions"), "mx_RoomSettingsDialog_rolesIcon", - , + , ), SettingsStore.getValue(UIFeature.AdvancedSettings) ? new Tab( SpaceSettingsTab.Advanced, _td("Advanced"), "mx_RoomSettingsDialog_warningIcon", - , + , ) : null, ].filter(Boolean) as NonEmptyArray; diff --git a/src/components/views/settings/tabs/room/AdvancedRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/AdvancedRoomSettingsTab.tsx index 869c51c9494..e8ce957c3b5 100644 --- a/src/components/views/settings/tabs/room/AdvancedRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/AdvancedRoomSettingsTab.tsx @@ -16,9 +16,9 @@ limitations under the License. import React from "react"; import { EventType } from "matrix-js-sdk/src/@types/event"; +import { Room } from "matrix-js-sdk/src/matrix"; import { _t } from "../../../../../languageHandler"; -import { MatrixClientPeg } from "../../../../../MatrixClientPeg"; import AccessibleButton, { ButtonEvent } from "../../../elements/AccessibleButton"; import RoomUpgradeDialog from "../../../dialogs/RoomUpgradeDialog"; import Modal from "../../../../../Modal"; @@ -29,7 +29,7 @@ import { ViewRoomPayload } from "../../../../../dispatcher/payloads/ViewRoomPayl import SettingsStore from "../../../../../settings/SettingsStore"; interface IProps { - roomId: string; + room: Room; closeSettingsFn(): void; } @@ -64,8 +64,8 @@ export default class AdvancedRoomSettingsTab extends React.Component { + const room = this.props.room; + room.getRecommendedVersion().then((v) => { const tombstone = room.currentState.getStateEvents(EventType.RoomTombstone, ""); const additionalStateChanges: Partial = {}; @@ -85,8 +85,7 @@ export default class AdvancedRoomSettingsTab extends React.Component { - const room = MatrixClientPeg.get().getRoom(this.props.roomId); - Modal.createDialog(RoomUpgradeDialog, { room }); + Modal.createDialog(RoomUpgradeDialog, { room: this.props.room }); }; private onOldRoomClicked = (e: ButtonEvent): void => { @@ -105,12 +104,11 @@ export default class AdvancedRoomSettingsTab extends React.Component{_t("This room is not accessible by remote Matrix servers")}; } @@ -143,9 +141,9 @@ export default class AdvancedRoomSettingsTab extends React.Component{_t("Advanced")}
- {room?.isSpaceRoom() ? _t("Space information") : _t("Room information")} + {room.isSpaceRoom() ? _t("Space information") : _t("Room information")}
{_t("Internal room ID")} - this.props.roomId}>{this.props.roomId} + this.props.room.roomId}> + {this.props.room.roomId} +
{unfederatableSection}
@@ -172,7 +172,7 @@ export default class AdvancedRoomSettingsTab extends React.Component{_t("Room version")}
{_t("Room version:")}  - {room?.getVersion()} + {room.getVersion()}
{oldRoomLink} {roomUpgradeButton} diff --git a/src/components/views/settings/tabs/room/BridgeSettingsTab.tsx b/src/components/views/settings/tabs/room/BridgeSettingsTab.tsx index 4eb676f4113..8731cee0a6a 100644 --- a/src/components/views/settings/tabs/room/BridgeSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/BridgeSettingsTab.tsx @@ -30,7 +30,7 @@ const BRIDGE_EVENT_TYPES = [ const BRIDGES_LINK = "https://matrix.org/bridges/"; interface IProps { - roomId: string; + room: Room; } export default class BridgeSettingsTab extends React.Component { @@ -51,9 +51,8 @@ export default class BridgeSettingsTab extends React.Component { public render(): React.ReactNode { // This settings tab will only be invoked if the following function returns more // than 0 events, so no validation is needed at this stage. - const bridgeEvents = BridgeSettingsTab.getBridgeStateEvents(this.props.roomId); - const client = MatrixClientPeg.get(); - const room = client.getRoom(this.props.roomId); + const bridgeEvents = BridgeSettingsTab.getBridgeStateEvents(this.props.room.roomId); + const room = this.props.room; let content: JSX.Element; if (bridgeEvents.length > 0) { diff --git a/src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx index 9c47a7432e6..a915aa42e38 100644 --- a/src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx @@ -15,6 +15,7 @@ limitations under the License. */ import React, { ContextType } from "react"; +import { Room } from "matrix-js-sdk/src/matrix"; import { _t } from "../../../../../languageHandler"; import RoomProfileSettings from "../../../room_settings/RoomProfileSettings"; @@ -28,7 +29,7 @@ import AliasSettings from "../../../room_settings/AliasSettings"; import PosthogTrackers from "../../../../../PosthogTrackers"; interface IProps { - roomId: string; + room: Room; } interface IState { @@ -50,7 +51,7 @@ export default class GeneralRoomSettingsTab extends React.Component { dis.dispatch({ action: "leave_room", - room_id: this.props.roomId, + room_id: this.props.room.roomId, }); PosthogTrackers.trackInteraction("WebRoomSettingsLeaveButton", ev); @@ -58,17 +59,18 @@ export default class GeneralRoomSettingsTab extends React.Component : null; + const urlPreviewSettings = SettingsStore.getValue(UIFeature.URLPreviews) ? ( + + ) : null; let leaveSection; - if (room?.getMyMembership() === "join") { + if (room.getMyMembership() === "join") { leaveSection = ( <> {_t("Leave room")} @@ -85,12 +87,12 @@ export default class GeneralRoomSettingsTab extends React.Component
{_t("General")}
- +
{_t("Room Addresses")}
void; } -export const PollHistoryTab: React.FC = ({ roomId, onFinished }) => { +export const PollHistoryTab: React.FC = ({ room, onFinished }) => { const matrixClient = useContext(MatrixClientContext); - const room = matrixClient.getRoom(roomId); - if (!room) { - return null; - } - const permalinkCreator = new RoomPermalinkCreator(room, roomId); + const permalinkCreator = new RoomPermalinkCreator(room, room.roomId); return (
diff --git a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx index 743cceff75d..58aa40610c3 100644 --- a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx @@ -22,6 +22,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { throttle, get } from "lodash"; import { compare } from "matrix-js-sdk/src/utils"; import { IContent } from "matrix-js-sdk/src/models/event"; +import { Room } from "matrix-js-sdk/src/matrix"; import { _t, _td } from "../../../../../languageHandler"; import { MatrixClientPeg } from "../../../../../MatrixClientPeg"; @@ -129,7 +130,7 @@ export class BannedUser extends React.Component { } interface IProps { - roomId: string; + room: Room; } export default class RolesRoomSettingsTab extends React.Component { @@ -145,7 +146,7 @@ export default class RolesRoomSettingsTab extends React.Component { } private onRoomStateUpdate = (state: RoomState): void => { - if (state.roomId !== this.props.roomId) return; + if (state.roomId !== this.props.room.roomId) return; this.onThisRoomMembership(); }; @@ -171,8 +172,8 @@ export default class RolesRoomSettingsTab extends React.Component { private onPowerLevelsChanged = (value: number, powerLevelKey: string): void => { const client = MatrixClientPeg.get(); - const room = client.getRoom(this.props.roomId); - const plEvent = room?.currentState.getStateEvents(EventType.RoomPowerLevels, ""); + const room = this.props.room; + const plEvent = room.currentState.getStateEvents(EventType.RoomPowerLevels, ""); let plContent = plEvent?.getContent() ?? {}; // Clone the power levels just in case @@ -198,7 +199,7 @@ export default class RolesRoomSettingsTab extends React.Component { parentObj[keyPath[keyPath.length - 1]] = value; } - client.sendStateEvent(this.props.roomId, EventType.RoomPowerLevels, plContent).catch((e) => { + client.sendStateEvent(this.props.room.roomId, EventType.RoomPowerLevels, plContent).catch((e) => { logger.error(e); Modal.createDialog(ErrorDialog, { @@ -213,8 +214,8 @@ export default class RolesRoomSettingsTab extends React.Component { private onUserPowerLevelChanged = (value: number, powerLevelKey: string): void => { const client = MatrixClientPeg.get(); - const room = client.getRoom(this.props.roomId); - const plEvent = room?.currentState.getStateEvents(EventType.RoomPowerLevels, ""); + const room = this.props.room; + const plEvent = room.currentState.getStateEvents(EventType.RoomPowerLevels, ""); let plContent = plEvent?.getContent() ?? {}; // Clone the power levels just in case @@ -224,7 +225,7 @@ export default class RolesRoomSettingsTab extends React.Component { if (!plContent["users"]) plContent["users"] = {}; plContent["users"][powerLevelKey] = value; - client.sendStateEvent(this.props.roomId, EventType.RoomPowerLevels, plContent).catch((e) => { + client.sendStateEvent(this.props.room.roomId, EventType.RoomPowerLevels, plContent).catch((e) => { logger.error(e); Modal.createDialog(ErrorDialog, { @@ -239,12 +240,12 @@ export default class RolesRoomSettingsTab extends React.Component { public render(): React.ReactNode { const client = MatrixClientPeg.get(); - const room = client.getRoom(this.props.roomId); - const isSpaceRoom = room?.isSpaceRoom(); + const room = this.props.room; + const isSpaceRoom = room.isSpaceRoom(); - const plEvent = room?.currentState.getStateEvents(EventType.RoomPowerLevels, ""); + const plEvent = room.currentState.getStateEvents(EventType.RoomPowerLevels, ""); const plContent = plEvent ? plEvent.getContent() || {} : {}; - const canChangeLevels = room?.currentState.mayClientSendStateEvent(EventType.RoomPowerLevels, client); + const canChangeLevels = room.currentState.mayClientSendStateEvent(EventType.RoomPowerLevels, client); const plEventsToLabels: Record = { // These will be translated for us later. @@ -392,7 +393,7 @@ export default class RolesRoomSettingsTab extends React.Component { } } - const banned = room?.getMembersWithMembership("ban"); + const banned = room.getMembersWithMembership("ban"); let bannedUsersSection: JSX.Element | undefined; if (banned?.length) { const canBanUsers = currentUserLevel >= banLevel; @@ -401,7 +402,7 @@ export default class RolesRoomSettingsTab extends React.Component {
    {banned.map((member) => { const banEvent = member.events.member?.getContent(); - const sender = room?.getMember(member.events.member.getSender()); + const sender = room.getMember(member.events.member.getSender()); let bannedBy = member.events.member?.getSender(); // start by falling back to mxid if (sender) bannedBy = sender.name; return ( @@ -443,7 +444,7 @@ export default class RolesRoomSettingsTab extends React.Component { .filter(Boolean); // hide the power level selector for enabling E2EE if it the room is already encrypted - if (client.isRoomEncrypted(this.props.roomId)) { + if (client.isRoomEncrypted(this.props.room.roomId)) { delete eventsLevels[EventType.RoomEncryption]; } @@ -481,9 +482,7 @@ export default class RolesRoomSettingsTab extends React.Component {
    {_t("Roles & Permissions")}
    {privilegedUsersSection} - {canChangeLevels && room !== null && ( - - )} + {canChangeLevels && } {mutedUsersSection} {bannedUsersSection} void; } @@ -61,7 +62,7 @@ export default class SecurityRoomSettingsTab extends React.Component) { super(props, context); - const state = context.getRoom(this.props.roomId)?.currentState; + const state = this.props.room.currentState; this.state = { guestAccess: this.pullContentPropertyFromEvent( @@ -75,7 +76,7 @@ export default class SecurityRoomSettingsTab extends React.Component => { - if (this.context.getRoom(this.props.roomId)?.getJoinRule() === JoinRule.Public) { + if (this.props.room.getJoinRule() === JoinRule.Public) { const dialog = Modal.createDialog(QuestionDialog, { title: _t("Are you sure you want to add encryption to this public room?"), description: ( @@ -172,7 +173,9 @@ export default class SecurityRoomSettingsTab extends React.Component { logger.error(e); this.setState({ encrypted: beforeEncrypted }); @@ -190,7 +193,7 @@ export default class SecurityRoomSettingsTab extends React.Component { - this.context.getRoom(this.props.roomId)?.setBlacklistUnverifiedDevices(checked); + this.props.room.setBlacklistUnverifiedDevices(checked); }; private async hasAliases(): Promise { const cli = this.context; - const response = await cli.getLocalAliases(this.props.roomId); + const response = await cli.getLocalAliases(this.props.room.roomId); const localAliases = response.aliases; return Array.isArray(localAliases) && localAliases.length !== 0; } private renderJoinRule(): JSX.Element { - const client = this.context; - const room = client.getRoom(this.props.roomId); + const room = this.props.room; let aliasWarning: JSX.Element | undefined; - if (room?.getJoinRule() === JoinRule.Public && !this.state.hasAliases) { + if (room.getJoinRule() === JoinRule.Public && !this.state.hasAliases) { aliasWarning = (
    @@ -260,7 +262,7 @@ export default class SecurityRoomSettingsTab extends React.Component ); } @@ -436,7 +438,7 @@ export default class SecurityRoomSettingsTab extends React.Component = ({ roomId }) => { - const room = useMemo(() => MatrixClientPeg.get().getRoom(roomId), [roomId]); - const isPublic = useMemo(() => room?.getJoinRule() === JoinRule.Public, [room]); +const ElementCallSwitch: React.FC = ({ room }) => { + const isPublic = useMemo(() => room.getJoinRule() === JoinRule.Public, [room]); const [content, events, maySend] = useRoomState( - room ?? undefined, + room, useCallback((state: RoomState) => { const content = state?.getStateEvents(EventType.RoomPowerLevels, "")?.getContent(); return [ @@ -68,12 +68,12 @@ const ElementCallSwitch: React.FC = ({ roomId }) => { events[ElementCall.MEMBER_EVENT_TYPE.name] = adminLevel; } - MatrixClientPeg.get().sendStateEvent(roomId, EventType.RoomPowerLevels, { + MatrixClientPeg.get().sendStateEvent(room.roomId, EventType.RoomPowerLevels, { events: events, ...content, }); }, - [roomId, content, events, isPublic], + [room.roomId, content, events, isPublic], ); const brand = SdkConfig.get("element_call").brand ?? DEFAULTS.element_call.brand; @@ -95,14 +95,14 @@ const ElementCallSwitch: React.FC = ({ roomId }) => { }; interface Props { - roomId: string; + room: Room; } -export const VoipRoomSettingsTab: React.FC = ({ roomId }) => { +export const VoipRoomSettingsTab: React.FC = ({ room }) => { return ( - + ); diff --git a/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx index d00b1a8f7e6..775aadf973b 100644 --- a/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx @@ -36,7 +36,7 @@ describe("AdvancedRoomSettingsTab", () => { let room: Room; const renderTab = (): RenderResult => { - return render(); + return render(); }; beforeEach(() => { diff --git a/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx index be95dbf56c0..0806b5a0bc4 100644 --- a/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx @@ -35,7 +35,7 @@ describe("RolesRoomSettingsTab", () => { let room: Room; const renderTab = (): RenderResult => { - return render(); + return render(); }; const getVoiceBroadcastsSelect = (): HTMLElement => { diff --git a/test/components/views/settings/tabs/room/VoipRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/VoipRoomSettingsTab-test.tsx index 88e49e02399..7e14a6f5dc4 100644 --- a/test/components/views/settings/tabs/room/VoipRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/VoipRoomSettingsTab-test.tsx @@ -27,13 +27,13 @@ import { MatrixClientPeg } from "../../../../../../src/MatrixClientPeg"; import { VoipRoomSettingsTab } from "../../../../../../src/components/views/settings/tabs/room/VoipRoomSettingsTab"; import { ElementCall } from "../../../../../../src/models/Call"; -describe("RolesRoomSettingsTab", () => { +describe("VoipRoomSettingsTab", () => { const roomId = "!room:example.com"; let cli: MatrixClient; let room: Room; const renderTab = (): RenderResult => { - return render(); + return render(); }; beforeEach(() => { From e434d645b2389e72147d116bd38d0a668e6e85c2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 19 Apr 2023 18:36:37 +1200 Subject: [PATCH 04/13] add errorboundary for roomsettingsdialog --- src/components/views/dialogs/RoomSettingsDialog.tsx | 11 ++++++++++- .../views/dialogs/RoomSettingsDialog-test.tsx | 11 ++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/components/views/dialogs/RoomSettingsDialog.tsx b/src/components/views/dialogs/RoomSettingsDialog.tsx index 998c369234b..f08fe72e401 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.tsx +++ b/src/components/views/dialogs/RoomSettingsDialog.tsx @@ -36,6 +36,7 @@ import { VoipRoomSettingsTab } from "../settings/tabs/room/VoipRoomSettingsTab"; import { ActionPayload } from "../../../dispatcher/payloads"; import { NonEmptyArray } from "../../../@types/common"; import { PollHistoryTab } from "../settings/tabs/room/PollHistoryTab"; +import ErrorBoundary from "../elements/ErrorBoundary"; export const ROOM_GENERAL_TAB = "ROOM_GENERAL_TAB"; export const ROOM_VOIP_TAB = "ROOM_VOIP_TAB"; @@ -57,7 +58,7 @@ interface IState { room: Room; } -export default class RoomSettingsDialog extends React.Component { +class RoomSettingsDialog extends React.Component { private dispatcherRef: string; public constructor(props: IProps) { @@ -229,3 +230,11 @@ export default class RoomSettingsDialog extends React.Component ); } } + +const WrappedRoomSettingsDialog: React.FC = (props) => ( + + + +); + +export default WrappedRoomSettingsDialog; diff --git a/test/components/views/dialogs/RoomSettingsDialog-test.tsx b/test/components/views/dialogs/RoomSettingsDialog-test.tsx index 5a4356cb334..7754e3d0245 100644 --- a/test/components/views/dialogs/RoomSettingsDialog-test.tsx +++ b/test/components/views/dialogs/RoomSettingsDialog-test.tsx @@ -44,18 +44,23 @@ describe("", () => { beforeEach(() => { jest.clearAllMocks(); - mockClient.getRoom.mockReturnValue(room); + mockClient.getRoom.mockImplementation((roomId) => (roomId === room.roomId ? room : null)); jest.spyOn(SettingsStore, "getValue").mockReset().mockReturnValue(false); }); - const getComponent = (onFinished = jest.fn()) => - render(, { + const getComponent = (onFinished = jest.fn(), propRoomId = roomId) => + render(, { wrapper: ({ children }) => ( {children} ), }); + it("catches errors when room is not found", () => { + getComponent(undefined, "!room-that-does-not-exist"); + expect(screen.getByText("Something went wrong!")).toBeInTheDocument(); + }); + describe("Settings tabs", () => { it("renders default tabs correctly", () => { const { container } = getComponent(); From 03779d4a1966c5d6db0bf23a5862a3d34c01ef9e Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 19 Apr 2023 19:02:45 +1200 Subject: [PATCH 05/13] apply strictnullchecks to tabs/room --- .../settings/tabs/room/NotificationSettingsTab.tsx | 2 +- .../views/settings/tabs/room/RolesRoomSettingsTab.tsx | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx b/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx index d5ab1075406..d8232210fa8 100644 --- a/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx @@ -53,7 +53,7 @@ export default class NotificationsSettingsTab extends React.Component) { super(props, context); - this.roomProps = EchoChamber.forRoom(context.getRoom(this.props.roomId)); + this.roomProps = EchoChamber.forRoom(context.getRoom(this.props.roomId)!); let currentSound = "default"; const soundData = Notifier.getSoundForRoom(this.props.roomId); diff --git a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx index 58aa40610c3..3f2c6d65fe0 100644 --- a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx @@ -187,7 +187,7 @@ export default class RolesRoomSettingsTab extends React.Component { plContent["events"][powerLevelKey.slice(eventsLevelPrefix.length)] = value; } else { const keyPath = powerLevelKey.split("."); - let parentObj: IContent | undefined; + let parentObj: IContent = {}; let currentObj = plContent; for (const key of keyPath) { if (!currentObj[key]) { @@ -402,16 +402,16 @@ export default class RolesRoomSettingsTab extends React.Component {
      {banned.map((member) => { const banEvent = member.events.member?.getContent(); - const sender = room.getMember(member.events.member.getSender()); - let bannedBy = member.events.member?.getSender(); // start by falling back to mxid - if (sender) bannedBy = sender.name; + const bannedById = member.events.member?.getSender(); + const sender = bannedById ? room.getMember(bannedById) : undefined; + const bannedBy = sender?.name || bannedById; // fallback to mxid return ( ); })} From 307a37f8dd72a62ea331854e1b6c1b1a218a2529 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Apr 2023 14:36:50 +1200 Subject: [PATCH 06/13] dedupe code to set toom in roomsettingdialog --- .../views/dialogs/RoomSettingsDialog.tsx | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/components/views/dialogs/RoomSettingsDialog.tsx b/src/components/views/dialogs/RoomSettingsDialog.tsx index f08fe72e401..bebb5340438 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.tsx +++ b/src/components/views/dialogs/RoomSettingsDialog.tsx @@ -1,6 +1,8 @@ /* Copyright 2019 New Vector Ltd Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> +Copyright 2023 The Matrix.org Foundation C.I.C. + Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -64,13 +66,7 @@ class RoomSettingsDialog extends React.Component { public constructor(props: IProps) { super(props); - const room = MatrixClientPeg.get().getRoom(this.props.roomId)!; - - // something is really wrong if we encounter this - if (!room) { - throw new Error(`Cannot find room ${this.props.roomId}`); - } - + const room = this.getRoom(); this.state = { roomName: "", room }; } @@ -82,12 +78,7 @@ class RoomSettingsDialog extends React.Component { public componentDidUpdate(): void { if (this.props.roomId !== this.state.room.roomId) { - const room = MatrixClientPeg.get().getRoom(this.props.roomId)!; - - // something is really wrong if we encounter this - if (!room) { - throw new Error(`Cannot find room ${this.props.roomId}`); - } + const room = this.getRoom(); this.setState({ room }); } } @@ -100,6 +91,21 @@ class RoomSettingsDialog extends React.Component { MatrixClientPeg.get().removeListener(RoomEvent.Name, this.onRoomName); } + /** + * Get room from client + * @returns Room + * @throws when room is not found + */ + private getRoom(): Room { + const room = MatrixClientPeg.get().getRoom(this.props.roomId)!; + + // something is really wrong if we encounter this + if (!room) { + throw new Error(`Cannot find room ${this.props.roomId}`); + } + return room; + } + private onAction = (payload: ActionPayload): void => { // When view changes below us, close the room settings // whilst the modal is open this can only be triggered when someone hits Leave Room From 2c7f243dc410861f403126dbdf5ed703f50074bf Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Apr 2023 14:36:56 +1200 Subject: [PATCH 07/13] add unit tests --- .../room/AdvancedRoomSettingsTab-test.tsx | 29 ++++- .../tabs/room/BridgeSettingsTab-test.tsx | 60 ++++++++++ .../BridgeSettingsTab-test.tsx.snap | 112 ++++++++++++++++++ 3 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 test/components/views/settings/tabs/room/BridgeSettingsTab-test.tsx create mode 100644 test/components/views/settings/tabs/room/__snapshots__/BridgeSettingsTab-test.tsx.snap diff --git a/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx index 775aadf973b..bd026db3678 100644 --- a/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/AdvancedRoomSettingsTab-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React from "react"; -import { fireEvent, render, RenderResult } from "@testing-library/react"; +import { fireEvent, render, RenderResult, screen } from "@testing-library/react"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { Room } from "matrix-js-sdk/src/models/room"; import { mocked } from "jest-mock"; @@ -69,6 +69,22 @@ describe("AdvancedRoomSettingsTab", () => { tab.getByText("custom_room_version_1"); }); + it("displays message when room cannot federate", () => { + const createEvent = new MatrixEvent({ + sender: "@a:b.com", + type: EventType.RoomCreate, + content: { "m.federate": false }, + room_id: room.roomId, + state_key: "", + }); + jest.spyOn(room.currentState, "getStateEvents").mockImplementation((type) => + type === EventType.RoomCreate ? createEvent : null, + ); + + renderTab(); + expect(screen.getByText("This room is not accessible by remote Matrix servers")).toBeInTheDocument(); + }); + function mockStateEvents(room: Room) { const createEvent = mkEvent({ event: true, @@ -143,5 +159,16 @@ describe("AdvancedRoomSettingsTab", () => { metricsViaKeyboard: false, }); }); + + it("handles when room is a space", async () => { + mockStateEvents(room); + jest.spyOn(room, "isSpaceRoom").mockReturnValue(true); + + mockStateEvents(room); + const tab = renderTab(); + const link = await tab.findByText("View older version of test room."); + expect(link).toBeInTheDocument(); + expect(screen.getByText("Space information")).toBeInTheDocument(); + }); }); }); diff --git a/test/components/views/settings/tabs/room/BridgeSettingsTab-test.tsx b/test/components/views/settings/tabs/room/BridgeSettingsTab-test.tsx new file mode 100644 index 00000000000..bf41be073a2 --- /dev/null +++ b/test/components/views/settings/tabs/room/BridgeSettingsTab-test.tsx @@ -0,0 +1,60 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { render } from "@testing-library/react"; +import { MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; + +import BridgeSettingsTab from "../../../../../../src/components/views/settings/tabs/room/BridgeSettingsTab"; +import { getMockClientWithEventEmitter } from "../../../../../test-utils"; + +describe("", () => { + const userId = "@alice:server.org"; + const client = getMockClientWithEventEmitter({ + getRoom: jest.fn(), + }); + const roomId = "!room:server.org"; + + const getComponent = (room: Room) => render(); + + it("renders when room is not bridging messages to any platform", () => { + const room = new Room(roomId, client, userId); + + const { container } = getComponent(room); + + expect(container).toMatchSnapshot(); + }); + + it("renders when room is bridging messages", () => { + const bridgeEvent = new MatrixEvent({ + type: "uk.half-shot.bridge", + content: { + channel: { id: "channel-test" }, + protocol: { id: "protocol-test" }, + bridgebot: "test", + }, + room_id: roomId, + state_key: "1", + }); + const room = new Room(roomId, client, userId); + room.currentState.setStateEvents([bridgeEvent]); + client.getRoom.mockReturnValue(room); + + const { container } = getComponent(room); + + expect(container).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/settings/tabs/room/__snapshots__/BridgeSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/room/__snapshots__/BridgeSettingsTab-test.tsx.snap new file mode 100644 index 00000000000..c38e07d15f8 --- /dev/null +++ b/test/components/views/settings/tabs/room/__snapshots__/BridgeSettingsTab-test.tsx.snap @@ -0,0 +1,112 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders when room is bridging messages 1`] = ` +
      +
      +
      + Bridges +
      +
      +
      +

      + + This room is bridging messages to the following platforms. + + Learn more. + + +

      +
        +
      • +
        +
        +
        +
        +

        + protocol-test +

        +

        + + + Channel: + + channel-test + + + +

        +
          + +
        • + + This bridge is managed by + . + +
          • +
        +
        +
        • +
      +
      +
      +
      +
      +`; + +exports[` renders when room is not bridging messages to any platform 1`] = ` +
      +
      +
      + Bridges +
      +
      +

      + + This room isn't bridging messages to any platforms. + + Learn more. + + +

      +
      +
      +
      +`; From 8103e9e42ab9e97f856efb3ddf347570dd9e2b90 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 24 Apr 2023 10:45:41 +1200 Subject: [PATCH 08/13] test SecurityRoomSettingsTab --- .../room/SecurityRoomSettingsTab-test.tsx | 410 +++++++++++++++ .../SecurityRoomSettingsTab-test.tsx.snap | 476 ++++++++++++++++++ 2 files changed, 886 insertions(+) create mode 100644 test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx create mode 100644 test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap diff --git a/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx new file mode 100644 index 00000000000..15e5dccf4e7 --- /dev/null +++ b/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx @@ -0,0 +1,410 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { fireEvent, render, screen, within } from "@testing-library/react"; +import { EventType, GuestAccess, HistoryVisibility, JoinRule, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; +import { logger } from "matrix-js-sdk/src/logger"; + +import SecurityRoomSettingsTab from "../../../../../../src/components/views/settings/tabs/room/SecurityRoomSettingsTab"; +import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext"; +import SettingsStore from "../../../../../../src/settings/SettingsStore"; +import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser } from "../../../../../test-utils"; + +describe("", () => { + const userId = "@alice:server.org"; + const client = getMockClientWithEventEmitter({ + ...mockClientMethodsUser(userId), + getRoom: jest.fn(), + isRoomEncrypted: jest.fn(), + getLocalAliases: jest.fn().mockReturnValue([]), + sendStateEvent: jest.fn(), + }); + const roomId = "!room:server.org"; + + const getComponent = (room: Room, closeSettingsFn = jest.fn()) => + render(, { + wrapper: ({ children }) => ( + {children} + ), + }); + + const setRoomStateEvents = ( + room: Room, + joinRule?: JoinRule, + guestAccess?: GuestAccess, + history?: HistoryVisibility, + ): void => { + const events = [ + new MatrixEvent({ + type: EventType.RoomCreate, + content: { version: "test" }, + sender: userId, + state_key: "", + room_id: room.roomId, + }), + guestAccess && + new MatrixEvent({ + type: EventType.RoomGuestAccess, + content: { guest_access: guestAccess }, + sender: userId, + state_key: "", + room_id: room.roomId, + }), + history && + new MatrixEvent({ + type: EventType.RoomHistoryVisibility, + content: { history_visibility: history }, + sender: userId, + state_key: "", + room_id: room.roomId, + }), + joinRule && + new MatrixEvent({ + type: EventType.RoomJoinRules, + content: { join_rule: joinRule }, + sender: userId, + state_key: "", + room_id: room.roomId, + }), + ].filter(Boolean); + + room.currentState.setStateEvents(events); + }; + + beforeEach(() => { + client.sendStateEvent.mockReset().mockResolvedValue(undefined); + client.isRoomEncrypted.mockReturnValue(false); + jest.spyOn(SettingsStore, "getValue").mockRestore(); + }); + + it("uses defaults for guest access and history visibility", () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room); + + const { container } = getComponent(room); + + expect(container).toMatchSnapshot(); + }); + + describe("join rule", () => { + it("warns when trying to make an encrypted room public", async () => { + const room = new Room(roomId, client, userId); + client.isRoomEncrypted.mockReturnValue(true); + setRoomStateEvents(room, JoinRule.Invite); + + getComponent(room); + + fireEvent.click(screen.getByLabelText("Public")); + + const modal = await screen.findByRole("dialog"); + + expect(modal).toMatchSnapshot(); + + fireEvent.click(screen.getByText("Cancel")); + + // join rule not updated + expect(screen.getByLabelText("Private (invite only)").hasAttribute("checked")).toBeTruthy(); + }); + + it("updates join rule", async () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room, JoinRule.Invite); + + getComponent(room); + + fireEvent.click(screen.getByLabelText("Public")); + + await flushPromises(); + + expect(client.sendStateEvent).toHaveBeenCalledWith( + room.roomId, + EventType.RoomJoinRules, + { + join_rule: JoinRule.Public, + }, + "", + ); + }); + + it("handles error when updating join rule fails", async () => { + const room = new Room(roomId, client, userId); + client.sendStateEvent.mockRejectedValue("oups"); + setRoomStateEvents(room, JoinRule.Invite); + + getComponent(room); + + fireEvent.click(screen.getByLabelText("Public")); + + await flushPromises(); + + const dialog = await screen.findByRole("dialog"); + + expect(dialog).toMatchSnapshot(); + + fireEvent.click(within(dialog).getByText("OK")); + }); + + it("displays advanced section toggle when join rule is public", () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room, JoinRule.Public); + + getComponent(room); + + expect(screen.getByText("Show advanced")).toBeInTheDocument(); + }); + + it("does not display advanced section toggle when join rule is not public", () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room, JoinRule.Invite); + + getComponent(room); + + expect(screen.queryByText("Show advanced")).not.toBeInTheDocument(); + }); + }); + + describe("guest access", () => { + it("uses forbidden by default when room has no guest access event", () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room, JoinRule.Public); + + getComponent(room); + + fireEvent.click(screen.getByText("Show advanced")); + + expect(screen.getByLabelText("Enable guest access").getAttribute("aria-checked")).toBe("false"); + }); + + it("updates guest access on toggle", () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room, JoinRule.Public); + getComponent(room); + fireEvent.click(screen.getByText("Show advanced")); + + fireEvent.click(screen.getByLabelText("Enable guest access")); + + // toggle set immediately + expect(screen.getByLabelText("Enable guest access").getAttribute("aria-checked")).toBe("true"); + + expect(client.sendStateEvent).toHaveBeenCalledWith( + room.roomId, + EventType.RoomGuestAccess, + { guest_access: GuestAccess.CanJoin }, + "", + ); + }); + + it("logs error and resets state when updating guest access fails", async () => { + client.sendStateEvent.mockRejectedValue("oups"); + jest.spyOn(logger, "error").mockImplementation(() => {}); + const room = new Room(roomId, client, userId); + setRoomStateEvents(room, JoinRule.Public, GuestAccess.CanJoin); + getComponent(room); + fireEvent.click(screen.getByText("Show advanced")); + + fireEvent.click(screen.getByLabelText("Enable guest access")); + + // toggle set immediately + expect(screen.getByLabelText("Enable guest access").getAttribute("aria-checked")).toBe("false"); + + await flushPromises(); + expect(client.sendStateEvent).toHaveBeenCalled(); + expect(logger.error).toHaveBeenCalledWith("oups"); + + // toggle reset to old value + expect(screen.getByLabelText("Enable guest access").getAttribute("aria-checked")).toBe("true"); + }); + }); + + describe("history visibility", () => { + it("does not render section when RoomHistorySettings feature is disabled", () => { + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + const room = new Room(roomId, client, userId); + setRoomStateEvents(room); + + getComponent(room); + + expect(screen.queryByText("Who can read history")).not.toBeInTheDocument(); + }); + + it("uses shared as default history visibility when no state event found", () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room); + + getComponent(room); + + expect(screen.getByText("Who can read history?").parentElement).toMatchSnapshot(); + expect(screen.getByDisplayValue(HistoryVisibility.Shared)).toBeChecked(); + }); + + it("does not render world readable option when room is encrypted", () => { + const room = new Room(roomId, client, userId); + client.isRoomEncrypted.mockReturnValue(true); + setRoomStateEvents(room); + + getComponent(room); + + expect(screen.queryByDisplayValue(HistoryVisibility.WorldReadable)).not.toBeInTheDocument(); + }); + + it("renders world readable option when room is encrypted and history is already set to world readable", () => { + const room = new Room(roomId, client, userId); + client.isRoomEncrypted.mockReturnValue(true); + setRoomStateEvents(room, undefined, undefined, HistoryVisibility.WorldReadable); + + getComponent(room); + + expect(screen.getByDisplayValue(HistoryVisibility.WorldReadable)).toBeInTheDocument(); + }); + + it("updates history visibility", () => { + const room = new Room(roomId, client, userId); + + getComponent(room); + + fireEvent.click(screen.getByDisplayValue(HistoryVisibility.Invited)); + + // toggle updated immediately + expect(screen.getByDisplayValue(HistoryVisibility.Invited)).toBeChecked(); + + expect(client.sendStateEvent).toHaveBeenCalledWith( + room.roomId, + EventType.RoomHistoryVisibility, + { + history_visibility: HistoryVisibility.Invited, + }, + "", + ); + }); + + it("handles error when updating history visibility", async () => { + const room = new Room(roomId, client, userId); + client.sendStateEvent.mockRejectedValue("oups"); + jest.spyOn(logger, "error").mockImplementation(() => {}); + + getComponent(room); + + fireEvent.click(screen.getByDisplayValue(HistoryVisibility.Invited)); + + // toggle updated immediately + expect(screen.getByDisplayValue(HistoryVisibility.Invited)).toBeChecked(); + + await flushPromises(); + + // reset to before updated value + expect(screen.getByDisplayValue(HistoryVisibility.Shared)).toBeChecked(); + expect(logger.error).toHaveBeenCalledWith("oups"); + }); + }); + + describe("encryption", () => { + it("displays encryption as enabled", () => { + const room = new Room(roomId, client, userId); + client.isRoomEncrypted.mockReturnValue(true); + setRoomStateEvents(room); + getComponent(room); + + expect(screen.getByLabelText("Encrypted")).toBeChecked(); + // can't disable encryption once enabled + expect(screen.getByLabelText("Encrypted").getAttribute("aria-disabled")).toEqual("true"); + }); + + it("asks users to confirm when setting room to encrypted", async () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room); + getComponent(room); + + expect(screen.getByLabelText("Encrypted")).not.toBeChecked(); + + fireEvent.click(screen.getByLabelText("Encrypted")); + + const dialog = await screen.findByRole("dialog"); + + fireEvent.click(within(dialog).getByText("Cancel")); + + expect(client.sendStateEvent).not.toHaveBeenCalled(); + expect(screen.getByLabelText("Encrypted")).not.toBeChecked(); + }); + + it("enables encryption after confirmation", async () => { + const room = new Room(roomId, client, userId); + setRoomStateEvents(room); + getComponent(room); + + expect(screen.getByLabelText("Encrypted")).not.toBeChecked(); + + fireEvent.click(screen.getByLabelText("Encrypted")); + + const dialog = await screen.findByRole("dialog"); + + fireEvent.click(within(dialog).getByText("OK")); + + expect(client.sendStateEvent).toHaveBeenCalledWith(room.roomId, EventType.RoomEncryption, { + algorithm: "m.megolm.v1.aes-sha2", + }); + }); + + it("renders world readable option when room is encrypted and history is already set to world readable", () => { + const room = new Room(roomId, client, userId); + client.isRoomEncrypted.mockReturnValue(true); + setRoomStateEvents(room, undefined, undefined, HistoryVisibility.WorldReadable); + + getComponent(room); + + expect(screen.getByDisplayValue(HistoryVisibility.WorldReadable)).toBeInTheDocument(); + }); + + it("updates history visibility", () => { + const room = new Room(roomId, client, userId); + + getComponent(room); + + fireEvent.click(screen.getByDisplayValue(HistoryVisibility.Invited)); + + // toggle updated immediately + expect(screen.getByDisplayValue(HistoryVisibility.Invited)).toBeChecked(); + + expect(client.sendStateEvent).toHaveBeenCalledWith( + room.roomId, + EventType.RoomHistoryVisibility, + { + history_visibility: HistoryVisibility.Invited, + }, + "", + ); + }); + + it("handles error when updating history visibility", async () => { + const room = new Room(roomId, client, userId); + client.sendStateEvent.mockRejectedValue("oups"); + jest.spyOn(logger, "error").mockImplementation(() => {}); + + getComponent(room); + + fireEvent.click(screen.getByDisplayValue(HistoryVisibility.Invited)); + + // toggle updated immediately + expect(screen.getByDisplayValue(HistoryVisibility.Invited)).toBeChecked(); + + await flushPromises(); + + // reset to before updated value + expect(screen.getByDisplayValue(HistoryVisibility.Shared)).toBeChecked(); + expect(logger.error).toHaveBeenCalledWith("oups"); + }); + }); +}); diff --git a/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap new file mode 100644 index 00000000000..f957e15bab4 --- /dev/null +++ b/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap @@ -0,0 +1,476 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` history visibility uses shared as default history visibility when no state event found 1`] = ` +
      + + Who can read history? + +
      + Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged. +
      +
      +`; + +exports[` join rule handles error when updating join rule fails 1`] = ` + `; - -exports[` uses defaults for guest access and history visibility 1`] = ` -
      -
      -
      - Security & Privacy -
      -
      - - Encryption - -
      - Once enabled, encryption cannot be disabled. -
      -
      - - Encrypted - -
      -
      -
      -
      -
      -
      - - Access - -
      - Decide who can join !room:server.org. -
      -
      -
      - - Who can read history? - -
      - Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged. -
      -
      -
      -
      -`; From 723f0311005c6fe1c9574bad6a9623d3cbd9b535 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 24 Apr 2023 13:20:57 +1200 Subject: [PATCH 10/13] strict fixes --- .../settings/tabs/room/SecurityRoomSettingsTab-test.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx index 69fdbba07fe..559d0703dc6 100644 --- a/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx @@ -23,6 +23,7 @@ import SecurityRoomSettingsTab from "../../../../../../src/components/views/sett import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext"; import SettingsStore from "../../../../../../src/settings/SettingsStore"; import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser } from "../../../../../test-utils"; +import { filterBoolean } from "../../../../../../src/utils/arrays"; describe("", () => { const userId = "@alice:server.org"; @@ -48,7 +49,7 @@ describe("", () => { guestAccess?: GuestAccess, history?: HistoryVisibility, ): void => { - const events = [ + const events = filterBoolean([ new MatrixEvent({ type: EventType.RoomCreate, content: { version: "test" }, @@ -80,13 +81,13 @@ describe("", () => { state_key: "", room_id: room.roomId, }), - ].filter(Boolean); + ]); room.currentState.setStateEvents(events); }; beforeEach(() => { - client.sendStateEvent.mockReset().mockResolvedValue(undefined); + client.sendStateEvent.mockReset().mockResolvedValue({ event_id: 'test'}); client.isRoomEncrypted.mockReturnValue(false); jest.spyOn(SettingsStore, "getValue").mockRestore(); }); From 4235d3bc02ed74d70a0229dd47fc08a761dd4e33 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 26 Apr 2023 16:41:54 +1200 Subject: [PATCH 11/13] more tests --- .../tabs/room/RolesRoomSettingsTab-test.tsx | 58 ++++++++++++++++++- .../room/SecurityRoomSettingsTab-test.tsx | 2 +- .../RolesRoomSettingsTab-test.tsx.snap | 26 +++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap diff --git a/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx index 0806b5a0bc4..3b8ea44e77b 100644 --- a/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/RolesRoomSettingsTab-test.tsx @@ -15,12 +15,13 @@ limitations under the License. */ import React from "react"; -import { fireEvent, render, RenderResult } from "@testing-library/react"; +import { fireEvent, render, RenderResult, screen } from "@testing-library/react"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { EventType } from "matrix-js-sdk/src/@types/event"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { Room } from "matrix-js-sdk/src/models/room"; import { mocked } from "jest-mock"; +import { RoomMember } from "matrix-js-sdk/src/matrix"; import RolesRoomSettingsTab from "../../../../../../src/components/views/settings/tabs/room/RolesRoomSettingsTab"; import { mkStubRoom, stubClient } from "../../../../../test-utils"; @@ -30,12 +31,13 @@ import SettingsStore from "../../../../../../src/settings/SettingsStore"; import { ElementCall } from "../../../../../../src/models/Call"; describe("RolesRoomSettingsTab", () => { + const userId = "@alice:server.org"; const roomId = "!room:example.com"; let cli: MatrixClient; let room: Room; - const renderTab = (): RenderResult => { - return render(); + const renderTab = (propRoom: Room = room): RenderResult => { + return render(); }; const getVoiceBroadcastsSelect = (): HTMLElement => { @@ -183,4 +185,54 @@ describe("RolesRoomSettingsTab", () => { expect(getJoinCallSelectedOption(tab)).toBeFalsy(); }); }); + + describe("Banned users", () => { + it("should not render banned section when no banned users", () => { + const room = new Room(roomId, cli, userId); + renderTab(room); + + expect(screen.queryByText("Banned users")).not.toBeInTheDocument(); + }); + + it("renders banned users", () => { + const bannedMember = new RoomMember(roomId, "@bob:server.org"); + bannedMember.setMembershipEvent( + new MatrixEvent({ + type: EventType.RoomMember, + content: { + membership: "ban", + reason: "just testing", + }, + sender: userId, + }), + ); + const room = new Room(roomId, cli, userId); + jest.spyOn(room, "getMembersWithMembership").mockReturnValue([bannedMember]); + renderTab(room); + + expect(screen.getByText("Banned users").parentElement).toMatchSnapshot(); + }); + + it("uses banners display name when available", () => { + const bannedMember = new RoomMember(roomId, "@bob:server.org"); + const senderMember = new RoomMember(roomId, "@alice:server.org"); + senderMember.name = "Alice"; + bannedMember.setMembershipEvent( + new MatrixEvent({ + type: EventType.RoomMember, + content: { + membership: "ban", + reason: "just testing", + }, + sender: userId, + }), + ); + const room = new Room(roomId, cli, userId); + jest.spyOn(room, "getMembersWithMembership").mockReturnValue([bannedMember]); + jest.spyOn(room, "getMember").mockReturnValue(senderMember); + renderTab(room); + + expect(screen.getByTitle("Banned by Alice")).toBeInTheDocument(); + }); + }); }); diff --git a/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx b/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx index 559d0703dc6..e2b4e494645 100644 --- a/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx @@ -87,7 +87,7 @@ describe("", () => { }; beforeEach(() => { - client.sendStateEvent.mockReset().mockResolvedValue({ event_id: 'test'}); + client.sendStateEvent.mockReset().mockResolvedValue({ event_id: "test" }); client.isRoomEncrypted.mockReturnValue(false); jest.spyOn(SettingsStore, "getValue").mockRestore(); }); diff --git a/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap new file mode 100644 index 00000000000..d771a152d2a --- /dev/null +++ b/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap @@ -0,0 +1,26 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`RolesRoomSettingsTab Banned users renders banned users 1`] = ` +
      + + Banned users + +
        +
      • + + + @bob:server.org + + + Reason: just testing + +
        • +
      +
      +`; From 46f381fbcbd28096c2f54c7a808022fd44657dd6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 26 Apr 2023 19:18:52 +1200 Subject: [PATCH 12/13] 2% more test coverage --- .../views/dialogs/RoomSettingsDialog.tsx | 2 +- .../views/dialogs/RoomSettingsDialog-test.tsx | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/components/views/dialogs/RoomSettingsDialog.tsx b/src/components/views/dialogs/RoomSettingsDialog.tsx index bebb5340438..feff294514a 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.tsx +++ b/src/components/views/dialogs/RoomSettingsDialog.tsx @@ -79,7 +79,7 @@ class RoomSettingsDialog extends React.Component { public componentDidUpdate(): void { if (this.props.roomId !== this.state.room.roomId) { const room = this.getRoom(); - this.setState({ room }); + this.setState({ room, roomName: room.name ?? "" }); } } diff --git a/test/components/views/dialogs/RoomSettingsDialog-test.tsx b/test/components/views/dialogs/RoomSettingsDialog-test.tsx index 7754e3d0245..566358de790 100644 --- a/test/components/views/dialogs/RoomSettingsDialog-test.tsx +++ b/test/components/views/dialogs/RoomSettingsDialog-test.tsx @@ -38,13 +38,20 @@ describe("", () => { const roomId = "!room:server.org"; const room = new Room(roomId, mockClient, userId); + room.name = "Test Room"; + const room2 = new Room("!room2:server.org", mockClient, userId); + room2.name = "Another Room"; jest.spyOn(SettingsStore, "getValue"); beforeEach(() => { jest.clearAllMocks(); - mockClient.getRoom.mockImplementation((roomId) => (roomId === room.roomId ? room : null)); + mockClient.getRoom.mockImplementation((roomId) => { + if (roomId === room.roomId) return room; + if (roomId === room2.roomId) return room2; + return null; + }); jest.spyOn(SettingsStore, "getValue").mockReset().mockReturnValue(false); }); @@ -61,6 +68,16 @@ describe("", () => { expect(screen.getByText("Something went wrong!")).toBeInTheDocument(); }); + it("updates when roomId prop changes", () => { + const { rerender, getByText } = getComponent(undefined, roomId); + + expect(getByText(`Room Settings - ${room.name}`)).toBeInTheDocument(); + + rerender(); + + expect(getByText(`Room Settings - ${room2.name}`)).toBeInTheDocument(); + }); + describe("Settings tabs", () => { it("renders default tabs correctly", () => { const { container } = getComponent(); From 67c0bb23ed5533d2f7909cd239c38b2a1729c0d1 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 26 Apr 2023 21:11:23 +1200 Subject: [PATCH 13/13] remove roomName from RoomSettingsDialogs state --- src/components/views/dialogs/RoomSettingsDialog.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/views/dialogs/RoomSettingsDialog.tsx b/src/components/views/dialogs/RoomSettingsDialog.tsx index feff294514a..c070292cfa4 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.tsx +++ b/src/components/views/dialogs/RoomSettingsDialog.tsx @@ -56,7 +56,6 @@ interface IProps { } interface IState { - roomName: string; room: Room; } @@ -67,7 +66,7 @@ class RoomSettingsDialog extends React.Component { super(props); const room = this.getRoom(); - this.state = { roomName: "", room }; + this.state = { room }; } public componentDidMount(): void { @@ -79,7 +78,7 @@ class RoomSettingsDialog extends React.Component { public componentDidUpdate(): void { if (this.props.roomId !== this.state.room.roomId) { const room = this.getRoom(); - this.setState({ room, roomName: room.name ?? "" }); + this.setState({ room }); } } @@ -115,9 +114,8 @@ class RoomSettingsDialog extends React.Component { }; private onRoomName = (): void => { - this.setState({ - roomName: this.state.room.name ?? "", - }); + // rerender when the room name changes + this.forceUpdate(); }; private getTabs(): NonEmptyArray { @@ -217,7 +215,7 @@ class RoomSettingsDialog extends React.Component { } public render(): React.ReactNode { - const roomName = this.state.roomName; + const roomName = this.state.room.name; return (