Cleanup tasks in the Settings code (#12125)

* inline call to `SettingsStore.isEnabled`

* Remove confusing `SettingsStore.isEnabled`

This was basically the same as `SettingsStore.canSetValue` only more confusing,
so let's get rid of it.

* Add `configDisablesSetting` value for Settings

The current magic where this only works for features (but not beta features!)
is, well, magical. And I need more flexibility here.

* Remove redundant levels constant `LEVELS_FEATURE`

I don't know if this was ever intended to have different semantics to
`LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG`, but if it was, those semantics
shuold have been written down. They now seem to be used entirely
interchangably.

src/components/views/elements/SettingsFlag.tsx

@@ -40,8 +40,6 @@ interface IProps {
 
 interface IState {
     value: boolean;
-    /** true if `SettingsStore.isEnabled` returned false. */
-    disabled: boolean;
 }
 
 export default class SettingsFlag extends React.Component<IProps, IState> {
@@ -52,7 +50,6 @@ export default class SettingsFlag extends React.Component<IProps, IState> {
 
         this.state = {
             value: this.getSettingValue(),
-            disabled: this.isSettingDisabled(),
         };
     }
 
@@ -72,15 +69,9 @@ export default class SettingsFlag extends React.Component<IProps, IState> {
             this.props.isExplicit,
         );
     }
-
-    private isSettingDisabled(): boolean {
-        return !SettingsStore.isEnabled(this.props.name);
-    }
-
     private onSettingChange = (): void => {
         this.setState({
             value: this.getSettingValue(),
-            disabled: this.isSettingDisabled(),
         });
     };
 
@@ -104,14 +95,13 @@ export default class SettingsFlag extends React.Component<IProps, IState> {
     };
 
     public render(): React.ReactNode {
-        const canChange = SettingsStore.canSetValue(this.props.name, this.props.roomId ?? null, this.props.level);
+        const disabled = !SettingsStore.canSetValue(this.props.name, this.props.roomId ?? null, this.props.level);
 
-        if (!canChange && this.props.hideIfCannotSet) return null;
+        if (disabled && this.props.hideIfCannotSet) return null;
 
         const label = this.props.label ?? SettingsStore.getDisplayName(this.props.name, this.props.level);
         const description = SettingsStore.getDescription(this.props.name);
         const shouldWarn = SettingsStore.shouldHaveWarning(this.props.name);
-        const disabled = this.state.disabled || !canChange;
 
         if (this.props.useCheckbox) {
             return (

src/components/views/settings/CryptographyPanel.tsx

@@ -62,7 +62,7 @@ export default class CryptographyPanel extends React.Component<IProps, IState> {
         }
 
         let noSendUnverifiedSetting: JSX.Element | undefined;
-        if (SettingsStore.isEnabled("blacklistUnverifiedDevices")) {
+        if (SettingsStore.canSetValue("blacklistUnverifiedDevices", null, SettingLevel.DEVICE)) {
             noSendUnverifiedSetting = (
                 <SettingsFlag
                     name="blacklistUnverifiedDevices"

src/components/views/settings/E2eAdvancedPanel.tsx

@@ -38,5 +38,5 @@ const E2eAdvancedPanel: React.FC = () => {
 export default E2eAdvancedPanel;
 
 export function isE2eAdvancedPanelPossible(): boolean {
-    return SettingsStore.isEnabled(SETTING_MANUALLY_VERIFY_ALL_SESSIONS);
+    return SettingsStore.canSetValue(SETTING_MANUALLY_VERIFY_ALL_SESSIONS, null, SettingLevel.DEVICE);
 }

src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx

@@ -427,7 +427,10 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
         const canEnableEncryption = !isEncrypted && !isEncryptionForceDisabled && hasEncryptionPermission;
 
         let encryptionSettings: JSX.Element | undefined;
-        if (isEncrypted && SettingsStore.isEnabled("blacklistUnverifiedDevices")) {
+        if (
+            isEncrypted &&
+            SettingsStore.canSetValue("blacklistUnverifiedDevices", this.props.room.roomId, SettingLevel.ROOM_DEVICE)
+        ) {
             encryptionSettings = (
                 <SettingsFlag
                     name="blacklistUnverifiedDevices"

src/settings/Settings.tsx

@@ -67,7 +67,6 @@ const LEVELS_ROOM_SETTINGS_WITH_ROOM = [
     SettingLevel.ROOM,
 ];
 const LEVELS_ACCOUNT_SETTINGS = [SettingLevel.DEVICE, SettingLevel.ACCOUNT, SettingLevel.CONFIG];
-const LEVELS_FEATURE = [SettingLevel.DEVICE, SettingLevel.CONFIG];
 const LEVELS_DEVICE_ONLY_SETTINGS = [SettingLevel.DEVICE];
 const LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG = [SettingLevel.DEVICE, SettingLevel.CONFIG];
 const LEVELS_UI_FEATURE = [
@@ -125,6 +124,17 @@ export type SettingValueType =
 export interface IBaseSetting<T extends SettingValueType = SettingValueType> {
     isFeature?: false | undefined;
 
+    /**
+     * If true, then the presence of this setting in `config.json` will disable the option in the UI.
+     *
+     * In other words, we prevent the user overriding the setting if an explicit value is given in `config.json`;
+     * though note that users who have already set a non-default value before `config.json` is update will continue
+     * to use that value (and, indeed, won't be able to change it!)
+     *
+     * Obviously, this only really makes sense if `supportedLevels` includes {@link SettingLevel.CONFIG}.
+     */
+    configDisablesSetting?: true;
+
     // Display names are strongly recommended for clarity.
     // Display name can also be an object for different levels.
     displayName?:
@@ -192,7 +202,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         isFeature: true,
         labsGroup: LabGroup.VoiceAndVideo,
         displayName: _td("labs|video_rooms"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
         // Reload to ensure that the left panel etc. get remounted
         controller: new ReloadOnChangeController(),
@@ -230,7 +240,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     [Features.NotificationSettings2]: {
         isFeature: true,
         labsGroup: LabGroup.Experimental,
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|notification_settings"),
         default: false,
         betaInfo: {
@@ -249,62 +259,70 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_msc3531_hide_messages_pending_moderation": {
         isFeature: true,
         labsGroup: LabGroup.Moderation,
+        configDisablesSetting: true,
         // Requires a reload since this setting is cached in EventUtils
         controller: new ReloadOnChangeController(),
         displayName: _td("labs|msc3531_hide_messages_pending_moderation"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_report_to_moderators": {
         isFeature: true,
         labsGroup: LabGroup.Moderation,
+        configDisablesSetting: true,
         displayName: _td("labs|report_to_moderators"),
         description: _td("labs|report_to_moderators_description"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_latex_maths": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
+        configDisablesSetting: true,
         displayName: _td("labs|latex_maths"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_pinning": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
+        configDisablesSetting: true,
         displayName: _td("labs|pinning"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_wysiwyg_composer": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
+        configDisablesSetting: true,
         displayName: _td("labs|wysiwyg_composer"),
         description: _td("labs|feature_wysiwyg_composer_description"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_mjolnir": {
         isFeature: true,
         labsGroup: LabGroup.Moderation,
+        configDisablesSetting: true,
         displayName: _td("labs|mjolnir"),
         description: _td("labs|currently_experimental"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_custom_themes": {
         isFeature: true,
         labsGroup: LabGroup.Themes,
+        configDisablesSetting: true,
         displayName: _td("labs|custom_themes"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_dehydration": {
         isFeature: true,
         labsGroup: LabGroup.Encryption,
+        configDisablesSetting: true,
         displayName: _td("labs|dehydration"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "useOnlyCurrentProfiles": {
@@ -323,22 +341,25 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_html_topic": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|html_topic"),
         default: false,
     },
     "feature_bridge_state": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|bridge_state"),
         default: false,
     },
     "feature_jump_to_date": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
+        configDisablesSetting: true,
         displayName: _td("labs|jump_to_date"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
         controller: new ServerSupportUnstableFeatureController(
             "feature_jump_to_date",
@@ -368,6 +389,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_sliding_sync": {
         isFeature: true,
         labsGroup: LabGroup.Developer,
+        configDisablesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|sliding_sync"),
         description: _td("labs|sliding_sync_description"),
@@ -382,32 +404,36 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     },
     "feature_element_call_video_rooms": {
         isFeature: true,
-        supportedLevels: LEVELS_FEATURE,
         labsGroup: LabGroup.VoiceAndVideo,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|element_call_video_rooms"),
         controller: new ReloadOnChangeController(),
         default: false,
     },
     "feature_group_calls": {
         isFeature: true,
-        supportedLevels: LEVELS_FEATURE,
         labsGroup: LabGroup.VoiceAndVideo,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|group_calls"),
         controller: new ReloadOnChangeController(),
         default: false,
     },
     "feature_disable_call_per_sender_encryption": {
         isFeature: true,
-        supportedLevels: LEVELS_FEATURE,
         labsGroup: LabGroup.VoiceAndVideo,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|feature_disable_call_per_sender_encryption"),
         default: false,
     },
     "feature_allow_screen_share_only_mode": {
         isFeature: true,
+        labsGroup: LabGroup.VoiceAndVideo,
+        configDisablesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         description: _td("labs|under_active_development"),
-        labsGroup: LabGroup.VoiceAndVideo,
         displayName: _td("labs|allow_screen_share_only_mode"),
         controller: new ReloadOnChangeController(),
         default: false,
@@ -415,7 +441,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_location_share_live": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|location_share_live"),
         description: _td("labs|location_share_live_description"),
         shouldWarn: true,
@@ -424,7 +451,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_dynamic_room_predecessors": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|dynamic_room_predecessors"),
         description: _td("labs|dynamic_room_predecessors_description"),
         shouldWarn: true,
@@ -433,7 +461,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     [Features.VoiceBroadcast]: {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|voice_broadcast"),
         default: false,
     },
@@ -445,7 +474,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     [Features.OidcNativeFlow]: {
         isFeature: true,
         labsGroup: LabGroup.Developer,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|oidc_native_flow"),
         description: _td("labs|oidc_native_flow_description"),
         default: false,
@@ -454,6 +484,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         // use the rust matrix-sdk-crypto-js for crypto.
         isFeature: true,
         labsGroup: LabGroup.Developer,
+        configDisablesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|rust_crypto"),
         description: _td("labs|under_active_development"),
@@ -470,9 +501,10 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_render_reaction_images": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
+        configDisablesSetting: true,
         displayName: _td("labs|render_reaction_images"),
         description: _td("labs|render_reaction_images_description"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     /**
@@ -521,33 +553,37 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     },
     "feature_hidebold": {
         isFeature: true,
+        labsGroup: LabGroup.Rooms,
+        configDisablesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|hidebold"),
-        labsGroup: LabGroup.Rooms,
         default: false,
     },
     "feature_ask_to_join": {
-        default: false,
-        displayName: _td("labs|ask_to_join"),
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        supportedLevels: LEVELS_FEATURE,
+        configDisablesSetting: true,
+        default: false,
+        displayName: _td("labs|ask_to_join"),
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
     },
     "feature_new_room_decoration_ui": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
+        configDisablesSetting: true,
         displayName: _td("labs|new_room_decoration_ui"),
         description: _td("labs|under_active_development"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
         controller: new ReloadOnChangeController(),
     },
     "feature_notifications": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
+        configDisablesSetting: true,
         displayName: _td("labs|notifications"),
         description: _td("labs|unrealiable_e2e"),
-        supportedLevels: LEVELS_FEATURE,
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "useCompactLayout": {

src/settings/SettingsStore.ts

@@ -320,19 +320,6 @@ export default class SettingsStore {
         }
     }
 
-    /**
-     * Determines if a setting is enabled.
-     * If a setting is disabled then it should normally be hidden from the user to de-clutter the user interface.
-     * This rule is intentionally ignored for labs flags to unveil what features are available with
-     * the right server support.
-     * @param {string} settingName The setting to look up.
-     * @return {boolean} True if the setting is enabled.
-     */
-    public static isEnabled(settingName: string): boolean {
-        if (!SETTINGS[settingName]) return false;
-        return !SETTINGS[settingName].controller?.settingDisabled ?? true;
-    }
-
     /**
      * Retrieves the reason a setting is disabled if one is assigned.
      * If a setting is not disabled, or no reason is given by the `SettingController`,
@@ -516,6 +503,15 @@ export default class SettingsStore {
      * Determines if the current user is permitted to set the given setting at the given
      * level for a particular room. The room ID is optional if the setting is not being
      * set for a particular room, otherwise it should be supplied.
+     *
+     * This takes into account both the value of {@link SettingController#settingDisabled} of the
+     * `SettingController`, if any; and, for settings where {@link IBaseSetting#configDisablesSetting} is true,
+     * whether the setting has been given a value in `config.json`.
+     *
+     * Typically, if the user cannot set the setting, it should be hidden, to declutter the UI;
+     * however some settings (typically, the labs flags) are exposed but greyed out, to unveil
+     * what features are available with the right server support.
+     *
      * @param {string} settingName The name of the setting to check.
      * @param {String} roomId The room ID to check in, may be null.
      * @param {SettingLevel} level The level to
@@ -528,12 +524,12 @@ export default class SettingsStore {
             throw new Error("Setting '" + settingName + "' does not appear to be a setting.");
         }
 
-        if (!SettingsStore.isEnabled(settingName)) {
+        if (SETTINGS[settingName].controller?.settingDisabled) {
             return false;
         }
 
         // When non-beta features are specified in the config.json, we force them as enabled or disabled.
-        if (SettingsStore.isFeature(settingName) && !SETTINGS[settingName]?.betaInfo) {
+        if (SETTINGS[settingName]?.configDisablesSetting) {
             const configVal = SettingsStore.getValueAt(SettingLevel.CONFIG, settingName, roomId, true, true);
             if (configVal === true || configVal === false) return false;
         }

test/components/views/settings/tabs/user/PreferencesUserSettingsTab-test.tsx

@@ -42,7 +42,6 @@ describe("PreferencesUserSettingsTab", () => {
         beforeEach(() => {
             stubClient();
             jest.spyOn(SettingsStore, "setValue");
-            jest.spyOn(SettingsStore, "canSetValue").mockReturnValue(true);
             jest.spyOn(window, "matchMedia").mockReturnValue({ matches: false } as MediaQueryList);
         });