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

Commit

Permalink
Fix config override of other settings levels (#12593)
Browse files Browse the repository at this point in the history 
* Make config override other settings levels and add tests

* fix documentation

* lint

* Use a const for finalLevel.

* respect the explicit parameter

* Use supportedLevelsAreOrdered for config overrides rather than a separate setting.

* Fix typos

* Fix mock in UserSetttingsDialog-test

* Special case disabling of setting tos use config overrides.

* remove logs
  • Loading branch information
@langleyd
langleyd authored Jun 14, 2024
1 parent 8e200dc commit d6b9e2a
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 70 deletions.
7 changes: 7 additions & 0 deletions src/components/views/elements/SettingsFlag.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ export default class SettingsFlag extends React.Component<IProps, IState> {
}

private getSettingValue(): boolean {
// If a level defined in props is overridden by a level at a high presedence, it gets disabled
// and we should show the overridding value.
if (
SettingsStore.settingIsOveriddenAtConfigLevel(this.props.name, this.props.roomId ?? null, this.props.level)
) {
return !!SettingsStore.getValue(this.props.name);
}
return !!SettingsStore.getValueAt(
this.props.level,
this.props.name,
Expand Down
109 changes: 49 additions & 60 deletions src/settings/Settings.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const LEVELS_ROOM_SETTINGS_WITH_ROOM = [
const LEVELS_ACCOUNT_SETTINGS = [SettingLevel.DEVICE, SettingLevel.ACCOUNT, SettingLevel.CONFIG];
const LEVELS_DEVICE_ONLY_SETTINGS = [SettingLevel.DEVICE];
const LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG = [SettingLevel.DEVICE, SettingLevel.CONFIG];
const LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED = [SettingLevel.CONFIG, SettingLevel.DEVICE];
const LEVELS_UI_FEATURE = [
SettingLevel.CONFIG,
// in future we might have a .well-known level or something
Expand Down Expand Up @@ -133,17 +134,6 @@ 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`.
* XXX: 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!): https://github.com/element-hq/element-web/issues/26877
*
* 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?:
Expand Down Expand Up @@ -268,70 +258,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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"feature_latex_maths": {
isFeature: true,
labsGroup: LabGroup.Messaging,
configDisablesSetting: true,
displayName: _td("labs|latex_maths"),
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"feature_pinning": {
isFeature: true,
labsGroup: LabGroup.Messaging,
configDisablesSetting: true,
displayName: _td("labs|pinning"),
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"feature_mjolnir": {
isFeature: true,
labsGroup: LabGroup.Moderation,
configDisablesSetting: true,
displayName: _td("labs|mjolnir"),
description: _td("labs|currently_experimental"),
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"feature_custom_themes": {
isFeature: true,
labsGroup: LabGroup.Themes,
configDisablesSetting: true,
displayName: _td("labs|custom_themes"),
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"feature_dehydration": {
isFeature: true,
labsGroup: LabGroup.Encryption,
configDisablesSetting: true,
displayName: _td("labs|dehydration"),
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"useOnlyCurrentProfiles": {
Expand All @@ -350,25 +340,25 @@ export const SETTINGS: { [setting: string]: ISetting } = {
"feature_html_topic": {
isFeature: true,
labsGroup: LabGroup.Rooms,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|html_topic"),
default: false,
},
"feature_bridge_state": {
isFeature: true,
labsGroup: LabGroup.Rooms,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
controller: new ServerSupportUnstableFeatureController(
"feature_jump_to_date",
Expand Down Expand Up @@ -398,8 +388,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
"feature_sliding_sync": {
isFeature: true,
labsGroup: LabGroup.Developer,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|sliding_sync"),
description: _td("labs|sliding_sync_description"),
shouldWarn: true,
Expand All @@ -414,34 +404,34 @@ export const SETTINGS: { [setting: string]: ISetting } = {
"feature_element_call_video_rooms": {
isFeature: true,
labsGroup: LabGroup.VoiceAndVideo,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|element_call_video_rooms"),
controller: new ReloadOnChangeController(),
default: false,
},
"feature_group_calls": {
isFeature: true,
labsGroup: LabGroup.VoiceAndVideo,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|group_calls"),
controller: new ReloadOnChangeController(),
default: false,
},
"feature_disable_call_per_sender_encryption": {
isFeature: true,
labsGroup: LabGroup.VoiceAndVideo,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
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,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
description: _td("labs|under_active_development"),
displayName: _td("labs|allow_screen_share_only_mode"),
controller: new ReloadOnChangeController(),
Expand All @@ -450,8 +440,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
"feature_location_share_live": {
isFeature: true,
labsGroup: LabGroup.Messaging,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|location_share_live"),
description: _td("labs|location_share_live_description"),
shouldWarn: true,
Expand All @@ -460,8 +450,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
"feature_dynamic_room_predecessors": {
isFeature: true,
labsGroup: LabGroup.Rooms,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|dynamic_room_predecessors"),
description: _td("labs|dynamic_room_predecessors_description"),
shouldWarn: true,
Expand All @@ -470,8 +460,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
[Features.VoiceBroadcast]: {
isFeature: true,
labsGroup: LabGroup.Messaging,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|voice_broadcast"),
default: false,
},
Expand All @@ -483,8 +473,8 @@ export const SETTINGS: { [setting: string]: ISetting } = {
[Features.OidcNativeFlow]: {
isFeature: true,
labsGroup: LabGroup.Developer,
configDisablesSetting: true,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
displayName: _td("labs|oidc_native_flow"),
description: _td("labs|oidc_native_flow_description"),
default: false,
Expand All @@ -493,7 +483,6 @@ export const SETTINGS: { [setting: string]: ISetting } = {
// use the rust matrix-sdk-crypto-wasm for crypto.
isFeature: true,
labsGroup: LabGroup.Developer,
// unlike most features, `configDisablesSetting` is false here.
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
displayName: _td("labs|rust_crypto"),
description: () => {
Expand Down Expand Up @@ -527,10 +516,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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
/**
Expand Down Expand Up @@ -609,28 +598,28 @@ export const SETTINGS: { [setting: string]: ISetting } = {
"feature_ask_to_join": {
isFeature: true,
labsGroup: LabGroup.Rooms,
configDisablesSetting: true,
default: false,
displayName: _td("labs|ask_to_join"),
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
},
"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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
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_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
supportedLevelsAreOrdered: true,
default: false,
},
"useCompactLayout": {
Expand Down
45 changes: 35 additions & 10 deletions src/settings/SettingsStore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,41 +505,66 @@ export default class SettingsStore {
* 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`.
* `SettingController`, if any; and, for settings where {@link IBaseSetting#supportedLevelsAreOrdered} is true,
* checks whether a level of higher precedence is set.
*
* 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
* check at.
* @param {SettingLevel} level The level to check at.
* @return {boolean} True if the user may set the setting, false otherwise.
*/
public static canSetValue(settingName: string, roomId: string | null, level: SettingLevel): boolean {
const setting = SETTINGS[settingName];
// Verify that the setting is actually a setting
if (!SETTINGS[settingName]) {
if (!setting) {
throw new Error("Setting '" + settingName + "' does not appear to be a setting.");
}

if (SETTINGS[settingName].controller?.settingDisabled) {
if (setting.controller?.settingDisabled) {
return false;
}

// For some config settings (mostly: non-beta features), a value in config.json overrides the local setting
// (ie: we force them as enabled or disabled).
if (SETTINGS[settingName]?.configDisablesSetting) {
const configVal = SettingsStore.getValueAt(SettingLevel.CONFIG, settingName, roomId, true, true);
if (configVal === true || configVal === false) return false;
// (ie: we force them as enabled or disabled). In this case we should not let the user change the setting.
if (
setting?.supportedLevelsAreOrdered &&
SettingsStore.settingIsOveriddenAtConfigLevel(settingName, roomId, level)
) {
return false;
}

const handler = SettingsStore.getHandler(settingName, level);
if (!handler) return false;
return handler.canSetValue(settingName, roomId);
}

/**
* Determines if the setting at the specified level is overidden by one at a config level.
* @param settingName The name of the setting to check.
* @param roomId The room ID to check in, may be null.
* @param level The level to check at.
* @returns
*/
public static settingIsOveriddenAtConfigLevel(
settingName: string,
roomId: string | null,
level: SettingLevel,
): boolean {
const setting = SETTINGS[settingName];
const levelOrders = getLevelOrder(setting);
const configIndex = levelOrders.indexOf(SettingLevel.CONFIG);
const levelIndex = levelOrders.indexOf(level);
if (configIndex === -1 || levelIndex === -1 || configIndex >= levelIndex) {
return false;
}
const configVal = SettingsStore.getValueAt(SettingLevel.CONFIG, settingName, roomId, true, true);
return configVal === true || configVal === false;
}

/**
* Determines if the given level is supported on this device.
* @param {SettingLevel} level The level
Expand Down
1 change: 1 addition & 0 deletions test/components/views/dialogs/UserSettingsDialog-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ jest.mock("../../../../src/settings/SettingsStore", () => ({
getDescription: jest.fn(),
shouldHaveWarning: jest.fn(),
disabledMessage: jest.fn(),
settingIsOveriddenAtConfigLevel: jest.fn(),
}));

jest.mock("../../../../src/SdkConfig", () => ({
Expand Down
Loading

0 comments on commit d6b9e2a

Please sign in to comment.