From 55b5b14830d635641390d820a7ab1bc072a29024 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Fri, 5 Nov 2021 12:08:52 +0100 Subject: [PATCH 01/31] fix: negative limit and pageSize handling --- packages/core/strapi/lib/core-api/service/pagination.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/pagination.js b/packages/core/strapi/lib/core-api/service/pagination.js index d9696b24d6..5fd055bccd 100644 --- a/packages/core/strapi/lib/core-api/service/pagination.js +++ b/packages/core/strapi/lib/core-api/service/pagination.js @@ -22,7 +22,7 @@ const applyMaxLimit = (limit, maxLimit) => { return maxLimit; } - return limit; + return Math.max(1, limit); }; const shouldCount = params => { @@ -72,7 +72,7 @@ const getPaginationInfo = params => { if (isPagedPagination(pagination)) { const pageSize = isUndefined(pagination.pageSize) ? defaultLimit - : Math.max(1, toNumber(pagination.pageSize)); + : toNumber(pagination.pageSize); return { page: Math.max(1, toNumber(pagination.page || 1)), @@ -80,9 +80,7 @@ const getPaginationInfo = params => { }; } - const limit = isUndefined(pagination.limit) - ? defaultLimit - : Math.max(1, toNumber(pagination.limit)); + const limit = isUndefined(pagination.limit) ? defaultLimit : toNumber(pagination.limit); return { start: Math.max(0, toNumber(pagination.start || 0)), From afe446843e8fb193fc611f4c77a618f4e4954d1e Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Mon, 8 Nov 2021 12:30:34 +0100 Subject: [PATCH 02/31] fix: negative limit and pageSize handling (graphql) --- packages/core/utils/lib/pagination.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/core/utils/lib/pagination.js b/packages/core/utils/lib/pagination.js index 158cbabf5d..faeed72e6a 100644 --- a/packages/core/utils/lib/pagination.js +++ b/packages/core/utils/lib/pagination.js @@ -34,6 +34,12 @@ const ensureMaxValues = (maxLimit = -1) => ({ start, limit }) => ({ limit: withMaxLimit(limit, maxLimit), }); +// Apply maxLimit as the limit when limit is -1 +const withNoLimit = (pagination, maxLimit = -1) => ({ + ...pagination, + limit: pagination.limit === -1 ? maxLimit : pagination.limit, +}); + const withDefaultPagination = (args, { defaults = {}, maxLimit = -1 } = {}) => { const defaultValues = merge(STRAPI_DEFAULTS, defaults); @@ -74,6 +80,9 @@ const withDefaultPagination = (args, { defaults = {}, maxLimit = -1 } = {}) => { }); } + // Handle -1 limit + Object.assign(pagination, withNoLimit(pagination, maxLimit)); + const replacePaginationAttributes = pipe( // Remove pagination attributes omit(paginationAttributes), From 3e5cea0cbcdff6b042a598eb2d075de473faf420 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Mon, 8 Nov 2021 15:46:51 +0100 Subject: [PATCH 03/31] fix: negative limit without maxLimit --- packages/core/strapi/lib/core-api/service/pagination.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/pagination.js b/packages/core/strapi/lib/core-api/service/pagination.js index 5fd055bccd..764bacabb4 100644 --- a/packages/core/strapi/lib/core-api/service/pagination.js +++ b/packages/core/strapi/lib/core-api/service/pagination.js @@ -17,9 +17,9 @@ const getLimitConfigDefaults = () => ({ * @param {number?} maxLimit - maxlimit used has capping * @returns {number} */ -const applyMaxLimit = (limit, maxLimit) => { - if (maxLimit && (limit === -1 || limit > maxLimit)) { - return maxLimit; +const applyMaxLimit = (limit, maxLimit = null) => { + if (limit === -1 || (maxLimit && limit > maxLimit)) { + return maxLimit || -1; } return Math.max(1, limit); From 73e5b672fd0ccb62a1659107bac2627f4c22be4e Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Mon, 8 Nov 2021 17:17:56 +0100 Subject: [PATCH 04/31] fix: negative limit without maxLimit (graphql) --- packages/core/utils/lib/pagination.js | 2 +- .../services/internals/types/response-collection-meta.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/utils/lib/pagination.js b/packages/core/utils/lib/pagination.js index faeed72e6a..c5d0cf8881 100644 --- a/packages/core/utils/lib/pagination.js +++ b/packages/core/utils/lib/pagination.js @@ -26,7 +26,7 @@ const withMaxLimit = (limit, maxLimit = -1) => { // Ensure minimum page & pageSize values (page >= 1, pageSize >= 0, start >= 0, limit >= 0) const ensureMinValues = ({ start, limit }) => ({ start: Math.max(start, 0), - limit: Math.max(limit, 1), + limit: limit === -1 ? limit : Math.max(limit, 1), }); const ensureMaxValues = (maxLimit = -1) => ({ start, limit }) => ({ diff --git a/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js b/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js index 828008aa2e..0b9c31851e 100644 --- a/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js +++ b/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js @@ -25,9 +25,9 @@ module.exports = ({ strapi }) => { const { start, limit } = args; const total = await strapi.entityService.count(resourceUID, args); - const pageSize = limit; - const pageCount = limit === 0 ? 0 : Math.ceil(total / limit); - const page = limit === 0 ? 1 : Math.floor(start / limit) + 1; + const pageSize = limit === -1 ? total : limit; + const pageCount = limit === -1 ? 1 : Math.ceil(total / limit); + const page = limit === -1 ? 1 : Math.floor(start / limit) + 1; return { total, page, pageSize, pageCount }; }, From d5eb04628521e87e6e20b69e0291354a135fd240 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Mon, 8 Nov 2021 17:32:40 +0100 Subject: [PATCH 05/31] fix: avoid sending -1 limit to db layer --- packages/core/utils/lib/convert-query-params.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/utils/lib/convert-query-params.js b/packages/core/utils/lib/convert-query-params.js index e4c9f94763..24292a8c44 100644 --- a/packages/core/utils/lib/convert-query-params.js +++ b/packages/core/utils/lib/convert-query-params.js @@ -110,6 +110,8 @@ const convertLimitQueryParams = limitQuery => { throw new Error(`convertLimitQueryParams expected a positive integer got ${limitAsANumber}`); } + if (limitAsANumber === -1) return null; + return limitAsANumber; }; From f42485b83973eb7f5e112a5165ec6b6f7b1d3fa6 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 9 Nov 2021 08:48:42 +0100 Subject: [PATCH 06/31] fix: removed unlimied query support for pagedPagination (pageSize=-1) --- packages/core/strapi/lib/core-api/service/pagination.js | 2 +- packages/core/utils/lib/pagination.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/pagination.js b/packages/core/strapi/lib/core-api/service/pagination.js index 764bacabb4..03aaefe1f4 100644 --- a/packages/core/strapi/lib/core-api/service/pagination.js +++ b/packages/core/strapi/lib/core-api/service/pagination.js @@ -76,7 +76,7 @@ const getPaginationInfo = params => { return { page: Math.max(1, toNumber(pagination.page || 1)), - pageSize: applyMaxLimit(pageSize, maxLimit), + pageSize: Math.max(1, applyMaxLimit(pageSize, maxLimit)), }; } diff --git a/packages/core/utils/lib/pagination.js b/packages/core/utils/lib/pagination.js index c5d0cf8881..1eff844b88 100644 --- a/packages/core/utils/lib/pagination.js +++ b/packages/core/utils/lib/pagination.js @@ -72,7 +72,10 @@ const withDefaultPagination = (args, { defaults = {}, maxLimit = -1 } = {}) => { // Page / PageSize if (usePagePagination) { - const { page, pageSize } = merge(defaultValues.page, args); + const { page, pageSize } = merge(defaultValues.page, { + ...args, + pageSize: Math.max(1, args.pageSize), + }); Object.assign(pagination, { start: (page - 1) * pageSize, From 28427b09c4613e48d61146f3f298b5d712075bfa Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 9 Nov 2021 11:05:47 +0100 Subject: [PATCH 07/31] fix: fixed unexpected behavior when maxLimit is set --- packages/core/strapi/lib/core-api/service/pagination.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/pagination.js b/packages/core/strapi/lib/core-api/service/pagination.js index 03aaefe1f4..9413e0ee45 100644 --- a/packages/core/strapi/lib/core-api/service/pagination.js +++ b/packages/core/strapi/lib/core-api/service/pagination.js @@ -72,11 +72,11 @@ const getPaginationInfo = params => { if (isPagedPagination(pagination)) { const pageSize = isUndefined(pagination.pageSize) ? defaultLimit - : toNumber(pagination.pageSize); + : Math.max(1, toNumber(pagination.pageSize)); return { page: Math.max(1, toNumber(pagination.page || 1)), - pageSize: Math.max(1, applyMaxLimit(pageSize, maxLimit)), + pageSize: applyMaxLimit(pageSize, maxLimit), }; } From 882db487097c424321fbf4a8044a8fcac5447bef Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 9 Nov 2021 11:09:27 +0100 Subject: [PATCH 08/31] test: added tests for pagination service --- .../lib/core-api/__tests__/service.test.js | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/packages/core/strapi/lib/core-api/__tests__/service.test.js b/packages/core/strapi/lib/core-api/__tests__/service.test.js index 4903c2d595..a8eb4be0f1 100644 --- a/packages/core/strapi/lib/core-api/__tests__/service.test.js +++ b/packages/core/strapi/lib/core-api/__tests__/service.test.js @@ -2,6 +2,7 @@ const _ = require('lodash'); const { createService } = require('../service'); +const { getPaginationInfo } = require('../service/pagination'); const maxLimit = 50; const defaultLimit = 20; @@ -146,3 +147,119 @@ describe('Default Service', () => { }); }); }); + +describe('Pagination service', () => { + test('Uses default limit', () => { + const pagination = {}; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: defaultLimit, + }); + }); + + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: pagination.pageSize, + }); + }); + + test('Uses maxLimit as pageSize', () => { + const pagination = { pageSize: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: maxLimit, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + }); + + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: pagination.limit, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); + }); + }); +}); From d6f8a6debcf6ecbd81d588bd9ed9471edaf4fa4b Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 9 Nov 2021 11:09:55 +0100 Subject: [PATCH 09/31] test: added tests for pagination util --- .../utils/lib/__tests__/pagination.test.js | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 packages/core/utils/lib/__tests__/pagination.test.js diff --git a/packages/core/utils/lib/__tests__/pagination.test.js b/packages/core/utils/lib/__tests__/pagination.test.js new file mode 100644 index 0000000000..450e67aaf1 --- /dev/null +++ b/packages/core/utils/lib/__tests__/pagination.test.js @@ -0,0 +1,126 @@ +'use strict'; + +const { withDefaultPagination } = require('../pagination'); + +const defaultLimit = 20; +const maxLimit = 50; +const defaults = { + offset: { limit: defaultLimit }, + page: { pageSize: defaultLimit }, +}; + +describe('Pagination util', () => { + test('Uses default limit', () => { + const pagination = {}; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: defaultLimit, + }); + }); + + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.pageSize, + }); + }); + + test('Uses maxLimit as pageSize', () => { + const pagination = { pageSize: 999 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + }); + + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.limit, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: 999 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: -1 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + }); +}); From 9b244f881645c95a55c2064477a728ad37fd9d58 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Mon, 22 Nov 2021 10:46:03 +0100 Subject: [PATCH 10/31] fix: pageSize meta improved --- .../server/services/internals/types/response-collection-meta.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js b/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js index 0b9c31851e..604c8f9180 100644 --- a/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js +++ b/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js @@ -25,7 +25,7 @@ module.exports = ({ strapi }) => { const { start, limit } = args; const total = await strapi.entityService.count(resourceUID, args); - const pageSize = limit === -1 ? total : limit; + const pageSize = limit === -1 ? total - start : limit; const pageCount = limit === -1 ? 1 : Math.ceil(total / limit); const page = limit === -1 ? 1 : Math.floor(start / limit) + 1; From abecfe8f16a642137a5466654a90260ae22e058c Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 23 Nov 2021 16:48:16 +0100 Subject: [PATCH 11/31] tests: moved and splitted test file --- .../__tests__/index.test.js} | 138 +----------------- .../service/__tests__/pagination.test.js | 138 ++++++++++++++++++ 2 files changed, 139 insertions(+), 137 deletions(-) rename packages/core/strapi/lib/core-api/{__tests__/service.test.js => service/__tests__/index.test.js} (51%) create mode 100644 packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js diff --git a/packages/core/strapi/lib/core-api/__tests__/service.test.js b/packages/core/strapi/lib/core-api/service/__tests__/index.test.js similarity index 51% rename from packages/core/strapi/lib/core-api/__tests__/service.test.js rename to packages/core/strapi/lib/core-api/service/__tests__/index.test.js index 960c6f0154..704699d23f 100644 --- a/packages/core/strapi/lib/core-api/__tests__/service.test.js +++ b/packages/core/strapi/lib/core-api/service/__tests__/index.test.js @@ -1,26 +1,6 @@ 'use strict'; -const _ = require('lodash'); -const { createService } = require('../service'); -const { getPaginationInfo } = require('../service/pagination'); - -const maxLimit = 50; -const defaultLimit = 20; - -// init global strapi -global.strapi = { - config: { - get(path, defaultValue) { - return _.get(this, path, defaultValue); - }, - api: { - rest: { - defaultLimit, - maxLimit, - }, - }, - }, -}; +const { createService } = require('..'); describe('Default Service', () => { describe('Collection Type', () => { @@ -145,119 +125,3 @@ describe('Default Service', () => { }); }); }); - -describe('Pagination service', () => { - test('Uses default limit', () => { - const pagination = {}; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: defaultLimit, - }); - }); - - describe('Paged pagination', () => { - test('Uses specified pageSize', () => { - const pagination = { pageSize: 5 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: pagination.pageSize, - }); - }); - - test('Uses maxLimit as pageSize', () => { - const pagination = { pageSize: 999 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: maxLimit, - }); - }); - - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: 0 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: 1, - }); - }); - - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: -1 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: 1, - }); - }); - - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: -2 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: 1, - }); - }); - }); - - describe('Offset pagination', () => { - test('Uses specified limit', () => { - const pagination = { limit: 5 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - start: 0, - limit: pagination.limit, - }); - }); - - test('Uses maxLimit as limit', () => { - const pagination = { limit: 999 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - start: 0, - limit: maxLimit, - }); - }); - - test('Uses 1 as limit', () => { - const pagination = { limit: 0 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - start: 0, - limit: 1, - }); - }); - - test('Uses maxLimit as limit', () => { - const pagination = { limit: -1 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - start: 0, - limit: maxLimit, - }); - }); - - test('Uses 1 as limit', () => { - const pagination = { limit: -2 }; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - start: 0, - limit: 1, - }); - }); - }); -}); diff --git a/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js b/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js new file mode 100644 index 0000000000..48b1e0ff38 --- /dev/null +++ b/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js @@ -0,0 +1,138 @@ +'use strict'; + +const _ = require('lodash'); +const { getPaginationInfo } = require('../pagination'); + +const maxLimit = 50; +const defaultLimit = 20; + +// init global strapi +global.strapi = { + config: { + get(path, defaultValue) { + return _.get(this, path, defaultValue); + }, + api: { + rest: { + defaultLimit, + maxLimit, + }, + }, + }, +}; + +describe('Pagination service', () => { + test('Uses default limit', () => { + const pagination = {}; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: defaultLimit, + }); + }); + + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: pagination.pageSize, + }); + }); + + test('Uses maxLimit as pageSize', () => { + const pagination = { pageSize: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: maxLimit, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + }); + + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: pagination.limit, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); + }); + }); +}); From 92e5a71256f95643299a7f7708aa1ef89c4c1bb4 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 23 Nov 2021 18:24:42 +0100 Subject: [PATCH 12/31] tests: added tests for when maxLimit not configured --- .../service/__tests__/pagination.test.js | 305 +++++++++++++----- 1 file changed, 221 insertions(+), 84 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js b/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js index 48b1e0ff38..3390a39d6f 100644 --- a/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js +++ b/packages/core/strapi/lib/core-api/service/__tests__/pagination.test.js @@ -6,132 +6,269 @@ const { getPaginationInfo } = require('../pagination'); const maxLimit = 50; const defaultLimit = 20; -// init global strapi -global.strapi = { - config: { - get(path, defaultValue) { - return _.get(this, path, defaultValue); - }, - api: { - rest: { - defaultLimit, - maxLimit, - }, - }, - }, -}; - describe('Pagination service', () => { - test('Uses default limit', () => { - const pagination = {}; - const paginationInfo = getPaginationInfo({ pagination }); - - expect(paginationInfo).toEqual({ - page: 1, - pageSize: defaultLimit, + describe('With maxLimit set globally', () => { + beforeAll(() => { + global.strapi = { + config: { + get(path, defaultValue) { + return _.get(this, path, defaultValue); + }, + api: { + rest: { + defaultLimit, + maxLimit, + }, + }, + }, + }; }); - }); - describe('Paged pagination', () => { - test('Uses specified pageSize', () => { - const pagination = { pageSize: 5 }; + test('Uses default limit', () => { + const pagination = {}; const paginationInfo = getPaginationInfo({ pagination }); expect(paginationInfo).toEqual({ page: 1, - pageSize: pagination.pageSize, + pageSize: defaultLimit, }); }); - test('Uses maxLimit as pageSize', () => { - const pagination = { pageSize: 999 }; - const paginationInfo = getPaginationInfo({ pagination }); + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - page: 1, - pageSize: maxLimit, + expect(paginationInfo).toEqual({ + page: 1, + pageSize: pagination.pageSize, + }); + }); + + test('Uses maxLimit as pageSize', () => { + const pagination = { pageSize: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: maxLimit, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); }); }); - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: 0 }; - const paginationInfo = getPaginationInfo({ pagination }); + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - page: 1, - pageSize: 1, + expect(paginationInfo).toEqual({ + start: 0, + limit: pagination.limit, + }); }); - }); - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: -1 }; - const paginationInfo = getPaginationInfo({ pagination }); + test('Uses maxLimit as limit', () => { + const pagination = { limit: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - page: 1, - pageSize: 1, + expect(paginationInfo).toEqual({ + start: 0, + limit: maxLimit, + }); }); - }); - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: -2 }; - const paginationInfo = getPaginationInfo({ pagination }); + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - page: 1, - pageSize: 1, + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); }); }); }); - describe('Offset pagination', () => { - test('Uses specified limit', () => { - const pagination = { limit: 5 }; + // Setting global strapi api conf + + describe('With maxLimit undefined', () => { + beforeAll(() => { + global.strapi = { + config: { + get(path, defaultValue) { + return _.get(this, path, defaultValue); + }, + api: { + rest: { + defaultLimit, + maxLimit: undefined, + }, + }, + }, + }; + }); + + test('Uses default limit', () => { + const pagination = {}; const paginationInfo = getPaginationInfo({ pagination }); expect(paginationInfo).toEqual({ - start: 0, - limit: pagination.limit, + page: 1, + pageSize: defaultLimit, }); }); - test('Uses maxLimit as limit', () => { - const pagination = { limit: 999 }; - const paginationInfo = getPaginationInfo({ pagination }); + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - start: 0, - limit: maxLimit, + expect(paginationInfo).toEqual({ + page: 1, + pageSize: pagination.pageSize, + }); + }); + + test('Uses specified pageSize', () => { + const pagination = { pageSize: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: pagination.pageSize, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + page: 1, + pageSize: 1, + }); }); }); - test('Uses 1 as limit', () => { - const pagination = { limit: 0 }; - const paginationInfo = getPaginationInfo({ pagination }); + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - start: 0, - limit: 1, + expect(paginationInfo).toEqual({ + start: 0, + limit: pagination.limit, + }); }); - }); - test('Uses maxLimit as limit', () => { - const pagination = { limit: -1 }; - const paginationInfo = getPaginationInfo({ pagination }); + test('Uses specified limit', () => { + const pagination = { limit: 999 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - start: 0, - limit: maxLimit, + expect(paginationInfo).toEqual({ + start: 0, + limit: pagination.limit, + }); }); - }); - test('Uses 1 as limit', () => { - const pagination = { limit: -2 }; - const paginationInfo = getPaginationInfo({ pagination }); + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const paginationInfo = getPaginationInfo({ pagination }); - expect(paginationInfo).toEqual({ - start: 0, - limit: 1, + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses -1 as limit', () => { + const pagination = { limit: -1 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: -1, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const paginationInfo = getPaginationInfo({ pagination }); + + expect(paginationInfo).toEqual({ + start: 0, + limit: 1, + }); }); }); }); From 34140bcae7877defc1a1ca63625d56d071e8b552 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 23 Nov 2021 18:27:18 +0100 Subject: [PATCH 13/31] refactor: moved maxLimit maxing outside of applyMaxLimit --- .../strapi/lib/core-api/service/pagination.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/pagination.js b/packages/core/strapi/lib/core-api/service/pagination.js index b3d762be90..73f22420d7 100644 --- a/packages/core/strapi/lib/core-api/service/pagination.js +++ b/packages/core/strapi/lib/core-api/service/pagination.js @@ -13,18 +13,13 @@ const getLimitConfigDefaults = () => ({ }); /** - * if there is max limit set and limit exceeds this number, return configured max limit + * Should maxLimit be used as the limit or not * @param {number} limit - limit you want to cap * @param {number?} maxLimit - maxlimit used has capping - * @returns {number} + * @returns {boolean} */ -const applyMaxLimit = (limit, maxLimit = null) => { - if (limit === -1 || (maxLimit && limit > maxLimit)) { - return maxLimit || -1; - } - - return Math.max(1, limit); -}; +const shouldApplyMaxLimit = (limit, maxLimit = null) => + limit === -1 || (maxLimit && limit > maxLimit); const shouldCount = params => { if (has('pagination.withCount', params)) { @@ -81,7 +76,7 @@ const getPaginationInfo = params => { return { page: Math.max(1, toNumber(pagination.page || 1)), - pageSize: applyMaxLimit(pageSize, maxLimit), + pageSize: shouldApplyMaxLimit(pageSize, maxLimit) ? maxLimit || -1 : Math.max(1, pageSize), }; } @@ -89,7 +84,7 @@ const getPaginationInfo = params => { return { start: Math.max(0, toNumber(pagination.start || 0)), - limit: applyMaxLimit(limit, maxLimit), + limit: shouldApplyMaxLimit(limit, maxLimit) ? maxLimit || -1 : Math.max(1, limit), }; }; From a97c831cc020942715e5b1f8623ac151125a40fd Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Wed, 24 Nov 2021 10:13:02 +0100 Subject: [PATCH 14/31] tests: added tests for when maxLimit not configured(graphql) --- .../utils/lib/__tests__/pagination.test.js | 261 +++++++++++++----- 1 file changed, 190 insertions(+), 71 deletions(-) diff --git a/packages/core/utils/lib/__tests__/pagination.test.js b/packages/core/utils/lib/__tests__/pagination.test.js index 450e67aaf1..709ffff821 100644 --- a/packages/core/utils/lib/__tests__/pagination.test.js +++ b/packages/core/utils/lib/__tests__/pagination.test.js @@ -3,123 +3,242 @@ const { withDefaultPagination } = require('../pagination'); const defaultLimit = 20; -const maxLimit = 50; const defaults = { offset: { limit: defaultLimit }, page: { pageSize: defaultLimit }, }; describe('Pagination util', () => { - test('Uses default limit', () => { - const pagination = {}; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + describe('With maxLimit set', () => { + const maxLimit = 50; - expect(defaultPagination).toEqual({ - start: 0, - limit: defaultLimit, - }); - }); - - describe('Paged pagination', () => { - test('Uses specified pageSize', () => { - const pagination = { pageSize: 5 }; + test('Uses default limit', () => { + const pagination = {}; const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); expect(defaultPagination).toEqual({ start: 0, - limit: pagination.pageSize, + limit: defaultLimit, }); }); - test('Uses maxLimit as pageSize', () => { - const pagination = { pageSize: 999 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); - expect(defaultPagination).toEqual({ - start: 0, - limit: maxLimit, + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.pageSize, + }); + }); + + test('Uses maxLimit as pageSize', () => { + const pagination = { pageSize: 999 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); }); }); - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: 0 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); - expect(defaultPagination).toEqual({ - start: 0, - limit: 1, + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.limit, + }); }); - }); - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: -1 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + test('Uses maxLimit as limit', () => { + const pagination = { limit: 999 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); - expect(defaultPagination).toEqual({ - start: 0, - limit: 1, + expect(defaultPagination).toEqual({ + start: 0, + limit: maxLimit, + }); }); - }); - test('Uses 1 as pageSize', () => { - const pagination = { pageSize: -2 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); - expect(defaultPagination).toEqual({ - start: 0, - limit: 1, + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses maxLimit as limit', () => { + const pagination = { limit: -1 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: maxLimit, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); }); }); }); - describe('Offset pagination', () => { - test('Uses specified limit', () => { - const pagination = { limit: 5 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + describe('With maxLimit undefined', () => { + test('Uses default limit', () => { + const pagination = {}; + const defaultPagination = withDefaultPagination(pagination, { defaults }); expect(defaultPagination).toEqual({ start: 0, - limit: pagination.limit, + limit: defaultLimit, }); }); - test('Uses maxLimit as limit', () => { - const pagination = { limit: 999 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + describe('Paged pagination', () => { + test('Uses specified pageSize', () => { + const pagination = { pageSize: 5 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); - expect(defaultPagination).toEqual({ - start: 0, - limit: maxLimit, + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.pageSize, + }); + }); + + test('Uses specified pageSize', () => { + const pagination = { pageSize: 999 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.pageSize, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: 0 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -1 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses 1 as pageSize', () => { + const pagination = { pageSize: -2 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); }); }); - test('Uses 1 as limit', () => { - const pagination = { limit: 0 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + describe('Offset pagination', () => { + test('Uses specified limit', () => { + const pagination = { limit: 5 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); - expect(defaultPagination).toEqual({ - start: 0, - limit: 1, + expect(defaultPagination).toEqual({ + start: 0, + limit: pagination.limit, + }); }); - }); - test('Uses maxLimit as limit', () => { - const pagination = { limit: -1 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + test('Uses apecified limit', () => { + const pagination = { limit: 999 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); - expect(defaultPagination).toEqual({ - start: 0, - limit: maxLimit, + expect(defaultPagination).toEqual({ + start: 0, + limit: 999, + }); }); - }); - test('Uses 1 as limit', () => { - const pagination = { limit: -2 }; - const defaultPagination = withDefaultPagination(pagination, { defaults, maxLimit }); + test('Uses 1 as limit', () => { + const pagination = { limit: 0 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); - expect(defaultPagination).toEqual({ - start: 0, - limit: 1, + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); + }); + + test('Uses -1 as limit', () => { + const pagination = { limit: -1 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: -1, + }); + }); + + test('Uses 1 as limit', () => { + const pagination = { limit: -2 }; + const defaultPagination = withDefaultPagination(pagination, { defaults }); + + expect(defaultPagination).toEqual({ + start: 0, + limit: 1, + }); }); }); }); From 56f3b4041ff05b743a9aa8c434f8701bbec4eb2b Mon Sep 17 00:00:00 2001 From: Vincent <89356961+vincentbpro@users.noreply.github.com> Date: Thu, 2 Dec 2021 16:02:01 +0100 Subject: [PATCH 15/31] Update packages/core/strapi/lib/core-api/service/__tests__/index.test.js Co-authored-by: Alexandre BODIN --- .../core/strapi/lib/core-api/service/__tests__/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/strapi/lib/core-api/service/__tests__/index.test.js b/packages/core/strapi/lib/core-api/service/__tests__/index.test.js index 704699d23f..fb0b176e80 100644 --- a/packages/core/strapi/lib/core-api/service/__tests__/index.test.js +++ b/packages/core/strapi/lib/core-api/service/__tests__/index.test.js @@ -1,6 +1,6 @@ 'use strict'; -const { createService } = require('..'); +const { createService } = require('../index'); describe('Default Service', () => { describe('Collection Type', () => { From 3ed7f1c293362337ac8958162c0e2b4aa83cb691 Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Thu, 2 Dec 2021 16:47:56 +0100 Subject: [PATCH 16/31] fix: removed -1 limit handling for REST --- packages/core/strapi/lib/core-api/service/pagination.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/strapi/lib/core-api/service/pagination.js b/packages/core/strapi/lib/core-api/service/pagination.js index 73f22420d7..872f4b3596 100644 --- a/packages/core/strapi/lib/core-api/service/pagination.js +++ b/packages/core/strapi/lib/core-api/service/pagination.js @@ -18,8 +18,8 @@ const getLimitConfigDefaults = () => ({ * @param {number?} maxLimit - maxlimit used has capping * @returns {boolean} */ -const shouldApplyMaxLimit = (limit, maxLimit = null) => - limit === -1 || (maxLimit && limit > maxLimit); +const shouldApplyMaxLimit = (limit, maxLimit = null, { isPagedPagination = false } = {}) => + (!isPagedPagination && limit === -1) || (maxLimit && limit > maxLimit); const shouldCount = params => { if (has('pagination.withCount', params)) { @@ -76,7 +76,9 @@ const getPaginationInfo = params => { return { page: Math.max(1, toNumber(pagination.page || 1)), - pageSize: shouldApplyMaxLimit(pageSize, maxLimit) ? maxLimit || -1 : Math.max(1, pageSize), + pageSize: shouldApplyMaxLimit(pageSize, maxLimit, { isPagedPagination: true }) + ? maxLimit + : Math.max(1, pageSize), }; } From 2461a73a6c603bc4ed02254e1b1b2ac29c7e3a1c Mon Sep 17 00:00:00 2001 From: harimkims Date: Fri, 3 Dec 2021 04:11:44 +0900 Subject: [PATCH 17/31] Fix unable to download original image in uploader Signed-off-by: harimkims --- .../admin/src/components/AssetCard/AssetCard.js | 2 +- .../components/EditAssetDialog/PreviewBox/index.js | 12 +++++++++--- .../MediaLibraryInput/Carousel/CarouselAsset.js | 4 ++-- .../core/upload/admin/src/utils/createAssetUrl.js | 11 +++++++++-- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/core/upload/admin/src/components/AssetCard/AssetCard.js b/packages/core/upload/admin/src/components/AssetCard/AssetCard.js index 41ce38ace9..c83b85313c 100644 --- a/packages/core/upload/admin/src/components/AssetCard/AssetCard.js +++ b/packages/core/upload/admin/src/components/AssetCard/AssetCard.js @@ -27,7 +27,7 @@ export const AssetCard = ({ allowedTypes, asset, isSelected, onSelect, onEdit, s key={asset.id} name={asset.name} extension={getFileExtension(asset.ext)} - url={local ? asset.url : createAssetUrl(asset)} + url={local ? asset.url : createAssetUrl(asset, true)} mime={asset.mime} onEdit={onEdit ? () => onEdit(asset) : undefined} onSelect={handleSelect} diff --git a/packages/core/upload/admin/src/components/EditAssetDialog/PreviewBox/index.js b/packages/core/upload/admin/src/components/EditAssetDialog/PreviewBox/index.js index 551a2a08f5..85963c0903 100644 --- a/packages/core/upload/admin/src/components/EditAssetDialog/PreviewBox/index.js +++ b/packages/core/upload/admin/src/components/EditAssetDialog/PreviewBox/index.js @@ -41,7 +41,8 @@ export const PreviewBox = ({ }) => { const { trackUsage } = useTracking(); const previewRef = useRef(null); - const [assetUrl, setAssetUrl] = useState(createAssetUrl(asset)); + const [assetUrl, setAssetUrl] = useState(createAssetUrl(asset, false)); + const [thumbnailUrl, setThumbnailUrl] = useState(createAssetUrl(asset, true)); const { formatMessage } = useIntl(); const [showConfirmDialog, setShowConfirmDialog] = useState(false); const { @@ -73,6 +74,7 @@ export const PreviewBox = ({ asset.url = fileLocalUrl; } setAssetUrl(fileLocalUrl); + setThumbnailUrl(fileLocalUrl); } }, [replacementFile, asset]); @@ -83,16 +85,19 @@ export const PreviewBox = ({ // Making sure that when persisting the new asset, the URL changes with width and height // So that the browser makes a request and handle the image caching correctly at the good size let optimizedCachingImage; + let optimizedCachingThumbnailImage; if (asset.isLocal) { optimizedCachingImage = URL.createObjectURL(file); + optimizedCachingThumbnailImage = optimizedCachingImage; asset.url = optimizedCachingImage; asset.rawFile = file; trackUsage('didCropFile', { duplicatedFile: null, location: trackedLocation }); } else { const updatedAsset = await editAsset(nextAsset, file); - optimizedCachingImage = createAssetUrl(updatedAsset); + optimizedCachingImage = createAssetUrl(updatedAsset, false); + optimizedCachingThumbnailImage = createAssetUrl(updatedAsset, true); trackUsage('didCropFile', { duplicatedFile: false, location: trackedLocation }); } @@ -100,6 +105,7 @@ export const PreviewBox = ({ stopCropping(); onCropCancel(); setAssetUrl(optimizedCachingImage); + setThumbnailUrl(optimizedCachingThumbnailImage); }; const isInCroppingMode = isCropping && !isLoading; @@ -192,7 +198,7 @@ export const PreviewBox = ({ )} - + { return ( @@ -39,7 +39,7 @@ export const CarouselAsset = ({ asset }) => { as="img" maxHeight="100%" maxWidth="100%" - src={createAssetUrl(asset)} + src={createAssetUrl(asset, true)} alt={asset.alternativeText || asset.name} /> ); diff --git a/packages/core/upload/admin/src/utils/createAssetUrl.js b/packages/core/upload/admin/src/utils/createAssetUrl.js index 5c878601c4..8560b80f11 100644 --- a/packages/core/upload/admin/src/utils/createAssetUrl.js +++ b/packages/core/upload/admin/src/utils/createAssetUrl.js @@ -1,11 +1,18 @@ import { prefixFileUrlWithBackendUrl } from '@strapi/helper-plugin'; -export const createAssetUrl = asset => { +/** + * Create image URL for asset + * @param {Object} asset + * @param {Boolean} forThumbnail - if true, return URL for thumbnail + * if there's no thumbnail, return the URL of the original image. + * @return {String} URL + */ +export const createAssetUrl = (asset, forThumbnail = true) => { if (asset.isLocal) { return asset.url; } - const assetUrl = asset?.formats?.thumbnail?.url || asset.url; + const assetUrl = forThumbnail ? asset?.formats?.thumbnail?.url || asset.url : asset.url; const backendUrl = prefixFileUrlWithBackendUrl(assetUrl); return `${backendUrl}?updated_at=${asset.updatedAt}`; From d68c4769e9c8ec3cf424c6163a5a2ea4e68fdb49 Mon Sep 17 00:00:00 2001 From: harimkims Date: Fri, 3 Dec 2021 04:39:33 +0900 Subject: [PATCH 18/31] Fix falling front unit test Signed-off-by: harimkims --- .../components/EditAssetDialog/tests/EditAssetDialog.test.js | 2 +- .../admin/src/components/EditAssetDialog/tests/index.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/upload/admin/src/components/EditAssetDialog/tests/EditAssetDialog.test.js b/packages/core/upload/admin/src/components/EditAssetDialog/tests/EditAssetDialog.test.js index 3df99dc985..f82579b1e8 100644 --- a/packages/core/upload/admin/src/components/EditAssetDialog/tests/EditAssetDialog.test.js +++ b/packages/core/upload/admin/src/components/EditAssetDialog/tests/EditAssetDialog.test.js @@ -201,7 +201,7 @@ describe('', () => { fireEvent.click(screen.getByLabelText('Download')); expect(downloadFile).toHaveBeenCalledWith( - 'http://localhost:1337/uploads/thumbnail_Screenshot_2_5d4a574d61.png?updated_at=2021-10-04T09:42:31.670Z', + 'http://localhost:1337/uploads/Screenshot_2_5d4a574d61.png?updated_at=2021-10-04T09:42:31.670Z', 'Screenshot 2.png' ); }); diff --git a/packages/core/upload/admin/src/components/EditAssetDialog/tests/index.test.js b/packages/core/upload/admin/src/components/EditAssetDialog/tests/index.test.js index daf1fc036f..76c9c02b59 100644 --- a/packages/core/upload/admin/src/components/EditAssetDialog/tests/index.test.js +++ b/packages/core/upload/admin/src/components/EditAssetDialog/tests/index.test.js @@ -156,7 +156,7 @@ describe('', () => { fireEvent.click(screen.getByLabelText('Download')); expect(downloadFile).toHaveBeenCalledWith( - 'http://localhost:1337/uploads/thumbnail_Screenshot_2_5d4a574d61.png?updated_at=2021-10-04T09:42:31.670Z', + 'http://localhost:1337/uploads/Screenshot_2_5d4a574d61.png?updated_at=2021-10-04T09:42:31.670Z', 'Screenshot 2.png' ); }); From 411a04398f9e513fd5d9b129d631d98c5e458201 Mon Sep 17 00:00:00 2001 From: harimkims Date: Sun, 5 Dec 2021 07:53:23 +0900 Subject: [PATCH 19/31] fix unable to get plugin config per env-config Signed-off-by: harimkims --- .../strapi/lib/core/loaders/plugins/index.js | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/index.js b/packages/core/strapi/lib/core/loaders/plugins/index.js index 9154d595dd..9a4fb6a4ac 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/index.js +++ b/packages/core/strapi/lib/core/loaders/plugins/index.js @@ -65,11 +65,24 @@ const formatContentTypes = plugins => { } }; +const getUserPluginConfigPath = async () => { + const globalUserConfigPath = join(strapi.dirs.config, 'plugins.js'); + const currentEnvUserConfigPath = join( + strapi.dirs.config, + `env/${process.env.NODE_ENV}/plugins.js` + ); + + if (await fse.pathExists(currentEnvUserConfigPath)) { + return loadConfigFile(currentEnvUserConfigPath); + } else if (await fse.pathExists(globalUserConfigPath)) { + return loadConfigFile(globalUserConfigPath); + } else { + return {}; + } +}; + const applyUserConfig = async plugins => { - const userPluginConfigPath = join(strapi.dirs.config, 'plugins.js'); - const userPluginsConfig = (await fse.pathExists(userPluginConfigPath)) - ? loadConfigFile(userPluginConfigPath) - : {}; + const userPluginsConfig = await getUserPluginConfigPath(); for (const pluginName in plugins) { const plugin = plugins[pluginName]; From ae58d85103805edebf39fe0a842a4e4c0287abae Mon Sep 17 00:00:00 2001 From: harimkims Date: Sun, 5 Dec 2021 08:19:15 +0900 Subject: [PATCH 20/31] rename function Signed-off-by: harimkims --- packages/core/strapi/lib/core/loaders/plugins/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/index.js b/packages/core/strapi/lib/core/loaders/plugins/index.js index 9a4fb6a4ac..5a06962c01 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/index.js +++ b/packages/core/strapi/lib/core/loaders/plugins/index.js @@ -65,7 +65,7 @@ const formatContentTypes = plugins => { } }; -const getUserPluginConfigPath = async () => { +const getUserPluginsConfig = async () => { const globalUserConfigPath = join(strapi.dirs.config, 'plugins.js'); const currentEnvUserConfigPath = join( strapi.dirs.config, @@ -82,7 +82,7 @@ const getUserPluginConfigPath = async () => { }; const applyUserConfig = async plugins => { - const userPluginsConfig = await getUserPluginConfigPath(); + const userPluginsConfig = await getUserPluginsConfig(); for (const pluginName in plugins) { const plugin = plugins[pluginName]; From 74b96a56dc9833625bbac8fb3fa5ebdb9edfcd5e Mon Sep 17 00:00:00 2001 From: matsubara Date: Tue, 7 Dec 2021 00:51:29 +0900 Subject: [PATCH 21/31] Update ja.json --- .../core/email/admin/src/translations/ja.json | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/core/email/admin/src/translations/ja.json b/packages/core/email/admin/src/translations/ja.json index 0967ef424b..e159c8de47 100644 --- a/packages/core/email/admin/src/translations/ja.json +++ b/packages/core/email/admin/src/translations/ja.json @@ -1 +1,22 @@ -{} +{ + "Settings.email.plugin.button.test-email": "メール送信のテスト", + "Settings.email.plugin.label.defaultFrom": "デフォルトの送信アドレス", + "Settings.email.plugin.label.defaultReplyTo": "デフォルトの返信アドレス", + "Settings.email.plugin.label.provider": "Eメール プロバイダ", + "Settings.email.plugin.label.testAddress": "メール送信のテストをする", + "Settings.email.plugin.notification.config.error": "Eメールの設定の取得に失敗しました", + "Settings.email.plugin.notification.data.loaded": "Eメールの設定データはすでに取得済みです", + "Settings.email.plugin.notification.test.error": "{to}へのテストメースの送信に失敗しました", + "Settings.email.plugin.notification.test.success": "テストメースの送信に成功しました、{to}のメールボックスを確認してください", + "Settings.email.plugin.placeholder.defaultFrom": "例: Strapi No-Reply ", + "Settings.email.plugin.placeholder.defaultReplyTo": "例: Strapi ", + "Settings.email.plugin.placeholder.testAddress": "例: developer@example.com", + "Settings.email.plugin.subTitle": "Eメールプラグインの設定のテスト", + "Settings.email.plugin.text.configuration": "このプラグインは{file}ファイルを用いて構成されました、このリンク {link} のドキュメントを確認してください。", + "Settings.email.plugin.title": "Eメールの設定", + "Settings.email.plugin.title.config": "構成", + "Settings.email.plugin.title.test": "テストメールの送信", + "SettingsNav.link.settings": "設定", + "SettingsNav.section-label": "Eメール プラグイン", + "components.Input.error.validation.email": "これは無効なEメールアドレスです" +} From b019507e58c80f8f9331c5ebc5ec2f48aeae9981 Mon Sep 17 00:00:00 2001 From: Yuku Takahashi Date: Wed, 8 Dec 2021 23:10:09 +0900 Subject: [PATCH 22/31] Skip if dependency doesn't export package.json --- .../lib/core/loaders/plugins/get-enabled-plugins.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js index 5318d43827..b3a91ac3ac 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js +++ b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js @@ -60,7 +60,13 @@ const getEnabledPlugins = async strapi => { const installedPlugins = {}; for (const dep in strapi.config.get('info.dependencies', {})) { const packagePath = join(dep, 'package.json'); - const packageInfo = require(packagePath); + let packageInfo + try { + packageInfo = require(packagePath); + } catch { + // Some packages, including firebase-admin, do not export package.json. + continue + } if (isStrapiPlugin(packageInfo)) { validatePluginName(packageInfo.strapi.name); From b56bd71be923f6568dbfe664d7737ba922898215 Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 9 Dec 2021 20:48:22 +0900 Subject: [PATCH 23/31] apply feedback Signed-off-by: harimkims --- .../strapi/lib/core/loaders/plugins/index.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/index.js b/packages/core/strapi/lib/core/loaders/plugins/index.js index 5a06962c01..a324f2bfbf 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/index.js +++ b/packages/core/strapi/lib/core/loaders/plugins/index.js @@ -3,6 +3,7 @@ const { join } = require('path'); const fse = require('fs-extra'); const { defaultsDeep, getOr, get } = require('lodash/fp'); +const { merge } = require('lodash'); const { env } = require('@strapi/utils'); const loadConfigFile = require('../../app-configuration/load-config-file'); const loadFiles = require('../../../load/load-files'); @@ -71,14 +72,19 @@ const getUserPluginsConfig = async () => { strapi.dirs.config, `env/${process.env.NODE_ENV}/plugins.js` ); + let config = {}; - if (await fse.pathExists(currentEnvUserConfigPath)) { - return loadConfigFile(currentEnvUserConfigPath); - } else if (await fse.pathExists(globalUserConfigPath)) { - return loadConfigFile(globalUserConfigPath); - } else { - return {}; + // assign global user config if exists + if (await fse.pathExists(globalUserConfigPath)) { + config = loadConfigFile(globalUserConfigPath); } + + // and merge user config by environment if exists + if (await fse.pathExists(currentEnvUserConfigPath)) { + config = merge(config, loadConfigFile(currentEnvUserConfigPath)); + } + + return config; }; const applyUserConfig = async plugins => { From bf621939126adf8f8bd53b19a7a2ea2b7d71d439 Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 9 Dec 2021 20:53:28 +0900 Subject: [PATCH 24/31] use lodash/fp merge instead lodash Signed-off-by: harimkims --- packages/core/strapi/lib/core/loaders/plugins/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/index.js b/packages/core/strapi/lib/core/loaders/plugins/index.js index a324f2bfbf..fe33749948 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/index.js +++ b/packages/core/strapi/lib/core/loaders/plugins/index.js @@ -2,8 +2,7 @@ const { join } = require('path'); const fse = require('fs-extra'); -const { defaultsDeep, getOr, get } = require('lodash/fp'); -const { merge } = require('lodash'); +const { defaultsDeep, getOr, get, merge } = require('lodash/fp'); const { env } = require('@strapi/utils'); const loadConfigFile = require('../../app-configuration/load-config-file'); const loadFiles = require('../../../load/load-files'); From 6bb5403ccb621f58f2b1f2dca3c2c46cb1573bdf Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 9 Dec 2021 21:55:41 +0900 Subject: [PATCH 25/31] simplify join Signed-off-by: harimkims --- packages/core/strapi/lib/core/loaders/plugins/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/index.js b/packages/core/strapi/lib/core/loaders/plugins/index.js index fe33749948..f44cdf0964 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/index.js +++ b/packages/core/strapi/lib/core/loaders/plugins/index.js @@ -68,8 +68,7 @@ const formatContentTypes = plugins => { const getUserPluginsConfig = async () => { const globalUserConfigPath = join(strapi.dirs.config, 'plugins.js'); const currentEnvUserConfigPath = join( - strapi.dirs.config, - `env/${process.env.NODE_ENV}/plugins.js` + strapi.dirs.config, 'env', process.env.NODE_ENV, 'plugins.js' ); let config = {}; From 5414bb87939607cab6664dd8cbd82e9743fcf178 Mon Sep 17 00:00:00 2001 From: harimkims Date: Fri, 10 Dec 2021 04:37:43 +0900 Subject: [PATCH 26/31] fix internal plugin's enabled flag ignore user plugin's enabled - make `get-user-plugins-config` function as independent function - when you load enabled plugins, use `get-user-plugins-config` function so that it recognizes user's plugin config per environment Signed-off-by: harimkims --- .../loaders/plugins/get-enabled-plugins.js | 8 ++-- .../plugins/get-user-plugins-config.js | 39 +++++++++++++++++++ .../strapi/lib/core/loaders/plugins/index.js | 23 +---------- 3 files changed, 44 insertions(+), 26 deletions(-) create mode 100644 packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js index 5318d43827..274e9499a1 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js +++ b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js @@ -5,7 +5,7 @@ const { statSync, existsSync } = require('fs'); const _ = require('lodash'); const { get, has, pick, pickBy, defaultsDeep, map, prop, pipe } = require('lodash/fp'); const { isKebabCase } = require('@strapi/utils'); -const loadConfigFile = require('../../app-configuration/load-config-file'); +const getUserPluginsConfig = require('./get-user-plugins-config'); const isStrapiPlugin = info => get('strapi.kind', info) === 'plugin'; const INTERNAL_PLUGINS = [ @@ -72,10 +72,7 @@ const getEnabledPlugins = async strapi => { } const declaredPlugins = {}; - const userPluginConfigPath = join(strapi.dirs.config, 'plugins.js'); - const userPluginsConfig = existsSync(userPluginConfigPath) - ? loadConfigFile(userPluginConfigPath) - : {}; + const userPluginsConfig = await getUserPluginsConfig(); _.forEach(userPluginsConfig, (declaration, pluginName) => { validatePluginName(pluginName); @@ -106,6 +103,7 @@ const getEnabledPlugins = async strapi => { const enabledPlugins = pipe( defaultsDeep(declaredPlugins), + pickBy((pValue, pName) => _.get(declaredPlugins, [pName, 'enabled'], true)), defaultsDeep(installedPluginsNotAlreadyUsed), pickBy(p => p.enabled) )(internalPlugins); diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js b/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js new file mode 100644 index 0000000000..7c019604e3 --- /dev/null +++ b/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js @@ -0,0 +1,39 @@ +'use strict'; + +const { join } = require('path'); +const fse = require('fs-extra'); +const { merge } = require('lodash/fp'); +const loadConfigFile = require('../../app-configuration/load-config-file'); + +/** + * Return user defined plugins' config + * first load config from `config/plugins.js` + * and then merge config from `config/env/{env}/plugins.js` + * @return {Promise<{}>} + */ +const getUserPluginsConfig = async () => { + const globalUserConfigPath = join(strapi.dirs.config, 'plugins.js'); + const currentEnvUserConfigPath = join( + strapi.dirs.config, + 'env', + process.env.NODE_ENV, + 'plugins.js' + ); + let config = {}; + + // assign global user config if exists + if (await fse.pathExists(globalUserConfigPath)) { + config = loadConfigFile(globalUserConfigPath); + } + console.log('global', config); + + // and merge user config by environment if exists + if (await fse.pathExists(currentEnvUserConfigPath)) { + config = merge(config, loadConfigFile(currentEnvUserConfigPath)); + console.log('env', config); + } + + return config; +}; + +module.exports = getUserPluginsConfig; diff --git a/packages/core/strapi/lib/core/loaders/plugins/index.js b/packages/core/strapi/lib/core/loaders/plugins/index.js index f44cdf0964..982805a1ae 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/index.js +++ b/packages/core/strapi/lib/core/loaders/plugins/index.js @@ -2,11 +2,12 @@ const { join } = require('path'); const fse = require('fs-extra'); -const { defaultsDeep, getOr, get, merge } = require('lodash/fp'); +const { defaultsDeep, getOr, get } = require('lodash/fp'); const { env } = require('@strapi/utils'); const loadConfigFile = require('../../app-configuration/load-config-file'); const loadFiles = require('../../../load/load-files'); const getEnabledPlugins = require('./get-enabled-plugins'); +const getUserPluginsConfig = require('./get-user-plugins-config'); const defaultPlugin = { bootstrap() {}, @@ -65,26 +66,6 @@ const formatContentTypes = plugins => { } }; -const getUserPluginsConfig = async () => { - const globalUserConfigPath = join(strapi.dirs.config, 'plugins.js'); - const currentEnvUserConfigPath = join( - strapi.dirs.config, 'env', process.env.NODE_ENV, 'plugins.js' - ); - let config = {}; - - // assign global user config if exists - if (await fse.pathExists(globalUserConfigPath)) { - config = loadConfigFile(globalUserConfigPath); - } - - // and merge user config by environment if exists - if (await fse.pathExists(currentEnvUserConfigPath)) { - config = merge(config, loadConfigFile(currentEnvUserConfigPath)); - } - - return config; -}; - const applyUserConfig = async plugins => { const userPluginsConfig = await getUserPluginsConfig(); From bcb2745733ac677bdec64aa3dec4eb6cda70cf31 Mon Sep 17 00:00:00 2001 From: Yuku Takahashi Date: Sat, 11 Dec 2021 00:27:32 +0900 Subject: [PATCH 27/31] Apply suggestions from code review --- .../strapi/lib/core/loaders/plugins/get-enabled-plugins.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js index b3a91ac3ac..c66bdf20cd 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js +++ b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js @@ -60,12 +60,12 @@ const getEnabledPlugins = async strapi => { const installedPlugins = {}; for (const dep in strapi.config.get('info.dependencies', {})) { const packagePath = join(dep, 'package.json'); - let packageInfo + let packageInfo; try { packageInfo = require(packagePath); } catch { // Some packages, including firebase-admin, do not export package.json. - continue + continue; } if (isStrapiPlugin(packageInfo)) { From 13435d25d05a7d2a4030b525f16bc682955b06e4 Mon Sep 17 00:00:00 2001 From: Yuku Takahashi Date: Tue, 14 Dec 2021 08:42:35 +0900 Subject: [PATCH 28/31] Remove unnecessary comment Co-authored-by: Alexandre BODIN --- .../core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js index c66bdf20cd..dae50a0f0e 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js +++ b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js @@ -64,7 +64,6 @@ const getEnabledPlugins = async strapi => { try { packageInfo = require(packagePath); } catch { - // Some packages, including firebase-admin, do not export package.json. continue; } From a5ac20dda05915ea69365da45ebd1188a9e6782f Mon Sep 17 00:00:00 2001 From: vincentbpro <89356961+vincentbpro@users.noreply.github.com> Date: Tue, 14 Dec 2021 10:41:47 +0100 Subject: [PATCH 29/31] fix: avoid divided by zero issue --- .../services/internals/types/response-collection-meta.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js b/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js index 604c8f9180..2a8beaec20 100644 --- a/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js +++ b/packages/plugins/graphql/server/services/internals/types/response-collection-meta.js @@ -23,11 +23,12 @@ module.exports = ({ strapi }) => { async resolve(parent) { const { args, resourceUID } = parent; const { start, limit } = args; + const safeLimit = Math.max(limit, 1); const total = await strapi.entityService.count(resourceUID, args); - const pageSize = limit === -1 ? total - start : limit; - const pageCount = limit === -1 ? 1 : Math.ceil(total / limit); - const page = limit === -1 ? 1 : Math.floor(start / limit) + 1; + const pageSize = limit === -1 ? total - start : safeLimit; + const pageCount = limit === -1 ? safeLimit : Math.ceil(total / safeLimit); + const page = limit === -1 ? safeLimit : Math.floor(start / safeLimit) + 1; return { total, page, pageSize, pageCount }; }, From ef063a1982fd15739a1bff68425c82e50ba0530f Mon Sep 17 00:00:00 2001 From: harimkims Date: Tue, 14 Dec 2021 18:41:49 +0900 Subject: [PATCH 30/31] remove console log Signed-off-by: harimkims --- .../strapi/lib/core/loaders/plugins/get-user-plugins-config.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js b/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js index 7c019604e3..10a0231089 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js +++ b/packages/core/strapi/lib/core/loaders/plugins/get-user-plugins-config.js @@ -25,12 +25,10 @@ const getUserPluginsConfig = async () => { if (await fse.pathExists(globalUserConfigPath)) { config = loadConfigFile(globalUserConfigPath); } - console.log('global', config); // and merge user config by environment if exists if (await fse.pathExists(currentEnvUserConfigPath)) { config = merge(config, loadConfigFile(currentEnvUserConfigPath)); - console.log('env', config); } return config; From e99079af24f03ae814ec98fcd844ec2fab2fe02e Mon Sep 17 00:00:00 2001 From: harimkims Date: Tue, 14 Dec 2021 19:46:34 +0900 Subject: [PATCH 31/31] remove unnecessary line --- .../core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js index 274e9499a1..1824025499 100644 --- a/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js +++ b/packages/core/strapi/lib/core/loaders/plugins/get-enabled-plugins.js @@ -103,7 +103,6 @@ const getEnabledPlugins = async strapi => { const enabledPlugins = pipe( defaultsDeep(declaredPlugins), - pickBy((pValue, pName) => _.get(declaredPlugins, [pName, 'enabled'], true)), defaultsDeep(installedPluginsNotAlreadyUsed), pickBy(p => p.enabled) )(internalPlugins);