Fix some features not being configurable via features (matrix-org#10276)

t3chguy · web-flow · commit ad8d27d2b2bc · 2023-03-03T13:31:51.000Z
diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts
@@ -40,6 +40,7 @@ import { SlidingSyncManager } from "./SlidingSyncManager";
 import CryptoStoreTooNewDialog from "./components/views/dialogs/CryptoStoreTooNewDialog";
 import { _t } from "./languageHandler";
 import { SettingLevel } from "./settings/SettingLevel";
+import MatrixClientBackedController from "./settings/controllers/MatrixClientBackedController";
 
 export interface IMatrixClientCreds {
     homeserverUrl: string;
@@ -237,6 +238,7 @@ class MatrixClientPegClass implements IMatrixClientPeg {
         // Connect the matrix client to the dispatcher and setting handlers
         MatrixActionCreators.start(this.matrixClient);
         MatrixClientBackedSettingsHandler.matrixClient = this.matrixClient;
+        MatrixClientBackedController.matrixClient = this.matrixClient;
 
         return opts;
     }
diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.tsx b/src/components/views/settings/tabs/user/LabsUserSettingsTab.tsx
@@ -23,60 +23,70 @@ import { SettingLevel } from "../../../../../settings/SettingLevel";
 import SdkConfig from "../../../../../SdkConfig";
 import BetaCard from "../../../beta/BetaCard";
 import SettingsFlag from "../../../elements/SettingsFlag";
-import { MatrixClientPeg } from "../../../../../MatrixClientPeg";
-import { LabGroup, labGroupNames } from "../../../../../settings/Settings";
+import { defaultWatchManager, LabGroup, labGroupNames } from "../../../../../settings/Settings";
 import { EnhancedMap } from "../../../../../utils/maps";
+import { arrayHasDiff } from "../../../../../utils/arrays";
 
-interface IState {
-    showJumpToDate: boolean;
-    showExploringPublicSpaces: boolean;
+interface State {
+    labs: string[];
+    betas: string[];
 }
 
-export default class LabsUserSettingsTab extends React.Component<{}, IState> {
+export default class LabsUserSettingsTab extends React.Component<{}, State> {
+    private readonly features = SettingsStore.getFeatureSettingNames();
+
     public constructor(props: {}) {
         super(props);
 
-        const cli = MatrixClientPeg.get();
-
-        cli.doesServerSupportUnstableFeature("org.matrix.msc3030").then((showJumpToDate) => {
-            this.setState({ showJumpToDate });
-        });
+        this.state = {
+            betas: [],
+            labs: [],
+        };
+    }
 
-        cli.doesServerSupportUnstableFeature("org.matrix.msc3827.stable").then((showExploringPublicSpaces) => {
-            this.setState({ showExploringPublicSpaces });
+    public componentDidMount(): void {
+        this.features.forEach((feature) => {
+            defaultWatchManager.watchSetting(feature, null, this.onChange);
         });
+        this.onChange();
+    }
 
-        this.state = {
-            showJumpToDate: false,
-            showExploringPublicSpaces: false,
-        };
+    public componentWillUnmount(): void {
+        defaultWatchManager.unwatchSetting(this.onChange);
     }
 
-    public render(): React.ReactNode {
-        const features = SettingsStore.getFeatureSettingNames();
-        const [labs, betas] = features.reduce(
+    private onChange = (): void => {
+        const features = SettingsStore.getFeatureSettingNames().filter((f) => SettingsStore.isEnabled(f));
+        const [_labs, betas] = features.reduce(
             (arr, f) => {
                 arr[SettingsStore.getBetaInfo(f) ? 1 : 0].push(f);
                 return arr;
             },
             [[], []] as [string[], string[]],
         );
 
-        let betaSection;
-        if (betas.length) {
+        const labs = SdkConfig.get("show_labs_settings") ? _labs : [];
+        if (arrayHasDiff(labs, this.state.labs) || arrayHasDiff(betas, this.state.betas)) {
+            this.setState({ labs, betas });
+        }
+    };
+
+    public render(): React.ReactNode {
+        let betaSection: JSX.Element | undefined;
+        if (this.state.betas.length) {
             betaSection = (
                 <div data-testid="labs-beta-section" className="mx_SettingsTab_section">
-                    {betas.map((f) => (
+                    {this.state.betas.map((f) => (
                         <BetaCard key={f} featureId={f} />
                     ))}
                 </div>
             );
         }
 
-        let labsSections;
-        if (SdkConfig.get("show_labs_settings")) {
+        let labsSections: JSX.Element | undefined;
+        if (this.state.labs.length) {
             const groups = new EnhancedMap<LabGroup, JSX.Element[]>();
-            labs.forEach((f) => {
+            this.state.labs.forEach((f) => {
                 groups
                     .getOrCreate(SettingsStore.getLabGroup(f), [])
                     .push(<SettingsFlag level={SettingLevel.DEVICE} name={f} key={f} />);
@@ -101,30 +111,6 @@ export default class LabsUserSettingsTab extends React.Component<{}, IState> {
                     />,
                 );
 
-            if (this.state.showJumpToDate) {
-                groups
-                    .getOrCreate(LabGroup.Messaging, [])
-                    .push(
-                        <SettingsFlag
-                            key="feature_jump_to_date"
-                            name="feature_jump_to_date"
-                            level={SettingLevel.DEVICE}
-                        />,
-                    );
-            }
-
-            if (this.state.showExploringPublicSpaces) {
-                groups
-                    .getOrCreate(LabGroup.Spaces, [])
-                    .push(
-                        <SettingsFlag
-                            key="feature_exploring_public_spaces"
-                            name="feature_exploring_public_spaces"
-                            level={SettingLevel.DEVICE}
-                        />,
-                    );
-            }
-
             labsSections = (
                 <>
                     {sortBy(Array.from(groups.entries()), "0").map(([group, flags]) => (
diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx
@@ -44,6 +44,10 @@ import SdkConfig from "../SdkConfig";
 import SlidingSyncController from "./controllers/SlidingSyncController";
 import { FontWatcher } from "./watchers/FontWatcher";
 import RustCryptoSdkController from "./controllers/RustCryptoSdkController";
+import ServerSupportUnstableFeatureController from "./controllers/ServerSupportUnstableFeatureController";
+import { WatchManager } from "./WatchManager";
+
+export const defaultWatchManager = new WatchManager();
 
 // These are just a bunch of helper arrays to avoid copy/pasting a bunch of times
 const LEVELS_ROOM_SETTINGS = [
@@ -218,9 +222,14 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         },
     },
     "feature_exploring_public_spaces": {
+        isFeature: true,
+        labsGroup: LabGroup.Spaces,
         displayName: _td("Explore public spaces in the new search dialog"),
         supportedLevels: LEVELS_FEATURE,
         default: false,
+        controller: new ServerSupportUnstableFeatureController("feature_exploring_public_spaces", defaultWatchManager, [
+            "org.matrix.msc3827.stable",
+        ]),
     },
     "feature_msc3531_hide_messages_pending_moderation": {
         isFeature: true,
@@ -359,13 +368,14 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         default: false,
     },
     "feature_jump_to_date": {
-        // We purposely leave out `isFeature: true` so it doesn't show in Labs
-        // by default. We will conditionally show it depending on whether we can
-        // detect MSC3030 support (see LabUserSettingsTab.tsx).
-        // labsGroup: LabGroup.Messaging,
+        isFeature: true,
+        labsGroup: LabGroup.Messaging,
         displayName: _td("Jump to date (adds /jumptodate and jump to date headers)"),
         supportedLevels: LEVELS_FEATURE,
         default: false,
+        controller: new ServerSupportUnstableFeatureController("feature_jump_to_date", defaultWatchManager, [
+            "org.matrix.msc3030",
+        ]),
     },
     "RoomList.backgroundImage": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
@@ -387,6 +397,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         controller: new SlidingSyncController(),
     },
     "feature_sliding_sync_proxy_url": {
+        // This is not a distinct feature, it is a setting for feature_sliding_sync above
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: "",
     },
diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts
@@ -27,9 +27,9 @@ import RoomSettingsHandler from "./handlers/RoomSettingsHandler";
 import ConfigSettingsHandler from "./handlers/ConfigSettingsHandler";
 import { _t } from "../languageHandler";
 import dis from "../dispatcher/dispatcher";
-import { IFeature, ISetting, LabGroup, SETTINGS } from "./Settings";
+import { IFeature, ISetting, LabGroup, SETTINGS, defaultWatchManager } from "./Settings";
 import LocalEchoWrapper from "./handlers/LocalEchoWrapper";
-import { CallbackFn as WatchCallbackFn, WatchManager } from "./WatchManager";
+import { CallbackFn as WatchCallbackFn } from "./WatchManager";
 import { SettingLevel } from "./SettingLevel";
 import SettingsHandler from "./handlers/SettingsHandler";
 import { SettingUpdatedPayload } from "../dispatcher/payloads/SettingUpdatedPayload";
@@ -39,8 +39,6 @@ import dispatcher from "../dispatcher/dispatcher";
 import { ActionPayload } from "../dispatcher/payloads";
 import { MatrixClientPeg } from "../MatrixClientPeg";
 
-const defaultWatchManager = new WatchManager();
-
 // Convert the settings to easier to manage objects for the handlers
 const defaultSettings: Record<string, any> = {};
 const invertedDefaultSettings: Record<string, boolean> = {};
diff --git a/src/settings/WatchManager.ts b/src/settings/WatchManager.ts
@@ -39,7 +39,7 @@ export class WatchManager {
     public unwatchSetting(cb: CallbackFn): void {
         this.watchers.forEach((map) => {
             map.forEach((callbacks) => {
-                let idx;
+                let idx: number;
                 while ((idx = callbacks.indexOf(cb)) !== -1) {
                     callbacks.splice(idx, 1);
                 }
diff --git a/src/settings/controllers/MatrixClientBackedController.ts b/src/settings/controllers/MatrixClientBackedController.ts
@@ -0,0 +1,52 @@
+/*
+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 { MatrixClient } from "matrix-js-sdk/src/client";
+
+import SettingController from "./SettingController";
+
+// Dev note: This whole class exists in the event someone logs out and back in - we want
+// to make sure the right MatrixClient is listening for changes.
+
+/**
+ * Represents the base class for settings controllers which need access to a MatrixClient.
+ * This class performs no logic and should be overridden.
+ */
+export default abstract class MatrixClientBackedController extends SettingController {
+    private static _matrixClient: MatrixClient;
+    private static instances: MatrixClientBackedController[] = [];
+
+    public static set matrixClient(client: MatrixClient) {
+        const oldClient = MatrixClientBackedController._matrixClient;
+        MatrixClientBackedController._matrixClient = client;
+
+        for (const instance of MatrixClientBackedController.instances) {
+            instance.initMatrixClient(oldClient, client);
+        }
+    }
+
+    protected constructor() {
+        super();
+
+        MatrixClientBackedController.instances.push(this);
+    }
+
+    public get client(): MatrixClient {
+        return MatrixClientBackedController._matrixClient;
+    }
+
+    protected abstract initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient): void;
+}
diff --git a/src/settings/controllers/ServerSupportUnstableFeatureController.ts b/src/settings/controllers/ServerSupportUnstableFeatureController.ts
@@ -0,0 +1,77 @@
+/*
+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 { MatrixClient } from "matrix-js-sdk/src/matrix";
+
+import { SettingLevel } from "../SettingLevel";
+import MatrixClientBackedController from "./MatrixClientBackedController";
+import { WatchManager } from "../WatchManager";
+import SettingsStore from "../SettingsStore";
+
+/**
+ * Disables a given setting if the server unstable feature it requires is not supported
+ * When a setting gets disabled or enabled from this controller it notifies the given WatchManager
+ */
+export default class ServerSupportUnstableFeatureController extends MatrixClientBackedController {
+    private enabled: boolean | undefined;
+
+    public constructor(
+        private readonly settingName: string,
+        private readonly watchers: WatchManager,
+        private readonly unstableFeatures: string[],
+        private readonly forcedValue: any = false,
+    ) {
+        super();
+    }
+
+    public get disabled(): boolean {
+        return !this.enabled;
+    }
+
+    public set disabled(v: boolean) {
+        if (!v === this.enabled) return;
+        this.enabled = !v;
+        const level = SettingsStore.firstSupportedLevel(this.settingName);
+        const settingValue = SettingsStore.getValue(this.settingName, null);
+        this.watchers.notifyUpdate(this.settingName, null, level, settingValue);
+    }
+
+    protected async initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient): Promise<void> {
+        this.disabled = true;
+        let supported = true;
+        for (const feature of this.unstableFeatures) {
+            supported = await this.client.doesServerSupportUnstableFeature(feature);
+            if (!supported) break;
+        }
+        this.disabled = !supported;
+    }
+
+    public getValueOverride(
+        level: SettingLevel,
+        roomId: string,
+        calculatedValue: any,
+        calculatedAtLevel: SettingLevel | null,
+    ): any {
+        if (this.settingDisabled) {
+            return this.forcedValue;
+        }
+        return null; // no override
+    }
+
+    public get settingDisabled(): boolean {
+        return this.disabled;
+    }
+}
diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts
@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import { logger } from "matrix-js-sdk/src/logger";
+import fetchMockJest from "fetch-mock-jest";
 
 import { advanceDateAndTime, stubClient } from "./test-utils";
 import { IMatrixClientPeg, MatrixClientPeg as peg } from "../src/MatrixClientPeg";
@@ -68,6 +69,7 @@ describe("MatrixClientPeg", () => {
             // instantiate a MatrixClientPegClass instance, with a new MatrixClient
             const PegClass = Object.getPrototypeOf(peg).constructor;
             testPeg = new PegClass();
+            fetchMockJest.get("http://example.com/_matrix/client/versions", {});
             testPeg.replaceUsingCreds({
                 accessToken: "SEKRET",
                 homeserverUrl: "http://example.com",
diff --git a/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx
diff --git a/test/settings/controllers/ServerSupportUnstableFeatureController-test.ts b/test/settings/controllers/ServerSupportUnstableFeatureController-test.ts
diff --git a/test/setup/setupManualMocks.ts b/test/setup/setupManualMocks.ts
diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ export class WatchManager {`
`39`	`39`	`public unwatchSetting(cb: CallbackFn): void {`
`40`	`40`	`this.watchers.forEach((map) => {`
`41`	`41`	`map.forEach((callbacks) => {`
`42`		`- let idx;`
	`42`	`+ let idx: number;`
`43`	`43`	`while ((idx = callbacks.indexOf(cb)) !== -1) {`
`44`	`44`	`callbacks.splice(idx, 1);`
`45`	`45`	`}`