From 4745d7634173ec680cbe1473ea6bf2b7f8d4c77d Mon Sep 17 00:00:00 2001 From: ivanThePleasant Date: Thu, 29 Sep 2022 14:03:41 +0300 Subject: [PATCH] Clean up unnecessary code --- .../core/admin/ee/server/controllers/role.js | 9 +-- .../server/controllers/__tests__/role.test.js | 70 ++++--------------- .../server/controllers/__tests__/user.test.js | 22 +----- .../core/admin/server/controllers/role.js | 6 +- .../core/admin/server/controllers/user.js | 2 - .../server/services/__tests__/user.test.js | 4 ++ packages/core/admin/server/services/role.js | 8 +-- packages/core/admin/server/services/user.js | 2 + .../server/controllers/content-types.js | 2 +- .../schema-builder/component-builder.js | 2 +- .../server/controllers/admin-folder-file.js | 2 +- .../server/controllers/admin-settings.js | 6 -- .../core/upload/server/services/upload.js | 10 ++- .../controllers/__tests__/locales.test.js | 18 +---- .../i18n/server/controllers/locales.js | 6 -- .../plugins/i18n/server/services/locales.js | 3 + 16 files changed, 43 insertions(+), 129 deletions(-) diff --git a/packages/core/admin/ee/server/controllers/role.js b/packages/core/admin/ee/server/controllers/role.js index b88ba21656..b025914dd0 100644 --- a/packages/core/admin/ee/server/controllers/role.js +++ b/packages/core/admin/ee/server/controllers/role.js @@ -92,14 +92,7 @@ module.exports = { return ctx.notFound('role.notFound'); } - const { permissions, shouldSendUpdate } = await roleService.assignPermissions( - role.id, - input.permissions - ); - - if (shouldSendUpdate) { - await getService('metrics').sendDidUpdateRolePermissions(ctx.state?.user); - } + const permissions = await roleService.assignPermissions(role.id, input.permissions); const sanitizedPermissions = permissions.map(permissionService.sanitizePermission); diff --git a/packages/core/admin/server/controllers/__tests__/role.test.js b/packages/core/admin/server/controllers/__tests__/role.test.js index 4840d71d3e..7a7f8f7e56 100644 --- a/packages/core/admin/server/controllers/__tests__/role.test.js +++ b/packages/core/admin/server/controllers/__tests__/role.test.js @@ -82,21 +82,10 @@ describe('Role controller', () => { test('Fails on missing permissions input', async () => { const findOne = jest.fn(() => Promise.resolve({ id: 1 })); - const state = { - user: { - id: 1, - }, - }; - - const ctx = createContext( - { - params: { id: 1 }, - body: {}, - }, - { - state, - } - ); + const ctx = createContext({ + params: { id: 1 }, + body: {}, + }); global.strapi = { admin: { @@ -124,23 +113,12 @@ describe('Role controller', () => { test('Fails on missing action permission', async () => { const findOne = jest.fn(() => Promise.resolve({ id: 1 })); - const state = { - user: { - id: 1, + const ctx = createContext({ + params: { id: 1 }, + body: { + permissions: [{}], }, - }; - - const ctx = createContext( - { - params: { id: 1 }, - body: { - permissions: [{}], - }, - }, - { - state, - } - ); + }); global.strapi = { admin: { services: { @@ -167,10 +145,7 @@ describe('Role controller', () => { test('Assign permissions if input is valid', async () => { const roleID = 1; const findOneRole = jest.fn(() => Promise.resolve({ id: roleID })); - const assignPermissions = jest.fn((roleID, permissions) => - Promise.resolve({ permissions, shouldSendUpdate: true }) - ); - const sendDidUpdateRolePermissions = jest.fn(); + const assignPermissions = jest.fn((roleID, permissions) => Promise.resolve(permissions)); const inputPermissions = [ { action: 'test', @@ -180,24 +155,12 @@ describe('Role controller', () => { }, ]; - const state = { - user: { - id: 1, - email: 'someTestEmailString', + const ctx = createContext({ + params: { id: roleID }, + body: { + permissions: inputPermissions, }, - }; - - const ctx = createContext( - { - params: { id: roleID }, - body: { - permissions: inputPermissions, - }, - }, - { - state, - } - ); + }); global.strapi = { admin: { @@ -221,9 +184,6 @@ describe('Role controller', () => { })), }, }, - metrics: { - sendDidUpdateRolePermissions, - }, }, }, }; diff --git a/packages/core/admin/server/controllers/__tests__/user.test.js b/packages/core/admin/server/controllers/__tests__/user.test.js index 50afaef023..db33b0d622 100644 --- a/packages/core/admin/server/controllers/__tests__/user.test.js +++ b/packages/core/admin/server/controllers/__tests__/user.test.js @@ -15,12 +15,7 @@ describe('User Controller', () => { test('Fails if user already exist', async () => { const exists = jest.fn(() => Promise.resolve(true)); - const state = { - user: { - id: 1, - }, - }; - const ctx = createContext({ body }, { state }); + const ctx = createContext({ body }); global.strapi = { admin: { @@ -49,15 +44,8 @@ describe('User Controller', () => { const exists = jest.fn(() => Promise.resolve(false)); const sanitizeUser = jest.fn((user) => Promise.resolve(user)); const created = jest.fn(); - const sendDidInviteUser = jest.fn(); - const state = { - user: { - id: 1, - email: 'someTestEmailString', - }, - }; - const ctx = createContext({ body }, { state, created }); - console.log(ctx); + const ctx = createContext({ body }, { created }); + global.strapi = { admin: { services: { @@ -66,9 +54,6 @@ describe('User Controller', () => { create, sanitizeUser, }, - metrics: { - sendDidInviteUser, - }, }, }, }; @@ -79,7 +64,6 @@ describe('User Controller', () => { expect(create).toHaveBeenCalledWith(body); expect(sanitizeUser).toHaveBeenCalled(); expect(created).toHaveBeenCalled(); - expect(sendDidInviteUser).toHaveBeenCalled(); }); }); diff --git a/packages/core/admin/server/controllers/role.js b/packages/core/admin/server/controllers/role.js index 7eb5aeca7a..013c91cf47 100644 --- a/packages/core/admin/server/controllers/role.js +++ b/packages/core/admin/server/controllers/role.js @@ -132,11 +132,7 @@ module.exports = { permissionsToAssign = input.permissions; } - const { permissions, shouldSendUpdate } = await assignPermissions(role.id, permissionsToAssign); - - if (shouldSendUpdate) { - await getService('metrics').sendDidUpdateRolePermissions(ctx.state?.user); - } + const permissions = await assignPermissions(role.id, permissionsToAssign); ctx.body = { data: permissions.map(sanitizePermission), diff --git a/packages/core/admin/server/controllers/user.js b/packages/core/admin/server/controllers/user.js index 9f680a1621..3fcf8912db 100644 --- a/packages/core/admin/server/controllers/user.js +++ b/packages/core/admin/server/controllers/user.js @@ -34,8 +34,6 @@ module.exports = { const createdUser = await getService('user').create(attributes); - getService('metrics').sendDidInviteUser(ctx.state?.user); - const userInfo = getService('user').sanitizeUser(createdUser); // Note: We need to assign manually the registrationToken to the diff --git a/packages/core/admin/server/services/__tests__/user.test.js b/packages/core/admin/server/services/__tests__/user.test.js index f81c81fe84..52d923c729 100644 --- a/packages/core/admin/server/services/__tests__/user.test.js +++ b/packages/core/admin/server/services/__tests__/user.test.js @@ -28,6 +28,7 @@ describe('User', () => { describe('create', () => { const count = jest.fn(() => Promise.resolve(1)); + const sendDidInviteUser = jest.fn(); test('Creates a user by merging given and default attributes', async () => { const create = jest.fn(({ data }) => Promise.resolve(data)); @@ -40,6 +41,7 @@ describe('User', () => { token: { createToken }, auth: { hashPassword }, role: { count }, + metrics: { sendDidInviteUser }, }, }, query() { @@ -68,6 +70,7 @@ describe('User', () => { token: { createToken }, auth: { hashPassword }, role: { count }, + metrics: { sendDidInviteUser }, }, }, query() { @@ -109,6 +112,7 @@ describe('User', () => { token: { createToken }, auth: { hashPassword }, role: { count }, + metrics: { sendDidInviteUser }, }, }, query() { diff --git a/packages/core/admin/server/services/role.js b/packages/core/admin/server/services/role.js index 68c72cbe09..163d74e231 100644 --- a/packages/core/admin/server/services/role.js +++ b/packages/core/admin/server/services/role.js @@ -315,7 +315,6 @@ const assignPermissions = async (roleId, permissions = []) => { const superAdmin = await getService('role').getSuperAdmin(); const isSuperAdmin = superAdmin && superAdmin.id === roleId; const assignRole = set('role', roleId); - let shouldSendUpdate = false; const permissionsWithRole = permissions // Add the role attribute to every permission @@ -352,13 +351,10 @@ const assignPermissions = async (roleId, permissions = []) => { } if (!isSuperAdmin && (permissionsToAdd.length || permissionsToDelete.length)) { - shouldSendUpdate = true; + await getService('metrics').sendDidUpdateRolePermissions(); } - return { - permissions: permissionsToReturn, - shouldSendUpdate, - }; + return permissionsToReturn; }; const addPermissions = async (roleId, permissions) => { diff --git a/packages/core/admin/server/services/user.js b/packages/core/admin/server/services/user.js index af0ab720eb..5970dbf5ee 100644 --- a/packages/core/admin/server/services/user.js +++ b/packages/core/admin/server/services/user.js @@ -41,6 +41,8 @@ const create = async (attributes) => { const createdUser = await strapi.query('admin::user').create({ data: user, populate: ['roles'] }); + getService('metrics').sendDidInviteUser(); + return createdUser; }; diff --git a/packages/core/content-manager/server/controllers/content-types.js b/packages/core/content-manager/server/controllers/content-types.js index f17306545b..eb79696ee0 100644 --- a/packages/core/content-manager/server/controllers/content-types.js +++ b/packages/core/content-manager/server/controllers/content-types.js @@ -105,7 +105,7 @@ module.exports = { const newConfiguration = await contentTypeService.updateConfiguration(contentType, input); - await metricsService.sendDidConfigureListView(contentType, newConfiguration, ctx.state?.user); + await metricsService.sendDidConfigureListView(contentType, newConfiguration); const confWithUpdatedMetadata = { ...newConfiguration, diff --git a/packages/core/content-type-builder/server/services/schema-builder/component-builder.js b/packages/core/content-type-builder/server/services/schema-builder/component-builder.js index aac249cbe2..589cdb394c 100644 --- a/packages/core/content-type-builder/server/services/schema-builder/component-builder.js +++ b/packages/core/content-type-builder/server/services/schema-builder/component-builder.js @@ -57,7 +57,7 @@ module.exports = function createComponentBuilder() { .set('config', infos.config) .setAttributes(this.convertAttributes(infos.attributes)); - if (strapi.components.size === 0) { + if (this.components.size === 0) { strapi.telemetry.send('didCreateFirstComponent'); } else { strapi.telemetry.send('didCreateComponent'); diff --git a/packages/core/upload/server/controllers/admin-folder-file.js b/packages/core/upload/server/controllers/admin-folder-file.js index 97e8795a1e..ca3d1117e9 100644 --- a/packages/core/upload/server/controllers/admin-folder-file.js +++ b/packages/core/upload/server/controllers/admin-folder-file.js @@ -231,7 +231,7 @@ module.exports = { }); strapi.telemetry.send('didBulkMoveMediaLibraryElements', { - groupProperties: { + eventProperties: { rootFolderNumber: updatedFolders.length, rootAssetNumber: updatedFiles.length, totalFolderNumber, diff --git a/packages/core/upload/server/controllers/admin-settings.js b/packages/core/upload/server/controllers/admin-settings.js index 0c9b54466f..395a072914 100644 --- a/packages/core/upload/server/controllers/admin-settings.js +++ b/packages/core/upload/server/controllers/admin-settings.js @@ -19,12 +19,6 @@ module.exports = { await getService('upload').setSettings(data); - if (data.responsiveDimensions === true) { - strapi.telemetry.send('didEnableResponsiveDimensions'); - } else { - strapi.telemetry.send('didDisableResponsiveDimensions'); - } - ctx.body = { data }; }, diff --git a/packages/core/upload/server/services/upload.js b/packages/core/upload/server/services/upload.js index a03fa9e775..9370772ea9 100644 --- a/packages/core/upload/server/services/upload.js +++ b/packages/core/upload/server/services/upload.js @@ -341,7 +341,7 @@ module.exports = ({ strapi }) => ({ fileValues[UPDATED_BY_ATTRIBUTE] = user.id; } - sendMediaMetrics(fileValues, user); + sendMediaMetrics(fileValues); const res = await strapi.entityService.update(FILE_MODEL_UID, id, { data: fileValues }); @@ -357,7 +357,7 @@ module.exports = ({ strapi }) => ({ fileValues[CREATED_BY_ATTRIBUTE] = user.id; } - sendMediaMetrics(fileValues, user); + sendMediaMetrics(fileValues); const res = await strapi.query(FILE_MODEL_UID).create({ data: fileValues }); @@ -442,6 +442,12 @@ module.exports = ({ strapi }) => ({ }, setSettings(value) { + if (value.responsiveDimensions === true) { + strapi.telemetry.send('didEnableResponsiveDimensions'); + } else { + strapi.telemetry.send('didDisableResponsiveDimensions'); + } + return strapi.store({ type: 'plugin', name: 'upload', key: 'settings' }).set({ value }); }, }); diff --git a/packages/plugins/i18n/server/controllers/__tests__/locales.test.js b/packages/plugins/i18n/server/controllers/__tests__/locales.test.js index a57d2a7561..d9ff092fd5 100644 --- a/packages/plugins/i18n/server/controllers/__tests__/locales.test.js +++ b/packages/plugins/i18n/server/controllers/__tests__/locales.test.js @@ -48,7 +48,6 @@ describe('Locales', () => { const expectedLocales = { code: 'af', name: 'Afrikaans (af)', isDefault: true }; const getDefaultLocale = jest.fn(() => Promise.resolve('af')); const setDefaultLocale = jest.fn(() => Promise.resolve()); - const sendDidUpdateI18nLocalesEvent = jest.fn(() => Promise.resolve()); const setIsDefault = jest.fn(() => expectedLocales); const findByCode = jest.fn(() => undefined); @@ -66,9 +65,6 @@ describe('Locales', () => { setDefaultLocale, create, }, - metrics: { - sendDidUpdateI18nLocalesEvent, - }, }, }, }, @@ -89,7 +85,6 @@ describe('Locales', () => { const locale = { code: 'af', name: 'Afrikaans (af)' }; const expectedLocale = { code: 'af', name: 'Afrikaans (af)', isDefault: false }; const getDefaultLocale = jest.fn(() => Promise.resolve('en')); - const sendDidUpdateI18nLocalesEvent = jest.fn(() => Promise.resolve()); const setIsDefault = jest.fn(() => expectedLocale); const findByCode = jest.fn(() => undefined); @@ -106,9 +101,6 @@ describe('Locales', () => { getDefaultLocale, create, }, - metrics: { - sendDidUpdateI18nLocalesEvent, - }, }, }, }, @@ -179,7 +171,6 @@ describe('Locales', () => { const updates = { name: 'Afrikaans' }; const expectedLocales = { code: 'af', name: 'Afrikaans', isDefault: true }; const setDefaultLocale = jest.fn(() => Promise.resolve()); - const sendDidUpdateI18nLocalesEvent = jest.fn(() => Promise.resolve()); const setIsDefault = jest.fn(() => expectedLocales); const findById = jest.fn(() => existingLocale); @@ -196,9 +187,6 @@ describe('Locales', () => { setDefaultLocale, update, }, - metrics: { - sendDidUpdateI18nLocalesEvent, - }, }, }, }, @@ -274,7 +262,6 @@ describe('Locales', () => { const locale = { code: 'af', name: 'Afrikaans (af)' }; const expectedLocales = { code: 'af', name: 'Afrikaans (af)', isDefault: false }; const getDefaultLocale = jest.fn(() => Promise.resolve('en')); - const sendDidUpdateI18nLocalesEvent = jest.fn(() => Promise.resolve()); const setIsDefault = jest.fn(() => expectedLocales); const findById = jest.fn(() => locale); @@ -291,9 +278,6 @@ describe('Locales', () => { getDefaultLocale, delete: deleteFn, }, - metrics: { - sendDidUpdateI18nLocalesEvent, - }, }, }, }, @@ -334,7 +318,7 @@ describe('Locales', () => { sanitizers, }; - const ctx = { params: { id: 1 }, state: { user: { id: 1 } } }; + const ctx = { params: { id: 1 } }; expect.assertions(5); diff --git a/packages/plugins/i18n/server/controllers/locales.js b/packages/plugins/i18n/server/controllers/locales.js index a8dce10565..2a0624b161 100644 --- a/packages/plugins/i18n/server/controllers/locales.js +++ b/packages/plugins/i18n/server/controllers/locales.js @@ -43,8 +43,6 @@ module.exports = { const locale = await localesService.create(localeToPersist); - getService('metrics').sendDidUpdateI18nLocalesEvent(user); - if (isDefault) { await localesService.setDefaultLocale(locale); } @@ -74,8 +72,6 @@ module.exports = { const updatedLocale = await localesService.update({ id }, cleanUpdates); - getService('metrics').sendDidUpdateI18nLocalesEvent(user); - if (isDefault) { await localesService.setDefaultLocale(updatedLocale); } @@ -102,8 +98,6 @@ module.exports = { await localesService.delete({ id }); - getService('metrics').sendDidUpdateI18nLocalesEvent(ctx.state?.user); - const sanitizedLocale = await sanitizeLocale(existingLocale); ctx.body = await localesService.setIsDefault(sanitizedLocale); diff --git a/packages/plugins/i18n/server/services/locales.js b/packages/plugins/i18n/server/services/locales.js index 4acc918c46..96d6e00123 100644 --- a/packages/plugins/i18n/server/services/locales.js +++ b/packages/plugins/i18n/server/services/locales.js @@ -16,11 +16,13 @@ const count = (params) => strapi.query('plugin::i18n.locale').count({ where: par const create = async (locale) => { const result = await strapi.query('plugin::i18n.locale').create({ data: locale }); + getService('metrics').sendDidUpdateI18nLocalesEvent(); return result; }; const update = async (params, updates) => { const result = await strapi.query('plugin::i18n.locale').update({ where: params, data: updates }); + getService('metrics').sendDidUpdateI18nLocalesEvent(); return result; }; @@ -30,6 +32,7 @@ const deleteFn = async ({ id }) => { if (localeToDelete) { await deleteAllLocalizedEntriesFor({ locale: localeToDelete.code }); const result = await strapi.query('plugin::i18n.locale').delete({ where: { id } }); + getService('metrics').sendDidUpdateI18nLocalesEvent(); return result; }