fix: multiple requests locale (#22273)

* fix: multiple requests locale

* fix: actions request

* fix: properly load locales

* fix: add status to localizations

* fix: remove unused types

* fix: front tests

* fix: add validation fields into localizations

* fix: validatable attributes

* fix: select nested fields when populating localizations

* fix: uncomment localizations populate

* fix: document-metadata

* fix: empty populate

* fix: revert to original proposal

* fix: do not select document ids on components (#22330)

* fix: do not select document ids on components

* chore: unit test

* fix: metadata test

* fix: populate

* fix: default fields

* fix: show current locale when bulk publishing

* fix: create locale

* Update packages/core/content-manager/server/src/services/document-metadata.ts

Co-authored-by: Jamie Howard <48524071+jhoward1994@users.noreply.github.com>

---------

Co-authored-by: Jamie Howard <48524071+jhoward1994@users.noreply.github.com>
This commit is contained in:
Marc Roig 2024-12-16 16:12:40 +01:00 committed by GitHub
parent 9a9885d211
commit 0d4051ce87
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 353 additions and 148 deletions

View File

@ -326,7 +326,7 @@ const ListViewPage = () => {
</Table.Cell>
);
})}
{/* we stop propogation here to allow the menu to trigger it's events without triggering the row redirect */}
{/* we stop propagation here to allow the menu to trigger it's events without triggering the row redirect */}
<ActionsCell onClick={(e) => e.stopPropagation()}>
<TableActions document={row} />
</ActionsCell>

View File

@ -1,15 +1,16 @@
import { groupBy, pick } from 'lodash/fp';
import { groupBy, pick, uniq } from 'lodash/fp';
import { async, contentTypes, traverseEntity } from '@strapi/utils';
import { async, contentTypes } from '@strapi/utils';
import type { Core, UID, Modules } from '@strapi/types';
import type { DocumentMetadata } from '../../../shared/contracts/collection-types';
import { getValidatableFieldsPopulate } from './utils/populate';
import { getPopulateForValidation } from './utils/populate';
export interface DocumentVersion {
id: string | number;
documentId: Modules.Documents.ID;
locale?: string;
localizations?: DocumentVersion[];
updatedAt?: string | null | Date;
publishedAt?: string | null | Date;
}
@ -29,7 +30,6 @@ const AVAILABLE_LOCALES_FIELDS = [
'locale',
'updatedAt',
'createdAt',
'status',
'publishedAt',
'documentId',
];
@ -79,8 +79,7 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
async getAvailableLocales(
uid: UID.ContentType,
version: DocumentVersion,
allVersions: DocumentVersion[],
validatableFields: string[] = []
allVersions: DocumentVersion[]
) {
// Group all versions by locale
const versionsByLocale = groupBy('locale', allVersions);
@ -94,38 +93,16 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
// There will not be a draft and a version counterpart if the content
// type does not have draft and publish
const model = strapi.getModel(uid);
const keysToKeep = [...AVAILABLE_LOCALES_FIELDS, ...validatableFields];
const traversalFunction = async (localeVersion: DocumentVersion) =>
traverseEntity(
({ key }, { remove }) => {
if (keysToKeep.includes(key)) {
// Keep the value if it is a field to pick
return;
}
// Otherwise remove this key from the data
remove(key);
},
{ schema: model, getModel: strapi.getModel.bind(strapi) },
// @ts-expect-error fix types DocumentVersion incompatible with Data
localeVersion
);
const mappingResult = await async.map(
Object.values(versionsByLocale),
async (localeVersions: DocumentVersion[]) => {
const mappedLocaleVersions: DocumentVersion[] = await async.map(
localeVersions,
traversalFunction
);
if (!contentTypes.hasDraftAndPublish(model)) {
return mappedLocaleVersions[0];
return localeVersions[0];
}
const draftVersion = mappedLocaleVersions.find((v) => v.publishedAt === null);
const otherVersions = mappedLocaleVersions.filter((v) => v.id !== draftVersion?.id);
const draftVersion = localeVersions.find((v) => v.publishedAt === null);
const otherVersions = localeVersions.filter((v) => v.id !== draftVersion?.id);
if (!draftVersion) {
return;
@ -167,6 +144,7 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
// Pick status fields (at fields, status, by fields), use lodash fp
return pick(AVAILABLE_STATUS_FIELDS, availableStatus);
},
/**
* Get the available status of many documents, useful for batch operations
* @param uid
@ -178,17 +156,17 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
// The status and locale of all documents should be the same
const status = documents[0].publishedAt !== null ? 'published' : 'draft';
const locale = documents[0]?.locale;
const otherStatus = status === 'published' ? 'draft' : 'published';
const locales = documents.map((d) => d.locale).filter(Boolean);
return strapi.documents(uid).findMany({
filters: {
return strapi.query(uid).findMany({
where: {
documentId: { $in: documents.map((d) => d.documentId).filter(Boolean) },
// NOTE: find the "opposite" status
publishedAt: { $null: status === 'published' },
locale: { $in: locales },
},
status: otherStatus,
locale,
fields: ['documentId', 'locale', 'updatedAt', 'createdAt', 'publishedAt'],
}) as unknown as DocumentMetadata['availableStatus'];
select: ['id', 'documentId', 'locale', 'updatedAt', 'createdAt', 'publishedAt'],
});
},
getStatus(version: DocumentVersion, otherDocumentStatuses?: DocumentMetadata['availableStatus']) {
@ -229,11 +207,10 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
) {
// TODO: Ignore publishedAt if availableStatus=false, and ignore locale if
// i18n is disabled
const populate = getValidatableFieldsPopulate(uid);
const versions = await strapi.db.query(uid).findMany({
where: { documentId: version.documentId },
const { populate = {}, fields = [] } = getPopulateForValidation(uid);
const params = {
populate: {
// Populate only fields that require validation for bulk locale actions
...populate,
// NOTE: creator fields are selected in this way to avoid exposing sensitive data
createdBy: {
@ -243,10 +220,18 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
select: ['id', 'firstname', 'lastname', 'email'],
},
},
});
fields: uniq([...AVAILABLE_LOCALES_FIELDS, ...fields]),
filters: {
documentId: version.documentId,
},
};
const dbParams = strapi.get('query-params').transform(uid, params);
const versions = await strapi.db.query(uid).findMany(dbParams);
// TODO: Remove use of available locales and use localizations instead
const availableLocalesResult = availableLocales
? await this.getAvailableLocales(uid, version, versions, Object.keys(populate))
? await this.getAvailableLocales(uid, version, versions)
: [];
const availableStatusResult = availableStatus
@ -288,6 +273,19 @@ export default ({ strapi }: { strapi: Core.Strapi }) => ({
const meta = await this.getMetadata(uid, document, opts);
// Populate localization statuses
if (document.localizations) {
const otherStatus = await this.getManyAvailableStatus(uid, document.localizations);
document.localizations = document.localizations.map((d) => {
const status = otherStatus.find((s) => s.documentId === d.documentId);
return {
...d,
status: this.getStatus(d, status ? [status] : []),
};
});
}
return {
data: {
...document,

View File

@ -0,0 +1,173 @@
import { getPopulateForValidation } from '../populate';
describe('getPopulateForValidation', () => {
const fakeModels = {
empty: {
modelName: 'Fake empty model',
attributes: {},
},
scalarOnly: {
modelName: 'Fake scalar-only model',
attributes: {
title: { type: 'string', required: true },
description: { type: 'text', required: false },
},
},
componentWithRequiredFields: {
modelName: 'Fake component with required fields',
attributes: {
componentAttrName: {
type: 'component',
component: 'componentFields',
},
},
},
componentFields: {
modelName: 'Fake component fields',
attributes: {
subfield1: { type: 'string', required: true },
subfield2: { type: 'number', required: false },
},
},
componentWithoutRequiredFields: {
modelName: 'Fake component without required fields',
attributes: {
componentAttrName: {
type: 'component',
component: 'empty',
},
},
},
} as any;
beforeEach(() => {
global.strapi = {
getModel: jest.fn((uid) => fakeModels[uid]),
} as any;
});
afterEach(() => {
jest.clearAllMocks();
});
test('with empty model', () => {
const uid = 'empty';
const result = getPopulateForValidation(uid as any);
expect(result).toEqual({});
});
test('with scalar-only model', () => {
const uid = 'scalarOnly';
const result = getPopulateForValidation(uid as any);
expect(result).toEqual({
fields: ['title'], // Only scalar fields requiring validation
});
});
describe('components', () => {
test('with component model containing required fields', () => {
const uid = 'componentWithRequiredFields';
const result = getPopulateForValidation(uid as any);
expect(result).toEqual({
populate: {
componentAttrName: {
fields: ['subfield1'], // Only required fields in the component
},
},
});
});
test('with component model without required fields', () => {
const uid = 'componentWithoutRequiredFields';
const result = getPopulateForValidation(uid as any);
expect(result).toEqual({}); // No required fields, so no populate
});
test('with nested components', () => {
fakeModels.nestedComponent = {
modelName: 'Fake nested component model',
attributes: {
nestedComponentAttr: {
type: 'component',
component: 'componentFields',
},
},
};
fakeModels.parentModel = {
modelName: 'Fake parent model',
attributes: {
parentComponent: {
type: 'component',
component: 'nestedComponent',
},
},
};
const uid = 'parentModel';
const result = getPopulateForValidation(uid as any);
expect(result).toEqual({
populate: {
parentComponent: {
populate: {
nestedComponentAttr: {
fields: ['subfield1'],
},
},
},
},
});
});
});
describe('dynamic zones', () => {
fakeModels.dynamicZone = {
modelName: 'Fake dynamic zone model',
attributes: {
dynZoneAttrName: {
type: 'dynamiczone',
components: [
'componentFields',
'componentWithRequiredFields',
'componentWithoutRequiredFields',
],
},
},
};
test('with dynamic zone model', () => {
const uid = 'dynamicZone';
const result = getPopulateForValidation(uid as any);
expect(result).toEqual({
populate: {
dynZoneAttrName: {
on: {
componentFields: {
fields: ['subfield1'],
},
componentWithRequiredFields: {
populate: {
componentAttrName: {
fields: ['subfield1'],
},
},
},
},
},
},
});
});
});
});

View File

@ -46,6 +46,15 @@ function getPopulateForRelation(
return initialPopulate;
}
// If populating localizations attribute, also include validatable fields
// Mainly needed for bulk locale publishing, so the Client has all the information necessary to perform validations
if (attributeName === 'localizations') {
const validationPopulate = getPopulateForValidation(model.uid as UID.Schema);
return {
populate: validationPopulate.populate,
};
}
// always populate createdBy, updatedBy, localizations etc.
if (!isVisibleAttribute(model, attributeName)) {
return true;
@ -153,6 +162,10 @@ const getDeepPopulate = (
const model = strapi.getModel(uid);
if (!model) {
return {};
}
return Object.keys(model.attributes).reduce(
(populateAcc, attributeName: string) =>
merge(
@ -180,49 +193,63 @@ const getDeepPopulate = (
* @param options - Options to apply while populating
* @param level - Current level of nested call
*/
const getValidatableFieldsPopulate = (
uid: UID.Schema,
{
initialPopulate = {} as any,
countMany = false,
countOne = false,
maxLevel = Infinity,
}: PopulateOptions = {},
level = 1
) => {
if (level > maxLevel) {
const getPopulateForValidation = (uid: UID.Schema): Record<string, any> => {
const model = strapi.getModel(uid);
if (!model) {
return {};
}
const model = strapi.getModel(uid);
return Object.entries(model.attributes).reduce((populateAcc, [attributeName, attribute]) => {
if (!getDoesAttributeRequireValidation(attribute)) {
// If the attribute does not require validation, skip it
return Object.entries(model.attributes).reduce((populateAcc: any, [attributeName, attribute]) => {
if (isScalarAttribute(attribute)) {
// If the scalar attribute requires validation, add it to the fields array
if (getDoesAttributeRequireValidation(attribute)) {
populateAcc.fields = populateAcc.fields || [];
populateAcc.fields.push(attributeName);
}
return populateAcc;
}
if (isScalarAttribute(attribute)) {
return merge(populateAcc, {
[attributeName]: true,
});
if (isComponent(attribute)) {
// @ts-expect-error - should be a component
const component = attribute.component;
// Get the validation result for this component
const componentResult = getPopulateForValidation(component);
if (Object.keys(componentResult).length > 0) {
populateAcc.populate = populateAcc.populate || {};
populateAcc.populate[attributeName] = componentResult;
}
return populateAcc;
}
return merge(
populateAcc,
getPopulateFor(
attributeName,
model,
{
// @ts-expect-error - improve types
initialPopulate: initialPopulate?.[attributeName],
countMany,
countOne,
maxLevel,
if (isDynamicZone(attribute)) {
const components = (attribute as Schema.Attribute.DynamicZone).components;
// Handle dynamic zone components
const componentsResult = (components || []).reduce(
(acc, componentUID) => {
// Get validation populate for this component
const componentResult = getPopulateForValidation(componentUID);
// Only include component if it has fields requiring validation
if (Object.keys(componentResult).length > 0) {
acc[componentUID] = componentResult;
}
return acc;
},
level
)
);
{} as Record<string, any>
);
// Only add to populate if we have components requiring validation
if (Object.keys(componentsResult).length > 0) {
populateAcc.populate = populateAcc.populate || {};
populateAcc.populate[attributeName] = { on: componentsResult };
}
}
return populateAcc;
}, {});
};
@ -341,7 +368,7 @@ const buildDeepPopulate = (uid: UID.CollectionType) => {
export {
getDeepPopulate,
getDeepPopulateDraftCount,
getPopulateForValidation,
getQueryPopulate,
buildDeepPopulate,
getValidatableFieldsPopulate,
};

View File

@ -18,6 +18,9 @@ const models = {
title: {
type: 'string',
},
one_to_one: { type: 'relation', relation: 'oneToOne', target: 'api::dog.dog' },
cpa: { type: 'component', component: 'default.cpa' },
cpb: { type: 'component', component: 'default.cpb' },
dz: { type: 'dynamiczone', components: ['default.cpa', 'default.cpb'] },
morph_to_one: { type: 'relation', relation: 'morphToOne' },
morph_to_many: { type: 'relation', relation: 'morphToMany' },
@ -90,6 +93,34 @@ describe('convert-query-params', () => {
test.todo('convertLimitQueryParams');
describe('convertPopulateQueryParams', () => {
describe('Fields selection', () => {
test('should not select documentId when selecting fields for components', () => {
const populate = {
cpa: { fields: ['field'] },
cpb: { fields: ['field'] },
};
const newPopulate = private_convertPopulateQueryParams(populate, models['api::dog.dog']);
expect(newPopulate).toStrictEqual({
cpa: { select: ['id', 'field'] },
cpb: { select: ['id', 'field'] },
});
});
test('should select documentId for non-component populate', () => {
const populate = {
one_to_one: { fields: ['title'] },
};
const newPopulate = private_convertPopulateQueryParams(populate, models['api::dog.dog']);
expect(newPopulate).toStrictEqual({
one_to_one: { select: ['id', 'documentId', 'title'] },
});
});
});
describe('Morph-Like Attributes', () => {
test.each<[label: string, key: string]>([
['dynamic zone', 'dz'],

View File

@ -501,7 +501,7 @@ const createTransformer = ({ getModel }: TransformerOptions) => {
}
if (fields) {
query.select = convertFieldsQueryParams(fields);
query.select = convertFieldsQueryParams(fields, schema);
}
if (populate) {
@ -538,23 +538,36 @@ const createTransformer = ({ getModel }: TransformerOptions) => {
};
// TODO: ensure field is valid in content types (will probably have to check strapi.contentTypes since it can be a string.path)
const convertFieldsQueryParams = (fields: FieldsParams, depth = 0): SelectQuery | undefined => {
const convertFieldsQueryParams = (
fields: FieldsParams,
schema?: Model,
depth = 0
): SelectQuery | undefined => {
if (depth === 0 && fields === '*') {
return undefined;
}
if (typeof fields === 'string') {
const fieldsValues = fields.split(',').map((value) => _.trim(value));
return _.uniq([ID_ATTRIBUTE, DOC_ID_ATTRIBUTE, ...fieldsValues]);
// NOTE: Only include the doc id if it's a content type
if (schema?.modelType === 'contentType') {
return _.uniq([ID_ATTRIBUTE, DOC_ID_ATTRIBUTE, ...fieldsValues]);
}
return _.uniq([ID_ATTRIBUTE, ...fieldsValues]);
}
if (isStringArray(fields)) {
// map convert
const fieldsValues = fields
.flatMap((value) => convertFieldsQueryParams(value, depth + 1))
.flatMap((value) => convertFieldsQueryParams(value, schema, depth + 1))
.filter((v) => !isNil(v)) as string[];
return _.uniq([ID_ATTRIBUTE, DOC_ID_ATTRIBUTE, ...fieldsValues]);
// NOTE: Only include the doc id if it's a content type
if (schema?.modelType === 'contentType') {
return _.uniq([ID_ATTRIBUTE, DOC_ID_ATTRIBUTE, ...fieldsValues]);
}
return _.uniq([ID_ATTRIBUTE, ...fieldsValues]);
}
throw new Error('Invalid fields parameter. Expected a string or an array of strings');
@ -700,7 +713,7 @@ const createTransformer = ({ getModel }: TransformerOptions) => {
}
if (!isNil(fields)) {
query.select = convertFieldsQueryParams(fields);
query.select = convertFieldsQueryParams(fields, schema);
}
if (!isNil(populate)) {

View File

@ -161,7 +161,7 @@ const LocalePickerAction = ({
const allCurrentLocales = [
{ status: getDocumentStatus(document, meta), locale: currentLocale?.code },
...(meta?.availableLocales ?? []),
...(document?.localizations ?? []),
];
if (!hasI18n || !Array.isArray(locales) || locales.length === 0) {
@ -472,14 +472,13 @@ interface ExtendedDocumentActionProps extends DocumentActionProps {
* -----------------------------------------------------------------------------------------------*/
const BulkLocaleAction: DocumentActionComponent = ({
document: baseDocument,
document,
documentId,
model,
collectionType,
action,
}: ExtendedDocumentActionProps) => {
const baseLocale = baseDocument?.locale ?? null;
const locale = document?.locale ?? null;
const [{ query }] = useQueryParams<{ status: 'draft' | 'published' }>();
const params = React.useMemo(() => buildValidParams(query), [query]);
@ -497,22 +496,18 @@ const BulkLocaleAction: DocumentActionComponent = ({
const { publishMany: publishManyAction, unpublishMany: unpublishManyAction } =
useDocumentActions();
const {
document,
meta: documentMeta,
schema,
validate,
} = useDocument(
const { schema, validate } = useDocument(
{
model,
collectionType,
documentId,
params: {
locale: baseLocale,
locale,
},
},
{
skip: !hasI18n || !baseLocale,
// No need to fetch the document, the data is already available in the `document` prop
skip: true,
}
);
@ -545,27 +540,27 @@ const BulkLocaleAction: DocumentActionComponent = ({
// Extract the rows for the bulk locale publish modal and any validation
// errors per locale
const [rows, validationErrors] = React.useMemo(() => {
if (!document || !documentMeta?.availableLocales) {
// If we don't have a document or available locales, we return empty rows
// and no validation errors
if (!document) {
return [[], {}];
}
const localizations = document.localizations ?? [];
// Build the rows for the bulk locale publish modal by combining the current
// document with all the available locales from the document meta
const rowsFromMeta: LocaleStatus[] = documentMeta?.availableLocales.map((doc) => {
const locales: LocaleStatus[] = localizations.map((doc: any) => {
const { locale, status } = doc;
return { locale, status };
});
rowsFromMeta.unshift({
// Add the current document locale
locales.unshift({
locale: document.locale,
status: document.status,
});
// Build the validation errors for each locale.
const allDocuments = [document, ...(documentMeta?.availableLocales ?? [])];
const allDocuments = [document, ...localizations];
const errors = allDocuments.reduce<FormErrors>((errs, document) => {
if (!document) {
return errs;
@ -579,8 +574,8 @@ const BulkLocaleAction: DocumentActionComponent = ({
return errs;
}, {});
return [rowsFromMeta, errors];
}, [document, documentMeta?.availableLocales, validate]);
return [locales, errors];
}, [document, validate]);
const isBulkPublish = action === 'bulk-publish';
const localesForAction = selectedRows.reduce((acc: string[], selectedRow: LocaleStatus) => {

View File

@ -1,4 +1,3 @@
import { unstable_useDocument as useDocument } from '@strapi/content-manager/strapi-admin';
import { Box, Flex, Popover, Typography, useCollator, Button } from '@strapi/design-system';
import { CaretDown } from '@strapi/icons';
import { useIntl } from 'react-intl';
@ -7,39 +6,23 @@ import { Locale } from '../../../shared/contracts/locales';
import { useGetLocalesQuery } from '../services/locales';
interface LocaleListCellProps {
documentId: string;
collectionType: string;
localizations: { locale: string }[];
locale: string;
model: string;
}
const LocaleListCell = ({
documentId,
locale: currentLocale,
collectionType,
model,
}: LocaleListCellProps) => {
// TODO: avoid loading availableLocales for each row but get that from the BE
const { meta, isLoading } = useDocument({
documentId,
collectionType,
model,
params: {
locale: currentLocale,
},
});
const LocaleListCell = ({ locale: currentLocale, localizations }: LocaleListCellProps) => {
const { locale: language } = useIntl();
const { data: locales = [] } = useGetLocalesQuery();
const formatter = useCollator(language, {
sensitivity: 'base',
});
if (!Array.isArray(locales) || isLoading) {
if (!Array.isArray(locales) || !localizations) {
return null;
}
const availableLocales = meta?.availableLocales.map((doc) => doc.locale) ?? [];
const availableLocales = localizations.map((loc) => loc.locale);
const localesForDocument = locales
.reduce<Locale[]>((acc, locale) => {
const createdLocale = [currentLocale, ...availableLocales].find((loc) => {

View File

@ -20,14 +20,7 @@ jest.mock('@strapi/content-manager/strapi-admin', () => ({
describe('LocaleListCell', () => {
it('renders a button with all the names of the locales that are available for the document', async () => {
render(
<LocaleListCell
documentId="12345"
collectionType="collection-types"
locale="en"
model="api::address.address"
/>
);
render(<LocaleListCell localizations={[{ locale: 'en' }, { locale: 'fr' }]} locale="en" />);
expect(
await screen.findByRole('button', { name: 'English (default), Français' })
@ -38,12 +31,7 @@ describe('LocaleListCell', () => {
it('renders a list of the locales available on the document when the button is clicked', async () => {
const { user } = render(
<LocaleListCell
documentId="12345"
collectionType="collection-types"
locale="en"
model="api::address.address"
/>
<LocaleListCell localizations={[{ locale: 'en' }, { locale: 'fr' }]} locale="en" />
);
expect(

View File

@ -317,10 +317,7 @@ describe('CM API - Document metadata', () => {
createdAt: expect.any(String),
updatedAt: expect.any(String),
};
expect(meta.availableLocales).toEqual(
expect.arrayContaining([expect.objectContaining(expectedLocaleData)])
);
expect(meta.availableLocales).toMatchObject([expectedLocaleData]);
// Ensure no unwanted keys are present
const unwantedKeys = ['shopName'];
unwantedKeys.forEach((key) => {