Skip to content

Commit

Permalink
Fix D&P population (#8249)
Browse files Browse the repository at this point in the history 
* Fix D&P population (gql, rest)

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

* Fix units tests

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

* Fix mongoose filtering on non draft & publish model

Signed-off-by: Alexandre Bodin <bodin.alex@gmail.com>

Co-authored-by: Alexandre Bodin <bodin.alex@gmail.com>
  • Loading branch information
@Convly @alexandrebodin
Convly and alexandrebodin committed Oct 8, 2020
1 parent 38126d9 commit b2b4515
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 44 deletions.
13 changes: 8 additions & 5 deletions packages/strapi-connector-mongoose/lib/buildQuery.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ const _ = require('lodash');
var semver = require('semver');
const utils = require('./utils')();
const populateQueries = require('./utils/populate-queries');
const { hasDeepFilters } = require('strapi-utils');
const {
hasDeepFilters,
contentTypes: { hasDraftAndPublish },
} = require('strapi-utils');

const combineSearchAndWhere = (search = [], wheres = []) => {
const criterias = {};
Expand Down Expand Up @@ -107,7 +110,7 @@ const buildSimpleQuery = ({ model, filters, search, populate }) => {
let query = model
.find(findCriteria, null, { publicationState: filters.publicationState })
.populate(populate);
query = applyQueryParams({ query, filters });
query = applyQueryParams({ model, query, filters });

return Object.assign(query, {
// Override count to use countDocuments on simple find query
Expand Down Expand Up @@ -167,7 +170,7 @@ const buildDeepQuery = ({ model, filters, search, populate }) => {
)
.populate(populate);

return applyQueryParams({ query, filters });
return applyQueryParams({ model, query, filters });
})
.then(...args);
},
Expand Down Expand Up @@ -205,7 +208,7 @@ const buildDeepQuery = ({ model, filters, search, populate }) => {
* @param {Object} options.query - Mongoose query
* @param {Object} options.filters - Filters object
*/
const applyQueryParams = ({ query, filters }) => {
const applyQueryParams = ({ model, query, filters }) => {
// Apply sort param
if (_.has(filters, 'sort')) {
const sortFilter = filters.sort.reduce((acc, sort) => {
Expand All @@ -231,7 +234,7 @@ const applyQueryParams = ({ query, filters }) => {
if (_.has(filters, 'publicationState')) {
const populateQuery = populateQueries.publicationState[filters.publicationState];

if (populateQuery) {
if (hasDraftAndPublish(model) && populateQuery) {
query = query.where(populateQuery);
}
}
Expand Down
49 changes: 27 additions & 22 deletions packages/strapi-plugin-graphql/services/type-definitions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ const {
constants: { DP_PUB_STATE_LIVE },
} = contentTypes;

const OPTIONS = Symbol();

const DynamicZoneScalar = require('../types/dynamiczoneScalar');

const { formatModelConnectionsGQL } = require('./build-aggregation');
Expand All @@ -26,6 +24,11 @@ const { toSingular, toPlural } = require('./naming');
const { buildQuery, buildMutation } = require('./resolvers-builder');
const { actionExists } = require('./utils');

const OPTIONS = Symbol();

const FIND_QUERY_ARGUMENTS = `(sort: String, limit: Int, start: Int, where: JSON, publicationState: PublicationState)`;
const FIND_ONE_QUERY_ARGUMENTS = `(id: ID!, publicationState: PublicationState)`;

const assignOptions = (element, parent) => {
if (Array.isArray(element)) {
return element.map(el => assignOptions(el, parent));
Expand All @@ -46,6 +49,13 @@ const isNotPrivate = _.curry((model, attributeName) => {
return !contentTypes.isPrivateAttribute(model, attributeName);
});

const wrapPublicationStateResolver = query => async (parent, args, ctx, ast) => {
const results = await query(parent, args, ctx, ast);

const queryOptions = _.pick(args, 'publicationState');
return assignOptions(results, { [OPTIONS]: queryOptions });
};

const buildTypeDefObj = model => {
const { associations = [], attributes, primaryKey, globalId } = model;

Expand Down Expand Up @@ -177,7 +187,7 @@ const buildAssocResolvers = model => {
case 'manyWay': {
resolver[association.alias] = async obj => {
if (obj[association.alias]) {
return obj[association.alias];
return assignOptions(obj[association.alias], obj);
}

const queryOpts = initQueryOptions(targetModel, obj);
Expand Down Expand Up @@ -336,17 +346,18 @@ const buildSingleType = model => {
}

if (isQueryEnabled(_schema, singularName)) {
const resolver = buildQuery(singularName, {
resolver: `${uid}.find`,
..._.get(_schema, `resolver.Query.${singularName}`, {}),
});

_.merge(localSchema, {
query: {
// TODO: support all the unique fields
[singularName]: model.globalId,
[`${singularName}(publicationState: PublicationState)`]: model.globalId,
},
resolvers: {
Query: {
[singularName]: buildQuery(singularName, {
resolver: `${uid}.find`,
..._.get(_schema, `resolver.Query.${singularName}`, {}),
}),
[singularName]: wrapPublicationStateResolver(resolver),
},
},
});
Expand Down Expand Up @@ -412,13 +423,15 @@ const buildCollectionType = model => {
..._.get(_schema, `resolver.Query.${singularName}`, {}),
};
if (actionExists(resolverOpts)) {
const resolver = buildQuery(singularName, resolverOpts);
_.merge(localSchema, {
query: {
[`${singularName}(id: ID!)`]: model.globalId,
// TODO: support all the unique fields
[`${singularName}${FIND_ONE_QUERY_ARGUMENTS}`]: model.globalId,
},
resolvers: {
Query: {
[singularName]: buildQuery(singularName, resolverOpts),
[singularName]: wrapPublicationStateResolver(resolver),
},
},
});
Expand All @@ -431,22 +444,14 @@ const buildCollectionType = model => {
..._.get(_schema, `resolver.Query.${pluralName}`, {}),
};
if (actionExists(resolverOpts)) {
const draftAndPublishArgument = contentTypes.hasDraftAndPublish(model)
? 'publicationState: PublicationState'
: '';
const queryArguments = `(sort: String, limit: Int, start: Int, where: JSON ${draftAndPublishArgument})`;
const resolver = buildQuery(pluralName, resolverOpts);
_.merge(localSchema, {
query: {
[`${pluralName}${queryArguments}`]: `[${model.globalId}]`,
[`${pluralName}${FIND_QUERY_ARGUMENTS}`]: `[${model.globalId}]`,
},
resolvers: {
Query: {
[pluralName]: async (parent, args, ctx, ast) => {
const results = await buildQuery(pluralName, resolverOpts)(parent, args, ctx, ast);

const queryOptions = _.pick(args, 'publicationState');
return assignOptions(results, { [OPTIONS]: queryOptions });
},
[pluralName]: wrapPublicationStateResolver(resolver),
},
},
});
Expand Down
2 changes: 1 addition & 1 deletion packages/strapi-plugin-users-permissions/controllers/User.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ module.exports = {
* @return {Number}
*/
async count(ctx) {
if (ctx.query._q) {
if (_.has(ctx.query, '_q')) {
return await strapi.plugins['users-permissions'].services.user.countSearch(ctx.query);
}
ctx.body = await strapi.plugins['users-permissions'].services.user.count(ctx.query);
Expand Down
6 changes: 3 additions & 3 deletions packages/strapi/lib/core-api/__tests__/service.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('Default Service', () => {
await service.createOrUpdate(input);

expect(strapi.entityService.find).toHaveBeenCalledWith(
{ populate: undefined },
{ populate: undefined, params: { _publicationState: 'live' } },
{
model: 'testModel',
}
Expand Down Expand Up @@ -95,7 +95,7 @@ describe('Default Service', () => {
await service.createOrUpdate(input);

expect(strapi.entityService.find).toHaveBeenCalledWith(
{ populate: undefined },
{ populate: undefined, params: { _publicationState: 'live' } },
{
model: 'testModel',
}
Expand Down Expand Up @@ -130,7 +130,7 @@ describe('Default Service', () => {
await service.delete();

expect(strapi.entityService.find).toHaveBeenCalledWith(
{ populate: undefined },
{ populate: undefined, params: { _publicationState: 'live' } },
{
model: 'testModel',
}
Expand Down
11 changes: 7 additions & 4 deletions packages/strapi/lib/core-api/controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ const createSingleTypeController = ({ model, service }) => {
*
* @return {Object|Array}
*/
async find() {
const entity = await service.find();
async find(ctx) {
const { query } = ctx;
const entity = await service.find(query);
return sanitize(entity);
},

Expand Down Expand Up @@ -90,7 +91,9 @@ const createCollectionTypeController = ({ model, service }) => {
* @return {Object}
*/
async findOne(ctx) {
const entity = await service.findOne({ id: ctx.params.id });
const { query, params } = ctx;
const entity = await service.findOne({ ...query, id: params.id });

return sanitize(entity);
},

Expand All @@ -100,7 +103,7 @@ const createCollectionTypeController = ({ model, service }) => {
* @return {Number}
*/
count(ctx) {
if (ctx.query._q) {
if (_.has(ctx.query, '_q')) {
return service.countSearch(ctx.query);
}
return service.count(ctx.query);
Expand Down
17 changes: 9 additions & 8 deletions packages/strapi/lib/core-api/service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ const {
},
} = require('strapi-utils');

const getFetchParams = (params, model) => {
const getFetchParams = params => {
const defaultParams = {};

if (hasDraftAndPublish(model)) {
Object.assign(defaultParams, {
_publicationState: DP_PUB_STATE_LIVE,
});
}
Object.assign(defaultParams, {
_publicationState: DP_PUB_STATE_LIVE,
});

return { ...defaultParams, ...params };
};
Expand Down Expand Up @@ -56,8 +54,11 @@ const createSingleTypeService = ({ model, strapi }) => {
*
* @return {Promise}
*/
find(populate) {
return strapi.entityService.find({ populate }, { model: modelName });
find(params, populate) {
return strapi.entityService.find(
{ params: getFetchParams(params, model), populate },
{ model: modelName }
);
},

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/strapi/lib/services/entity-service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = ({ db, eventHub, entityValidator }) => ({

// return first element and ignore filters
if (kind === 'singleType') {
const results = await db.query(model).find({ _limit: 1 }, populate);
const results = await db.query(model).find({ _limit: 1, ...params }, populate);
return _.first(results) || null;
}

Expand Down

0 comments on commit b2b4515

Please sign in to comment.