diff --git a/packages/strapi-connector-bookshelf/lib/buildQuery.js b/packages/strapi-connector-bookshelf/lib/buildQuery.js index 3a48aafd65..5f2a5f7a43 100644 --- a/packages/strapi-connector-bookshelf/lib/buildQuery.js +++ b/packages/strapi-connector-bookshelf/lib/buildQuery.js @@ -1,7 +1,7 @@ 'use strict'; const _ = require('lodash'); -const { each, prop, reject, isEmpty } = require('lodash/fp'); +const { each, prop, isEmpty } = require('lodash/fp'); const { singular } = require('pluralize'); const { toQueries, runPopulateQueries } = require('./utils/populate-queries'); @@ -14,15 +14,21 @@ const BOOLEAN_OPERATORS = ['or']; * @param {Object} options.filters - Filters params (start, limit, sort, where) */ const buildQuery = ({ model, filters }) => qb => { + const joinsTree = buildJoinsAndFilter(qb, model, filters); + if (_.has(filters, 'where') && Array.isArray(filters.where) && filters.where.length > 0) { qb.distinct(); } - const joinsTree = buildJoinsAndFilter(qb, model, filters); - if (_.has(filters, 'sort')) { - const clauses = filters.sort.map(buildSortClauseFromTree(joinsTree)); - qb.orderBy(reject(isEmpty, clauses)); + const clauses = filters.sort.map(buildSortClauseFromTree(joinsTree)).filter(c => !isEmpty(c)); + const orderBy = clauses.map(({ order, alias }) => ({ order, column: alias })); + const orderColumns = clauses.map(({ alias, column }) => ({ [alias]: column })); + const columns = [`${joinsTree.alias}.*`, ...orderColumns]; + + qb.distinct() + .column(columns) + .orderBy(orderBy); } if (_.has(filters, 'start')) { @@ -47,13 +53,21 @@ const buildQuery = ({ model, filters }) => qb => { */ const buildSortClauseFromTree = tree => ({ field, order }) => { if (!field.includes('.')) { - return { column: field, order }; + return { + column: `${tree.alias}.${field}`, + order, + alias: `_strapi_tmp_${tree.alias}_${field}`, + }; } const [relation, attribute] = field.split('.'); for (const { alias, assoc } of Object.values(tree.joins)) { if (relation === assoc.alias) { - return { column: `${alias}.${attribute}`, order }; + return { + column: `${alias}.${attribute}`, + order, + alias: `_strapi_tmp_${alias}_${attribute}`, + }; } } diff --git a/packages/strapi-connector-bookshelf/lib/mount-models.js b/packages/strapi-connector-bookshelf/lib/mount-models.js index 7b89cbd7c4..2d40eef526 100644 --- a/packages/strapi-connector-bookshelf/lib/mount-models.js +++ b/packages/strapi-connector-bookshelf/lib/mount-models.js @@ -610,6 +610,10 @@ module.exports = async ({ models, target }, ctx, { selfFinalize = false } = {}) const formatValue = createFormatter(definition.client); function formatEntry(entry) { Object.keys(entry.attributes).forEach(key => { + if (key.startsWith('_strapi_tmp_')) { + delete entry.attributes[key]; + return; + } const attr = definition.attributes[key] || {}; entry.attributes[key] = formatValue(attr, entry.attributes[key]); }); diff --git a/packages/strapi-plugin-content-manager/tests/api/basic-count-relations.test.e2e.js b/packages/strapi-plugin-content-manager/tests/api/basic-count-relations.test.e2e.js deleted file mode 100644 index c25d670627..0000000000 --- a/packages/strapi-plugin-content-manager/tests/api/basic-count-relations.test.e2e.js +++ /dev/null @@ -1,171 +0,0 @@ -'use strict'; - -const { createAuthRequest } = require('../../../../test/helpers/request'); -const { createStrapiInstance } = require('../../../../test/helpers/strapi'); -const { createTestBuilder } = require('../../../../test/helpers/builder'); - -let strapi; -let rq; -const builder = createTestBuilder(); - -let data = { - stamps: [], - collectors: [], -}; - -const stamp = { - name: 'stamp', - kind: 'collectionType', - attributes: { - name: { - type: 'string', - }, - }, -}; - -const collector = { - name: 'collector', - kind: 'collectionType', - attributes: { - name: { - type: 'string', - }, - age: { - type: 'integer', - }, - stamps: { - nature: 'manyWay', - target: 'application::stamp.stamp', - unique: false, - }, - stamps_m2m: { - nature: 'manyToMany', - targetAttribute: 'collectors', - target: 'application::stamp.stamp', - unique: false, - dominant: true, - }, - stamps_one_many: { - nature: 'oneToMany', - targetAttribute: 'collector', - target: 'application::stamp.stamp', - unique: false, - }, - }, -}; - -const stampFixtures = [ - { - name: '1946', - }, - { - name: '1947', - }, - { - name: '1948', - }, -]; - -const collectorFixtures = ({ stamp }) => [ - { - name: 'Bernard', - age: 25, - stamps: [stamp[0].id, stamp[1].id], - stamps_m2m: [stamp[0].id], - stamps_one_many: [], - }, - { - name: 'Isabelle', - age: 55, - stamps: [stamp[0].id], - stamps_m2m: [], - stamps_one_many: [stamp[1].id, stamp[2].id], - }, - { - name: 'Emma', - age: 23, - stamps: [], - stamps_m2m: [stamp[0].id, stamp[1].id], - stamps_one_many: [stamp[0].id], - }, -]; - -const getCollectorByName = (collectors, name) => collectors.find(c => c.name === name); -const getStampByName = (stamps, name) => stamps.find(s => s.name === name); - -describe('CM API - Count relations', () => { - beforeAll(async () => { - await builder - .addContentTypes([stamp, collector]) - .addFixtures(stamp.name, stampFixtures) - .addFixtures(collector.name, collectorFixtures) - .build(); - - strapi = await createStrapiInstance(); - rq = await createAuthRequest({ strapi }); - - data.collectors = builder.sanitizedFixturesFor(collector.name, strapi); - data.stamps = builder.sanitizedFixturesFor(stamp.name, strapi); - }, 60000); - - afterAll(async () => { - await strapi.destroy(); - await builder.cleanup(); - }, 60000); - - test('many-way', async () => { - const res = await rq({ - method: 'GET', - url: '/content-manager/collection-types/application::collector.collector', - }); - - expect(res.statusCode).toBe(200); - expect(Array.isArray(res.body.results)).toBe(true); - expect(res.body.results).toHaveLength(3); - expect(getCollectorByName(res.body.results, 'Bernard').stamps.count).toBe(2); - expect(getCollectorByName(res.body.results, 'Isabelle').stamps.count).toBe(1); - expect(getCollectorByName(res.body.results, 'Emma').stamps.count).toBe(0); - }); - - test('many-to-many (collector -> stamps)', async () => { - const res = await rq({ - method: 'GET', - url: '/content-manager/collection-types/application::collector.collector', - }); - - expect(res.statusCode).toBe(200); - expect(Array.isArray(res.body.results)).toBe(true); - expect(res.body.results).toHaveLength(3); - expect(getCollectorByName(res.body.results, 'Bernard').stamps_m2m.count).toBe(1); - expect(getCollectorByName(res.body.results, 'Isabelle').stamps_m2m.count).toBe(0); - expect(getCollectorByName(res.body.results, 'Emma').stamps_m2m.count).toBe(2); - }); - - test('many-to-many (stamp -> collectors)', async () => { - const res = await rq({ - method: 'GET', - url: '/content-manager/collection-types/application::stamp.stamp', - }); - - expect(res.statusCode).toBe(200); - expect(Array.isArray(res.body.results)).toBe(true); - expect(res.body.results).toHaveLength(3); - expect(getStampByName(res.body.results, '1946').collectors.count).toBe(2); - expect(getStampByName(res.body.results, '1947').collectors.count).toBe(1); - expect(getStampByName(res.body.results, '1948').collectors.count).toBe(0); - }); - - test('one-to-many', async () => { - const res = await rq({ - method: 'GET', - url: '/content-manager/collection-types/application::collector.collector', - }); - - expect(res.statusCode).toBe(200); - expect(Array.isArray(res.body.results)).toBe(true); - expect(res.body.results).toHaveLength(3); - expect(getCollectorByName(res.body.results, 'Bernard').stamps_one_many.count).toBe(0); - expect(getCollectorByName(res.body.results, 'Isabelle').stamps_one_many.count).toBe(2); - expect(getCollectorByName(res.body.results, 'Emma').stamps_one_many.count).toBe(1); - }); -}); diff --git a/packages/strapi-plugin-content-manager/tests/api/basic-relations.test.e2e.js b/packages/strapi-plugin-content-manager/tests/api/basic-relations.test.e2e.js new file mode 100644 index 0000000000..d738c97e96 --- /dev/null +++ b/packages/strapi-plugin-content-manager/tests/api/basic-relations.test.e2e.js @@ -0,0 +1,404 @@ +'use strict'; + +const { createAuthRequest } = require('../../../../test/helpers/request'); +const { createStrapiInstance } = require('../../../../test/helpers/strapi'); +const { createTestBuilder } = require('../../../../test/helpers/builder'); + +let strapi; +let rq; +const builder = createTestBuilder(); + +let data = { + stamps: [], + collectors: [], +}; + +const stamp = { + name: 'stamp', + kind: 'collectionType', + attributes: { + name: { + type: 'string', + }, + }, +}; + +const collector = { + name: 'collector', + kind: 'collectionType', + attributes: { + name: { + type: 'string', + }, + age: { + type: 'integer', + }, + stamps: { + nature: 'manyWay', + target: 'application::stamp.stamp', + unique: false, + }, + stamps_one_way: { + nature: 'oneWay', + target: 'application::stamp.stamp', + unique: false, + }, + stamps_m2m: { + nature: 'manyToMany', + targetAttribute: 'collectors', + target: 'application::stamp.stamp', + unique: false, + dominant: true, + }, + stamps_one_many: { + nature: 'oneToMany', + targetAttribute: 'collector', + target: 'application::stamp.stamp', + unique: false, + }, + stamps_one_one: { + nature: 'oneToOne', + targetAttribute: 'collector_one_one', + target: 'application::stamp.stamp', + unique: false, + }, + }, +}; + +const stampFixtures = [ + { + name: '1946', + }, + { + name: '1947', + }, + { + name: '1948', + }, +]; + + + + +const collectorFixtures = ({ stamp }) => [ + { + name: 'Bernard', + age: 25, + stamps: [stamp[0].id, stamp[1].id], + stamps_m2m: [stamp[0].id], + stamps_one_many: [], + stamps_one_way: stamp[0].id, + stamps_one_one: stamp[0].id, + }, + { + name: 'Isabelle', + age: 55, + stamps: [stamp[0].id], + stamps_m2m: [], + stamps_one_many: [stamp[1].id, stamp[2].id], + stamps_one_way: stamp[1].id, + stamps_one_one: stamp[1].id, + }, + { + name: 'Emma', + age: 23, + stamps: [], + stamps_m2m: [stamp[0].id, stamp[1].id], + stamps_one_many: [stamp[0].id], + stamps_one_way: stamp[2].id, + stamps_one_one: stamp[2].id, + }, +]; + + +const getCollectorByName = (collectors, name) => collectors.find(c => c.name === name); +const getStampByName = (stamps, name) => stamps.find(s => s.name === name); + +describe('CM API', () => { + beforeAll(async () => { + await builder + .addContentTypes([stamp, collector]) + .addFixtures(stamp.name, stampFixtures) + .addFixtures(collector.name, collectorFixtures) + .build(); + + + strapi = await createStrapiInstance(); + rq = await createAuthRequest({ strapi }); + + data.collectors = builder.sanitizedFixturesFor(collector.name, strapi); + data.stamps = builder.sanitizedFixturesFor(stamp.name, strapi); + }, 60000); + + afterAll(async () => { + await strapi.destroy(); + await builder.cleanup(); + }, 60000); + + describe('Count relations', () => { + test('many-way', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(getCollectorByName(res.body.results, 'Bernard').stamps.count).toBe(2); + expect(getCollectorByName(res.body.results, 'Isabelle').stamps.count).toBe(1); + expect(getCollectorByName(res.body.results, 'Emma').stamps.count).toBe(0); + }); + + test('many-to-many (collector -> stamps)', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(getCollectorByName(res.body.results, 'Bernard').stamps_m2m.count).toBe(1); + expect(getCollectorByName(res.body.results, 'Isabelle').stamps_m2m.count).toBe(0); + expect(getCollectorByName(res.body.results, 'Emma').stamps_m2m.count).toBe(2); + }); + + test('many-to-many (stamp -> collectors)', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::stamp.stamp', + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(getStampByName(res.body.results, '1946').collectors.count).toBe(2); + expect(getStampByName(res.body.results, '1947').collectors.count).toBe(1); + expect(getStampByName(res.body.results, '1948').collectors.count).toBe(0); + }); + + test('one-to-many', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(getCollectorByName(res.body.results, 'Bernard').stamps_one_many.count).toBe(0); + expect(getCollectorByName(res.body.results, 'Isabelle').stamps_one_many.count).toBe(2); + expect(getCollectorByName(res.body.results, 'Emma').stamps_one_many.count).toBe(1); + }); + }); + + describe('Filter relations', () => { + test('many-way', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _where: { 'stamps.name': '1946' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(2); + expect(res.body.results[0].name).toBe('Bernard'); + expect(res.body.results[1].name).toBe('Isabelle'); + }); + + test('many-to-many (collector -> stamps)', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _where: { 'stamps_m2m.name': '1946' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(2); + expect(getCollectorByName(res.body.results, 'Bernard')).toBeDefined(); + expect(getCollectorByName(res.body.results, 'Emma')).toBeDefined(); + }); + + test('many-to-many (stamp -> collectors)', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::stamp.stamp', + qs: { + _where: { 'collectors.name': 'Emma' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(2); + expect(getStampByName(res.body.results, '1946')).toBeDefined(); + expect(getStampByName(res.body.results, '1947')).toBeDefined(); + }); + + test('one-to-many', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _where: { 'stamps_one_many.name': '1947' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(1); + expect(res.body.results[0].name).toBe('Isabelle'); + }); + + test('many-to-one', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::stamp.stamp', + qs: { + _where: { 'collector.name': 'Isabelle' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(2); + expect(getStampByName(res.body.results, '1947')).toBeDefined(); + expect(getStampByName(res.body.results, '1948')).toBeDefined(); + }); + + test('one-way', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _where: { 'stamps_one_way.name': '1947' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(1); + expect(getCollectorByName(res.body.results, 'Isabelle')).toBeDefined(); + }); + + test('one-one', async () => { + const res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _where: { 'stamps_one_one.name': '1947' }, + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(1); + expect(getCollectorByName(res.body.results, 'Isabelle')).toBeDefined(); + }); + }); + + describe('Sort relations', () => { + test('many-to-one', async () => { + let res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::stamp.stamp', + qs: { + _sort: 'collector.name:ASC', + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(res.body.results[0].collector.name).toBe('Emma'); + expect(res.body.results[1].collector.name).toBe('Isabelle'); + expect(res.body.results[2].collector.name).toBe('Isabelle'); + + res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::stamp.stamp', + qs: { + _sort: 'collector.name:DESC', + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(res.body.results[0].collector.name).toBe('Isabelle'); + expect(res.body.results[1].collector.name).toBe('Isabelle'); + expect(res.body.results[2].collector.name).toBe('Emma'); + }); + + test('one-way', async () => { + let res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _sort: 'stamps_one_way.name:ASC', + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(res.body.results[0].stamps_one_way.name).toBe('1946'); + expect(res.body.results[1].stamps_one_way.name).toBe('1947'); + expect(res.body.results[2].stamps_one_way.name).toBe('1948'); + + res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _sort: 'stamps_one_way.name:DESC', + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(res.body.results[0].stamps_one_way.name).toBe('1948'); + expect(res.body.results[1].stamps_one_way.name).toBe('1947'); + expect(res.body.results[2].stamps_one_way.name).toBe('1946'); + }); + + test('one-one', async () => { + let res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _sort: 'stamps_one_one.name:ASC', + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(res.body.results[0].stamps_one_one.name).toBe('1946'); + expect(res.body.results[1].stamps_one_one.name).toBe('1947'); + expect(res.body.results[2].stamps_one_one.name).toBe('1948'); + + res = await rq({ + method: 'GET', + url: '/content-manager/collection-types/application::collector.collector', + qs: { + _sort: 'stamps_one_one.name:DESC', + }, + }); + + expect(res.statusCode).toBe(200); + expect(Array.isArray(res.body.results)).toBe(true); + expect(res.body.results).toHaveLength(3); + expect(res.body.results[0].stamps_one_one.name).toBe('1948'); + expect(res.body.results[1].stamps_one_one.name).toBe('1947'); + expect(res.body.results[2].stamps_one_one.name).toBe('1946'); + }); + }); +}); diff --git a/packages/strapi-utils/lib/build-query.js b/packages/strapi-utils/lib/build-query.js index 0f42ea80c9..d366118804 100644 --- a/packages/strapi-utils/lib/build-query.js +++ b/packages/strapi-utils/lib/build-query.js @@ -146,9 +146,17 @@ const normalizeSortClauses = (clauses, { model }) => { })); normalizedClauses.forEach(({ field }) => { - if (field.includes('.')) { + const fieldDepth = field.split('.').length - 1; + if (fieldDepth === 1) { // Check if the relational field exists getAssociationFromFieldKey({ model, field }); + } else if (fieldDepth > 1) { + const err = new Error( + `Sorting on ${field} is not possible: you cannot sort at a depth greater than 1` + ); + + err.status = 400; + throw err; } });