fix PR comments, move pagination logic to the database layer, handle searches

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

packages/strapi-admin/controllers/user.js

@@ -32,10 +32,15 @@ module.exports = {
   },
 
   async find(ctx) {
-    const users = await strapi.query('user', 'admin').findPage(ctx.query);
+    const method = _.has(ctx.query, '_q') ? 'searchPage' : 'findPage';
+
+    const { results, pagination } = await strapi.admin.services.user[method](ctx.query);
 
     return {
-      data: users,
+      data: {
+        results: results.map(strapi.admin.services.user.sanitizeUser),
+        pagination,
+      },
     };
   },
 };

packages/strapi-admin/services/__tests__/user.test.js

@@ -129,48 +129,63 @@ describe('User', () => {
     });
   });
 
-  describe('Find users (paginated)', () => {
-    const defaults = {page: 1, pageSize: 100};
+  describe('Fetch users (paginated)', () => {
+    const defaults = { page: 1, pageSize: 100 };
 
     beforeEach(() => {
-      const findPage = jest.fn(({page = defaults.page, pageSize = defaults.pageSize} = {}) => {
+      const fetchPage = jest.fn(({ page = defaults.page, pageSize = defaults.pageSize } = {}) => {
         return {
-          results: Array.from({length: pageSize}).map((_, i) => i + (page - 1) * pageSize),
-          pagination: {page, pageSize, total: page * pageSize, pageCount: page},
+          results: Array.from({ length: pageSize }).map((_, i) => i + (page - 1) * pageSize),
+          pagination: { page, pageSize, total: page * pageSize, pageCount: page },
         };
       });
 
       global.strapi = {
         query() {
-          return {findPage};
+          return { findPage: fetchPage, searchPage: fetchPage };
         },
       };
     });
 
-    test('Find users with custom pagination', async () => {
-      const pagination = {page: 2, pageSize: 15};
-      const page = await userService.findPage(pagination);
+    test('Fetch users with custom pagination', async () => {
+      const pagination = { page: 2, pageSize: 15 };
+      const foundPage = await userService.findPage(pagination);
+      const searchedPage = await userService.searchPage(pagination);
 
-      expect(page.results.length).toBe(15);
-      expect(page.results[0]).toBe(15);
-      expect((page.pagination.total = 30));
+      expect(foundPage.results.length).toBe(15);
+      expect(foundPage.results[0]).toBe(15);
+      expect((foundPage.pagination.total = 30));
+
+      expect(searchedPage.results.length).toBe(15);
+      expect(searchedPage.results[0]).toBe(15);
+      expect((searchedPage.pagination.total = 30));
     });
 
-    test('Find users with default pagination', async () => {
-      const page = await userService.findPage();
+    test('Fetch users with default pagination', async () => {
+      const foundPage = await userService.findPage();
+      const searchedPage = await userService.searchPage();
 
-      expect(page.results.length).toBe(100);
-      expect(page.results[0]).toBe(0);
-      expect((page.pagination.total = 100));
+      expect(foundPage.results.length).toBe(100);
+      expect(foundPage.results[0]).toBe(0);
+      expect((foundPage.pagination.total = 100));
+
+      expect(searchedPage.results.length).toBe(100);
+      expect(searchedPage.results[0]).toBe(0);
+      expect((searchedPage.pagination.total = 100));
     });
 
-    test('Find users with partial pagination', async () => {
-      const pagination = {page: 2};
-      const page = await userService.findPage(pagination);
+    test('Fetch users with partial pagination', async () => {
+      const pagination = { page: 2 };
+      const foundPage = await userService.findPage(pagination);
+      const searchedPage = await userService.searchPage(pagination);
 
-      expect(page.results.length).toBe(100);
-      expect(page.results[0]).toBe(100);
-      expect((page.pagination.total = 200));
+      expect(foundPage.results.length).toBe(100);
+      expect(foundPage.results[0]).toBe(100);
+      expect((foundPage.pagination.total = 200));
+
+      expect(searchedPage.results.length).toBe(100);
+      expect(searchedPage.results[0]).toBe(100);
+      expect((searchedPage.pagination.total = 200));
     });
   });
 

packages/strapi-admin/services/user.js

@@ -62,11 +62,11 @@ const findRegistrationInfo = async registrationToken => {
 /**
  * Registers a user based on a registrationToken and some informations to update
  * @param {Object} params
- * @param {Object} params.registrationInfo registration token
+ * @param {Object} params.registrationToken registration token
  * @param {Object} params.userInfo user info
  */
 const register = async ({ registrationToken, userInfo }) => {
-  const matchingUser = await strapi.query('user', 'admin').findOne({registrationToken});
+  const matchingUser = await strapi.query('user', 'admin').findOne({ registrationToken });
 
   if (!matchingUser) {
     throw strapi.errors.badRequest('Invalid registration info');
@@ -75,7 +75,7 @@ const register = async ({ registrationToken, userInfo }) => {
   const hashedPassword = await strapi.admin.services.auth.hashPassword(userInfo.password);
 
   return strapi.admin.services.user.update(
-    {id: matchingUser.id},
+    { id: matchingUser.id },
     {
       password: hashedPassword,
       firstname: userInfo.firstname,
@@ -84,7 +84,7 @@ const register = async ({ registrationToken, userInfo }) => {
       isActive: true,
     }
   );
-}
+};
 
 /** Find many users (paginated)
  * @param query
@@ -94,6 +94,14 @@ const findPage = async query => {
   return strapi.query('user', 'admin').findPage(query);
 };
 
+/** Search for many users (paginated)
+ * @param query
+ * @returns {Promise<user>}
+ */
+const searchPage = async query => {
+  return strapi.query('user', 'admin').searchPage(query);
+};
+
 module.exports = {
   create,
   update,
@@ -102,4 +110,5 @@ module.exports = {
   register,
   sanitizeUser,
   findPage,
+  searchPage,
 };

packages/strapi-connector-bookshelf/lib/queries.js

@@ -8,7 +8,6 @@ const {
   convertRestQueryParams,
   buildQuery,
   escapeQuery,
-  findPageQueryFactory,
 } = require('strapi-utils');
 
 module.exports = function createQueryBuilder({ model, strapi }) {
@@ -70,8 +69,6 @@ module.exports = function createQueryBuilder({ model, strapi }) {
       .then(results => results.toJSON());
   }
 
-  const findPage = findPageQueryFactory(find, count);
-
   /**
    * Count entries based on filters
    */
@@ -623,7 +620,6 @@ module.exports = function createQueryBuilder({ model, strapi }) {
   return {
     findOne,
     find,
-    findPage,
     create,
     update,
     delete: deleteMany,

packages/strapi-connector-mongoose/lib/queries.js

@@ -4,7 +4,7 @@
  */
 
 const _ = require('lodash');
-const { convertRestQueryParams, buildQuery, findPageQueryFactory } = require('strapi-utils');
+const { convertRestQueryParams, buildQuery } = require('strapi-utils');
 
 const { findComponentByGlobalId } = require('./utils/helpers');
 
@@ -395,8 +395,6 @@ module.exports = ({ model, strapi }) => {
     }).then(results => results.map(result => (result ? result.toObject() : null)));
   }
 
-  const findPage = findPageQueryFactory(find, count);
-
   async function findOne(params, populate) {
     const entries = await find({ ...params, _limit: 1 }, populate);
     return entries[0] || null;
@@ -507,7 +505,6 @@ module.exports = ({ model, strapi }) => {
   return {
     findOne,
     find,
-    findPage,
     create,
     update,
     delete: deleteMany,

packages/strapi-database/lib/queries/__tests__/create-query.test.js

@@ -4,7 +4,7 @@ const _ = require('lodash');
 const createQuery = require('../create-query');
 
 describe('Database queries', () => {
-  describe('Subsitute id with primaryKey in paramters', () => {
+  describe('Substitute id with primaryKey in parameters', () => {
     test.each(['create', 'update', 'delete', 'find', 'findOne', 'search', 'count', 'countSearch'])(
       'Calling "%s" replaces id by the primaryKey in the params of the model before calling the underlying connector',
       async method => {

packages/strapi-database/lib/queries/__tests__/paginated-queries.test.js

@@ -0,0 +1,132 @@
+'use strict';
+
+const {
+  createFindPageQuery,
+  createSearchPageQuery,
+  createPaginatedQuery,
+  getPaginationInfos,
+} = require('../paginated-queries');
+
+describe('Paginated Queries', () => {
+  describe('createPaginatedQuery', () => {
+    test('Successfully create a paginated query based on given fetch and count', async () => {
+      const fetch = jest.fn(() => [1, 2]);
+      const count = jest.fn(() => 2);
+
+      const paginatedQuery = createPaginatedQuery({ fetch, count });
+
+      const data = await paginatedQuery({});
+
+      expect(fetch).toHaveBeenCalled();
+      expect(count).toHaveBeenCalled();
+      expect(data).toMatchObject({
+        results: [1, 2],
+        pagination: {
+          page: 1,
+          pageSize: 100,
+          total: 2,
+          pageCount: 1,
+        },
+      });
+    });
+
+    test('Use custom pagination options to find a specific page', async () => {
+      const fetch = jest.fn(() => [5, 6]);
+      const count = jest.fn(() => 6);
+
+      const paginatedQuery = createPaginatedQuery({ fetch, count });
+
+      const data = await paginatedQuery({ page: 2, pageSize: 4 });
+
+      expect(fetch).toHaveBeenCalled();
+      expect(count).toHaveBeenCalled();
+      expect(data).toMatchObject({
+        results: [5, 6],
+        pagination: {
+          page: 2,
+          pageSize: 4,
+          total: 6,
+          pageCount: 2,
+        },
+      });
+    });
+  });
+
+  describe('createFindPageQuery', () => {
+    test('Successfully create a findPage query based on given find and count', async () => {
+      const find = jest.fn(() => [1, 2]);
+      const count = jest.fn(() => 2);
+
+      const paginatedQuery = createFindPageQuery({ find, count });
+
+      const data = await paginatedQuery({});
+
+      expect(find).toHaveBeenCalled();
+      expect(count).toHaveBeenCalled();
+      expect(data).toMatchObject({
+        results: [1, 2],
+        pagination: {
+          page: 1,
+          pageSize: 100,
+          total: 2,
+          pageCount: 1,
+        },
+      });
+    });
+  });
+
+  describe('createSearchPageQuery', () => {
+    test('Successfully create a searchPage query based on given search and countSearch', async () => {
+      const search = jest.fn(() => [1, 2]);
+      const countSearch = jest.fn(() => 2);
+
+      const paginatedQuery = createSearchPageQuery({ search, countSearch });
+
+      const data = await paginatedQuery({});
+
+      expect(search).toHaveBeenCalled();
+      expect(countSearch).toHaveBeenCalled();
+      expect(data).toMatchObject({
+        results: [1, 2],
+        pagination: {
+          page: 1,
+          pageSize: 100,
+          total: 2,
+          pageCount: 1,
+        },
+      });
+    });
+  });
+
+  describe('getPaginationInfos', () => {
+    test('Incomplete last page', async () => {
+      const queryParams = { page: 2, pageSize: 6 };
+      const count = jest.fn(() => 8);
+
+      const pagination = await getPaginationInfos(queryParams, count);
+
+      expect(count).toHaveBeenCalled();
+      expect(pagination).toEqual({
+        page: 2,
+        pageSize: 6,
+        total: 8,
+        pageCount: 2,
+      });
+    });
+
+    test('Complete last page', async () => {
+      const queryParams = { page: 2, pageSize: 6 };
+      const count = jest.fn(() => 18);
+
+      const pagination = await getPaginationInfos(queryParams, count);
+
+      expect(count).toHaveBeenCalled();
+      expect(pagination).toEqual({
+        page: 2,
+        pageSize: 6,
+        total: 18,
+        pageCount: 3,
+      });
+    });
+  });
+});

packages/strapi-database/lib/queries/create-query.js

@@ -1,7 +1,7 @@
 'use strict';
 
-const { replaceIdByPrimaryKey } = require('../utils/primary-key');
-const { executeBeforeLifecycle, executeAfterLifecycle } = require('../utils/lifecycles');
+const { createQueryWithLifecycles, withLifecycles } = require('./helpers');
+const { createFindPageQuery, createSearchPageQuery } = require('./paginated-queries');
 
 /**
  * @param {Object} opts options
@@ -52,30 +52,15 @@ module.exports = function createQuery(opts) {
     delete: createQueryWithLifecycles({ query: 'delete', model, connectorQuery }),
     find: createQueryWithLifecycles({ query: 'find', model, connectorQuery }),
     findOne: createQueryWithLifecycles({ query: 'findOne', model, connectorQuery }),
-    findPage: createQueryWithLifecycles({ query: 'findPage', model, connectorQuery }),
     count: createQueryWithLifecycles({ query: 'count', model, connectorQuery }),
     search: createQueryWithLifecycles({ query: 'search', model, connectorQuery }),
     countSearch: createQueryWithLifecycles({ query: 'countSearch', model, connectorQuery }),
+
+    findPage: withLifecycles({ query: 'findPage', model, fn: createFindPageQuery(connectorQuery) }),
+    searchPage: withLifecycles({
+      query: 'searchPage',
+      model,
+      fn: createSearchPageQuery(connectorQuery),
+    }),
   };
 };
-
-// wraps a connectorQuery call with:
-// - param substitution
-// - lifecycle hooks
-const createQueryWithLifecycles = ({ query, model, connectorQuery }) => async (params, ...rest) => {
-  // substitute id for primaryKey value in params
-  const newParams = replaceIdByPrimaryKey(params, model);
-  const queryArguments = [newParams, ...rest];
-
-  // execute before hook
-  await executeBeforeLifecycle(query, model, ...queryArguments);
-
-  // execute query
-  const result = await connectorQuery[query](...queryArguments);
-
-  // execute after hook with result and arguments
-  await executeAfterLifecycle(query, model, result, ...queryArguments);
-
-  // return result
-  return result;
-};

packages/strapi-database/lib/queries/helpers.js

@@ -0,0 +1,35 @@
+'use strict';
+
+const { replaceIdByPrimaryKey } = require('../utils/primary-key');
+const { executeBeforeLifecycle, executeAfterLifecycle } = require('../utils/lifecycles');
+
+const withLifecycles = ({ query, model, fn }) => async (params, ...rest) => {
+  // substitute id for primaryKey value in params
+  const newParams = replaceIdByPrimaryKey(params, model);
+  const queryArguments = [newParams, ...rest];
+
+  // execute before hook
+  await executeBeforeLifecycle(query, model, ...queryArguments);
+
+  // execute query
+  const result = await fn(...queryArguments);
+
+  // execute after hook with result and arguments
+  await executeAfterLifecycle(query, model, result, ...queryArguments);
+
+  // return result
+  return result;
+};
+
+// wraps a connectorQuery call with:
+// - param substitution
+// - lifecycle hooks
+const createQueryWithLifecycles = ({ query, model, connectorQuery }) => {
+  return withLifecycles({
+    query,
+    model,
+    fn: (...queryParameters) => connectorQuery[query](...queryParameters),
+  });
+};
+
+module.exports = { withLifecycles, createQueryWithLifecycles };

packages/strapi-database/lib/queries/paginated-queries.js

@@ -0,0 +1,55 @@
+'use strict';
+
+const _ = require('lodash');
+
+const createPaginatedQuery = ({ fetch, count }) => async (queryParams, ...args) => {
+  const params = _.omit(queryParams, ['page', 'pageSize']);
+  const pagination = await getPaginationInfos(queryParams, count);
+
+  Object.assign(params, paginationToQueryParams(pagination));
+
+  const results = await fetch(params, ...args);
+
+  return { results, pagination };
+};
+
+const createSearchPageQuery = ({ search, countSearch }) =>
+  createPaginatedQuery({ fetch: search, count: countSearch });
+
+const createFindPageQuery = ({ find, count }) => createPaginatedQuery({ fetch: find, count });
+
+const getPaginationInfos = async (queryParams, count) => {
+  const { page, pageSize, ...params } = withDefaultPagination(queryParams);
+
+  const total = await count(params);
+
+  return {
+    page,
+    pageSize,
+    pageCount: Math.ceil(total / pageSize),
+    total,
+  };
+};
+
+const withDefaultPagination = params => {
+  const { page = 1, pageSize = 100, ...rest } = params;
+
+  return {
+    page: parseInt(page),
+    pageSize: parseInt(pageSize),
+    ...rest,
+  };
+};
+
+const paginationToQueryParams = ({ page, pageSize }) => ({
+  _start: Math.max(page - 1, 0) * pageSize,
+  _limit: pageSize,
+});
+
+module.exports = {
+  getPaginationInfos,
+  withDefaultPagination,
+  createPaginatedQuery,
+  createFindPageQuery,
+  createSearchPageQuery,
+};

packages/strapi-utils/lib/__tests__/findPageQueryFactory.test.js

@@ -1,47 +0,0 @@
-'use strict';
-
-const findPageQueryFactory = require('../findPageQueryFactory');
-
-describe('findPageQueryFactory', () => {
-  test('Successfully create a query based on given find and count', async () => {
-    const find = jest.fn(() => [1, 2]);
-    const count = jest.fn(() => 2);
-
-    const findPage = findPageQueryFactory(find, count);
-
-    const data = await findPage({});
-
-    expect(find).toHaveBeenCalled();
-    expect(count).toHaveBeenCalled();
-    expect(data).toMatchObject({
-      results: [1, 2],
-      pagination: {
-        page: 1,
-        pageSize: 100,
-        total: 2,
-        pageCount: 1,
-      },
-    });
-  });
-
-  test('Use custom pagination options to find a specific page', async () => {
-    const find = jest.fn(() => [5, 6]);
-    const count = jest.fn(() => 6);
-
-    const findPage = findPageQueryFactory(find, count);
-
-    const data = await findPage({ page: 2, pageSize: 4 });
-
-    expect(find).toHaveBeenCalled();
-    expect(count).toHaveBeenCalled();
-    expect(data).toMatchObject({
-      results: [5, 6],
-      pagination: {
-        page: 2,
-        pageSize: 4,
-        total: 6,
-        pageCount: 2,
-      },
-    });
-  });
-});

packages/strapi-utils/lib/findPageQueryFactory.js

@@ -1,47 +0,0 @@
-'use strict';
-
-const DEFAULT_PAGINATION_OPTS = { page: 1, pageSize: 100 };
-
-/**
- * Create a findPage query based on the find and count queries provided to the factory
- * @param find The find query
- * @param count The count query
- * @returns {function(*, ...[*]): {pagination: {pageCount: number, total: number, pageSize: number, page: number}, results: [*]}}
- */
-const findPageQueryFactory = (find, count) => async (queryParams, ...args) => {
-  const { page, pageSize, ...params } = withDefaultPagination(queryParams);
-
-  const total = await count(params);
-
-  const pagination = {
-    page,
-    pageSize,
-    pageCount: Math.ceil(total / pageSize),
-    total,
-  };
-
-  Object.assign(params, {
-    _start: Math.max(pagination.page - 1, 0) * pagination.pageSize,
-    _limit: pagination.pageSize,
-  });
-
-  const results = await find(params, ...args);
-
-  return { results, pagination };
-};
-
-function withDefaultPagination(params) {
-  const {
-    page = DEFAULT_PAGINATION_OPTS.page,
-    pageSize = DEFAULT_PAGINATION_OPTS.pageSize,
-    ...rest
-  } = params;
-
-  return {
-    page: parseInt(page),
-    pageSize: parseInt(pageSize),
-    ...rest,
-  };
-}
-
-module.exports = findPageQueryFactory;

packages/strapi-utils/lib/index.js

@@ -23,7 +23,6 @@ const {
 } = require('./stringFormatting');
 const { removeUndefined } = require('./objectFormatting');
 const { getConfigUrls, getAbsoluteAdminUrl, getAbsoluteServerUrl } = require('./config');
-const findPageQueryFactory = require('./findPageQueryFactory');
 
 module.exports = {
   yup,
@@ -46,5 +45,4 @@ module.exports = {
   removeUndefined,
   getAbsoluteAdminUrl,
   getAbsoluteServerUrl,
-  findPageQueryFactory,
 };