Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(history): refactor restore and add tests for relations #20179

Merged
merged 10 commits into from May 6, 2024
Merged

chore(history): refactor restore and add tests for relations #20179

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
433f88c
chore(history): refactor restore
markkaylor Apr 19, 2024
a83c4ff
chore: refactor lifecycles into service
markkaylor Apr 23, 2024
11e7684
chore: rename utils function
markkaylor Apr 23, 2024
964f662
chore: update variable names
markkaylor Apr 23, 2024
fd71d90
chore: update missed variable names
markkaylor Apr 24, 2024
d0e45bc
Merge branch 'v5/main' of github.com:strapi/strapi into v5/history-re…
markkaylor Apr 30, 2024
13aa7ad
chore: test sanitized relations
markkaylor Apr 30, 2024
5c8c735
fix: prettify and remove ts-expects
markkaylor Apr 30, 2024
39f7b14
chore: remi feedback
markkaylor May 6, 2024
8b35806
Merge branch 'v5/main' into v5/history-refactor-restore
markkaylor May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/core/content-manager/server/src/history/services/__tests__/history.test.ts
View file
Open in desktop
Expand Up @@ -138,7 +138,6 @@ describe('history-version service', () => {
status: 'draft' as const,
};

// @ts-expect-error - ignore
await historyService.createVersion(historyVersionData);
expect(createMock).toHaveBeenCalledWith({
data: {
Expand Down Expand Up @@ -172,7 +171,6 @@ describe('history-version service', () => {

mockGetRequestContext.mockReturnValueOnce(null as any);

// @ts-expect-error - ignore
await historyService.createVersion(historyVersionData);
expect(createMock).toHaveBeenCalledWith({
data: {
Expand Down
170 changes: 41 additions & 129 deletions packages/core/content-manager/server/src/history/services/history.ts
View file
Open in desktop
@@ -1,6 +1,6 @@
import type { Core, Data, Schema, Struct } from '@strapi/types';
import { async, errors } from '@strapi/utils';
import { omit, pick } from 'lodash/fp';
import { omit } from 'lodash/fp';

import { FIELDS_TO_IGNORE, HISTORY_VERSION_UID } from '../constants';
import type { HistoryVersions } from '../../../../shared/contracts';
Expand All @@ -9,14 +9,15 @@ import {
HistoryVersionDataResponse,
} from '../../../../shared/contracts/history-versions';
import { createServiceUtils } from './utils';
import { getService as getContentManagerService } from '../../utils';

// Needed because the query engine doesn't return any types yet
type HistoryVersionQueryResult = Omit<HistoryVersionDataResponse, 'locale'> &
Pick<CreateHistoryVersion, 'locale'>;

const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
const query = strapi.db.query(HISTORY_VERSION_UID);
const historyUtils = createServiceUtils({ strapi });
const serviceUtils = createServiceUtils({ strapi });

return {
async createVersion(historyVersionData: HistoryVersions.CreateHistoryVersion) {
Expand All @@ -33,7 +34,7 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
results: HistoryVersions.HistoryVersionDataResponse[];
pagination: HistoryVersions.Pagination;
}> {
const locale = params.locale || (await historyUtils.getDefaultLocale());
const locale = params.query.locale || (await serviceUtils.getDefaultLocale());
const [{ results, pagination }, localeDictionary] = await Promise.all([
query.findPage({
...params.query,
Expand All @@ -47,122 +48,9 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
populate: ['createdBy'],
orderBy: [{ createdAt: 'desc' }],
}),
historyUtils.getLocaleDictionary(),
serviceUtils.getLocaleDictionary(),
]);

/**
* Get an object with two keys:
* - results: an array with the current values of the relations
* - meta: an object with the count of missing relations
*/
// TODO: Move outside this function to utils
const buildRelationReponse = async (
values: {
documentId: string;
locale: string | null;
}[],
attributeSchema: Schema.Attribute.RelationWithTarget
): Promise<{ results: any[]; meta: { missingCount: number } }> => {
return (
values
// Until we implement proper pagination, limit relations to an arbitrary amount
.slice(0, 25)
.reduce(
async (currentRelationDataPromise, entry) => {
const currentRelationData = await currentRelationDataPromise;

// Entry can be null if it's a toOne relation
if (!entry) {
return currentRelationData;
}

const relatedEntry = await strapi
.documents(attributeSchema.target)
.findOne({ documentId: entry.documentId, locale: entry.locale || undefined });

const permissionChecker = getContentManagerService('permission-checker').create({
userAbility: params.state.userAbility,
model: attributeSchema.target,
});
const sanitizedEntry = await permissionChecker.sanitizeOutput(relatedEntry);

if (sanitizedEntry) {
currentRelationData.results.push({
...relatedEntry,
...(isNormalRelation
? {
status: await historyUtils.getVersionStatus(
attributeSchema.target,
relatedEntry
),
}
: {}),
});
} else {
// The related content has been deleted
currentRelationData.meta.missingCount += 1;
}

return currentRelationData;
},
Promise.resolve({
results: [] as any[],
meta: { missingCount: 0 },
})
)
);
};

/**
* Get an object with two keys:
* - results: an array with the current values of the relations
* - meta: an object with the count of missing relations
*/
// TODO: Move outside this function to utils
const buildMediaResponse = async (
values: { id: Data.ID }[]
): Promise<{ results: any[]; meta: { missingCount: number } }> => {
return (
values
// Until we implement proper pagination, limit relations to an arbitrary amount
.slice(0, 25)
.reduce(
async (currentRelationDataPromise, entry) => {
const currentRelationData = await currentRelationDataPromise;

// Entry can be null if it's a toOne relation
if (!entry) {
return currentRelationData;
}

const permissionChecker = getContentManagerService('permission-checker').create({
userAbility: params.state.userAbility,
model: 'plugin::upload.file',
});

const relatedEntry = await strapi.db
.query('plugin::upload.file')
.findOne({ where: { id: entry.id } });

const sanitizedEntry = await permissionChecker.sanitizeOutput(relatedEntry);

if (sanitizedEntry) {
currentRelationData.results.push(sanitizedEntry);
} else {
// The related content has been deleted
currentRelationData.meta.missingCount += 1;
}

return currentRelationData;
},
Promise.resolve({
results: [] as any[],
meta: { missingCount: 0 },
})
)
);
};

const populateEntryRelations = async (
entry: HistoryVersionQueryResult
): Promise<CreateHistoryVersion['data']> => {
Expand All @@ -174,9 +62,22 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
: [attributeValue];

if (attributeSchema.type === 'media') {
const permissionChecker = getContentManagerService('permission-checker').create({
userAbility: params.state.userAbility,
model: 'plugin::upload.file',
});

const response = await serviceUtils.buildMediaResponse(attributeValues);
const sanitizedResults = await Promise.all(
response.results.map((media) => permissionChecker.sanitizeOutput(media))
);

return {
...(await currentDataWithRelations),
[attributeKey]: await buildMediaResponse(attributeValues),
[attributeKey]: {
results: sanitizedResults,
meta: response.meta,
},
};
}

Expand All @@ -192,7 +93,7 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
*/
if (attributeSchema.target === 'admin::user') {
const adminUsers = await Promise.all(
attributeValues.map(async (userToPopulate) => {
attributeValues.map((userToPopulate) => {
if (userToPopulate == null) {
return null;
}
Expand All @@ -205,18 +106,29 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {

return {
...(await currentDataWithRelations),
/**
* Ideally we would return the same "{results: [], meta: {}}" shape, however,
* when sanitizing the data as a whole in the controller before sending to the client,
* the data for admin relation user is completely sanitized if we return an object here as opposed to an array.
*/
markkaylor marked this conversation as resolved.
Show resolved Hide resolved
[attributeKey]: adminUsers,
};
}

const permissionChecker = getContentManagerService('permission-checker').create({
userAbility: params.state.userAbility,
model: attributeSchema.target,
});

const response = await serviceUtils.buildRelationReponse(
attributeValues,
attributeSchema
);
const sanitizedResults = await Promise.all(
response.results.map((media) => permissionChecker.sanitizeOutput(media))
);

return {
...(await currentDataWithRelations),
[attributeKey]: await buildRelationReponse(attributeValues, attributeSchema),
[attributeKey]: {
results: sanitizedResults,
meta: response.meta,
},
};
}

Expand All @@ -235,7 +147,7 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
...result,
data: await populateEntryRelations(result),
meta: {
unknownAttributes: historyUtils.getSchemaAttributesDiff(
unknownAttributes: serviceUtils.getSchemaAttributesDiff(
result.schema,
strapi.getModel(params.query.contentType).attributes
),
Expand All @@ -254,7 +166,7 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
async restoreVersion(versionId: Data.ID) {
const version = await query.findOne({ where: { id: versionId } });
const contentTypeSchemaAttributes = strapi.getModel(version.contentType).attributes;
const schemaDiff = historyUtils.getSchemaAttributesDiff(
const schemaDiff = serviceUtils.getSchemaAttributesDiff(
version.schema,
contentTypeSchemaAttributes
);
Expand Down Expand Up @@ -291,12 +203,12 @@ const createHistoryService = ({ strapi }: { strapi: Core.Strapi }) => {
attribute.relation !== 'morphToOne' &&
attribute.relation !== 'morphToMany'
) {
const data = await historyUtils.getRelationRestoreValue(versionRelationData, attribute);
const data = await serviceUtils.getRelationRestoreValue(versionRelationData, attribute);
previousRelationAttributes[name] = data;
}

if (attribute.type === 'media') {
const data = await historyUtils.getMediaRestoreValue(versionRelationData, attribute);
const data = await serviceUtils.getMediaRestoreValue(versionRelationData, attribute);
previousRelationAttributes[name] = data;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/content-manager/server/src/history/services/index.ts
View file
Open in desktop
Expand Up @@ -4,5 +4,5 @@ import { createLifecyclesService } from './lifecycles';

export const services = {
history: createHistoryService,
lifecycles: createLifecyclesService
lifecycles: createLifecyclesService,
} satisfies Plugin.LoadedPlugin['services'];
94 changes: 94 additions & 0 deletions packages/core/content-manager/server/src/history/services/utils.ts
View file
Open in desktop
Expand Up @@ -8,6 +8,11 @@ import { HistoryVersions } from '../../../../shared/contracts';

const DEFAULT_RETENTION_DAYS = 90;

type RelationResponse = {
results: any[];
meta: { missingCount: number };
};
markkaylor marked this conversation as resolved.
Show resolved Hide resolved

export const createServiceUtils = ({ strapi }: { strapi: Core.Strapi }) => {
/**
* @description
Expand Down Expand Up @@ -220,6 +225,93 @@ export const createServiceUtils = ({ strapi }: { strapi: Core.Strapi }) => {
}, {});
};

/**
* @description
* Builds a response object for relations containing the related data and a count of missing relations
*/
const buildMediaResponse = async (values: { id: Data.ID }[]): Promise<RelationResponse> => {
return (
values
// Until we implement proper pagination, limit relations to an arbitrary amount
.slice(0, 25)
.reduce(
async (currentRelationDataPromise, entry) => {
const currentRelationData = await currentRelationDataPromise;

// Entry can be null if it's a toOne relation
if (!entry) {
return currentRelationData;
}

const relatedEntry = await strapi.db
.query('plugin::upload.file')
.findOne({ where: { id: entry.id } });

if (relatedEntry) {
currentRelationData.results.push(relatedEntry);
} else {
// The related content has been deleted
currentRelationData.meta.missingCount += 1;
}

return currentRelationData;
},
Promise.resolve<RelationResponse>({
results: [],
meta: { missingCount: 0 },
})
)
);
};

/**
* @description
* Builds a response object for media containing the media assets data and a count of missing media assets
*/
const buildRelationReponse = async (
values: {
documentId: string;
locale: string | null;
}[],
attributeSchema: Schema.Attribute.RelationWithTarget
): Promise<RelationResponse> => {
return (
values
// Until we implement proper pagination, limit relations to an arbitrary amount
.slice(0, 25)
.reduce(
async (currentRelationDataPromise, entry) => {
const currentRelationData = await currentRelationDataPromise;

// Entry can be null if it's a toOne relation
if (!entry) {
return currentRelationData;
}

const relatedEntry = await strapi
.documents(attributeSchema.target)
.findOne({ documentId: entry.documentId, locale: entry.locale || undefined });

if (relatedEntry) {
currentRelationData.results.push({
...relatedEntry,
status: await getVersionStatus(attributeSchema.target, relatedEntry),
});
} else {
// The related content has been deleted
currentRelationData.meta.missingCount += 1;
}

return currentRelationData;
},
Promise.resolve<RelationResponse>({
results: [],
meta: { missingCount: 0 },
})
)
);
};

return {
getSchemaAttributesDiff,
getRelationRestoreValue,
Expand All @@ -229,5 +321,7 @@ export const createServiceUtils = ({ strapi }: { strapi: Core.Strapi }) => {
getRetentionDays,
getVersionStatus,
getDeepPopulate,
buildMediaResponse,
buildRelationReponse,
};
};