From ad8d27d2b2bc534c0a5f9fd862d9975768ad40a4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 3 Mar 2023 13:31:51 +0000 Subject: [PATCH] Fix some features not being configurable via `features` (#10276) --- src/MatrixClientPeg.ts | 2 + .../tabs/user/LabsUserSettingsTab.tsx | 88 ++++++------- src/settings/Settings.tsx | 19 ++- src/settings/SettingsStore.ts | 6 +- src/settings/WatchManager.ts | 2 +- .../MatrixClientBackedController.ts | 52 ++++++++ .../ServerSupportUnstableFeatureController.ts | 77 ++++++++++++ test/MatrixClientPeg-test.ts | 2 + .../tabs/user/LabsUserSettingsTab-test.tsx | 21 +++- ...erSupportUnstableFeatureController-test.ts | 119 ++++++++++++++++++ test/setup/setupManualMocks.ts | 1 + test/test-utils/test-utils.ts | 1 + 12 files changed, 329 insertions(+), 61 deletions(-) create mode 100644 src/settings/controllers/MatrixClientBackedController.ts create mode 100644 src/settings/controllers/ServerSupportUnstableFeatureController.ts create mode 100644 test/settings/controllers/ServerSupportUnstableFeatureController-test.ts diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index f9070fb598..13c75f0ba3 100644 --- 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 index e4380f2aa6..889b1e4387 100644 --- a/src/components/views/settings/tabs/user/LabsUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/LabsUserSettingsTab.tsx @@ -23,38 +23,41 @@ 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 }); - }); - - cli.doesServerSupportUnstableFeature("org.matrix.msc3827.stable").then((showExploringPublicSpaces) => { - this.setState({ showExploringPublicSpaces }); - }); - this.state = { - showJumpToDate: false, - showExploringPublicSpaces: false, + betas: [], + labs: [], }; } - public render(): React.ReactNode { - const features = SettingsStore.getFeatureSettingNames(); - const [labs, betas] = features.reduce( + public componentDidMount(): void { + this.features.forEach((feature) => { + defaultWatchManager.watchSetting(feature, null, this.onChange); + }); + this.onChange(); + } + + public componentWillUnmount(): void { + defaultWatchManager.unwatchSetting(this.onChange); + } + + 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; @@ -62,21 +65,28 @@ export default class LabsUserSettingsTab extends React.Component<{}, IState> { [[], []] 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 = (
- {betas.map((f) => ( + {this.state.betas.map((f) => ( ))}
); } - let labsSections; - if (SdkConfig.get("show_labs_settings")) { + let labsSections: JSX.Element | undefined; + if (this.state.labs.length) { const groups = new EnhancedMap(); - labs.forEach((f) => { + this.state.labs.forEach((f) => { groups .getOrCreate(SettingsStore.getLabGroup(f), []) .push(); @@ -101,30 +111,6 @@ export default class LabsUserSettingsTab extends React.Component<{}, IState> { />, ); - if (this.state.showJumpToDate) { - groups - .getOrCreate(LabGroup.Messaging, []) - .push( - , - ); - } - - if (this.state.showExploringPublicSpaces) { - groups - .getOrCreate(LabGroup.Spaces, []) - .push( - , - ); - } - labsSections = ( <> {sortBy(Array.from(groups.entries()), "0").map(([group, flags]) => ( diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 83a9cfcc99..b8d69f6c26 100644 --- 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 index 962078ec7e..d03f45bcc5 100644 --- 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 = {}; const invertedDefaultSettings: Record = {}; diff --git a/src/settings/WatchManager.ts b/src/settings/WatchManager.ts index 0e3cae8435..a917d513b5 100644 --- 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 new file mode 100644 index 0000000000..6a68e729d0 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e50b673d82 --- /dev/null +++ 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 { + 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 index 46b9757ad9..5608b803d6 100644 --- 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 index 7fa008d4a3..7bd5062cc4 100644 --- a/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx @@ -16,6 +16,7 @@ limitations under the License. import React from "react"; import { render } from "@testing-library/react"; +import { defer } from "matrix-js-sdk/src/utils"; import LabsUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/LabsUserSettingsTab"; import SettingsStore from "../../../../../../src/settings/SettingsStore"; @@ -25,6 +26,7 @@ import { mockClientMethodsUser, } from "../../../../../test-utils"; import SdkConfig from "../../../../../../src/SdkConfig"; +import MatrixClientBackedController from "../../../../../../src/settings/controllers/MatrixClientBackedController"; describe("", () => { const sdkConfigSpy = jest.spyOn(SdkConfig, "get"); @@ -35,7 +37,7 @@ describe("", () => { const getComponent = () => ; const userId = "@alice:server.org"; - getMockClientWithEventEmitter({ + const cli = getMockClientWithEventEmitter({ ...mockClientMethodsUser(userId), ...mockClientMethodsServer(), }); @@ -70,4 +72,21 @@ describe("", () => { const labsSections = container.getElementsByClassName("mx_SettingsTab_section"); expect(labsSections.length).toEqual(11); }); + + it("renders a labs flag which requires unstable support once support is confirmed", async () => { + // enable labs + sdkConfigSpy.mockImplementation((configName) => configName === "show_labs_settings"); + + const deferred = defer(); + cli.doesServerSupportUnstableFeature.mockImplementation(async (featureName) => { + return featureName === "org.matrix.msc3827.stable" ? deferred.promise : false; + }); + MatrixClientBackedController.matrixClient = cli; + + const { queryByText, findByText } = render(getComponent()); + + expect(queryByText("Explore public spaces in the new search dialog")).toBeFalsy(); + deferred.resolve(true); + await expect(findByText("Explore public spaces in the new search dialog")).resolves.toBeDefined(); + }); }); diff --git a/test/settings/controllers/ServerSupportUnstableFeatureController-test.ts b/test/settings/controllers/ServerSupportUnstableFeatureController-test.ts new file mode 100644 index 0000000000..91f6083da6 --- /dev/null +++ b/test/settings/controllers/ServerSupportUnstableFeatureController-test.ts @@ -0,0 +1,119 @@ +/* +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 { defer } from "matrix-js-sdk/src/utils"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; + +import ServerSupportUnstableFeatureController from "../../../src/settings/controllers/ServerSupportUnstableFeatureController"; +import { SettingLevel } from "../../../src/settings/SettingLevel"; +import { LabGroup, SETTINGS } from "../../../src/settings/Settings"; +import { stubClient } from "../../test-utils"; +import { WatchManager } from "../../../src/settings/WatchManager"; +import MatrixClientBackedController from "../../../src/settings/controllers/MatrixClientBackedController"; + +describe("ServerSupportUnstableFeatureController", () => { + const watchers = new WatchManager(); + const setting = "setting_name"; + + async function prepareSetting( + cli: MatrixClient, + controller: ServerSupportUnstableFeatureController, + ): Promise { + SETTINGS[setting] = { + isFeature: true, + labsGroup: LabGroup.Messaging, + displayName: "name of some kind", + supportedLevels: [SettingLevel.DEVICE, SettingLevel.CONFIG], + default: false, + controller, + }; + + const deferred = defer(); + watchers.watchSetting(setting, null, deferred.resolve); + MatrixClientBackedController.matrixClient = cli; + await deferred.promise; + } + + describe("getValueOverride()", () => { + it("should return forced value is setting is disabled", async () => { + const cli = stubClient(); + cli.doesServerSupportUnstableFeature = jest.fn(async () => false); + + const controller = new ServerSupportUnstableFeatureController( + setting, + watchers, + ["feature"], + "other_value", + ); + await prepareSetting(cli, controller); + + expect(controller.getValueOverride(SettingLevel.DEVICE, null, true, SettingLevel.ACCOUNT)).toEqual( + "other_value", + ); + }); + + it("should pass through to the handler if setting is not disabled", async () => { + const cli = stubClient(); + cli.doesServerSupportUnstableFeature = jest.fn(async () => true); + + const controller = new ServerSupportUnstableFeatureController( + setting, + watchers, + ["feature"], + "other_value", + ); + await prepareSetting(cli, controller); + + expect(controller.getValueOverride(SettingLevel.DEVICE, null, true, SettingLevel.ACCOUNT)).toEqual(null); + }); + }); + + describe("settingDisabled()", () => { + it("returns true if there is no matrix client", () => { + const controller = new ServerSupportUnstableFeatureController(setting, watchers, ["org.matrix.msc3030"]); + expect(controller.settingDisabled).toEqual(true); + }); + + it("returns true if not all required features are supported", async () => { + const cli = stubClient(); + cli.doesServerSupportUnstableFeature = jest.fn(async (featureName) => { + return featureName === "org.matrix.msc3827.stable"; + }); + + const controller = new ServerSupportUnstableFeatureController(setting, watchers, [ + "org.matrix.msc3827.stable", + "org.matrix.msc3030", + ]); + await prepareSetting(cli, controller); + + expect(controller.settingDisabled).toEqual(true); + }); + + it("returns false if all required features are supported", async () => { + const cli = stubClient(); + cli.doesServerSupportUnstableFeature = jest.fn(async (featureName) => { + return featureName === "org.matrix.msc3827.stable" || featureName === "org.matrix.msc3030"; + }); + const controller = new ServerSupportUnstableFeatureController(setting, watchers, [ + "org.matrix.msc3827.stable", + "org.matrix.msc3030", + ]); + await prepareSetting(cli, controller); + + expect(controller.settingDisabled).toEqual(false); + }); + }); +}); diff --git a/test/setup/setupManualMocks.ts b/test/setup/setupManualMocks.ts index b0cac01cbd..2d4d453256 100644 --- a/test/setup/setupManualMocks.ts +++ b/test/setup/setupManualMocks.ts @@ -85,6 +85,7 @@ window.HTMLElement.prototype.scrollIntoView = jest.fn(); fetchMock.config.overwriteRoutes = false; fetchMock.catch(""); fetchMock.get("/image-file-stub", "image file stub"); +fetchMock.get("/_matrix/client/versions", {}); // @ts-ignore window.fetch = fetchMock.sandbox(); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 342949e768..f7f0fe9c7c 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -509,6 +509,7 @@ export function mkStubRoom( return { canInvite: jest.fn(), client, + findThreadForEvent: jest.fn(), createThreadsTimelineSets: jest.fn().mockReturnValue(new Promise(() => {})), currentState: { getStateEvents: jest.fn((_type, key) => (key === undefined ? [] : null)),