From 7077995bd8f9a487fd3f81b6c00a4744422fec74 Mon Sep 17 00:00:00 2001 From: mfrachet Date: Mon, 15 Mar 2021 09:13:44 +0100 Subject: [PATCH 1/5] Testing getInitialLocale --- .../src/containers/RBACManager/useSyncRbac.js | 4 +- .../src/components/LocalePicker/index.js | 13 +- .../middlewares/localePermissionMiddleware.js | 9 +- .../admin/src/utils/getInitialLocale.js | 13 ++ .../admin/src/utils/getLocaleFromQuery.js | 7 ++ .../src/utils/tests/getInitialLocale.test.js | 115 ++++++++++++++++++ 6 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js create mode 100644 packages/strapi-plugin-i18n/admin/src/utils/getLocaleFromQuery.js create mode 100644 packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js b/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js index 6fe164b729..6b28287bb8 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js @@ -8,7 +8,7 @@ const selectPermissions = state => state.get(`${pluginId}_rbacManager`).permissi const selectCollectionTypePermissions = state => state.get('permissionsManager').collectionTypesRelatedPermissions; -const useSyncRbac = (query, collectionTypeUID = 'listView') => { +const useSyncRbac = (query, collectionTypeUID) => { const collectionTypesRelatedPermissions = useSelector(selectCollectionTypePermissions); const permissions = useSelector(selectPermissions); const dispatch = useDispatch(); @@ -16,7 +16,7 @@ const useSyncRbac = (query, collectionTypeUID = 'listView') => { const relatedPermissions = collectionTypesRelatedPermissions[collectionTypeUID]; useEffect(() => { - if (query && query.pluginOptions && relatedPermissions) { + if (query && relatedPermissions) { dispatch(setPermissions(relatedPermissions, query.pluginOptions, 'listView')); return () => { diff --git a/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js b/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js index fc4a34f40a..69c51a1e1f 100644 --- a/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js +++ b/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js @@ -4,6 +4,7 @@ import { Picker, Padded, Text, Flex } from '@buffetjs/core'; import { Carret, useQueryParams } from 'strapi-helper-plugin'; import styled from 'styled-components'; import get from 'lodash/get'; +import getInitialLocale from '../../utils/getInitialLocale'; const List = styled.ul` list-style-type: none; @@ -34,16 +35,6 @@ const EllipsisParagraph = styled(Text)` text-align: left; `; -const getInitialLocale = (query, locales = []) => { - const localeFromQuery = get(query, 'query.pluginOptions.locale', undefined); - - if (localeFromQuery) { - return locales.find(locale => locale.code === localeFromQuery); - } - - return locales[0]; -}; - const selectContentManagerListViewPluginOptions = state => state.get('content-manager_listView').contentType.pluginOptions; @@ -68,6 +59,8 @@ const LocalePicker = () => { return null; } + console.log('lol', locales); + return ( permission => - permission.properties.locales.indexOf(locale) !== -1; + get(permission, 'properties.locales', []).indexOf(locale) !== -1; const localePermissionMiddleware = () => () => next => action => { if (action.type !== 'ContentManager/RBACManager/SET_PERMISSIONS') { @@ -12,11 +12,12 @@ const localePermissionMiddleware = () => () => next => action => { return next(action); } - if (!get(action, '__meta__.pluginOptions.locale', false)) { + const locale = get(action, '__meta__.pluginOptions.locale', null); + + if (!locale) { return next(action); } - const locale = action.__meta__.pluginOptions.locale; const permissions = action.permissions; const nextPermissions = Object.keys(permissions).reduce((acc, key) => { @@ -30,8 +31,6 @@ const localePermissionMiddleware = () => () => next => action => { return acc; }, {}); - - return next({ ...action, permissions: nextPermissions }); }; diff --git a/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js b/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js new file mode 100644 index 0000000000..576a7277a3 --- /dev/null +++ b/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js @@ -0,0 +1,13 @@ +import getLocaleFromQuery from './getLocaleFromQuery'; + +const getInitialLocale = (query, locales = []) => { + const localeFromQuery = getLocaleFromQuery(query); + + if (localeFromQuery) { + return locales.find(locale => locale.code === localeFromQuery); + } + + return locales[0]; +}; + +export default getInitialLocale; diff --git a/packages/strapi-plugin-i18n/admin/src/utils/getLocaleFromQuery.js b/packages/strapi-plugin-i18n/admin/src/utils/getLocaleFromQuery.js new file mode 100644 index 0000000000..83d977b2aa --- /dev/null +++ b/packages/strapi-plugin-i18n/admin/src/utils/getLocaleFromQuery.js @@ -0,0 +1,7 @@ +import get from 'lodash/get'; + +const getLocaleFromQuery = query => { + return get(query, 'query.pluginOptions.locale', undefined); +}; + +export default getLocaleFromQuery; diff --git a/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js b/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js new file mode 100644 index 0000000000..b90c548990 --- /dev/null +++ b/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js @@ -0,0 +1,115 @@ +import getInitialLocale from '../getInitialLocale'; + +describe('getInitialLocale', () => { + it('gives "fr-FR" when the query.pluginOptions.locale is "fr-FR"', () => { + const query = { + query: { + page: '1', + pageSize: '10', + _sort: 'Name:ASC', + pluginOptions: { + locale: 'fr-FR', + }, + }, + rawQuery: '?page=1&pageSize=10&_sort=Name:ASC&pluginOptions[locale]=fr-FR', + }; + + const locales = [ + { + id: 1, + name: 'English', + code: 'en', + created_at: '2021-03-09T14:57:03.016Z', + updated_at: '2021-03-09T14:57:03.016Z', + isDefault: true, + }, + { + id: 2, + name: 'French (France) (fr-FR)', + code: 'fr-FR', + created_at: '2021-03-09T15:03:06.992Z', + updated_at: '2021-03-09T15:03:06.996Z', + isDefault: false, + }, + ]; + + const expected = { + id: 2, + name: 'French (France) (fr-FR)', + code: 'fr-FR', + created_at: '2021-03-09T15:03:06.992Z', + updated_at: '2021-03-09T15:03:06.996Z', + isDefault: false, + }; + const actual = getInitialLocale(query, locales); + + expect(actual).toEqual(expected); + }); + + it('gives the default locale ("en") when there s no locale in the query', () => { + const query = { + query: { + page: '1', + pageSize: '10', + _sort: 'Name:ASC', + pluginOptions: { + something: 'great', + }, + }, + rawQuery: '?page=1&pageSize=10&_sort=Name:ASC&pluginOptions[something]=great', + }; + + const locales = [ + { + id: 1, + name: 'English', + code: 'en', + created_at: '2021-03-09T14:57:03.016Z', + updated_at: '2021-03-09T14:57:03.016Z', + isDefault: true, + }, + { + id: 2, + name: 'French (France) (fr-FR)', + code: 'fr-FR', + created_at: '2021-03-09T15:03:06.992Z', + updated_at: '2021-03-09T15:03:06.996Z', + isDefault: false, + }, + ]; + + const expected = { + id: 1, + name: 'English', + code: 'en', + created_at: '2021-03-09T14:57:03.016Z', + updated_at: '2021-03-09T14:57:03.016Z', + isDefault: true, + }; + + const actual = getInitialLocale(query, locales); + + expect(actual).toEqual(expected); + }); + + it('gives "undefined" when theres no locale. IMPORTANT: such case should not exist since at least one locale is created on the backend when plug-in i18n', () => { + const query = { + query: { + page: '1', + pageSize: '10', + _sort: 'Name:ASC', + pluginOptions: { + something: 'great', + }, + }, + rawQuery: '?page=1&pageSize=10&_sort=Name:ASC&pluginOptions[something]=great', + }; + + const locales = []; + + const expected = undefined; + const actual = getInitialLocale(query, locales); + + expect(actual).toEqual(expected); + }); +}); From 285db5d3e863e50f2deb617dd09900cf87b264c6 Mon Sep 17 00:00:00 2001 From: mfrachet Date: Mon, 15 Mar 2021 09:14:52 +0100 Subject: [PATCH 2/5] fixing tests --- .../admin/src/utils/getInitialLocale.js | 3 ++- .../src/utils/tests/getInitialLocale.test.js | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js b/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js index 576a7277a3..66fa768730 100644 --- a/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js +++ b/packages/strapi-plugin-i18n/admin/src/utils/getInitialLocale.js @@ -7,7 +7,8 @@ const getInitialLocale = (query, locales = []) => { return locales.find(locale => locale.code === localeFromQuery); } - return locales[0]; + // Returns the default locale when nothing is in the query + return locales.find(locale => locale.isDefault); }; export default getInitialLocale; diff --git a/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js b/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js index b90c548990..b71bc1b03e 100644 --- a/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js +++ b/packages/strapi-plugin-i18n/admin/src/utils/tests/getInitialLocale.test.js @@ -60,14 +60,6 @@ describe('getInitialLocale', () => { }; const locales = [ - { - id: 1, - name: 'English', - code: 'en', - created_at: '2021-03-09T14:57:03.016Z', - updated_at: '2021-03-09T14:57:03.016Z', - isDefault: true, - }, { id: 2, name: 'French (France) (fr-FR)', @@ -76,6 +68,14 @@ describe('getInitialLocale', () => { updated_at: '2021-03-09T15:03:06.996Z', isDefault: false, }, + { + id: 1, + name: 'English', + code: 'en', + created_at: '2021-03-09T14:57:03.016Z', + updated_at: '2021-03-09T14:57:03.016Z', + isDefault: true, + }, ]; const expected = { From 84ec6945bcc50965bf6fc77c0d169c83e859f4bf Mon Sep 17 00:00:00 2001 From: mfrachet Date: Mon, 15 Mar 2021 09:23:13 +0100 Subject: [PATCH 3/5] adding tests for selectors --- .../src/containers/RBACManager/useSyncRbac.js | 7 +-- .../admin/src/selectors.js | 6 +++ .../admin/src/tests/selectors.test.js | 43 +++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 packages/strapi-plugin-content-manager/admin/src/selectors.js create mode 100644 packages/strapi-plugin-content-manager/admin/src/tests/selectors.test.js diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js b/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js index 6b28287bb8..12b44f16a6 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/RBACManager/useSyncRbac.js @@ -1,12 +1,7 @@ import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import pluginId from '../../pluginId'; import { resetPermissions, setPermissions } from './actions'; - -const selectPermissions = state => state.get(`${pluginId}_rbacManager`).permissions; - -const selectCollectionTypePermissions = state => - state.get('permissionsManager').collectionTypesRelatedPermissions; +import { selectPermissions, selectCollectionTypePermissions } from '../../selectors'; const useSyncRbac = (query, collectionTypeUID) => { const collectionTypesRelatedPermissions = useSelector(selectCollectionTypePermissions); diff --git a/packages/strapi-plugin-content-manager/admin/src/selectors.js b/packages/strapi-plugin-content-manager/admin/src/selectors.js new file mode 100644 index 0000000000..c9c1d3ca77 --- /dev/null +++ b/packages/strapi-plugin-content-manager/admin/src/selectors.js @@ -0,0 +1,6 @@ +import pluginId from './pluginId'; + +export const selectPermissions = state => state.get(`${pluginId}_rbacManager`).permissions; + +export const selectCollectionTypePermissions = state => + state.get('permissionsManager').collectionTypesRelatedPermissions; diff --git a/packages/strapi-plugin-content-manager/admin/src/tests/selectors.test.js b/packages/strapi-plugin-content-manager/admin/src/tests/selectors.test.js new file mode 100644 index 0000000000..e26e982b17 --- /dev/null +++ b/packages/strapi-plugin-content-manager/admin/src/tests/selectors.test.js @@ -0,0 +1,43 @@ +import { selectPermissions, selectCollectionTypePermissions } from '../selectors'; + +describe('selectors', () => { + let store; + + beforeEach(() => { + store = new Map(); + }); + + describe('selectPermissions', () => { + it('resolves the permissions key of the "content-manager_rbacManager" store key', () => { + store.set('content-manager_rbacManager', { + permissions: { + some: 'permission', + }, + }); + + const actual = selectPermissions(store); + const expected = { + some: 'permission', + }; + + expect(actual).toEqual(expected); + }); + }); + + describe('selectCollectionTypePermissions', () => { + it('resolves the permissions key of the "permissionsManager" store key', () => { + store.set('permissionsManager', { + collectionTypesRelatedPermissions: { + some: 'permission again', + }, + }); + + const actual = selectCollectionTypePermissions(store); + const expected = { + some: 'permission again', + }; + + expect(actual).toEqual(expected); + }); + }); +}); From 952927c91e9c12f6cac8efa254e9e985887c9a51 Mon Sep 17 00:00:00 2001 From: mfrachet Date: Mon, 15 Mar 2021 09:34:31 +0100 Subject: [PATCH 4/5] partial tests --- .../middlewares/localePermissionMiddleware.js | 4 +- .../tests/localePermissionMiddleware.test.js | 65 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 packages/strapi-plugin-i18n/admin/src/middlewares/tests/localePermissionMiddleware.test.js diff --git a/packages/strapi-plugin-i18n/admin/src/middlewares/localePermissionMiddleware.js b/packages/strapi-plugin-i18n/admin/src/middlewares/localePermissionMiddleware.js index 45e0396d73..b206f4c73b 100644 --- a/packages/strapi-plugin-i18n/admin/src/middlewares/localePermissionMiddleware.js +++ b/packages/strapi-plugin-i18n/admin/src/middlewares/localePermissionMiddleware.js @@ -8,7 +8,9 @@ const localePermissionMiddleware = () => () => next => action => { return next(action); } - if (action.__meta__.containerName !== 'listView') { + const containerName = get(action, '__meta__.containerName', null); + + if (containerName !== 'listView') { return next(action); } diff --git a/packages/strapi-plugin-i18n/admin/src/middlewares/tests/localePermissionMiddleware.test.js b/packages/strapi-plugin-i18n/admin/src/middlewares/tests/localePermissionMiddleware.test.js new file mode 100644 index 0000000000..310533493c --- /dev/null +++ b/packages/strapi-plugin-i18n/admin/src/middlewares/tests/localePermissionMiddleware.test.js @@ -0,0 +1,65 @@ +import localePermissionMiddleware from '../localePermissionMiddleware'; + +describe('localePermissionMiddleware', () => { + it('does not modify the action when the type is not "ContentManager/RBACManager/SET_PERMISSIONS"', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'UNKNOWN_TYPE', + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toBe(action); + }); + + it('does not modify the action when it the __meta__ key is not set', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: undefined, + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toBe(action); + }); + + it('does not modify the action when it the __meta__.containerName is not "listView"', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { containerName: undefined }, + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toBe(action); + }); + + it('does not modify the action when it the __meta__.pluginOptions is not set', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { containerName: 'listView' }, + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toBe(action); + }); + + it('does not modify the action when it the __meta__.pluginOptions.locale is not set', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { + containerName: 'listView', + pluginOptions: {}, + }, + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toBe(action); + }); +}); From 6e733b753246ce40ca267f528ff6c59c718bdb1a Mon Sep 17 00:00:00 2001 From: mfrachet Date: Mon, 15 Mar 2021 09:48:58 +0100 Subject: [PATCH 5/5] adding testrs for locale --- .../src/components/LocalePicker/index.js | 2 - .../tests/localePermissionMiddleware.test.js | 71 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js b/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js index 69c51a1e1f..b18b20e862 100644 --- a/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js +++ b/packages/strapi-plugin-i18n/admin/src/components/LocalePicker/index.js @@ -59,8 +59,6 @@ const LocalePicker = () => { return null; } - console.log('lol', locales); - return ( { expect(nextAction).toBe(action); }); + + it('creates an empty permissions object from an empty array', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { + containerName: 'listView', + pluginOptions: { + locale: 'en', + }, + }, + permissions: {}, + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toEqual({ + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { containerName: 'listView', pluginOptions: { locale: 'en' } }, + permissions: {}, + }); + }); + + it('creates a valid permissions object from a filled array', () => { + const nextFn = jest.fn(x => x); + const action = { + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { + containerName: 'listView', + pluginOptions: { + locale: 'en', + }, + }, + permissions: { + 'plugins::content-manager.explorer.create': [ + { + id: 459, + action: 'plugins::content-manager.explorer.create', + subject: 'application::article.article', + properties: { + fields: ['Name'], + locales: ['en'], + }, + conditions: [], + }, + ], + }, + }; + + const nextAction = localePermissionMiddleware()()(nextFn)(action); + + expect(nextAction).toEqual({ + type: 'ContentManager/RBACManager/SET_PERMISSIONS', + __meta__: { containerName: 'listView', pluginOptions: { locale: 'en' } }, + permissions: { + 'plugins::content-manager.explorer.create': [ + { + id: 459, + action: 'plugins::content-manager.explorer.create', + subject: 'application::article.article', + properties: { + fields: ['Name'], + locales: ['en'], + }, + + conditions: [], + }, + ], + }, + }); + }); });