From 4ab6c6594464a0399b9c8b4a32dde9e04ee84d99 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 18 Apr 2023 13:21:52 +0100 Subject: [PATCH 1/3] Honour feature toggles in guest mode --- .../handlers/DeviceSettingsHandler.ts | 11 +-- .../handlers/DeviceSettingsHandler-test.ts | 81 +++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 test/settings/handlers/DeviceSettingsHandler-test.ts diff --git a/src/settings/handlers/DeviceSettingsHandler.ts b/src/settings/handlers/DeviceSettingsHandler.ts index e50015f847e..978c83c1a6f 100644 --- a/src/settings/handlers/DeviceSettingsHandler.ts +++ b/src/settings/handlers/DeviceSettingsHandler.ts @@ -16,7 +16,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MatrixClientPeg } from "../../MatrixClientPeg"; import { SettingLevel } from "../SettingLevel"; import { CallbackFn, WatchManager } from "../WatchManager"; import AbstractLocalStorageSettingsHandler from "./AbstractLocalStorageSettingsHandler"; @@ -117,10 +116,12 @@ export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsH // public for access to migrations - not exposed from the SettingsHandler interface public readFeature(featureName: string): boolean | null { - if (MatrixClientPeg.get() && MatrixClientPeg.get().isGuest()) { - // Guests should not have any labs features enabled. - return false; - } + // Previously, we disabled all features for guests, but since different + // installations can have site-specific config files which might set up + // different behaviour that is relevant to guests, we removed that + // special behaviour. See + // https://github.com/vector-im/element-web/issues/24513 for the + // discussion. // XXX: This turns they key names into `mx_labs_feature_feature_x` (double feature). // This is because all feature names start with `feature_` as a matter of policy. diff --git a/test/settings/handlers/DeviceSettingsHandler-test.ts b/test/settings/handlers/DeviceSettingsHandler-test.ts new file mode 100644 index 00000000000..19d19d4c819 --- /dev/null +++ b/test/settings/handlers/DeviceSettingsHandler-test.ts @@ -0,0 +1,81 @@ +/* +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 { mocked } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/client"; + +import { MatrixClientPeg } from "../../../src/MatrixClientPeg"; +import DeviceSettingsHandler from "../../../src/settings/handlers/DeviceSettingsHandler"; +import { CallbackFn, WatchManager } from "../../../src/settings/WatchManager"; +import { stubClient } from "../../test-utils/test-utils"; + +describe("DeviceSettingsHandler", () => { + const ROOM_ID_IS_UNUSED = ""; + + const unknownSettingKey = "unknown_setting"; + const featureKey = "my_feature"; + + let watchers: WatchManager; + let handler: DeviceSettingsHandler; + let settingListener: CallbackFn; + + beforeEach(() => { + watchers = new WatchManager(); + handler = new DeviceSettingsHandler([featureKey], watchers); + settingListener = jest.fn(); + }); + + afterEach(() => { + watchers.unwatchSetting(settingListener); + }); + + it("Returns undefined for an unknown setting", () => { + expect(handler.getValue(unknownSettingKey, ROOM_ID_IS_UNUSED)).toBeUndefined(); + }); + + it("Returns the value for a disabled feature", () => { + handler.setValue(featureKey, ROOM_ID_IS_UNUSED, false); + expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(false); + }); + + it("Returns the value for an enabled feature", () => { + handler.setValue(featureKey, ROOM_ID_IS_UNUSED, true); + expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(true); + }); + + describe("If I am a guest", () => { + let client: MatrixClient; + + beforeEach(() => { + client = stubClient(); + mocked(client.isGuest).mockReturnValue(true); + }); + + afterEach(() => { + MatrixClientPeg.get = () => null; + }); + + it("Returns the value for a disabled feature", () => { + handler.setValue(featureKey, ROOM_ID_IS_UNUSED, false); + expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(false); + }); + + it("Returns the value for an enabled feature", () => { + handler.setValue(featureKey, ROOM_ID_IS_UNUSED, true); + expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(true); + }); + }); +}); From 0c657e6afd4c69d07c39bc2783184f36feaf940f Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 18 Apr 2023 13:33:32 +0100 Subject: [PATCH 2/3] Suppress TS warning about returning null MatrixClient --- test/settings/handlers/DeviceSettingsHandler-test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/settings/handlers/DeviceSettingsHandler-test.ts b/test/settings/handlers/DeviceSettingsHandler-test.ts index 19d19d4c819..735e8b54abd 100644 --- a/test/settings/handlers/DeviceSettingsHandler-test.ts +++ b/test/settings/handlers/DeviceSettingsHandler-test.ts @@ -65,6 +65,7 @@ describe("DeviceSettingsHandler", () => { }); afterEach(() => { + // @ts-ignore Allow returning a null MatrixClient MatrixClientPeg.get = () => null; }); From 79d9a7442357fd22f7540d7deb5beb2d6422088e Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 18 Apr 2023 13:37:00 +0100 Subject: [PATCH 3/3] Revert "Suppress TS warning about returning null MatrixClient" Don't ts-ignore this - we will eventually fix it via the strict work. This reverts commit 0c657e6afd4c69d07c39bc2783184f36feaf940f. --- test/settings/handlers/DeviceSettingsHandler-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/settings/handlers/DeviceSettingsHandler-test.ts b/test/settings/handlers/DeviceSettingsHandler-test.ts index 735e8b54abd..19d19d4c819 100644 --- a/test/settings/handlers/DeviceSettingsHandler-test.ts +++ b/test/settings/handlers/DeviceSettingsHandler-test.ts @@ -65,7 +65,6 @@ describe("DeviceSettingsHandler", () => { }); afterEach(() => { - // @ts-ignore Allow returning a null MatrixClient MatrixClientPeg.get = () => null; });