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

Fix config override of other settings levels #12593

Merged
merged 19 commits into from
Jun 14, 2024
Merged

Fix config override of other settings levels #12593

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
86e1914
Make config override other settings levels and add tests
langleyd Jun 10, 2024
c65c6ef
fix documentation
langleyd Jun 10, 2024
845a2c3
lint
langleyd Jun 10, 2024
c7bda77
Use a const for finalLevel.
langleyd Jun 10, 2024
c63522a
respect the explicit parameter
langleyd Jun 10, 2024
871c0b6
Merge branch 'develop' into langleyd/fix_config_override_setting_value
langleyd Jun 10, 2024
1147afb
Use supportedLevelsAreOrdered for config overrides rather than a sepa…
langleyd Jun 10, 2024
eeb2795
Fix typos
langleyd Jun 11, 2024
53bc93d
Merge branch 'develop' into langleyd/fix_config_override_setting_value
langleyd Jun 11, 2024
c4e8931
Fix mock in UserSetttingsDialog-test
langleyd Jun 11, 2024
0f7cf3c
Merge branch 'langleyd/fix_config_override_setting_value' of https://…
langleyd Jun 11, 2024
ee18a7d
Merge branch 'develop' into langleyd/fix_config_override_setting_value
langleyd Jun 11, 2024
bedf253
Special case disabling of setting tos use config overrides.
langleyd Jun 13, 2024
d90c36c
Merge branch 'langleyd/fix_config_override_setting_value' of https://…
langleyd Jun 13, 2024
6fc476b
Merge branch 'develop' into langleyd/fix_config_override_setting_value
langleyd Jun 13, 2024
d8955de
remove logs
langleyd Jun 14, 2024
49c65cd
Merge branch 'langleyd/fix_config_override_setting_value' of https://…
langleyd Jun 14, 2024
0fa1b98
Merge branch 'develop' into langleyd/fix_config_override_setting_value
langleyd Jun 14, 2024
2350c01
Merge branch 'develop' into langleyd/fix_config_override_setting_value
langleyd Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/components/views/elements/SettingsFlag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ 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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need the !! here given its a boolean

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identified by Sonar: https://sonarcloud.io/project/issues?id=matrix-react-sdk&pullRequest=12593&resolved=false&sinceLeakPeriod=true

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
Open in desktop
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
49 changes: 39 additions & 10 deletions src/settings/SettingsStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,41 +505,70 @@ 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);
console.log("settingIsOveriddenAtConfigLevel");
console.log(settingName);
console.log(configIndex);
console.log(levelIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should probably not land

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
Loading
Loading