From e2bc2f296641a55ef2894a95ea551ac2c2addc64 Mon Sep 17 00:00:00 2001 From: Thenkei Date: Tue, 15 Jun 2021 11:39:26 +0200 Subject: [PATCH] refactor(permissions-formatter): add a new service that handle permissions formatting --- src/context/init.js | 2 + src/services/permissions-formatter.js | 43 ++++++++ src/services/permissions-getter.js | 44 +------- test/services/permissions-getter.unit.test.js | 100 +++++++++++++----- test/services/permissions.test.js | 22 ++-- 5 files changed, 134 insertions(+), 77 deletions(-) create mode 100644 src/services/permissions-formatter.js diff --git a/src/context/init.js b/src/context/init.js index 0e6c90b0e..10e81e0ba 100644 --- a/src/context/init.js +++ b/src/context/init.js @@ -26,6 +26,7 @@ const baseFilterParser = require('../services/base-filters-parser'); const ConfigStore = require('../services/config-store'); const PermissionsChecker = require('../services/permissions-checker'); const PermissionsGetter = require('../services/permissions-getter'); +const permissionsFormatter = require('../services/permissions-formatter'); const SchemaFileUpdater = require('../services/schema-file-updater'); const SmartActionHook = require('../services/smart-action-hook'); const schemasGenerator = require('../generators/schemas'); @@ -133,6 +134,7 @@ function initServices(context) { context.addInstance('forestServerRequester', forestServerRequester); context.addInstance('schemasGenerator', schemasGenerator); context.addInstance('baseFilterParser', baseFilterParser); + context.addInstance('permissionsFormatter', permissionsFormatter); context.addClass(ProjectDirectoryFinder); context.addClass(ConfigStore); context.addClass(PermissionsGetter); diff --git a/src/services/permissions-formatter.js b/src/services/permissions-formatter.js new file mode 100644 index 000000000..99b557141 --- /dev/null +++ b/src/services/permissions-formatter.js @@ -0,0 +1,43 @@ + +const _transformActionsPermissionsFromOldToNewFormat = (smartActionsPermissions) => { + const newSmartActionsPermissions = {}; + Object.keys(smartActionsPermissions).forEach((actionName) => { + const action = smartActionsPermissions[actionName]; + newSmartActionsPermissions[actionName] = { + triggerEnabled: action.users ? action.allowed && action.users : action.allowed, + }; + }); + return newSmartActionsPermissions; +}; + +const transformPermissionsFromOldToNewFormat = (permissions) => { + const newPermissions = {}; + + Object.keys(permissions).forEach((modelName) => { + const modelPermissions = permissions[modelName]; + const { collection } = modelPermissions; + + newPermissions[modelName] = { + collection: { + browseEnabled: collection.list || collection.searchToEdit, + readEnabled: collection.show, + editEnabled: collection.update, + addEnabled: collection.create, + deleteEnabled: collection.delete, + exportEnabled: collection.export, + }, + scope: modelPermissions.scope, + }; + + if (modelPermissions.actions) { + newPermissions[modelName] + .actions = _transformActionsPermissionsFromOldToNewFormat(modelPermissions.actions); + } + }); + + return newPermissions; +}; + +module.exports = { + transformPermissionsFromOldToNewFormat, +}; diff --git a/src/services/permissions-getter.js b/src/services/permissions-getter.js index 264095b10..c795bef19 100644 --- a/src/services/permissions-getter.js +++ b/src/services/permissions-getter.js @@ -1,9 +1,10 @@ class PermissionsGetter { constructor({ - configStore, env, forestServerRequester, moment, VError, + configStore, env, permissionsFormatter, forestServerRequester, moment, VError, }) { this.configStore = configStore; this.forestServerRequester = forestServerRequester; + this.permissionsFormatter = permissionsFormatter; this.moment = moment; this.VError = VError; @@ -92,45 +93,6 @@ class PermissionsGetter { }; } - static _transformActionsPermissionsFromOldToNewFormat(smartActionsPermissions) { - const newSmartActionsPermissions = {}; - Object.keys(smartActionsPermissions).forEach((actionName) => { - const action = smartActionsPermissions[actionName]; - newSmartActionsPermissions[actionName] = { - triggerEnabled: action.users ? action.allowed && action.users : action.allowed, - }; - }); - return newSmartActionsPermissions; - } - - static _transformPermissionsFromOldToNewFormat(permissions) { - const newPermissions = {}; - - Object.keys(permissions).forEach((modelName) => { - const modelPermissions = permissions[modelName]; - const { collection } = modelPermissions; - - newPermissions[modelName] = { - collection: { - browseEnabled: collection.list || collection.searchToEdit, - readEnabled: collection.show, - editEnabled: collection.update, - addEnabled: collection.create, - deleteEnabled: collection.delete, - exportEnabled: collection.export, - }, - scope: modelPermissions.scope, - }; - - if (modelPermissions.actions) { - newPermissions[modelName].actions = PermissionsGetter - ._transformActionsPermissionsFromOldToNewFormat(modelPermissions.actions); - } - }); - - return newPermissions; - } - _setRenderingPermissions(renderingId, permissions, { environmentId } = {}) { const options = { environmentId, initIfNotExisting: true }; if (!this._getPermissions(options).renderings) { @@ -166,7 +128,7 @@ class PermissionsGetter { this._setRolesACLPermissions(renderingId, permissions, { environmentId }); } else { const newFormatPermissions = permissions - ? PermissionsGetter._transformPermissionsFromOldToNewFormat(permissions) + ? this.permissionsFormatter.transformPermissionsFromOldToNewFormat(permissions) : null; this._setRenderingPermissions(renderingId, newFormatPermissions, { environmentId }); } diff --git a/test/services/permissions-getter.unit.test.js b/test/services/permissions-getter.unit.test.js index 0f459dc87..4dfd150c9 100644 --- a/test/services/permissions-getter.unit.test.js +++ b/test/services/permissions-getter.unit.test.js @@ -7,6 +7,7 @@ describe('services > PermissionsGetter', () => { env: {}, configStore: {}, forestServerRequester: {}, + permissionsFormatter: {}, moment, VError, }; @@ -169,19 +170,24 @@ describe('services > PermissionsGetter', () => { }, }; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); + permissionsGetter.isRolesACLActivated = true; jest.spyOn(permissionsGetter, '_setRolesACLPermissions'); - jest.spyOn(PermissionsGetter, '_transformPermissionsFromOldToNewFormat'); permissionsGetter._setPermissions(1, permissions); expect(permissionsGetter._setRolesACLPermissions).toHaveBeenCalledTimes(1); expect(permissionsGetter._setRolesACLPermissions) .toHaveBeenCalledWith(1, permissions, { environmentId: undefined }); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat).not.toHaveBeenCalled(); - - jest.restoreAllMocks(); + expect(mockTransformPermissionsFromOldToNewFormat) + .not.toHaveBeenCalled(); }); describe('with environmentId', () => { @@ -196,17 +202,24 @@ describe('services > PermissionsGetter', () => { }, }; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); + permissionsGetter.isRolesACLActivated = true; jest.spyOn(permissionsGetter, '_setRolesACLPermissions'); - jest.spyOn(PermissionsGetter, '_transformPermissionsFromOldToNewFormat'); permissionsGetter._setPermissions(1, permissions, { environmentId }); expect(permissionsGetter._setRolesACLPermissions).toHaveBeenCalledTimes(1); expect(permissionsGetter._setRolesACLPermissions) .toHaveBeenCalledWith(1, permissions, { environmentId }); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat).not.toHaveBeenCalled(); + expect(mockTransformPermissionsFromOldToNewFormat) + .not.toHaveBeenCalled(); jest.restoreAllMocks(); }); @@ -227,17 +240,24 @@ describe('services > PermissionsGetter', () => { queries: ['someQuery'], }; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); + permissionsGetter.isRolesACLActivated = true; jest.spyOn(permissionsGetter, '_setRolesACLPermissions'); - jest.spyOn(PermissionsGetter, '_transformPermissionsFromOldToNewFormat'); permissionsGetter._setPermissions(1, permissions, { environmentId }, stats); expect(permissionsGetter._setRolesACLPermissions).toHaveBeenCalledTimes(1); expect(permissionsGetter._setRolesACLPermissions) .toHaveBeenCalledWith(1, permissions, { environmentId }); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat).not.toHaveBeenCalled(); + expect(mockTransformPermissionsFromOldToNewFormat) + .not.toHaveBeenCalled(); const renderingPermissions = permissionsGetter ._getPermissionsInRendering(1, { environmentId }).data; @@ -295,11 +315,17 @@ describe('services > PermissionsGetter', () => { const permissions = {}; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); + permissionsGetter.isRolesACLActivated = false; jest.spyOn(permissionsGetter, '_setRenderingPermissions'); jest.spyOn(permissionsGetter, '_setCollectionsPermissions'); - jest.spyOn(PermissionsGetter, '_transformPermissionsFromOldToNewFormat'); permissionsGetter._setPermissions(1, permissions); @@ -307,8 +333,9 @@ describe('services > PermissionsGetter', () => { expect(permissionsGetter._setRenderingPermissions) .toHaveBeenCalledWith(1, permissions, { environmentId: undefined }); expect(permissionsGetter._setCollectionsPermissions).not.toHaveBeenCalled(); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat).toHaveBeenCalledTimes(1); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat) + expect(mockTransformPermissionsFromOldToNewFormat) + .toHaveBeenCalledTimes(1); + expect(mockTransformPermissionsFromOldToNewFormat) .toHaveBeenCalledWith(permissions); jest.restoreAllMocks(); @@ -322,11 +349,17 @@ describe('services > PermissionsGetter', () => { const environmentId = 100; const permissions = {}; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); + permissionsGetter.isRolesACLActivated = false; jest.spyOn(permissionsGetter, '_setRenderingPermissions'); jest.spyOn(permissionsGetter, '_setCollectionsPermissions'); - jest.spyOn(PermissionsGetter, '_transformPermissionsFromOldToNewFormat'); permissionsGetter._setPermissions(1, permissions, { environmentId }); @@ -334,8 +367,9 @@ describe('services > PermissionsGetter', () => { expect(permissionsGetter._setRenderingPermissions) .toHaveBeenCalledWith(1, permissions, { environmentId }); expect(permissionsGetter._setCollectionsPermissions).not.toHaveBeenCalled(); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat).toHaveBeenCalledTimes(1); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat) + expect(mockTransformPermissionsFromOldToNewFormat) + .toHaveBeenCalledTimes(1); + expect(mockTransformPermissionsFromOldToNewFormat) .toHaveBeenCalledWith(permissions); jest.restoreAllMocks(); @@ -352,19 +386,24 @@ describe('services > PermissionsGetter', () => { queries: ['someQuery'], }; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); + permissionsGetter.isRolesACLActivated = false; jest.spyOn(permissionsGetter, '_setRenderingPermissions'); jest.spyOn(permissionsGetter, '_setCollectionsPermissions'); - jest.spyOn(PermissionsGetter, '_transformPermissionsFromOldToNewFormat'); permissionsGetter._setPermissions(1, permissions, { environmentId }, stats); expect(permissionsGetter._setRenderingPermissions).toHaveBeenCalledTimes(1); expect(permissionsGetter._setCollectionsPermissions).not.toHaveBeenCalled(); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat).toHaveBeenCalledTimes(1); - expect(PermissionsGetter._transformPermissionsFromOldToNewFormat) - .toHaveBeenCalledWith(permissions); + expect(mockTransformPermissionsFromOldToNewFormat).toHaveBeenCalledTimes(1); + expect(mockTransformPermissionsFromOldToNewFormat).toHaveBeenCalledWith(permissions); const renderingPermissions = permissionsGetter ._getPermissionsInRendering(1, { environmentId }).data; @@ -386,7 +425,13 @@ describe('services > PermissionsGetter', () => { queries: ['someOtherQuery'], }; - const permissionsGetter = new PermissionsGetter(defaultDependencies); + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); + const permissionsGetter = new PermissionsGetter({ + ...defaultDependencies, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, + }); permissionsGetter.isRolesACLActivated = false; @@ -679,6 +724,8 @@ describe('services > PermissionsGetter', () => { data: {}, meta: {}, }; + + const mockTransformPermissionsFromOldToNewFormat = jest.fn((p) => p); const mockForestServerRequesterPerform = jest.fn(async () => fakeResponse); const permissionsGetter = new PermissionsGetter({ ...defaultDependencies, @@ -690,6 +737,9 @@ describe('services > PermissionsGetter', () => { forestServerRequester: { perform: mockForestServerRequesterPerform, }, + permissionsFormatter: { + transformPermissionsFromOldToNewFormat: mockTransformPermissionsFromOldToNewFormat, + }, }); await permissionsGetter._retrievePermissions(1); diff --git a/test/services/permissions.test.js b/test/services/permissions.test.js index b617ed82d..58afde808 100644 --- a/test/services/permissions.test.js +++ b/test/services/permissions.test.js @@ -610,7 +610,7 @@ describe('services > permissions', () => { it('should retrieve the permissions', async () => { expect.assertions(4); resetNock(); - const { permissionsGetter } = context.inject(); + const { permissionsGetter, permissionsFormatter } = context.inject(); let lastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); let retrievedPermissions = permissionsGetter._getPermissionsInRendering(1); @@ -644,8 +644,8 @@ describe('services > permissions', () => { lastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); expect(lastRetrieve).not.toBeNull(); - const permissionsInNewFormat = permissionsGetter.constructor - ._transformPermissionsFromOldToNewFormat(permissions.data); + const permissionsInNewFormat = permissionsFormatter + .transformPermissionsFromOldToNewFormat(permissions.data); expect(retrievedPermissions.data).toStrictEqual(permissionsInNewFormat); }); }); @@ -655,7 +655,7 @@ describe('services > permissions', () => { it('should re-retrieve the permissions', async () => { expect.assertions(6); resetNock(); - const { permissionsGetter } = context.inject(); + const { permissionsGetter, permissionsFormatter } = context.inject(); permissionsGetter.expirationInSeconds = 1; const intialLastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); @@ -704,8 +704,8 @@ describe('services > permissions', () => { const firstRetrievedPermissions = permissionsGetter._getPermissionsInRendering(1); const firstLastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); - const permissions1InNewFormat = permissionsGetter.constructor - ._transformPermissionsFromOldToNewFormat(permissions1.data); + const permissions1InNewFormat = permissionsFormatter + .transformPermissionsFromOldToNewFormat(permissions1.data); expect(firstRetrievedPermissions.data).toStrictEqual(permissions1InNewFormat); expect(firstLastRetrieve).not.toBeNull(); @@ -725,8 +725,8 @@ describe('services > permissions', () => { .then(() => { const secondRetrievedPermissions = permissionsGetter._getPermissionsInRendering(1); const secondLastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); - const permissions2InNewFormat = permissionsGetter.constructor - ._transformPermissionsFromOldToNewFormat(permissions2.data); + const permissions2InNewFormat = permissionsFormatter + .transformPermissionsFromOldToNewFormat(permissions2.data); expect(secondRetrievedPermissions.data).toStrictEqual(permissions2InNewFormat); expect(secondLastRetrieve - firstLastRetrieve > 0).toStrictEqual(true); @@ -738,7 +738,7 @@ describe('services > permissions', () => { it('should not re-retrieve the permissions', async () => { expect.assertions(6); resetNock(); - const { permissionsGetter } = context.inject(); + const { permissionsGetter, permissionsFormatter } = context.inject(); permissionsGetter.expirationInSeconds = 1000; const intialLastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); @@ -787,8 +787,8 @@ describe('services > permissions', () => { const firstRetrievedPermissions = permissionsGetter._getPermissionsInRendering(1); const firstLastRetrieve = permissionsGetter._getLastRetrieveTimeInRendering(1); - const permissions1InNewFormat = permissionsGetter.constructor - ._transformPermissionsFromOldToNewFormat(permissions1.data); + const permissions1InNewFormat = permissionsFormatter + .transformPermissionsFromOldToNewFormat(permissions1.data); expect(firstRetrievedPermissions.data).toStrictEqual(permissions1InNewFormat); expect(firstLastRetrieve).not.toBeNull();