Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Bail out of RoomSettingsDialog when room is not found #10662

Merged
merged 20 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 48 additions & 17 deletions src/components/views/dialogs/RoomSettingsDialog.tsx
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -16,7 +18,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";
Expand All @@ -36,6 +38,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";
Expand All @@ -54,14 +57,17 @@ interface IProps {

interface IState {
roomName: string;
room: Room;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the changes to this file. If we're storing the room in state though, couldn't we also remove the roomName from state and read this.state.room.name wherever this.state.roomName is currently used?

I think the onRoomName method could possibly disappear then too

}

export default class RoomSettingsDialog extends React.Component<IProps, IState> {
class RoomSettingsDialog extends React.Component<IProps, IState> {
private dispatcherRef: string;

public constructor(props: IProps) {
super(props);
this.state = { roomName: "" };

const room = this.getRoom();
this.state = { roomName: "", room };
}

public componentDidMount(): void {
Expand All @@ -70,6 +76,13 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
this.onRoomName();
}

public componentDidUpdate(): void {
if (this.props.roomId !== this.state.room.roomId) {
const room = this.getRoom();
this.setState({ room });
}
}

public componentWillUnmount(): void {
if (this.dispatcherRef) {
dis.unregister(this.dispatcherRef);
Expand All @@ -78,6 +91,21 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
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
Expand All @@ -88,7 +116,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>

private onRoomName = (): void => {
this.setState({
roomName: MatrixClientPeg.get().getRoom(this.props.roomId)?.name ?? "",
roomName: this.state.room.name ?? "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be overtaken by my other comment in this file, but could we omit the ?? "" here? It looks to me like a Room always has a .name string

});
};

Expand All @@ -100,7 +128,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
ROOM_GENERAL_TAB,
_td("General"),
"mx_RoomSettingsDialog_settingsIcon",
<GeneralRoomSettingsTab roomId={this.props.roomId} />,
<GeneralRoomSettingsTab room={this.state.room} />,
"RoomSettingsGeneral",
),
);
Expand All @@ -110,7 +138,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
ROOM_VOIP_TAB,
_td("Voice & Video"),
"mx_RoomSettingsDialog_voiceIcon",
<VoipRoomSettingsTab roomId={this.props.roomId} />,
<VoipRoomSettingsTab room={this.state.room} />,
),
);
}
Expand All @@ -119,12 +147,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
ROOM_SECURITY_TAB,
_td("Security & Privacy"),
"mx_RoomSettingsDialog_securityIcon",
(
<SecurityRoomSettingsTab
roomId={this.props.roomId}
closeSettingsFn={() => this.props.onFinished(true)}
/>
),
<SecurityRoomSettingsTab room={this.state.room} closeSettingsFn={() => this.props.onFinished(true)} />,
"RoomSettingsSecurityPrivacy",
),
);
Expand All @@ -133,7 +156,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
ROOM_ROLES_TAB,
_td("Roles & Permissions"),
"mx_RoomSettingsDialog_rolesIcon",
<RolesRoomSettingsTab roomId={this.props.roomId} />,
<RolesRoomSettingsTab room={this.state.room} />,
"RoomSettingsRolesPermissions",
),
);
Expand All @@ -144,7 +167,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
"mx_RoomSettingsDialog_notificationsIcon",
(
<NotificationSettingsTab
roomId={this.props.roomId}
roomId={this.state.room.roomId}
closeSettingsFn={() => this.props.onFinished(true)}
/>
),
Expand All @@ -158,7 +181,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
ROOM_BRIDGES_TAB,
_td("Bridges"),
"mx_RoomSettingsDialog_bridgesIcon",
<BridgeSettingsTab roomId={this.props.roomId} />,
<BridgeSettingsTab room={this.state.room} />,
"RoomSettingsBridges",
),
);
Expand All @@ -169,7 +192,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
ROOM_POLL_HISTORY_TAB,
_td("Poll history"),
"mx_RoomSettingsDialog_pollsIcon",
<PollHistoryTab roomId={this.props.roomId} onFinished={() => this.props.onFinished(true)} />,
<PollHistoryTab room={this.state.room} onFinished={() => this.props.onFinished(true)} />,
),
);

Expand All @@ -181,7 +204,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
"mx_RoomSettingsDialog_warningIcon",
(
<AdvancedRoomSettingsTab
roomId={this.props.roomId}
room={this.state.room}
closeSettingsFn={() => this.props.onFinished(true)}
/>
),
Expand Down Expand Up @@ -213,3 +236,11 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
);
}
}

const WrappedRoomSettingsDialog: React.FC<IProps> = (props) => (
<ErrorBoundary>
<RoomSettingsDialog {...props} />
</ErrorBoundary>
);

export default WrappedRoomSettingsDialog;
4 changes: 2 additions & 2 deletions src/components/views/dialogs/SpaceSettingsDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ const SpaceSettingsDialog: React.FC<IProps> = ({ matrixClient: cli, space, onFin
SpaceSettingsTab.Roles,
_td("Roles & Permissions"),
"mx_RoomSettingsDialog_rolesIcon",
<RolesRoomSettingsTab roomId={space.roomId} />,
<RolesRoomSettingsTab room={space} />,
),
SettingsStore.getValue(UIFeature.AdvancedSettings)
? new Tab(
SpaceSettingsTab.Advanced,
_td("Advanced"),
"mx_RoomSettingsDialog_warningIcon",
<AdvancedRoomSettingsTab roomId={space.roomId} closeSettingsFn={onFinished} />,
<AdvancedRoomSettingsTab room={space} closeSettingsFn={onFinished} />,
)
: null,
].filter(Boolean) as NonEmptyArray<Tab>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -29,7 +29,7 @@ import { ViewRoomPayload } from "../../../../../dispatcher/payloads/ViewRoomPayl
import SettingsStore from "../../../../../settings/SettingsStore";

interface IProps {
roomId: string;
room: Room;
closeSettingsFn(): void;
}

Expand Down Expand Up @@ -64,8 +64,8 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
this.state = {};

// we handle lack of this object gracefully later, so don't worry about it failing here.
const room = MatrixClientPeg.get().getRoom(this.props.roomId);
room?.getRecommendedVersion().then((v) => {
const room = this.props.room;
room.getRecommendedVersion().then((v) => {
const tombstone = room.currentState.getStateEvents(EventType.RoomTombstone, "");

const additionalStateChanges: Partial<IState> = {};
Expand All @@ -85,8 +85,7 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
}

private upgradeRoom = (): void => {
const room = MatrixClientPeg.get().getRoom(this.props.roomId);
if (room) Modal.createDialog(RoomUpgradeDialog, { room });
Modal.createDialog(RoomUpgradeDialog, { room: this.props.room });
};

private onOldRoomClicked = (e: ButtonEvent): void => {
Expand All @@ -105,12 +104,11 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
};

public render(): React.ReactNode {
const client = MatrixClientPeg.get();
const room = client.getRoom(this.props.roomId);
const isSpace = room?.isSpaceRoom();
const room = this.props.room;
const isSpace = room.isSpaceRoom();

let unfederatableSection: JSX.Element | undefined;
if (room?.currentState.getStateEvents(EventType.RoomCreate, "")?.getContent()["m.federate"] === false) {
if (room.currentState.getStateEvents(EventType.RoomCreate, "")?.getContent()["m.federate"] === false) {
unfederatableSection = <div>{_t("This room is not accessible by remote Matrix servers")}</div>;
}

Expand Down Expand Up @@ -143,9 +141,9 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
if (this.state.oldRoomId) {
let copy: string;
if (isSpace) {
copy = _t("View older version of %(spaceName)s.", { spaceName: room?.name ?? this.state.oldRoomId });
copy = _t("View older version of %(spaceName)s.", { spaceName: room.name ?? this.state.oldRoomId });
} else {
copy = _t("View older messages in %(roomName)s.", { roomName: room?.name ?? this.state.oldRoomId });
copy = _t("View older messages in %(roomName)s.", { roomName: room.name ?? this.state.oldRoomId });
}

oldRoomLink = (
Expand All @@ -160,19 +158,21 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
<div className="mx_SettingsTab_heading">{_t("Advanced")}</div>
<div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
<span className="mx_SettingsTab_subheading">
{room?.isSpaceRoom() ? _t("Space information") : _t("Room information")}
{room.isSpaceRoom() ? _t("Space information") : _t("Room information")}
</span>
<div>
<span>{_t("Internal room ID")}</span>
<CopyableText getTextToCopy={() => this.props.roomId}>{this.props.roomId}</CopyableText>
<CopyableText getTextToCopy={() => this.props.room.roomId}>
{this.props.room.roomId}
</CopyableText>
</div>
{unfederatableSection}
</div>
<div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
<span className="mx_SettingsTab_subheading">{_t("Room version")}</span>
<div>
<span>{_t("Room version:")}</span>&nbsp;
{room?.getVersion()}
{room.getVersion()}
</div>
{oldRoomLink}
{roomUpgradeButton}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<IProps> {
Expand All @@ -51,9 +51,8 @@ export default class BridgeSettingsTab extends React.Component<IProps> {
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -28,7 +29,7 @@ import AliasSettings from "../../../room_settings/AliasSettings";
import PosthogTrackers from "../../../../../PosthogTrackers";

interface IProps {
roomId: string;
room: Room;
}

interface IState {
Expand All @@ -50,25 +51,26 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
private onLeaveClick = (ev: ButtonEvent): void => {
dis.dispatch({
action: "leave_room",
room_id: this.props.roomId,
room_id: this.props.room.roomId,
});

PosthogTrackers.trackInteraction("WebRoomSettingsLeaveButton", ev);
};

public render(): React.ReactNode {
const client = this.context;
const room = client.getRoom(this.props.roomId);
const room = this.props.room;

const canSetAliases = true; // Previously, we arbitrarily only allowed admins to do this
const canSetCanonical = room?.currentState.mayClientSendStateEvent("m.room.canonical_alias", client);
const canonicalAliasEv = room?.currentState.getStateEvents("m.room.canonical_alias", "") ?? undefined;
const canSetCanonical = room.currentState.mayClientSendStateEvent("m.room.canonical_alias", client);
const canonicalAliasEv = room.currentState.getStateEvents("m.room.canonical_alias", "") ?? undefined;

const urlPreviewSettings =
room && SettingsStore.getValue(UIFeature.URLPreviews) ? <UrlPreviewSettings room={room} /> : null;
const urlPreviewSettings = SettingsStore.getValue(UIFeature.URLPreviews) ? (
<UrlPreviewSettings room={room} />
) : null;

let leaveSection;
if (room?.getMyMembership() === "join") {
if (room.getMyMembership() === "join") {
leaveSection = (
<>
<span className="mx_SettingsTab_subheading">{_t("Leave room")}</span>
Expand All @@ -85,12 +87,12 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
<div className="mx_SettingsTab mx_GeneralRoomSettingsTab">
<div className="mx_SettingsTab_heading">{_t("General")}</div>
<div className="mx_SettingsTab_section mx_GeneralRoomSettingsTab_profileSection">
<RoomProfileSettings roomId={this.props.roomId} />
<RoomProfileSettings roomId={room.roomId} />
</div>

<div className="mx_SettingsTab_heading">{_t("Room Addresses")}</div>
<AliasSettings
roomId={this.props.roomId}
roomId={room.roomId}
canSetCanonicalAlias={canSetCanonical}
canSetAliases={canSetAliases}
canonicalAliasEvent={canonicalAliasEv}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default class NotificationsSettingsTab extends React.Component<IProps, IS
public constructor(props: IProps, context: React.ContextType<typeof MatrixClientContext>) {
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);
Expand Down
Loading