From 49dfad0f201f17775666aa13def13cce27293fdc Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Fri, 3 Apr 2020 23:23:57 -0300 Subject: [PATCH 01/12] [FIX] Uncessary updates on Settings, Roles and Permissions on startup --- app/authorization/server/startup.js | 23 +++++---- .../server/lib/startup.js | 4 +- app/channel-settings/server/startup.js | 6 +-- app/discussion/server/permissions.js | 4 +- app/lib/server/startup/settings.js | 2 +- app/mail-messages/server/startup.js | 7 +-- app/message-pin/server/settings.js | 6 +-- .../server/startup/settings.js | 6 +-- app/models/server/models/Permissions.js | 12 ++++- app/models/server/models/Roles.js | 27 +++++----- app/settings/server/functions/settings.js | 51 +++++++++++++------ ee/app/auditing/server/index.js | 6 +-- 12 files changed, 84 insertions(+), 70 deletions(-) diff --git a/app/authorization/server/startup.js b/app/authorization/server/startup.js index 6781accad030..de1686d47997 100644 --- a/app/authorization/server/startup.js +++ b/app/authorization/server/startup.js @@ -1,7 +1,7 @@ /* eslint no-multi-spaces: 0 */ import { Meteor } from 'meteor/meteor'; -import { Roles, Permissions, Settings } from '../../models'; +import { Roles, Permissions, Settings } from '../../models/server'; import { settings } from '../../settings/server'; import { getSettingPermissionId, CONSTANTS } from '../lib'; import { clearCache } from './functions/hasPermission'; @@ -114,9 +114,7 @@ Meteor.startup(function() { ]; for (const permission of permissions) { - if (!Permissions.findOneById(permission._id)) { - Permissions.upsert(permission._id, { $set: permission }); - } + Permissions.createOrUpdate(permission._id, permission.roles); } const defaultRoles = [ @@ -134,7 +132,7 @@ Meteor.startup(function() { ]; for (const role of defaultRoles) { - Roles.upsert({ _id: role.name }, { $setOnInsert: { scope: role.scope, description: role.description || '', protected: true, mandatory2fa: false } }); + Roles.createOrUpdate(role.name, role.scope, role.description, true, false); } const getPreviousPermissions = function(settingId) { @@ -155,19 +153,17 @@ Meteor.startup(function() { const createSettingPermission = function(setting, previousSettingPermissions) { const permissionId = getSettingPermissionId(setting._id); const permission = { - _id: permissionId, level: CONSTANTS.SETTINGS_LEVEL, // copy those setting-properties which are needed to properly publish the setting-based permissions settingId: setting._id, group: setting.group, section: setting.section, sorter: setting.sorter, + roles: [], }; // copy previously assigned roles if available if (previousSettingPermissions[permissionId] && previousSettingPermissions[permissionId].roles) { permission.roles = previousSettingPermissions[permissionId].roles; - } else { - permission.roles = []; } if (setting.group) { permission.groupPermissionId = getSettingPermissionId(setting.group); @@ -175,7 +171,16 @@ Meteor.startup(function() { if (setting.section) { permission.sectionPermissionId = getSettingPermissionId(setting.section); } - Permissions.upsert(permission._id, { $set: permission }); + + const existent = Permissions.findOne({ + _id: permissionId, + ...permission, + }, { fields: { _id: 1 } }); + + if (!existent) { + Permissions.upsert({ _id: permissionId }, { $set: permission }); + } + delete previousSettingPermissions[permissionId]; }; diff --git a/app/channel-settings-mail-messages/server/lib/startup.js b/app/channel-settings-mail-messages/server/lib/startup.js index a04875ad90da..2db17d146aba 100644 --- a/app/channel-settings-mail-messages/server/lib/startup.js +++ b/app/channel-settings-mail-messages/server/lib/startup.js @@ -7,7 +7,5 @@ Meteor.startup(function() { _id: 'mail-messages', roles: ['admin'], }; - return Permissions.upsert(permission._id, { - $setOnInsert: permission, - }); + return Permissions.createOrUpdate(permission._id, permission.roles); }); diff --git a/app/channel-settings/server/startup.js b/app/channel-settings/server/startup.js index 698723ebe994..207f94a3479c 100644 --- a/app/channel-settings/server/startup.js +++ b/app/channel-settings/server/startup.js @@ -3,7 +3,7 @@ import { Meteor } from 'meteor/meteor'; import { Permissions } from '../../models'; Meteor.startup(function() { - Permissions.upsert('post-readonly', { $setOnInsert: { roles: ['admin', 'owner', 'moderator'] } }); - Permissions.upsert('set-readonly', { $setOnInsert: { roles: ['admin', 'owner'] } }); - Permissions.upsert('set-react-when-readonly', { $setOnInsert: { roles: ['admin', 'owner'] } }); + Permissions.createOrUpdate('post-readonly', ['admin', 'owner', 'moderator']); + Permissions.createOrUpdate('set-readonly', ['admin', 'owner']); + Permissions.createOrUpdate('set-react-when-readonly', ['admin', 'owner']); }); diff --git a/app/discussion/server/permissions.js b/app/discussion/server/permissions.js index 49bfe798b0b4..8492b749d380 100644 --- a/app/discussion/server/permissions.js +++ b/app/discussion/server/permissions.js @@ -10,8 +10,6 @@ Meteor.startup(() => { ]; for (const permission of permissions) { - if (!Permissions.findOneById(permission._id)) { - Permissions.upsert(permission._id, { $set: permission }); - } + Permissions.createOrUpdate(permission._id, permission.roles); } }); diff --git a/app/lib/server/startup/settings.js b/app/lib/server/startup/settings.js index 7f42f30ee2dc..afdf391eaed8 100644 --- a/app/lib/server/startup/settings.js +++ b/app/lib/server/startup/settings.js @@ -2766,7 +2766,7 @@ settings.addGroup('Setup_Wizard', function() { secret: true, }); - this.add('Cloud_Workspace_Access_Token_Expires_At', new Date(), { + this.add('Cloud_Workspace_Access_Token_Expires_At', new Date(0), { type: 'date', hidden: true, readonly: true, diff --git a/app/mail-messages/server/startup.js b/app/mail-messages/server/startup.js index 10dd89d2b8c0..95a58dc3faad 100644 --- a/app/mail-messages/server/startup.js +++ b/app/mail-messages/server/startup.js @@ -3,10 +3,5 @@ import { Meteor } from 'meteor/meteor'; import { Permissions } from '../../models'; Meteor.startup(function() { - return Permissions.upsert('access-mailer', { - $setOnInsert: { - _id: 'access-mailer', - roles: ['admin'], - }, - }); + return Permissions.createOrUpdate('access-mailer', ['admin']); }); diff --git a/app/message-pin/server/settings.js b/app/message-pin/server/settings.js index c2af7eaf2761..e274696c513f 100644 --- a/app/message-pin/server/settings.js +++ b/app/message-pin/server/settings.js @@ -9,9 +9,5 @@ Meteor.startup(function() { group: 'Message', public: true, }); - return Permissions.upsert('pin-message', { - $setOnInsert: { - roles: ['owner', 'moderator', 'admin'], - }, - }); + return Permissions.createOrUpdate('pin-message', ['owner', 'moderator', 'admin']); }); diff --git a/app/message-snippet/server/startup/settings.js b/app/message-snippet/server/startup/settings.js index 04047eb56bfc..05a633ee4d4b 100644 --- a/app/message-snippet/server/startup/settings.js +++ b/app/message-snippet/server/startup/settings.js @@ -9,9 +9,5 @@ Meteor.startup(function() { public: true, group: 'Message', }); - Permissions.upsert('snippet-message', { - $setOnInsert: { - roles: ['owner', 'moderator', 'admin'], - }, - }); + Permissions.createOrUpdate('snippet-message', ['owner', 'moderator', 'admin']); }); diff --git a/app/models/server/models/Permissions.js b/app/models/server/models/Permissions.js index d1b17ce64698..9de372dd214b 100644 --- a/app/models/server/models/Permissions.js +++ b/app/models/server/models/Permissions.js @@ -15,15 +15,23 @@ export class Permissions extends Base { } createOrUpdate(name, roles) { + const exists = this.findOne({ + _id: name, + }, { fields: { _id: 1 } }); + + if (exists) { + return exists._id; + } + this.upsert({ _id: name }, { $set: { roles } }); } addRole(permission, role) { - this.update({ _id: permission }, { $addToSet: { roles: role } }); + this.update({ _id: permission, roles: { $ne: role } }, { $addToSet: { roles: role } }); } removeRole(permission, role) { - this.update({ _id: permission }, { $pull: { roles: role } }); + this.update({ _id: permission, roles: role }, { $pull: { roles: role } }); } } diff --git a/app/models/server/models/Roles.js b/app/models/server/models/Roles.js index 2715ab05d7c1..799a91426612 100644 --- a/app/models/server/models/Roles.js +++ b/app/models/server/models/Roles.js @@ -29,21 +29,22 @@ export class Roles extends Base { }); } - createOrUpdate(name, scope = 'Users', description, protectedRole, mandatory2fa) { - const updateData = {}; - updateData.name = name; - updateData.scope = scope; - - if (description != null) { - updateData.description = description; - } + createOrUpdate(name, scope = 'Users', description = '', protectedRole = true, mandatory2fa = false) { + const updateData = { + name, + scope, + description, + protected: protectedRole, + mandatory2fa, + }; - if (protectedRole) { - updateData.protected = protectedRole; - } + const exists = this.findOne({ + _id: name, + ...updateData, + }, { fields: { _id: 1 } }); - if (mandatory2fa != null) { - updateData.mandatory2fa = mandatory2fa; + if (exists) { + return exists._id; } this.upsert({ _id: name }, { $set: updateData }); diff --git a/app/settings/server/functions/settings.js b/app/settings/server/functions/settings.js index 7a7ae4a47c12..300e513c988a 100644 --- a/app/settings/server/functions/settings.js +++ b/app/settings/server/functions/settings.js @@ -120,9 +120,12 @@ settings.add = function(_id, value, { editor, ...options } = {}) { updateOperations.$setOnInsert.value = value; } } - const query = _.extend({ + + const query = { _id, - }, updateOperations.$set); + ...updateOperations.$set, + }; + if (options.section == null) { updateOperations.$unset = { section: 1, @@ -131,15 +134,19 @@ settings.add = function(_id, value, { editor, ...options } = {}) { $exists: false, }; } + const existentSetting = Settings.db.findOne(query); - if (existentSetting != null) { - if (existentSetting.editor == null && updateOperations.$setOnInsert.editor != null) { - updateOperations.$set.editor = updateOperations.$setOnInsert.editor; - delete updateOperations.$setOnInsert.editor; + if (existentSetting) { + if (existentSetting.editor || !updateOperations.$setOnInsert.editor) { + return; } - } else { - updateOperations.$set.ts = new Date(); + + updateOperations.$set.editor = updateOperations.$setOnInsert.editor; + delete updateOperations.$setOnInsert.editor; } + + updateOperations.$set.ts = new Date(); + return Settings.upsert({ _id, }, updateOperations); @@ -165,7 +172,7 @@ settings.addGroup = function(_id, options = {}, cb) { if (options.i18nDescription == null) { options.i18nDescription = `${ _id }_Description`; } - options.ts = new Date(); + options.blocked = false; options.hidden = false; if (blockedSettings[_id] != null) { @@ -174,15 +181,27 @@ settings.addGroup = function(_id, options = {}, cb) { if (hiddenSettings[_id] != null) { options.hidden = true; } - Settings.upsert({ + + const existentGroup = Settings.findOne({ _id, - }, { - $set: options, - $setOnInsert: { - type: 'group', - createdAt: new Date(), - }, + type: 'group', + ...options, }); + + if (!existentGroup) { + options.ts = new Date(); + + Settings.upsert({ + _id, + }, { + $set: options, + $setOnInsert: { + type: 'group', + createdAt: new Date(), + }, + }); + } + if (cb != null) { cb.call({ add(id, value, options) { diff --git a/ee/app/auditing/server/index.js b/ee/app/auditing/server/index.js index dd531c96949b..83e882893097 100644 --- a/ee/app/auditing/server/index.js +++ b/ee/app/auditing/server/index.js @@ -19,13 +19,11 @@ onLicense('auditing', () => { ]; permissions.forEach((permission) => { - if (!Permissions.findOneById(permission._id)) { - Permissions.upsert(permission._id, { $set: permission }); - } + Permissions.createOrUpdate(permission._id, permission.roles); }); defaultRoles.forEach((role) => - Roles.upsert({ _id: role.name }, { $setOnInsert: { scope: role.scope, description: role.description || '', protected: true } }), + Roles.createOrUpdate(role.name, role.scope, role.description), ); }); }); From 4f5bba7d397d30c34a4b0c5e72a04db4b2e43475 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Fri, 3 Apr 2020 23:56:06 -0300 Subject: [PATCH 02/12] Fix update of permissions --- app/api/server/v1/permissions.js | 2 +- app/api/server/v1/settings.js | 2 +- app/apps/server/orchestrator.js | 2 +- app/authorization/server/startup.js | 2 +- .../server/lib/startup.js | 2 +- app/channel-settings/server/startup.js | 6 +++--- app/cloud/server/index.js | 2 +- app/custom-sounds/server/startup/permissions.js | 2 +- app/discussion/server/permissions.js | 2 +- app/mail-messages/server/startup.js | 2 +- app/message-pin/server/settings.js | 2 +- app/message-snippet/server/startup/settings.js | 2 +- app/models/server/models/Permissions.js | 11 +++++++++++ ee/app/auditing/server/index.js | 2 +- ee/app/canned-responses/server/permissions.js | 6 +++--- ee/app/livechat-enterprise/server/permissions.js | 6 +++--- 16 files changed, 32 insertions(+), 21 deletions(-) diff --git a/app/api/server/v1/permissions.js b/app/api/server/v1/permissions.js index c5d790533287..e74c95662517 100644 --- a/app/api/server/v1/permissions.js +++ b/app/api/server/v1/permissions.js @@ -2,7 +2,7 @@ import { Meteor } from 'meteor/meteor'; import { Match, check } from 'meteor/check'; import { hasPermission } from '../../../authorization'; -import { Permissions, Roles } from '../../../models'; +import { Permissions, Roles } from '../../../models/server'; import { API } from '../api'; /** diff --git a/app/api/server/v1/settings.js b/app/api/server/v1/settings.js index 1fd8dba80c0b..42db479903fe 100644 --- a/app/api/server/v1/settings.js +++ b/app/api/server/v1/settings.js @@ -3,7 +3,7 @@ import { Match, check } from 'meteor/check'; import { ServiceConfiguration } from 'meteor/service-configuration'; import _ from 'underscore'; -import { Settings } from '../../../models'; +import { Settings } from '../../../models/server'; import { hasPermission } from '../../../authorization'; import { API } from '../api'; diff --git a/app/apps/server/orchestrator.js b/app/apps/server/orchestrator.js index 502ea2537a75..da9fca47fe61 100644 --- a/app/apps/server/orchestrator.js +++ b/app/apps/server/orchestrator.js @@ -24,7 +24,7 @@ class AppServerOrchestrator { initialize() { this._rocketchatLogger = new Logger('Rocket.Chat Apps'); - Permissions.createOrUpdate('manage-apps', ['admin']); + Permissions.create('manage-apps', ['admin']); this._marketplaceUrl = 'https://marketplace.rocket.chat'; diff --git a/app/authorization/server/startup.js b/app/authorization/server/startup.js index de1686d47997..a5e1867cf8c0 100644 --- a/app/authorization/server/startup.js +++ b/app/authorization/server/startup.js @@ -114,7 +114,7 @@ Meteor.startup(function() { ]; for (const permission of permissions) { - Permissions.createOrUpdate(permission._id, permission.roles); + Permissions.create(permission._id, permission.roles); } const defaultRoles = [ diff --git a/app/channel-settings-mail-messages/server/lib/startup.js b/app/channel-settings-mail-messages/server/lib/startup.js index 2db17d146aba..a079a2c4bc76 100644 --- a/app/channel-settings-mail-messages/server/lib/startup.js +++ b/app/channel-settings-mail-messages/server/lib/startup.js @@ -7,5 +7,5 @@ Meteor.startup(function() { _id: 'mail-messages', roles: ['admin'], }; - return Permissions.createOrUpdate(permission._id, permission.roles); + return Permissions.create(permission._id, permission.roles); }); diff --git a/app/channel-settings/server/startup.js b/app/channel-settings/server/startup.js index 207f94a3479c..5e9aeb7baaf2 100644 --- a/app/channel-settings/server/startup.js +++ b/app/channel-settings/server/startup.js @@ -3,7 +3,7 @@ import { Meteor } from 'meteor/meteor'; import { Permissions } from '../../models'; Meteor.startup(function() { - Permissions.createOrUpdate('post-readonly', ['admin', 'owner', 'moderator']); - Permissions.createOrUpdate('set-readonly', ['admin', 'owner']); - Permissions.createOrUpdate('set-react-when-readonly', ['admin', 'owner']); + Permissions.create('post-readonly', ['admin', 'owner', 'moderator']); + Permissions.create('set-readonly', ['admin', 'owner']); + Permissions.create('set-react-when-readonly', ['admin', 'owner']); }); diff --git a/app/cloud/server/index.js b/app/cloud/server/index.js index 58bd467b4dfd..f783f4887923 100644 --- a/app/cloud/server/index.js +++ b/app/cloud/server/index.js @@ -11,7 +11,7 @@ import { Permissions } from '../../models'; import { settings } from '../../settings/server'; if (Permissions) { - Permissions.createOrUpdate('manage-cloud', ['admin']); + Permissions.create('manage-cloud', ['admin']); } const licenseCronName = 'Cloud Workspace Sync'; diff --git a/app/custom-sounds/server/startup/permissions.js b/app/custom-sounds/server/startup/permissions.js index 7a9ba36a56c7..9e66458ea367 100644 --- a/app/custom-sounds/server/startup/permissions.js +++ b/app/custom-sounds/server/startup/permissions.js @@ -4,6 +4,6 @@ import { Permissions } from '../../../models'; Meteor.startup(() => { if (Permissions) { - Permissions.createOrUpdate('manage-sounds', ['admin']); + Permissions.create('manage-sounds', ['admin']); } }); diff --git a/app/discussion/server/permissions.js b/app/discussion/server/permissions.js index 8492b749d380..2f17e05d9414 100644 --- a/app/discussion/server/permissions.js +++ b/app/discussion/server/permissions.js @@ -10,6 +10,6 @@ Meteor.startup(() => { ]; for (const permission of permissions) { - Permissions.createOrUpdate(permission._id, permission.roles); + Permissions.create(permission._id, permission.roles); } }); diff --git a/app/mail-messages/server/startup.js b/app/mail-messages/server/startup.js index 95a58dc3faad..eadfc796e776 100644 --- a/app/mail-messages/server/startup.js +++ b/app/mail-messages/server/startup.js @@ -3,5 +3,5 @@ import { Meteor } from 'meteor/meteor'; import { Permissions } from '../../models'; Meteor.startup(function() { - return Permissions.createOrUpdate('access-mailer', ['admin']); + return Permissions.create('access-mailer', ['admin']); }); diff --git a/app/message-pin/server/settings.js b/app/message-pin/server/settings.js index e274696c513f..c16f82a183c3 100644 --- a/app/message-pin/server/settings.js +++ b/app/message-pin/server/settings.js @@ -9,5 +9,5 @@ Meteor.startup(function() { group: 'Message', public: true, }); - return Permissions.createOrUpdate('pin-message', ['owner', 'moderator', 'admin']); + return Permissions.create('pin-message', ['owner', 'moderator', 'admin']); }); diff --git a/app/message-snippet/server/startup/settings.js b/app/message-snippet/server/startup/settings.js index 05a633ee4d4b..15f8c349e8e6 100644 --- a/app/message-snippet/server/startup/settings.js +++ b/app/message-snippet/server/startup/settings.js @@ -9,5 +9,5 @@ Meteor.startup(function() { public: true, group: 'Message', }); - Permissions.createOrUpdate('snippet-message', ['owner', 'moderator', 'admin']); + Permissions.create('snippet-message', ['owner', 'moderator', 'admin']); }); diff --git a/app/models/server/models/Permissions.js b/app/models/server/models/Permissions.js index 9de372dd214b..009f29d37f9d 100644 --- a/app/models/server/models/Permissions.js +++ b/app/models/server/models/Permissions.js @@ -17,6 +17,7 @@ export class Permissions extends Base { createOrUpdate(name, roles) { const exists = this.findOne({ _id: name, + roles, }, { fields: { _id: 1 } }); if (exists) { @@ -26,6 +27,16 @@ export class Permissions extends Base { this.upsert({ _id: name }, { $set: { roles } }); } + create(name, roles) { + const exists = this.findOneById(name, { fields: { _id: 1 } }); + + if (exists) { + return exists._id; + } + + this.upsert({ _id: name }, { $set: { roles } }); + } + addRole(permission, role) { this.update({ _id: permission, roles: { $ne: role } }, { $addToSet: { roles: role } }); } diff --git a/ee/app/auditing/server/index.js b/ee/app/auditing/server/index.js index 83e882893097..a816e3a1bc2c 100644 --- a/ee/app/auditing/server/index.js +++ b/ee/app/auditing/server/index.js @@ -19,7 +19,7 @@ onLicense('auditing', () => { ]; permissions.forEach((permission) => { - Permissions.createOrUpdate(permission._id, permission.roles); + Permissions.create(permission._id, permission.roles); }); defaultRoles.forEach((role) => diff --git a/ee/app/canned-responses/server/permissions.js b/ee/app/canned-responses/server/permissions.js index 4adec4cd7c13..5c8100a514d0 100644 --- a/ee/app/canned-responses/server/permissions.js +++ b/ee/app/canned-responses/server/permissions.js @@ -4,8 +4,8 @@ import { Permissions } from '../../../../app/models'; Meteor.startup(() => { if (Permissions) { - Permissions.createOrUpdate('view-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); - Permissions.createOrUpdate('save-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); - Permissions.createOrUpdate('remove-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); + Permissions.create('view-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); + Permissions.create('save-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); + Permissions.create('remove-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); } }); diff --git a/ee/app/livechat-enterprise/server/permissions.js b/ee/app/livechat-enterprise/server/permissions.js index 79f6b16c7657..f6da9acdb5e0 100644 --- a/ee/app/livechat-enterprise/server/permissions.js +++ b/ee/app/livechat-enterprise/server/permissions.js @@ -39,7 +39,7 @@ export const createPermissions = () => { permissions.map((p) => Permissions.addRole(p, livechatMonitorRole)); - Permissions.createOrUpdate('manage-livechat-units', [adminRole, livechatManagerRole]); - Permissions.createOrUpdate('manage-livechat-monitors', [adminRole, livechatManagerRole]); - Permissions.createOrUpdate('manage-livechat-tags', [adminRole, livechatManagerRole]); + Permissions.create('manage-livechat-units', [adminRole, livechatManagerRole]); + Permissions.create('manage-livechat-monitors', [adminRole, livechatManagerRole]); + Permissions.create('manage-livechat-tags', [adminRole, livechatManagerRole]); }; From 8f35512d87e0b67ff64b0f4b00333f66f3925390 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sat, 4 Apr 2020 23:40:23 -0300 Subject: [PATCH 03/12] Rename files for the TS commit --- app/settings/client/{index.js => index.ts} | 0 app/settings/client/lib/{settings.js => settings.ts} | 0 app/settings/lib/{settings.js => settings.ts} | 0 app/settings/server/functions/{settings.js => settings.ts} | 0 app/settings/server/{index.js => index.ts} | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename app/settings/client/{index.js => index.ts} (100%) rename app/settings/client/lib/{settings.js => settings.ts} (100%) rename app/settings/lib/{settings.js => settings.ts} (100%) rename app/settings/server/functions/{settings.js => settings.ts} (100%) rename app/settings/server/{index.js => index.ts} (100%) diff --git a/app/settings/client/index.js b/app/settings/client/index.ts similarity index 100% rename from app/settings/client/index.js rename to app/settings/client/index.ts diff --git a/app/settings/client/lib/settings.js b/app/settings/client/lib/settings.ts similarity index 100% rename from app/settings/client/lib/settings.js rename to app/settings/client/lib/settings.ts diff --git a/app/settings/lib/settings.js b/app/settings/lib/settings.ts similarity index 100% rename from app/settings/lib/settings.js rename to app/settings/lib/settings.ts diff --git a/app/settings/server/functions/settings.js b/app/settings/server/functions/settings.ts similarity index 100% rename from app/settings/server/functions/settings.js rename to app/settings/server/functions/settings.ts diff --git a/app/settings/server/index.js b/app/settings/server/index.ts similarity index 100% rename from app/settings/server/index.js rename to app/settings/server/index.ts From 9bbf059eb9ef3279ef1ab73f764a1d57717f3f3c Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sat, 4 Apr 2020 23:43:01 -0300 Subject: [PATCH 04/12] Convert Settings to TS # Conflicts: # app/settings/client/lib/settings.ts # app/settings/lib/settings.ts # app/settings/server/functions/settings.ts --- app/settings/client/lib/settings.ts | 70 +-- app/settings/index.js | 4 +- app/settings/lib/settings.ts | 84 ++- app/settings/server/functions/settings.d.ts | 4 - app/settings/server/functions/settings.ts | 527 +++++++++--------- .../client/models/CachedCollection.js | 16 +- client/main.d.ts | 12 + 7 files changed, 392 insertions(+), 325 deletions(-) delete mode 100644 app/settings/server/functions/settings.d.ts create mode 100644 client/main.d.ts diff --git a/app/settings/client/lib/settings.ts b/app/settings/client/lib/settings.ts index bd824705baec..712eadc6c37d 100644 --- a/app/settings/client/lib/settings.ts +++ b/app/settings/client/lib/settings.ts @@ -2,45 +2,49 @@ import { Meteor } from 'meteor/meteor'; import { ReactiveDict } from 'meteor/reactive-dict'; import { CachedCollection } from '../../../ui-cached-collection'; -import { settings } from '../../lib/settings'; +import { SettingsBase, SettingValue } from '../../lib/settings'; -settings.cachedCollection = new CachedCollection({ +const cachedCollection = new CachedCollection({ name: 'public-settings', eventType: 'onAll', userRelated: false, listenChangesForLoggedUsersOnly: true, }); -settings.collection = settings.cachedCollection.collection; - -settings.dict = new ReactiveDict('settings'); - -settings.get = function(_id) { - return settings.dict.get(_id); -}; - -settings.init = function() { - let initialLoad = true; - settings.collection.find().observe({ - added(record) { - Meteor.settings[record._id] = record.value; - settings.dict.set(record._id, record.value); - settings.load(record._id, record.value, initialLoad); - }, - changed(record) { - Meteor.settings[record._id] = record.value; - settings.dict.set(record._id, record.value); - settings.load(record._id, record.value, initialLoad); - }, - removed(record) { - delete Meteor.settings[record._id]; - settings.dict.set(record._id, null); - settings.load(record._id, null, initialLoad); - }, - }); - initialLoad = false; -}; +class Settings extends SettingsBase { + cachedCollection = cachedCollection + + collection = cachedCollection.collection; + + dict = new ReactiveDict('settings'); + + get(_id: string): any { + return this.dict.get(_id); + } + + init(): void { + let initialLoad = true; + this.collection.find().observe({ + added: (record: {_id: string; value: SettingValue}) => { + Meteor.settings[record._id] = record.value; + this.dict.set(record._id, record.value); + this.load(record._id, record.value, initialLoad); + }, + changed: (record: {_id: string; value: SettingValue}) => { + Meteor.settings[record._id] = record.value; + this.dict.set(record._id, record.value); + this.load(record._id, record.value, initialLoad); + }, + removed: (record: {_id: string}) => { + delete Meteor.settings[record._id]; + this.dict.set(record._id, null); + this.load(record._id, undefined, initialLoad); + }, + }); + initialLoad = false; + } +} + +export const settings = new Settings(); settings.init(); - -export { settings }; diff --git a/app/settings/index.js b/app/settings/index.js index a67eca871efb..e4c33cf37ac3 100644 --- a/app/settings/index.js +++ b/app/settings/index.js @@ -1,8 +1,8 @@ import { Meteor } from 'meteor/meteor'; if (Meteor.isClient) { - module.exports = require('./client/index.js'); + module.exports = require('./client/index'); } if (Meteor.isServer) { - module.exports = require('./server/index.js'); + module.exports = require('./server/index'); } diff --git a/app/settings/lib/settings.ts b/app/settings/lib/settings.ts index ab5595f90ac9..d06e889619ff 100644 --- a/app/settings/lib/settings.ts +++ b/app/settings/lib/settings.ts @@ -1,13 +1,33 @@ import { Meteor } from 'meteor/meteor'; import _ from 'underscore'; -export const settings = { - callbacks: {}, - regexCallbacks: {}, - ts: new Date(), - get(_id, callback) { +export type SettingValueMultiSelect = Array<{key: string; i18nLabel: string}> +export type SettingValueRoomPick = Array<{_id: string; name: string}> | string +export type SettingValue = string | boolean | number | SettingValueMultiSelect | undefined; +export type SettingComposedValue = {key: string; value: SettingValue}; +export type SettingCallback = (key: string, value: SettingValue, initialLoad?: boolean) => void; + +interface ISettingCallbacks { + [key: string]: SettingCallback[]; +} + +interface ISettingRegexCallbacks { + [key: string]: { + regex: RegExp; + callbacks: SettingCallback[]; + }; +} + +export class SettingsBase { + private callbacks: ISettingCallbacks = {} + + private regexCallbacks: ISettingRegexCallbacks = {} + + // private ts = new Date() + + public get(_id: string, callback?: SettingCallback): SettingValue | SettingComposedValue[] | void { if (callback != null) { - settings.onload(_id, callback); + this.onload(_id, callback); if (!Meteor.settings) { return; } @@ -26,13 +46,16 @@ export const settings = { callback(key, value); }); } + return Meteor.settings[_id] != null && callback(_id, Meteor.settings[_id]); } + if (!Meteor.settings) { return; } + if (_.isRegExp(_id)) { - return Object.keys(Meteor.settings).reduce((items, key) => { + return Object.keys(Meteor.settings).reduce((items: SettingComposedValue[], key) => { const value = Meteor.settings[key]; if (_id.test(key)) { items.push({ @@ -43,46 +66,51 @@ export const settings = { return items; }, []); } + return Meteor.settings && Meteor.settings[_id]; - }, - set(_id, value, callback) { - return Meteor.call('saveSetting', _id, value, callback); - }, - batchSet(settings, callback) { - return Meteor.call('saveSettings', settings, callback); - }, - load(key, value, initialLoad) { + } + + set(_id: string, value: SettingValue, callback: () => void): void { + Meteor.call('saveSetting', _id, value, callback); + } + + batchSet(settings: Array<{_id: string; value: SettingValue}>, callback: () => void): void { + Meteor.call('saveSettings', settings, callback); + } + + load(key: string, value: SettingValue, initialLoad: boolean): void { ['*', key].forEach((item) => { - if (settings.callbacks[item]) { - settings.callbacks[item].forEach((callback) => callback(key, value, initialLoad)); + if (this.callbacks[item]) { + this.callbacks[item].forEach((callback) => callback(key, value, initialLoad)); } }); - Object.keys(settings.regexCallbacks).forEach((cbKey) => { - const cbValue = settings.regexCallbacks[cbKey]; + Object.keys(this.regexCallbacks).forEach((cbKey) => { + const cbValue = this.regexCallbacks[cbKey]; if (!cbValue.regex.test(key)) { return; } cbValue.callbacks.forEach((callback) => callback(key, value, initialLoad)); }); - }, - onload(key, callback) { + } + + onload(key: string | string[] | RegExp | RegExp[], callback: SettingCallback): void { // if key is '*' // for key, value in Meteor.settings // callback key, value, false // else if Meteor.settings?[_id]? // callback key, Meteor.settings[_id], false - const keys = [].concat(key); + const keys: Array = Array.isArray(key) ? key : [key]; keys.forEach((k) => { if (_.isRegExp(k)) { - settings.regexCallbacks[name = k.source] = settings.regexCallbacks[name = k.source] || { + this.regexCallbacks[k.source] = this.regexCallbacks[k.source] || { regex: k, callbacks: [], }; - settings.regexCallbacks[k.source].callbacks.push(callback); + this.regexCallbacks[k.source].callbacks.push(callback); } else { - settings.callbacks[k] = settings.callbacks[k] || []; - settings.callbacks[k].push(callback); + this.callbacks[k] = this.callbacks[k] || []; + this.callbacks[k].push(callback); } }); - }, -}; + } +} diff --git a/app/settings/server/functions/settings.d.ts b/app/settings/server/functions/settings.d.ts deleted file mode 100644 index aab7040d6ccd..000000000000 --- a/app/settings/server/functions/settings.d.ts +++ /dev/null @@ -1,4 +0,0 @@ -export namespace settings { - export function get(name: string, callback?: (key: string, value: any) => void): string; - export function updateById(_id: string, value: any, editor?: string): number; -} diff --git a/app/settings/server/functions/settings.ts b/app/settings/server/functions/settings.ts index 300e513c988a..270e5e487c08 100644 --- a/app/settings/server/functions/settings.ts +++ b/app/settings/server/functions/settings.ts @@ -1,35 +1,33 @@ import { Meteor } from 'meteor/meteor'; import _ from 'underscore'; -import { settings } from '../../lib/settings'; -import Settings from '../../../models/server/models/Settings'; +import { SettingsBase, SettingValue } from '../../lib/settings'; +import SettingsModel from '../../../models/server/models/Settings'; -const blockedSettings = {}; +const blockedSettings = new Set(); +const hiddenSettings = new Set(); if (process.env.SETTINGS_BLOCKED) { - process.env.SETTINGS_BLOCKED.split(',').forEach((settingId) => { blockedSettings[settingId] = 1; }); + process.env.SETTINGS_BLOCKED.split(',').forEach((settingId) => blockedSettings.add(settingId.trim())); } -const hiddenSettings = {}; if (process.env.SETTINGS_HIDDEN) { - process.env.SETTINGS_HIDDEN.split(',').forEach((settingId) => { hiddenSettings[settingId] = 1; }); + process.env.SETTINGS_HIDDEN.split(',').forEach((settingId) => hiddenSettings.add(settingId.trim())); } -settings._sorter = {}; - -const overrideSetting = (_id, value, options) => { - if (typeof process !== 'undefined' && process.env && process.env[_id]) { - value = process.env[_id]; - if (value.toLowerCase() === 'true') { +const overrideSetting = (_id: string, value: SettingValue, options: ISettingAddOptions): SettingValue => { + const envValue = process?.env?.[_id]; + if (envValue) { + if (envValue.toLowerCase() === 'true') { value = true; - } else if (value.toLowerCase() === 'false') { + } else if (envValue.toLowerCase() === 'false') { value = false; } else if (options.type === 'int') { - value = parseInt(value); + value = parseInt(envValue); } options.processEnvValue = value; options.valueSource = 'processEnvValue'; - } else if (Meteor.settings && typeof Meteor.settings[_id] !== 'undefined') { + } else if (typeof Meteor.settings?.[_id] !== 'undefined') { if (Meteor.settings[_id] == null) { return false; } @@ -39,14 +37,14 @@ const overrideSetting = (_id, value, options) => { options.valueSource = 'meteorSettingsValue'; } - if (typeof process !== 'undefined' && process.env && process.env[`OVERWRITE_SETTING_${ _id }`]) { - let value = process.env[`OVERWRITE_SETTING_${ _id }`]; - if (value.toLowerCase() === 'true') { + const meteorValue = process?.env?.[`OVERWRITE_SETTING_${ _id }`]; + if (meteorValue) { + if (meteorValue.toLowerCase() === 'true') { value = true; - } else if (value.toLowerCase() === 'false') { + } else if (meteorValue.toLowerCase() === 'false') { value = false; } else if (options.type === 'int') { - value = parseInt(value); + value = parseInt(meteorValue); } options.value = value; options.processEnvValue = value; @@ -56,273 +54,302 @@ const overrideSetting = (_id, value, options) => { return value; }; +export interface ISettingAddOptions { + _id?: string; + type?: 'group' | 'boolean' | 'int' | 'string' | 'asset' | 'code' | 'select' | 'password' | 'action' | 'relativeUrl' | 'language' | 'date' | 'color' | 'font' | 'roomPick' | 'multiSelect'; + editor?: string; + packageEditor?: string; + packageValue?: SettingValue; + valueSource?: string; + hidden?: boolean; + blocked?: boolean; + secret?: boolean; + sorter?: number; + i18nLabel?: string; + i18nDescription?: string; + autocomplete?: boolean; + force?: boolean; + group?: string; + section?: string; + enableQuery?: any; + processEnvValue?: SettingValue; + meteorSettingsValue?: SettingValue; + value?: SettingValue; + ts?: Date; +} -/* -* Add a setting -* @param {String} _id -* @param {Mixed} value -* @param {Object} setting -*/ +export interface ISettingAddGroupOptions { + hidden?: boolean; + blocked?: boolean; + ts?: Date; + i18nLabel?: string; + i18nDescription?: string; +} -settings.add = function(_id, value, { editor, ...options } = {}) { - if (!_id || value == null) { - return false; - } - if (settings._sorter[options.group] == null) { - settings._sorter[options.group] = 0; - } - options.packageValue = value; - options.valueSource = 'packageValue'; - options.hidden = options.hidden || false; - options.blocked = options.blocked || false; - options.secret = options.secret || false; - if (options.sorter == null) { - options.sorter = settings._sorter[options.group]++; - } - if (options.enableQuery != null) { - options.enableQuery = JSON.stringify(options.enableQuery); - } - if (options.i18nDefaultQuery != null) { - options.i18nDefaultQuery = JSON.stringify(options.i18nDefaultQuery); - } - if (options.i18nLabel == null) { - options.i18nLabel = _id; - } - if (options.i18nDescription == null) { - options.i18nDescription = `${ _id }_Description`; - } - if (blockedSettings[_id] != null) { - options.blocked = true; - } - if (hiddenSettings[_id] != null) { - options.hidden = true; - } - if (options.autocomplete == null) { - options.autocomplete = true; - } +interface IUpdateOperator { + $set: ISettingAddOptions; + $setOnInsert: ISettingAddOptions & { + createdAt: Date; + }; + $unset?: { + section?: 1; + }; +} - value = overrideSetting(_id, value, options); +type QueryExpression = { + $exists: boolean; +} - const updateOperations = { - $set: options, - $setOnInsert: { - createdAt: new Date(), - }, - }; - if (editor != null) { - updateOperations.$setOnInsert.editor = editor; - updateOperations.$setOnInsert.packageEditor = editor; - } - if (options.value == null) { - if (options.force === true) { - updateOperations.$set.value = options.packageValue; - } else { - updateOperations.$setOnInsert.value = value; +type Query = { + [P in keyof T]?: T[P] | QueryExpression; +} + +class Settings extends SettingsBase { + private afterInitialLoad: Array<(settings: Meteor.Settings) => void> = []; + + private _sorter: {[key: string]: number} = {}; + + private initialLoad = false; + + /* + * Add a setting + */ + add(_id: string, value: SettingValue, { editor, ...options }: ISettingAddOptions = {}): boolean { + if (!_id || value == null) { + return false; + } + if (options.group && this._sorter[options.group] == null) { + this._sorter[options.group] = 0; + } + options.packageValue = value; + options.valueSource = 'packageValue'; + options.hidden = options.hidden || false; + options.blocked = options.blocked || false; + options.secret = options.secret || false; + if (options.group && options.sorter == null) { + options.sorter = this._sorter[options.group]++; + } + if (options.enableQuery != null) { + options.enableQuery = JSON.stringify(options.enableQuery); + } + if (options.i18nLabel == null) { + options.i18nLabel = _id; + } + if (options.i18nDescription == null) { + options.i18nDescription = `${ _id }_Description`; + } + if (blockedSettings.has(_id)) { + options.blocked = true; + } + if (hiddenSettings.has(_id)) { + options.hidden = true; + } + if (options.autocomplete == null) { + options.autocomplete = true; } - } - const query = { - _id, - ...updateOperations.$set, - }; + value = overrideSetting(_id, value, options); - if (options.section == null) { - updateOperations.$unset = { - section: 1, - }; - query.section = { - $exists: false, + const updateOperations: IUpdateOperator = { + $set: options, + $setOnInsert: { + createdAt: new Date(), + }, }; - } + if (editor != null) { + updateOperations.$setOnInsert.editor = editor; + updateOperations.$setOnInsert.packageEditor = editor; + } - const existentSetting = Settings.db.findOne(query); - if (existentSetting) { - if (existentSetting.editor || !updateOperations.$setOnInsert.editor) { - return; + if (options.value == null) { + if (options.force === true) { + updateOperations.$set.value = options.packageValue; + } else { + updateOperations.$setOnInsert.value = value; + } } - updateOperations.$set.editor = updateOperations.$setOnInsert.editor; - delete updateOperations.$setOnInsert.editor; - } + const query: Query = { + _id, + ...updateOperations.$set, + }; - updateOperations.$set.ts = new Date(); + if (options.section == null) { + updateOperations.$unset = { + section: 1, + }; + query.section = { + $exists: false, + }; + } - return Settings.upsert({ - _id, - }, updateOperations); -}; + const existentSetting = SettingsModel.db.findOne(query); + if (existentSetting) { + if (existentSetting.editor || !updateOperations.$setOnInsert.editor) { + return true; + } + updateOperations.$set.editor = updateOperations.$setOnInsert.editor; + delete updateOperations.$setOnInsert.editor; + } -/* -* Add a setting group -* @param {String} _id -*/ + updateOperations.$set.ts = new Date(); -settings.addGroup = function(_id, options = {}, cb) { - if (!_id) { - return false; - } - if (_.isFunction(options)) { - cb = options; - options = {}; - } - if (options.i18nLabel == null) { - options.i18nLabel = _id; - } - if (options.i18nDescription == null) { - options.i18nDescription = `${ _id }_Description`; + SettingsModel.upsert({ + _id, + }, updateOperations); + return true; } - options.blocked = false; - options.hidden = false; - if (blockedSettings[_id] != null) { - options.blocked = true; - } - if (hiddenSettings[_id] != null) { - options.hidden = true; - } + /* + * Add a setting group + */ + addGroup(_id: string, cb: () => void): boolean; - const existentGroup = Settings.findOne({ - _id, - type: 'group', - ...options, - }); + // eslint-disable-next-line no-dupe-class-members + addGroup(_id: string, options: ISettingAddGroupOptions | (() => void) = {}, cb?: () => void): boolean { + if (!_id) { + return false; + } + if (_.isFunction(options)) { + cb = options; + options = {}; + } + if (options.i18nLabel == null) { + options.i18nLabel = _id; + } + if (options.i18nDescription == null) { + options.i18nDescription = `${ _id }_Description`; + } - if (!existentGroup) { - options.ts = new Date(); + options.blocked = false; + options.hidden = false; + if (blockedSettings.has(_id)) { + options.blocked = true; + } + if (hiddenSettings.has(_id)) { + options.hidden = true; + } - Settings.upsert({ + const existentGroup = SettingsModel.findOne({ _id, - }, { - $set: options, - $setOnInsert: { - type: 'group', - createdAt: new Date(), - }, + type: 'group', + ...options, }); - } - if (cb != null) { - cb.call({ - add(id, value, options) { - if (options == null) { - options = {}; - } - options.group = _id; - return settings.add(id, value, options); - }, - section(section, cb) { - return cb.call({ - add(id, value, options) { - if (options == null) { - options = {}; - } + if (!existentGroup) { + options.ts = new Date(); + + SettingsModel.upsert({ + _id, + }, { + $set: options, + $setOnInsert: { + type: 'group', + createdAt: new Date(), + }, + }); + } + + if (cb != null) { + cb.call({ + add: (id: string, value: SettingValue, options: ISettingAddOptions = {}) => { + options.group = _id; + return this.add(id, value, options); + }, + section: (section: string, cb: () => void) => cb.call({ + add: (id: string, value: SettingValue, options: ISettingAddOptions = {}) => { options.group = _id; options.section = section; - return settings.add(id, value, options); + return this.add(id, value, options); }, - }); - }, - }); + }), + }); + } + return true; } -}; - -/* -* Remove a setting by id -* @param {String} _id -*/ - -settings.removeById = function(_id) { - if (!_id) { - return false; + /* + * Remove a setting by id + */ + removeById(_id: string): boolean { + if (!_id) { + return false; + } + return SettingsModel.removeById(_id); } - return Settings.removeById(_id); -}; - -/* -* Update a setting by id -* @param {String} _id -*/ - -settings.updateById = function(_id, value, editor) { - if (!_id || value == null) { - return false; - } - if (editor != null) { - return Settings.updateValueAndEditorById(_id, value, editor); + /* + * Update a setting by id + */ + updateById(_id: string, value: SettingValue, editor: string): boolean { + if (!_id || value == null) { + return false; + } + if (editor != null) { + return SettingsModel.updateValueAndEditorById(_id, value, editor); + } + return SettingsModel.updateValueById(_id, value); } - return Settings.updateValueById(_id, value); -}; - -/* -* Update options of a setting by id -* @param {String} _id -*/ - -settings.updateOptionsById = function(_id, options) { - if (!_id || options == null) { - return false; + /* + * Update options of a setting by id + */ + updateOptionsById(_id: string, options: object): boolean { + if (!_id || options == null) { + return false; + } + return SettingsModel.updateOptionsById(_id, options); } - return Settings.updateOptionsById(_id, options); -}; - - -/* -* Update a setting by id -* @param {String} _id -*/ -settings.clearById = function(_id) { - if (_id == null) { - return false; + /* + * Update a setting by id + */ + clearById(_id: string): boolean { + if (_id == null) { + return false; + } + return SettingsModel.updateValueById(_id, undefined); } - return Settings.updateValueById(_id, undefined); -}; - -/* -* Update a setting by id -*/ - -settings.init = function() { - settings.initialLoad = true; - Settings.find().observe({ - added(record) { - Meteor.settings[record._id] = record.value; - if (record.env === true) { - process.env[record._id] = record.value; - } - return settings.load(record._id, record.value, settings.initialLoad); - }, - changed(record) { - Meteor.settings[record._id] = record.value; - if (record.env === true) { - process.env[record._id] = record.value; - } - return settings.load(record._id, record.value, settings.initialLoad); - }, - removed(record) { - delete Meteor.settings[record._id]; - if (record.env === true) { - delete process.env[record._id]; - } - return settings.load(record._id, undefined, settings.initialLoad); - }, - }); - settings.initialLoad = false; - settings.afterInitialLoad.forEach((fn) => fn(Meteor.settings)); -}; - -settings.afterInitialLoad = []; + /* + * Update a setting by id + */ + init(): void { + this.initialLoad = true; + SettingsModel.find().observe({ + added: (record: {_id: string; env: boolean; value: SettingValue}) => { + Meteor.settings[record._id] = record.value; + if (record.env === true) { + process.env[record._id] = String(record.value); + } + return this.load(record._id, record.value, this.initialLoad); + }, + changed: (record: {_id: string; env: boolean; value: SettingValue}) => { + Meteor.settings[record._id] = record.value; + if (record.env === true) { + process.env[record._id] = String(record.value); + } + return this.load(record._id, record.value, this.initialLoad); + }, + removed: (record: {_id: string; env: boolean}) => { + delete Meteor.settings[record._id]; + if (record.env === true) { + delete process.env[record._id]; + } + return this.load(record._id, undefined, this.initialLoad); + }, + }); + this.initialLoad = false; + this.afterInitialLoad.forEach((fn) => fn(Meteor.settings)); + } -settings.onAfterInitialLoad = function(fn) { - settings.afterInitialLoad.push(fn); - if (settings.initialLoad === false) { - return fn(Meteor.settings); + onAfterInitialLoad(fn: (settings: Meteor.Settings) => void): void { + this.afterInitialLoad.push(fn); + if (this.initialLoad === false) { + fn(Meteor.settings); + } } -}; +} -export { settings }; +export const settings = new Settings(); diff --git a/app/ui-cached-collection/client/models/CachedCollection.js b/app/ui-cached-collection/client/models/CachedCollection.js index f6c21c91049a..f9a2a7f17524 100644 --- a/app/ui-cached-collection/client/models/CachedCollection.js +++ b/app/ui-cached-collection/client/models/CachedCollection.js @@ -122,11 +122,11 @@ const log = (...args) => console.log(`CachedCollection ${ this.name } =>`, ...ar export class CachedCollection extends EventEmitter { constructor({ - collection, + collection = new Mongo.Collection(null), name, - methodName, - syncMethodName, - eventName, + methodName = `${ name }/get`, + syncMethodName = `${ name }/get`, + eventName = `${ name }-changed`, eventType = 'onUser', userRelated = true, listenChangesForLoggedUsersOnly = false, @@ -136,13 +136,13 @@ export class CachedCollection extends EventEmitter { onSyncData = (/* action, record */) => {}, }) { super(); - this.collection = collection || new Mongo.Collection(null); + this.collection = collection; this.ready = new ReactiveVar(false); this.name = name; - this.methodName = methodName || `${ name }/get`; - this.syncMethodName = syncMethodName || `${ name }/get`; - this.eventName = eventName || `${ name }-changed`; + this.methodName = methodName; + this.syncMethodName = syncMethodName; + this.eventName = eventName; this.eventType = eventType; this.useSync = useSync; this.listenChangesForLoggedUsersOnly = listenChangesForLoggedUsersOnly; diff --git a/client/main.d.ts b/client/main.d.ts new file mode 100644 index 000000000000..fbcc7568aa94 --- /dev/null +++ b/client/main.d.ts @@ -0,0 +1,12 @@ +/* eslint-disable @typescript-eslint/interface-name-prefix */ + +declare module 'meteor/reactive-dict' { + const ReactiveDict: ReactiveDictStatic; + interface ReactiveDictStatic { + new (name: string, initialValue?: T): ReactiveDict; + } + interface ReactiveDict { + get(name: string): T; + set(name: string, newValue: T): void; + } +} From dd6932a712ceecb3d7edda1ac9bfa8dbb46760ad Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sat, 4 Apr 2020 23:57:57 -0300 Subject: [PATCH 05/12] Show stack on API error when testing --- app/api/server/api.js | 2 +- app/api/server/v1/chat.js | 2 +- app/settings/index.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/api/server/api.js b/app/api/server/api.js index eab220650d6b..bc1232b0ec0b 100644 --- a/app/api/server/api.js +++ b/app/api/server/api.js @@ -373,7 +373,7 @@ export class APIClass extends Restivus { 'error-unauthorized': 'unauthorized', }[e.error] || 'failure'; - result = API.v1[apiMethod](typeof e === 'string' ? e : e.message, e.error, undefined, e); + result = API.v1[apiMethod](typeof e === 'string' ? e : e.message, e.error, process.env.TEST_MODE ? e.stack : undefined, e); } finally { delete Accounts._accountData[connection.id]; } diff --git a/app/api/server/v1/chat.js b/app/api/server/v1/chat.js index 8e5ac669d229..69685a3f6f97 100644 --- a/app/api/server/v1/chat.js +++ b/app/api/server/v1/chat.js @@ -4,7 +4,7 @@ import { Match, check } from 'meteor/check'; import { Messages } from '../../../models'; import { canAccessRoom, hasPermission } from '../../../authorization'; import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser'; -import { processWebhookMessage } from '../../../lib'; +import { processWebhookMessage } from '../../../lib/server'; import { API } from '../api'; import Rooms from '../../../models/server/models/Rooms'; import Users from '../../../models/server/models/Users'; diff --git a/app/settings/index.js b/app/settings/index.js index e4c33cf37ac3..1f3257e45429 100644 --- a/app/settings/index.js +++ b/app/settings/index.js @@ -1,8 +1,8 @@ import { Meteor } from 'meteor/meteor'; if (Meteor.isClient) { - module.exports = require('./client/index'); + module.exports = require('./client/index.ts'); } if (Meteor.isServer) { - module.exports = require('./server/index'); + module.exports = require('./server/index.ts'); } From dde16b0f5cb78f3d009d39d16deb7db5643c76f9 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sun, 5 Apr 2020 00:33:03 -0300 Subject: [PATCH 06/12] Fix tests --- app/lib/server/functions/createDirectRoom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/server/functions/createDirectRoom.js b/app/lib/server/functions/createDirectRoom.js index c8b5cd5cf081..2e1c569cbef4 100644 --- a/app/lib/server/functions/createDirectRoom.js +++ b/app/lib/server/functions/createDirectRoom.js @@ -1,5 +1,5 @@ import { Rooms, Subscriptions } from '../../../models/server'; -import { settings } from '../../../settings/lib/settings'; +import { settings } from '../../../settings/server'; import { getDefaultSubscriptionPref } from '../../../utils/server'; import { callbacks } from '../../../callbacks/server'; From 2f73e1b9ac61337d1d8a2cdc097943583e9efc12 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sat, 11 Apr 2020 17:30:45 -0300 Subject: [PATCH 07/12] Add unit tests --- .../server/functions/settings.mocks.ts | 33 +++ .../server/functions/settings.tests.ts | 206 ++++++++++++++++++ app/settings/server/functions/settings.ts | 27 ++- mocha.opts | 4 + package-lock.json | 78 ++++++- package.json | 20 +- tsconfig.json | 2 +- 7 files changed, 346 insertions(+), 24 deletions(-) create mode 100644 app/settings/server/functions/settings.mocks.ts create mode 100644 app/settings/server/functions/settings.tests.ts diff --git a/app/settings/server/functions/settings.mocks.ts b/app/settings/server/functions/settings.mocks.ts new file mode 100644 index 000000000000..16bad40274cb --- /dev/null +++ b/app/settings/server/functions/settings.mocks.ts @@ -0,0 +1,33 @@ +import mock from 'mock-require'; + +type Dictionary = { + [index: string]: any; +} + +class SettingsClass { + public data = new Map() + + public upsertCalls = 0; + + findOne(query: Dictionary): any { + return [...this.data.values()].find((data) => Object.entries(query).every(([key, value]) => data[key] === value)); + } + + upsert(query: any, update: any): void { + const existent = this.findOne(query); + + const data = { ...existent, ...query, ...update.$set }; + + if (!existent) { + Object.assign(data, update.$setOnInsert); + } + + // console.log(query, data); + this.data.set(query._id, data); + this.upsertCalls++; + } +} + +export const Settings = new SettingsClass(); + +mock('../../../models/server/models/Settings', Settings); diff --git a/app/settings/server/functions/settings.tests.ts b/app/settings/server/functions/settings.tests.ts new file mode 100644 index 000000000000..1a99c9e669fc --- /dev/null +++ b/app/settings/server/functions/settings.tests.ts @@ -0,0 +1,206 @@ +/* eslint-disable @typescript-eslint/camelcase */ +/* eslint-env mocha */ +import { Meteor } from 'meteor/meteor'; +import { expect } from 'chai'; + +import { Settings } from './settings.mocks'; +import { settings } from './settings'; + +describe('Settings', () => { + beforeEach(() => { + Settings.upsertCalls = 0; + Settings.data.clear(); + Meteor.settings = { public: {} }; + process.env = {}; + }); + + it('should not insert the same setting twice', () => { + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', true, { + type: 'boolean', + sorter: 0, + }); + }); + }); + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' })).to.be.include({ + type: 'boolean', + sorter: 0, + group: 'group', + section: 'section', + packageValue: true, + value: true, + valueSource: 'packageValue', + hidden: false, + blocked: false, + secret: false, + i18nLabel: 'my_setting', + i18nDescription: 'my_setting_Description', + autocomplete: true, + }); + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', true, { + type: 'boolean', + sorter: 0, + }); + }); + }); + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' }).value).to.be.equal(true); + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting2', false, { + type: 'boolean', + sorter: 0, + }); + }); + }); + + expect(Settings.data.size).to.be.equal(3); + expect(Settings.upsertCalls).to.be.equal(3); + expect(Settings.findOne({ _id: 'my_setting' }).value).to.be.equal(true); + expect(Settings.findOne({ _id: 'my_setting2' }).value).to.be.equal(false); + }); + + it('should respect override via environment', () => { + process.env.OVERWRITE_SETTING_my_setting = 'false'; + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', true, { + type: 'boolean', + sorter: 0, + }); + }); + }); + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' })).to.include({ + value: false, + processEnvValue: false, + valueSource: 'processEnvValue', + type: 'boolean', + sorter: 0, + group: 'group', + section: 'section', + packageValue: true, + hidden: false, + blocked: false, + secret: false, + i18nLabel: 'my_setting', + i18nDescription: 'my_setting_Description', + autocomplete: true, + }); + }); + + it('should respect initial value via environment', () => { + process.env.my_setting = '1'; + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + const expectedSetting = { + value: 1, + processEnvValue: 1, + valueSource: 'processEnvValue', + type: 'int', + sorter: 0, + group: 'group', + section: 'section', + packageValue: 0, + hidden: false, + blocked: false, + secret: false, + i18nLabel: 'my_setting', + i18nDescription: 'my_setting_Description', + autocomplete: true, + }; + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); + + process.env.my_setting = '2'; + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + expectedSetting.processEnvValue = 2; + + 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 respect initial value via Meteor.settings', () => { + Meteor.settings.my_setting = 1; + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + const expectedSetting = { + value: 1, + meteorSettingsValue: 1, + valueSource: 'meteorSettingsValue', + type: 'int', + sorter: 0, + group: 'group', + section: 'section', + packageValue: 0, + hidden: false, + blocked: false, + secret: false, + i18nLabel: 'my_setting', + i18nDescription: 'my_setting_Description', + autocomplete: true, + }; + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); + + Meteor.settings.my_setting = 2; + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + expectedSetting.meteorSettingsValue = 2; + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(3); + expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); + }); +}); diff --git a/app/settings/server/functions/settings.ts b/app/settings/server/functions/settings.ts index 270e5e487c08..2265e9c5f225 100644 --- a/app/settings/server/functions/settings.ts +++ b/app/settings/server/functions/settings.ts @@ -37,14 +37,14 @@ const overrideSetting = (_id: string, value: SettingValue, options: ISettingAddO options.valueSource = 'meteorSettingsValue'; } - const meteorValue = process?.env?.[`OVERWRITE_SETTING_${ _id }`]; - if (meteorValue) { - if (meteorValue.toLowerCase() === 'true') { + const overwriteValue = process?.env?.[`OVERWRITE_SETTING_${ _id }`]; + if (overwriteValue) { + if (overwriteValue.toLowerCase() === 'true') { value = true; - } else if (meteorValue.toLowerCase() === 'false') { + } else if (overwriteValue.toLowerCase() === 'false') { value = false; } else if (options.type === 'int') { - value = parseInt(meteorValue); + value = parseInt(overwriteValue); } options.value = value; options.processEnvValue = value; @@ -104,6 +104,15 @@ type Query = { [P in keyof T]?: T[P] | QueryExpression; } +type addSectionCallback = (this: { + add(id: string, value: SettingValue, options: ISettingAddOptions): void; +}) => void; + +type addGroupCallback = (this: { + add(id: string, value: SettingValue, options: ISettingAddOptions): void; + section(section: string, cb: addSectionCallback): void; +}) => void; + class Settings extends SettingsBase { private afterInitialLoad: Array<(settings: Meteor.Settings) => void> = []; @@ -183,7 +192,7 @@ class Settings extends SettingsBase { }; } - const existentSetting = SettingsModel.db.findOne(query); + const existentSetting = SettingsModel.findOne(query); if (existentSetting) { if (existentSetting.editor || !updateOperations.$setOnInsert.editor) { return true; @@ -204,10 +213,10 @@ class Settings extends SettingsBase { /* * Add a setting group */ - addGroup(_id: string, cb: () => void): boolean; + addGroup(_id: string, cb: addGroupCallback): boolean; // eslint-disable-next-line no-dupe-class-members - addGroup(_id: string, options: ISettingAddGroupOptions | (() => void) = {}, cb?: () => void): boolean { + addGroup(_id: string, options: ISettingAddGroupOptions | addGroupCallback = {}, cb?: addGroupCallback): boolean { if (!_id) { return false; } @@ -257,7 +266,7 @@ class Settings extends SettingsBase { options.group = _id; return this.add(id, value, options); }, - section: (section: string, cb: () => void) => cb.call({ + section: (section: string, cb: addSectionCallback) => cb.call({ add: (id: string, value: SettingValue, options: ISettingAddOptions = {}) => { options.group = _id; options.section = section; diff --git a/mocha.opts b/mocha.opts index 98d12fa40c7b..a2b834891a7d 100644 --- a/mocha.opts +++ b/mocha.opts @@ -1,4 +1,8 @@ +--require ts-node/register --require babel-mocha-es6-compiler --require babel-polyfill --reporter spec --ui bdd +--watch-extensions ts +--extension ts +app/**/*.tests.js app/**/*.tests.ts diff --git a/package-lock.json b/package-lock.json index b873b3375689..d889e2225174 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "Rocket.Chat", - "version": "3.1.0-develop", + "version": "3.2.0-develop", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -6112,6 +6112,12 @@ "resolved": "https://registry.npmjs.org/@types/caseless/-/caseless-0.12.1.tgz", "integrity": "sha512-FhlMa34NHp9K5MY1Uz8yb+ZvuX0pnvn3jScRSNAb75KHGB8d3rEU6hqMs3Z2vjuytcMfRg6c5CHMc3wtYyD2/A==" }, + "@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 + }, "@types/color-name": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/@types/color-name/-/color-name-1.1.1.tgz", @@ -6240,6 +6246,21 @@ "resolved": "https://registry.npmjs.org/@types/mime/-/mime-2.0.1.tgz", "integrity": "sha512-FwI9gX75FgVBJ7ywgnq/P7tw+/o1GUbtP0KzbtusLigAOgIgNISRK0ZPl4qertvXSIE8YbsVJueQ90cDt9YYyw==" }, + "@types/mocha": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-7.0.2.tgz", + "integrity": "sha512-ZvO2tAcjmMi8V/5Z3JsyofMe3hasRcaw88cto5etSVMwVQfeivGAlEYmaQgceUSVYFofVjT+ioHsATjdWcFt1w==", + "dev": true + }, + "@types/mock-require": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@types/mock-require/-/mock-require-2.0.0.tgz", + "integrity": "sha512-nOgjoE5bBiDeiA+z41i95makyHUSMWQMOPocP+J67Pqx/68HAXaeWN1NFtrAYYV6LrISIZZ8vKHm/a50k0f6Sg==", + "dev": true, + "requires": { + "@types/node": "*" + } + }, "@types/node": { "version": "9.6.40", "resolved": "https://registry.npmjs.org/@types/node/-/node-9.6.40.tgz", @@ -7093,6 +7114,12 @@ "readable-stream": "^2.0.6" } }, + "arg": { + "version": "4.1.3", + "resolved": "https://registry.npmjs.org/arg/-/arg-4.1.3.tgz", + "integrity": "sha512-58S9QDqG0Xx27YwPSt9fJxivjYl432YCwfDMfZ+71RAqUrZef7LrKQZ3LHLOwCS4FLNBplP533Zx895SeOCHvA==", + "dev": true + }, "argparse": { "version": "1.0.10", "resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.10.tgz", @@ -12932,6 +12959,12 @@ } } }, + "diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true + }, "diffie-hellman": { "version": "5.0.3", "resolved": "https://registry.npmjs.org/diffie-hellman/-/diffie-hellman-5.0.3.tgz", @@ -20428,6 +20461,12 @@ "pify": "^3.0.0" } }, + "make-error": { + "version": "1.3.6", + "resolved": "https://registry.npmjs.org/make-error/-/make-error-1.3.6.tgz", + "integrity": "sha512-s8UhlNe7vPKomQhC1qFelMokr/Sc3AgNbso3n74mVPA5LTZwkB9NlXf4XPamLxJE8h0gh73rM94xvwRT2CVInw==", + "dev": true + }, "make-plural": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/make-plural/-/make-plural-6.1.0.tgz", @@ -29440,6 +29479,37 @@ "integrity": "sha512-UGTRZu1evMw4uTPyYF66/KFd22XiU+jMaIuHrkIHQ2GivAXVlLV0v/vHrpOuTRf9BmpNHi/SO7Vd0rLu0y57jg==", "dev": true }, + "ts-node": { + "version": "8.8.2", + "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-8.8.2.tgz", + "integrity": "sha512-duVj6BpSpUpD/oM4MfhO98ozgkp3Gt9qIp3jGxwU2DFvl/3IRaEAvbLa8G60uS7C77457e/m5TMowjedeRxI1Q==", + "dev": true, + "requires": { + "arg": "^4.1.0", + "diff": "^4.0.1", + "make-error": "^1.1.1", + "source-map-support": "^0.5.6", + "yn": "3.1.1" + }, + "dependencies": { + "source-map": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", + "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==", + "dev": true + }, + "source-map-support": { + "version": "0.5.16", + "resolved": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.5.16.tgz", + "integrity": "sha512-efyLRJDr68D9hBBNIPWFjhpFzURh+KJykQwvMyW5UiZzYwoF6l4YMMDIJJEyFWxWCqfyxLzz6tSfUFR+kXXsVQ==", + "dev": true, + "requires": { + "buffer-from": "^1.0.0", + "source-map": "^0.6.0" + } + } + } + }, "ts-pnp": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/ts-pnp/-/ts-pnp-1.1.5.tgz", @@ -31024,6 +31094,12 @@ } } }, + "yn": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/yn/-/yn-3.1.1.tgz", + "integrity": "sha512-Ux4ygGWsu2c7isFWe8Yu1YluJmqVhxqK2cLXNQA5AcC3QfbGNpM7fu0Y8b/z16pXLnFxZYvWhd3fhBY9DLmC6Q==", + "dev": true + }, "zip-stream": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/zip-stream/-/zip-stream-2.0.1.tgz", diff --git a/package.json b/package.json index cda18917ced2..42fa2c04ad2f 100644 --- a/package.json +++ b/package.json @@ -6,16 +6,6 @@ "name": "Rocket.Chat", "url": "https://rocket.chat/" }, - "mocha": { - "tests": [ - "app/**/*.tests.js" - ], - "files": [ - "app/**/*.mocks.js", - "app/**/*.js", - "!app/**/*.tests.js" - ] - }, "keywords": [ "rocketchat", "rocket", @@ -32,11 +22,11 @@ "stylelint": "stylelint \"app/**/*.css\" \"client/**/*.css\" \"app/**/*.less\" \"client/**/*.less\" \"ee/**/*.less\"", "deploy": "npm run build && pm2 startOrRestart pm2.json", "postinstall": "node .scripts/npm-postinstall.js", - "testunit-watch": "mocha --watch --opts ./mocha.opts \"`node -e \"console.log(require('./package.json').mocha.tests.join(' '))\"`\"", - "coverage": "nyc -r html mocha --opts ./mocha.opts \"`node -e \"console.log(require('./package.json').mocha.tests.join(' '))\"`\"", + "coverage": "nyc -r html mocha --opts ./mocha.opts", "test": "node .scripts/start.js", "testui": "cypress run --project tests", - "testunit": "mocha --opts ./mocha.opts \"`node -e \"console.log(require('./package.json').mocha.tests.join(' '))\"`\"", + "testunit": "mocha --opts ./mocha.opts", + "testunit-watch": "mocha --watch --opts ./mocha.opts", "testapi": "mocha --opts ./mocha_api.opts", "testapps": "mocha --opts ./mocha_apps.opts", "testci": "npm run testapi && npm run testapps && npm run testui", @@ -72,7 +62,10 @@ "@storybook/addons": "^5.2.8", "@storybook/react": "^5.2.8", "@types/bcrypt": "^3.0.0", + "@types/chai": "^4.2.11", "@types/meteor": "^1.4.37", + "@types/mocha": "^7.0.2", + "@types/mock-require": "^2.0.0", "@typescript-eslint/eslint-plugin": "^2.11.0", "@typescript-eslint/parser": "^2.11.0", "acorn": "^6.4.1", @@ -114,6 +107,7 @@ "stylelint": "^9.9.0", "stylelint-order": "^2.0.0", "supertest": "^3.3.0", + "ts-node": "^8.8.2", "typescript": "^3.7.3", "webpack": "^4.29.3" }, diff --git a/tsconfig.json b/tsconfig.json index 84d64aaa3aaa..42fa5fc63b8f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,6 @@ { "compilerOptions": { - "module": "esNext", + "module": "CommonJS", "target": "es2018", "lib": ["esnext", "dom"], From e97536b2ba41ca31e3d2a931420934b3ff22d524 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sat, 11 Apr 2020 17:35:10 -0300 Subject: [PATCH 08/12] Improve overwrite test --- .../server/functions/settings.tests.ts | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/app/settings/server/functions/settings.tests.ts b/app/settings/server/functions/settings.tests.ts index 1a99c9e669fc..e17333f9bfcf 100644 --- a/app/settings/server/functions/settings.tests.ts +++ b/app/settings/server/functions/settings.tests.ts @@ -71,35 +71,55 @@ describe('Settings', () => { }); it('should respect override via environment', () => { - process.env.OVERWRITE_SETTING_my_setting = 'false'; + process.env.OVERWRITE_SETTING_my_setting = '1'; settings.addGroup('group', function() { this.section('section', function() { - this.add('my_setting', true, { - type: 'boolean', + this.add('my_setting', 0, { + type: 'int', sorter: 0, }); }); }); - expect(Settings.data.size).to.be.equal(2); - expect(Settings.upsertCalls).to.be.equal(2); - expect(Settings.findOne({ _id: 'my_setting' })).to.include({ - value: false, - processEnvValue: false, + const expectedSetting = { + value: 1, + processEnvValue: 1, valueSource: 'processEnvValue', - type: 'boolean', + type: 'int', sorter: 0, group: 'group', section: 'section', - packageValue: true, + packageValue: 0, hidden: false, blocked: false, secret: false, i18nLabel: 'my_setting', i18nDescription: 'my_setting_Description', autocomplete: true, + }; + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); + + process.env.OVERWRITE_SETTING_my_setting = '2'; + + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); }); + + expectedSetting.value = 2; + expectedSetting.processEnvValue = 2; + + 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 respect initial value via environment', () => { From a9e1bc730e3899f0c03c963788bcb0d46eccb54d Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Sat, 11 Apr 2020 17:41:20 -0300 Subject: [PATCH 09/12] Add more tests --- .../server/functions/settings.tests.ts | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/app/settings/server/functions/settings.tests.ts b/app/settings/server/functions/settings.tests.ts index e17333f9bfcf..2cc47778628a 100644 --- a/app/settings/server/functions/settings.tests.ts +++ b/app/settings/server/functions/settings.tests.ts @@ -223,4 +223,97 @@ describe('Settings', () => { expect(Settings.upsertCalls).to.be.equal(3); expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); }); + + it('should keep original value if value on code was changed', () => { + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + const expectedSetting = { + value: 0, + valueSource: 'packageValue', + type: 'int', + sorter: 0, + group: 'group', + section: 'section', + packageValue: 0, + hidden: false, + blocked: false, + secret: false, + i18nLabel: 'my_setting', + i18nDescription: 'my_setting_Description', + autocomplete: true, + }; + + expect(Settings.data.size).to.be.equal(2); + 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); + }); + + it('should change group and section', () => { + settings.addGroup('group', function() { + this.section('section', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + const expectedSetting = { + value: 0, + valueSource: 'packageValue', + type: 'int', + sorter: 0, + group: 'group', + section: 'section', + packageValue: 0, + hidden: false, + blocked: false, + secret: false, + i18nLabel: 'my_setting', + i18nDescription: 'my_setting_Description', + autocomplete: true, + }; + + expect(Settings.data.size).to.be.equal(2); + expect(Settings.upsertCalls).to.be.equal(2); + expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); + + settings.addGroup('group2', function() { + this.section('section2', function() { + this.add('my_setting', 0, { + type: 'int', + sorter: 0, + }); + }); + }); + + expectedSetting.group = 'group2'; + expectedSetting.section = 'section2'; + + expect(Settings.data.size).to.be.equal(3); + expect(Settings.upsertCalls).to.be.equal(4); + expect(Settings.findOne({ _id: 'my_setting' })).to.include(expectedSetting); + }); }); From c1e1f85a943288c20bc4d4ce862e17c556ecc1b4 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Mon, 4 May 2020 11:32:22 -0300 Subject: [PATCH 10/12] Remove types from tsconfig --- tsconfig.json | 1 - 1 file changed, 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index 42c070946eb8..ce035f036be6 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -29,7 +29,6 @@ }, "moduleResolution": "node", "resolveJsonModule": true, - "types": ["node"], "esModuleInterop": true, "preserveSymlinks": true, From d5a6b9f7f8617a71b776404e003b4ebd52caf937 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Mon, 4 May 2020 13:23:06 -0300 Subject: [PATCH 11/12] Use Map for settings callbacks --- app/settings/lib/settings.ts | 41 +++++++++++------------ app/settings/server/functions/settings.ts | 6 ++-- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/app/settings/lib/settings.ts b/app/settings/lib/settings.ts index d06e889619ff..f3aa1f2d0bf5 100644 --- a/app/settings/lib/settings.ts +++ b/app/settings/lib/settings.ts @@ -7,21 +7,15 @@ export type SettingValue = string | boolean | number | SettingValueMultiSelect | export type SettingComposedValue = {key: string; value: SettingValue}; export type SettingCallback = (key: string, value: SettingValue, initialLoad?: boolean) => void; -interface ISettingCallbacks { - [key: string]: SettingCallback[]; -} - interface ISettingRegexCallbacks { - [key: string]: { - regex: RegExp; - callbacks: SettingCallback[]; - }; + regex: RegExp; + callbacks: SettingCallback[]; } export class SettingsBase { - private callbacks: ISettingCallbacks = {} + private callbacks = new Map(); - private regexCallbacks: ISettingRegexCallbacks = {} + private regexCallbacks = new Map(); // private ts = new Date() @@ -80,13 +74,14 @@ export class SettingsBase { load(key: string, value: SettingValue, initialLoad: boolean): void { ['*', key].forEach((item) => { - if (this.callbacks[item]) { - this.callbacks[item].forEach((callback) => callback(key, value, initialLoad)); + const callbacks = this.callbacks.get(item); + if (callbacks) { + callbacks.forEach((callback) => callback(key, value, initialLoad)); } }); Object.keys(this.regexCallbacks).forEach((cbKey) => { - const cbValue = this.regexCallbacks[cbKey]; - if (!cbValue.regex.test(key)) { + const cbValue = this.regexCallbacks.get(cbKey); + if (!cbValue?.regex.test(key)) { return; } cbValue.callbacks.forEach((callback) => callback(key, value, initialLoad)); @@ -102,14 +97,18 @@ export class SettingsBase { const keys: Array = Array.isArray(key) ? key : [key]; keys.forEach((k) => { if (_.isRegExp(k)) { - this.regexCallbacks[k.source] = this.regexCallbacks[k.source] || { - regex: k, - callbacks: [], - }; - this.regexCallbacks[k.source].callbacks.push(callback); + if (!this.regexCallbacks.has(k.source)) { + this.regexCallbacks.set(k.source, { + regex: k, + callbacks: [], + }); + } + this.regexCallbacks.get(k.source)?.callbacks.push(callback); } else { - this.callbacks[k] = this.callbacks[k] || []; - this.callbacks[k].push(callback); + if (!this.callbacks.has(k)) { + this.callbacks.set(k, []); + } + this.callbacks.get(k)?.push(callback); } }); } diff --git a/app/settings/server/functions/settings.ts b/app/settings/server/functions/settings.ts index 2265e9c5f225..27a9881b8c36 100644 --- a/app/settings/server/functions/settings.ts +++ b/app/settings/server/functions/settings.ts @@ -16,7 +16,7 @@ if (process.env.SETTINGS_HIDDEN) { } const overrideSetting = (_id: string, value: SettingValue, options: ISettingAddOptions): SettingValue => { - const envValue = process?.env?.[_id]; + const envValue = process.env[_id]; if (envValue) { if (envValue.toLowerCase() === 'true') { value = true; @@ -27,7 +27,7 @@ const overrideSetting = (_id: string, value: SettingValue, options: ISettingAddO } options.processEnvValue = value; options.valueSource = 'processEnvValue'; - } else if (typeof Meteor.settings?.[_id] !== 'undefined') { + } else if (typeof Meteor.settings[_id] !== 'undefined') { if (Meteor.settings[_id] == null) { return false; } @@ -37,7 +37,7 @@ const overrideSetting = (_id: string, value: SettingValue, options: ISettingAddO options.valueSource = 'meteorSettingsValue'; } - const overwriteValue = process?.env?.[`OVERWRITE_SETTING_${ _id }`]; + const overwriteValue = process.env[`OVERWRITE_SETTING_${ _id }`]; if (overwriteValue) { if (overwriteValue.toLowerCase() === 'true') { value = true; From d630bbea6fa324b6d685d6083d6d904d2ae44278 Mon Sep 17 00:00:00 2001 From: Rodrigo Nascimento Date: Mon, 4 May 2020 16:08:30 -0300 Subject: [PATCH 12/12] =?UTF-8?q?Do=20not=20always=20update=20the=20role?= =?UTF-8?q?=E2=80=99s=20description=20and=20mandatory2fa?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/server/models/Roles.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/server/models/Roles.js b/app/models/server/models/Roles.js index 799a91426612..f14c0b9ac15e 100644 --- a/app/models/server/models/Roles.js +++ b/app/models/server/models/Roles.js @@ -30,17 +30,21 @@ export class Roles extends Base { } createOrUpdate(name, scope = 'Users', description = '', protectedRole = true, mandatory2fa = false) { - const updateData = { + const queryData = { name, scope, - description, protected: protectedRole, + }; + + const updateData = { + ...queryData, + description, mandatory2fa, }; const exists = this.findOne({ _id: name, - ...updateData, + ...queryData, }, { fields: { _id: 1 } }); if (exists) {