Regression: RegExp callbacks of settings were not being called (#17552)

app/settings/lib/settings.ts

@@ -19,7 +19,7 @@ export class SettingsBase {
 
 	// private ts = new Date()
 
-	public get(_id: string, callback?: SettingCallback): SettingValue | SettingComposedValue[] | void {
+	public get(_id: string | RegExp, callback?: SettingCallback): SettingValue | SettingComposedValue[] | void {
 		if (callback != null) {
 			this.onload(_id, callback);
 			if (!Meteor.settings) {
@@ -41,7 +41,9 @@ export class SettingsBase {
 				});
 			}
 
-			return Meteor.settings[_id] != null && callback(_id, Meteor.settings[_id]);
+			if (typeof _id === 'string') {
+				return Meteor.settings[_id] != null && callback(_id, Meteor.settings[_id]);
+			}
 		}
 
 		if (!Meteor.settings) {
@@ -79,8 +81,7 @@ export class SettingsBase {
 				callbacks.forEach((callback) => callback(key, value, initialLoad));
 			}
 		});
-		Object.keys(this.regexCallbacks).forEach((cbKey) => {
-			const cbValue = this.regexCallbacks.get(cbKey);
+		this.regexCallbacks.forEach((cbValue) => {
 			if (!cbValue?.regex.test(key)) {
 				return;
 			}

app/settings/server/functions/settings.mocks.ts

@@ -1,3 +1,4 @@
+import { Meteor } from 'meteor/meteor';
 import mock from 'mock-require';
 
 type Dictionary = {
@@ -24,8 +25,25 @@ class SettingsClass {
 
 		// console.log(query, data);
 		this.data.set(query._id, data);
+		Meteor.settings[query._id] = data.value;
+
+		// Can't import before the mock command on end of this file!
+		// eslint-disable-next-line @typescript-eslint/no-var-requires
+		const { settings } = require('./settings');
+		settings.load(query._id, data.value, !existent);
+
 		this.upsertCalls++;
 	}
+
+	updateValueById(id: string, value: any): void {
+		this.data.set(id, { ...this.data.get(id), value });
+
+		// Can't import before the mock command on end of this file!
+		// eslint-disable-next-line @typescript-eslint/no-var-requires
+		const { settings } = require('./settings');
+		Meteor.settings[id] = value;
+		settings.load(id, value, false);
+	}
 }
 
 export const Settings = new SettingsClass();

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

@@ -1,11 +1,14 @@
 /* eslint-disable @typescript-eslint/camelcase */
 /* eslint-env mocha */
 import { Meteor } from 'meteor/meteor';
-import { expect } from 'chai';
+import chai, { expect } from 'chai';
+import spies from 'chai-spies';
 
 import { Settings } from './settings.mocks';
 import { settings } from './settings';
 
+chai.use(spies);
+
 describe('Settings', () => {
 	beforeEach(() => {
 		Settings.upsertCalls = 0;
@@ -254,20 +257,21 @@ describe('Settings', () => {
 		expect(Settings.upsertCalls).to.be.equal(2);
 		expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting);
 
-		settings.addGroup('group', function() {
-			this.section('section', function() {
-				this.add('my_setting', 1, {
-					type: 'int',
-					sorter: 0,
-				});
-			});
-		});
-
-		expectedSetting.packageValue = 1;
-
-		expect(Settings.data.size).to.be.equal(2);
-		expect(Settings.upsertCalls).to.be.equal(3);
-		expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting);
+		// Can't reset setting because the Meteor.setting will have the first value and will act to enforce his value
+		// settings.addGroup('group', function() {
+		// 	this.section('section', function() {
+		// 		this.add('my_setting', 1, {
+		// 			type: 'int',
+		// 			sorter: 0,
+		// 		});
+		// 	});
+		// });
+
+		// expectedSetting.packageValue = 1;
+
+		// expect(Settings.data.size).to.be.equal(2);
+		// expect(Settings.upsertCalls).to.be.equal(3);
+		// expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting);
 	});
 
 	it('should change group and section', () => {
@@ -316,4 +320,41 @@ describe('Settings', () => {
 		expect(Settings.upsertCalls).to.be.equal(4);
 		expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting);
 	});
+
+	it('should call `settings.get` callback on setting added', () => {
+		settings.addGroup('group', function() {
+			this.section('section', function() {
+				this.add('setting_callback', 'value1', {
+					type: 'string',
+				});
+			});
+		});
+
+		const spy = chai.spy();
+		settings.get('setting_callback', spy);
+		settings.get(/setting_callback/, spy);
+
+		expect(spy).to.have.been.called.exactly(2);
+		expect(spy).to.have.been.called.always.with('setting_callback', 'value1');
+	});
+
+	it('should call `settings.get` callback on setting changed', () => {
+		const spy = chai.spy();
+		settings.get('setting_callback', spy);
+		settings.get(/setting_callback/, spy);
+
+		settings.addGroup('group', function() {
+			this.section('section', function() {
+				this.add('setting_callback', 'value2', {
+					type: 'string',
+				});
+			});
+		});
+
+		settings.updateById('setting_callback', 'value3');
+
+		expect(spy).to.have.been.called.exactly(4);
+		expect(spy).to.have.been.called.with('setting_callback', 'value2');
+		expect(spy).to.have.been.called.with('setting_callback', 'value3');
+	});
 });

app/settings/server/functions/settings.ts

@@ -27,11 +27,7 @@ const overrideSetting = (_id: string, value: SettingValue, options: ISettingAddO
 		}
 		options.processEnvValue = value;
 		options.valueSource = 'processEnvValue';
-	} else if (typeof Meteor.settings[_id] !== 'undefined') {
-		if (Meteor.settings[_id] == null) {
-			return false;
-		}
-
+	} else if (Meteor.settings[_id] != null && Meteor.settings[_id] !== value) {
 		value = Meteor.settings[_id];
 		options.meteorSettingsValue = value;
 		options.valueSource = 'meteorSettingsValue';
@@ -291,7 +287,7 @@ class Settings extends SettingsBase {
 	/*
 	* Update a setting by id
 	*/
-	updateById(_id: string, value: SettingValue, editor: string): boolean {
+	updateById(_id: string, value: SettingValue, editor?: string): boolean {
 		if (!_id || value == null) {
 			return false;
 		}

package-lock.json

@@ -6060,8 +6060,15 @@
 		"@types/chai": {
 			"version": "4.2.11",
 			"resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.2.11.tgz",
-			"integrity": "sha512-t7uW6eFafjO+qJ3BIV2gGUyZs27egcNRkUdalkud+Qa3+kg//f129iuOFivHDXQ+vnU3fDXuwgv0cqMCbcE8sw==",
-			"dev": true
+			"integrity": "sha512-t7uW6eFafjO+qJ3BIV2gGUyZs27egcNRkUdalkud+Qa3+kg//f129iuOFivHDXQ+vnU3fDXuwgv0cqMCbcE8sw=="
+		},
+		"@types/chai-spies": {
+			"version": "1.0.1",
+			"resolved": "https://registry.npmjs.org/@types/chai-spies/-/chai-spies-1.0.1.tgz",
+			"integrity": "sha512-vvlTzisMpzmPZaKDd0pFybCJB5bzx398JEdPsVD9Rwq4a7dcGSUDlNG2PAVhsanRBPl4hezqnlaFtL1/YwBGgw==",
+			"requires": {
+				"@types/chai": "*"
+			}
 		},
 		"@types/color-name": {
 			"version": "1.1.1",
@@ -10492,6 +10499,11 @@
 				"chai": ">1.9.0"
 			}
 		},
+		"chai-spies": {
+			"version": "1.0.0",
+			"resolved": "https://registry.npmjs.org/chai-spies/-/chai-spies-1.0.0.tgz",
+			"integrity": "sha512-elF2ZUczBsFoP07qCfMO/zeggs8pqCf3fZGyK5+2X4AndS8jycZYID91ztD9oQ7d/0tnS963dPkd0frQEThDsg=="
+		},
 		"chalk": {
 			"version": "1.1.3",
 			"resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz",

package.json

@@ -63,6 +63,7 @@
 		"@storybook/react": "^5.3.18",
 		"@types/bcrypt": "^3.0.0",
 		"@types/chai": "^4.2.11",
+		"@types/chai-spies": "^1.0.1",
 		"@types/meteor": "^1.4.37",
 		"@types/mocha": "^7.0.2",
 		"@types/mock-require": "^2.0.0",
@@ -78,6 +79,7 @@
 		"babel-polyfill": "^6.26.0",
 		"chai": "^4.2.0",
 		"chai-datetime": "^1.5.0",
+		"chai-spies": "^1.0.0",
 		"cypress": "^4.0.2",
 		"emojione-assets": "^4.5.0",
 		"eslint": "^6.7.2",