Skip to content

Commit

Permalink
fix: cm relation bugs (#19842)
Browse files Browse the repository at this point in the history 
* fix: relations

* fix: hardcode english locale

* fix(cm): don't try and be clever with caching relations

* fix: locale when target is not localized

---------

Co-authored-by: Josh <37798644+joshuaellis@users.noreply.github.com>
  • Loading branch information
@Marc-Roig @joshuaellis
Marc-Roig and joshuaellis committed Mar 19, 2024
1 parent 49e84a9 commit 66c1011
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 67 deletions.
20 changes: 4 additions & 16 deletions packages/core/admin/admin/src/content-manager/services/documents.ts
View file
Expand Up @@ -61,10 +61,7 @@ const documentApi = contentManagerApi.injectEndpoints({
}),
invalidatesTags: (result, _error, { model }) => [
{ type: 'Document', id: `${model}_LIST` },
{
type: 'Relations',
id: `${model}_${result?.data.documentId}`,
},
'Relations',
],
}),
deleteDocument: builder.mutation<
Expand Down Expand Up @@ -127,10 +124,7 @@ const documentApi = contentManagerApi.injectEndpoints({
id: collectionType !== SINGLE_TYPES ? `${model}_${documentId}` : model,
},
{ type: 'Document', id: `${model}_LIST` },
{
type: 'Relations',
id: `${model}_${documentId}`,
},
'Relations',
];
},
}),
Expand Down Expand Up @@ -278,10 +272,7 @@ const documentApi = contentManagerApi.injectEndpoints({
id: collectionType !== SINGLE_TYPES ? `${model}_${documentId}` : model,
},
{ type: 'Document', id: `${model}_LIST` },
{
type: 'Relations',
id: `${model}_${documentId}`,
},
'Relations',
];
},
}),
Expand Down Expand Up @@ -321,10 +312,7 @@ const documentApi = contentManagerApi.injectEndpoints({
type: 'Document',
id: collectionType !== SINGLE_TYPES ? `${model}_${documentId}` : model,
},
{
type: 'Relations',
id: `${model}_${documentId}`,
},
'Relations',
];
},
}),
Expand Down
4 changes: 1 addition & 3 deletions packages/core/admin/admin/src/content-manager/services/relations.ts
View file
Expand Up @@ -97,9 +97,7 @@ const relationsApi = contentManagerApi.injectEndpoints({
return response;
}
},
providesTags: (result, error, args) => [
{ type: 'Relations', id: `${args.model}_${args.id}` },
],
providesTags: ['Relations'],
}),
searchRelations: build.query<
Contracts.Relations.FindAvailable.Response,
Expand Down
122 changes: 75 additions & 47 deletions packages/core/content-manager/server/src/controllers/relations.ts
View file
Expand Up @@ -78,42 +78,66 @@ const addStatusToRelations = async (uid: Common.UID.ContentType, relations: Rela
});
};

const getPublishedAtClause = (
status: Documents.Params.PublicationStatus.Kind,
sourceUid: Common.UID.Schema,
targetUid: Common.UID.ContentType
) => {
const sourceModel = strapi.getModel(sourceUid);
const targetModel = strapi.getModel(targetUid);
const getPublishedAtClause = (status: string, uid: Common.UID.Schema) => {
const model = strapi.getModel(uid);

/**
* If target does not have dp, status should be published.
* As it only contains entries with publishedAt set.
* If dp is disabled, ignore the filter
*/
if (!contentTypes.hasDraftAndPublish(targetModel)) {
if (!model || !contentTypes.hasDraftAndPublish(model)) {
return {};
}

/**
* If source does not have dp, status should be draft.
* Both draft and publish entries are connectable, but we are doing it like
* this atm.
*/
if (!contentTypes.hasDraftAndPublish(sourceModel)) {
return { $null: true };
}

// Prioritize the draft status in case it's not provided
return status === 'published' ? { $notNull: true } : { $null: true };
};

const validateLocale = (
sourceUid: Common.UID.Schema,
targetUid: Common.UID.ContentType,
locale?: string
) => {
const sourceModel = strapi.getModel(sourceUid);
const targetModel = strapi.getModel(targetUid);

const isLocalized = strapi.plugin('i18n').service('content-types').isLocalizedContentType;
const isSourceLocalized = isLocalized(sourceModel);
const isTargetLocalized = isLocalized(targetModel);

let validatedLocale = locale;

if (!targetModel || !isTargetLocalized) validatedLocale = undefined;

return {
locale: validatedLocale,
isSourceLocalized,
isTargetLocalized,
};
};

const validateStatus = (
sourceUid: Common.UID.Schema,
status?: Documents.Params.PublicationStatus.Kind
) => {
const sourceModel = strapi.getModel(sourceUid);

const isDP = contentTypes.hasDraftAndPublish;
const isSourceDP = isDP(sourceModel);

// Default to draft if not set
if (!isSourceDP) return { status: undefined };

switch (status) {
case 'published':
return { status: 'published' };
default:
// Assign to draft if the status is not valid
return { status: 'draft' };
}
};

export default {
async extractAndValidateRequestInfo(
ctx: any,
id?: Entity.ID,
locale?: Documents.Params.Locale,
status?: Documents.Params.PublicationStatus.Kind
) {
async extractAndValidateRequestInfo(ctx: any, id?: Entity.ID) {
const { userAbility } = ctx.state;
const { model, targetField } = ctx.params;

Expand All @@ -129,6 +153,16 @@ export default {
);
}

const sourceUid = model;
const targetUid = attribute.target;

const { locale, isSourceLocalized, isTargetLocalized } = validateLocale(
sourceUid,
targetUid,
ctx.request?.query?.locale
);
const { status } = validateStatus(sourceUid, ctx.request?.query?.status);

const permissionChecker = getService('permission-checker').create({
userAbility,
model,
Expand All @@ -150,13 +184,9 @@ export default {
where.documentId = id;

if (status) {
where.publishedAt = getPublishedAtClause(status, sourceSchema.uid, attribute.target);
where.publishedAt = getPublishedAtClause(status, sourceUid);
}

const isSourceLocalized = strapi
.plugin('i18n')
.service('content-types')
.isLocalizedContentType(sourceSchema);
if (locale && isSourceLocalized) {
where.locale = locale;
}
Expand Down Expand Up @@ -197,7 +227,7 @@ export default {
? await getService('components').findConfiguration(sourceSchema)
: await getService('content-types').findConfiguration(sourceSchema);

const targetSchema = strapi.getModel(attribute.target);
const targetSchema = strapi.getModel(targetUid);

const mainField = flow(
prop(`metadatas.${targetField}.edit.mainField`),
Expand All @@ -212,18 +242,14 @@ export default {
'documentId',
]);

const isTargetLocalized = strapi
.plugin('i18n')
.service('content-types')
.isLocalizedContentType(targetSchema);

// TODO: Locale is always present, should we set it regardless?
if (isTargetLocalized) {
fieldsToSelect.push('locale');
}

return {
entryId,
locale,
status,
attribute,
fieldsToSelect,
mainField,
Expand All @@ -242,12 +268,12 @@ export default {
*/
async findAvailable(ctx: any) {
const { id } = ctx.request.query;
const locale = ctx.request?.query?.locale || null;
const status = ctx.request?.query?.status;

await validateFindAvailable(ctx.request.query);

const {
locale,
status,
targetField,
fieldsToSelect,
mainField,
Expand All @@ -258,7 +284,7 @@ export default {
schema: { uid: targetUid },
isLocalized: isTargetLocalized,
},
} = await this.extractAndValidateRequestInfo(ctx, id, locale, status);
} = await this.extractAndValidateRequestInfo(ctx, id);

const { idsToOmit, idsToInclude, _q, ...query } = ctx.request.query;

Expand All @@ -278,7 +304,7 @@ export default {
// If no status is requested, we find all the draft relations and later update them
// with the latest available status
addFiltersClause(queryParams, {
publishedAt: getPublishedAtClause(status, sourceUid, targetUid),
publishedAt: getPublishedAtClause(status, targetUid),
});

// We will only filter by locale if the target content type is localized
Expand Down Expand Up @@ -316,7 +342,7 @@ export default {

// Add the status and locale filters if they are provided
if (status) {
where[`${alias}.published_at`] = getPublishedAtClause(status, sourceUid, targetUid);
where[`${alias}.published_at`] = getPublishedAtClause(status, targetUid);
}
if (filterByLocale) {
where[`${alias}.locale`] = locale;
Expand Down Expand Up @@ -374,9 +400,6 @@ export default {

await validateFindExisting(ctx.request.query);

const locale = ctx.request?.query?.locale || null;
const status = ctx.request?.query?.status;

const {
entryId,
attribute,
Expand All @@ -388,7 +411,7 @@ export default {
target: {
schema: { uid: targetUid },
},
} = await this.extractAndValidateRequestInfo(ctx, id, locale, status);
} = await this.extractAndValidateRequestInfo(ctx, id);

const permissionQuery = await getService('permission-checker')
.create({ userAbility, model: targetUid })
Expand Down Expand Up @@ -443,7 +466,12 @@ export default {
const relationsUnion = uniqBy('id', concat(sanitizedRes.results, res.results));

ctx.body = {
pagination: res.pagination,
pagination: res.pagination || {
page: 1,
pageCount: 1,
pageSize: 10,
total: relationsUnion.length,
},
results: await addStatusToRelations(targetUid, relationsUnion),
};
},
Expand Down
3 changes: 2 additions & 1 deletion packages/core/core/src/services/document-service/transform/relations/utils/i18n.ts
View file
Expand Up @@ -9,7 +9,8 @@ export const isLocalizedContentType = (uid: Common.UID.Schema) => {

export const getDefaultLocale = () => {
// TODO: V5 make this more performant
return strapi.plugin('i18n').service('locales').getDefaultLocale();
// return strapi.plugin('i18n').service('locales').getDefaultLocale();
return 'en';
};

export const getRelationTargetLocale = (
Expand Down

0 comments on commit 66c1011

Please sign in to comment.