Regression: Prevent settings from getting updated (#23556)

app/settings/server/SettingsRegistry.ts

@@ -75,6 +75,7 @@ const compareSettingsIgnoringKeys = (keys: Array<keyof ISetting>) =>
 			.every((key) => isEqual(a[key as keyof ISetting], b[key as keyof ISetting]));
 
 const compareSettings = compareSettingsIgnoringKeys(['value', 'ts', 'createdAt', 'valueSource', 'packageValue', 'processEnvValue', '_updatedAt']);
+
 export class SettingsRegistry {
 	private model: typeof SettingsModel;
 
@@ -108,7 +109,15 @@ export class SettingsRegistry {
 			this._sorter[sorterKey] = this._sorter[sorterKey] ?? -1;
 		}
 
-		const settingFromCode = getSettingDefaults({ _id, type: 'string', section, value, sorter: sorter ?? (sorterKey?.length && this._sorter[sorterKey]++), group, ...options }, blockedSettings, hiddenSettings, wizardRequiredSettings);
+		const settingFromCode = getSettingDefaults({
+			_id,
+			type: 'string',
+			value,
+			sorter: sorter ?? (sorterKey?.length && this._sorter[sorterKey]++),
+			group,
+			section,
+			...options,
+		}, blockedSettings, hiddenSettings, wizardRequiredSettings);
 
 		if (isSettingEnterprise(settingFromCode) && !('invalidValue' in settingFromCode)) {
 			SystemLogger.error(`Enterprise setting ${ _id } is missing the invalidValue option`);
@@ -117,6 +126,7 @@ export class SettingsRegistry {
 
 		const settingStored = this.store.getSetting(_id);
 		const settingOverwritten = overwriteSetting(settingFromCode);
+
 		try {
 			validateSetting(settingFromCode._id, settingFromCode.type, settingFromCode.value);
 		} catch (e) {
@@ -129,7 +139,16 @@ export class SettingsRegistry {
 
 		if (settingStored && !compareSettings(settingStored, settingOverwritten)) {
 			const { value: _value, ...settingOverwrittenProps } = settingOverwritten;
-			this.model.upsert({ _id }, { $set: { ...settingOverwrittenProps } });
+
+			const overwrittenKeys = Object.keys(settingOverwritten);
+			const removedKeys = Object.keys(settingStored).filter((key) => !['_updatedAt'].includes(key) && !overwrittenKeys.includes(key));
+
+			this.model.upsert({ _id }, {
+				$set: { ...settingOverwrittenProps },
+				...removedKeys.length && {
+					$unset: removedKeys.reduce((unset, key) => ({ ...unset, [key]: 1 }), {}),
+				},
+			});
 			return;
 		}
 

app/settings/server/functions/getSettingDefaults.tests.ts

@@ -85,4 +85,14 @@ describe('getSettingDefaults', () => {
 
 		expect(setting).to.have.property('blocked').to.be.equal(true);
 	});
+
+	it('should not return undefined options', () => {
+		const setting = getSettingDefaults({ _id: 'test', value: true, type: 'string', section: undefined, group: undefined }, new Set(['test']));
+
+		expect(setting).to.be.an('object');
+		expect(setting).to.have.property('_id');
+
+		expect(setting).to.not.have.property('section');
+		expect(setting).to.not.have.property('group');
+	});
 });

app/settings/server/functions/getSettingDefaults.ts

@@ -1,7 +1,15 @@
 import { ISetting, ISettingColor, isSettingColor } from '../../../../definition/ISetting';
 
-export const getSettingDefaults = (setting: Partial<ISetting> & Pick<ISetting, '_id' | 'value' | 'type'>, blockedSettings: Set<string> = new Set(), hiddenSettings: Set<string> = new Set(), wizardRequiredSettings: Set<string> = new Set()): ISetting => {
-	const { _id, value, sorter, ...options } = setting;
+export const getSettingDefaults = (
+	setting: Partial<ISetting> & Pick<ISetting, '_id' | 'value' | 'type'>,
+	blockedSettings: Set<string> = new Set(),
+	hiddenSettings: Set<string> = new Set(),
+	wizardRequiredSettings: Set<string> = new Set(),
+): ISetting => {
+	const { _id, value, sorter, ...props } = setting;
+
+	const options = Object.fromEntries(Object.entries(props).filter(([, value]) => value !== undefined));
+
 	return {
 		_id,
 		value,