fix(history): handle sanitization when fetching versions (#20212)

Co-authored-by: Rémi de Juvigny <remi.dejuvigny@strapi.io>
strapi · Apr 30, 2024 · 4a26739 · 4a26739
1 parent 19fab61
commit 4a26739
Show file tree

Hide file tree

Showing 8 changed files with 218 additions and 107 deletions.
diff --git a/packages/core/content-manager/admin/src/history/components/VersionInputRenderer.tsx b/packages/core/content-manager/admin/src/history/components/VersionInputRenderer.tsx
@@ -59,16 +59,46 @@ const LinkEllipsis = styled(Link)`
 
 const CustomRelationInput = (props: RelationsFieldProps) => {
   const { formatMessage } = useIntl();
-  const field = useField<{ results: RelationResult[]; meta: { missingCount: number } }>(props.name);
-  const { results, meta } = field.value!;
+  const field = useField<
+    { results: RelationResult[]; meta: { missingCount: number } } | RelationResult[]
+  >(props.name);
+
+  /**
+   * Ideally the server would return the correct shape, however, for admin user relations
+   * it sanitizes everything out when it finds an object for the relation value.
+   */
+  const formattedFieldValue = Array.isArray(field.value)
+    ? {
+        results: field.value,
+        meta: { missingCount: 0 },
+      }
+    : field.value;
+
+  if (!formattedFieldValue || formattedFieldValue.results.length === 0) {
+    return (
+      <>
+        <FieldLabel>{props.label}</FieldLabel>
+        <Box marginTop={1}>
+          <StyledAlert variant="default">
+            {formatMessage({
+              id: 'content-manager.history.content.no-relations',
+              defaultMessage: 'No relations.',
+            })}
+          </StyledAlert>
+        </Box>
+      </>
+    );
+  }
+
+  const { results, meta } = formattedFieldValue;
 
   return (
     <Box>
       <FieldLabel>{props.label}</FieldLabel>
       {results.length > 0 && (
         <Flex direction="column" gap={2} marginTop={1} alignItems="stretch">
           {results.map((relationData) => {
-            // @ts-expect-error – targetModel does exist on the attribute. But it's not typed.
+            // @ts-expect-error - targetModel does exist on the attribute. But it's not typed.
             const href = `../${COLLECTION_TYPES}/${props.attribute.targetModel}/${relationData.documentId}`;
             const label = getRelationLabel(relationData, props.mainField);
 

diff --git a/packages/core/content-manager/admin/src/history/components/tests/VersionHeader.test.tsx b/packages/core/content-manager/admin/src/history/components/tests/VersionHeader.test.tsx
@@ -55,6 +55,8 @@ describe('VersionHeader', () => {
       componentsSchemas: {},
       locale: null,
       data: {
+        documentId: '1234',
+        id: 1,
         title: 'Test Title',
       },
       meta: {
@@ -117,7 +119,7 @@ describe('VersionHeader', () => {
 
     it('should display the correct subtitle without an entry title (mainField)', async () => {
       render(
-        { selectedVersion, mainField: 'id' },
+        { selectedVersion, mainField: 'plop' },
         '/collection-types/api::kitchensink.kitchensink/pcwmq3rlmp5w0be3cuplhnpr/history'
       );
 
@@ -138,6 +140,8 @@ describe('VersionHeader', () => {
       componentsSchemas: {},
       locale: null,
       data: {
+        documentId: '1234',
+        id: 1,
         title: 'Test Title',
       },
       meta: {

diff --git a/packages/core/content-manager/server/src/history/constants.ts b/packages/core/content-manager/server/src/history/constants.ts
@@ -5,7 +5,6 @@ export const FIELDS_TO_IGNORE = [
   'publishedAt',
   'createdBy',
   'updatedBy',
-  'locale',
   'strapi_stage',
   'strapi_assignee',
 ];
diff --git a/packages/core/content-manager/server/src/history/controllers/history-version.ts b/packages/core/content-manager/server/src/history/controllers/history-version.ts
@@ -1,5 +1,6 @@
-import { errors } from '@strapi/utils';
+import { async, errors } from '@strapi/utils';
 import type { Core, UID } from '@strapi/types';
+import { pick } from 'lodash/fp';
 import { getService as getContentManagerService } from '../../utils';
 import { getService } from '../utils';
 import { HistoryVersions } from '../../../../shared/contracts';
@@ -60,21 +61,32 @@ const createHistoryVersionController = ({ strapi }: { strapi: Core.Strapi }) =>
         return ctx.forbidden();
       }
 
-      const params: HistoryVersions.GetHistoryVersions.Request['query'] =
+      const query: HistoryVersions.GetHistoryVersions.Request['query'] =
         await permissionChecker.sanitizeQuery(ctx.query);
 
       const { results, pagination } = await getService(strapi, 'history').findVersionsPage({
-        ...params,
-        ...getValidPagination({ page: params.page, pageSize: params.pageSize }),
+        query: {
+          ...query,
+          ...getValidPagination({ page: query.page, pageSize: query.pageSize }),
+        },
+        state: { userAbility: ctx.state.userAbility },
       });
 
+      const sanitizedResults = await async.map(
+        results,
+        async (version: HistoryVersions.HistoryVersionDataResponse & { locale: string }) => {
+          return {
+            ...version,
+            data: await permissionChecker.sanitizeOutput(version.data),
+            createdBy: version.createdBy
+              ? pick(['id', 'firstname', 'lastname', 'username', 'email'], version.createdBy)
+              : undefined,
+          };
+        }
+      );
+
       return {
-        data: await Promise.all(
-          results.map(async (result) => ({
-            ...result,
-            data: await permissionChecker.sanitizeOutput(result.data),
-          }))
-        ),
+        data: sanitizedResults,
         meta: { pagination },
       };
     },

diff --git a/packages/core/content-manager/server/src/history/services/__tests__/history.test.ts b/packages/core/content-manager/server/src/history/services/__tests__/history.test.ts
@@ -246,6 +246,8 @@ describe('history-version service', () => {
     const historyVersionData = {
       contentType: 'api::article.article' as UID.ContentType,
       data: {
+        documentId: '1234',
+        id: 1,
         title: 'My article',
       },
       locale: 'en',
@@ -275,6 +277,8 @@ describe('history-version service', () => {
     const historyVersionData = {
       contentType: 'api::article.article' as UID.ContentType,
       data: {
+        documentId: '1234',
+        id: 1,
         title: 'My article',
       },
       locale: 'en',