feat(history): diff schemas to get unknown attributes (#19849)

2025-09-25 16:29:34 +00:00 · 2024-03-21 15:20:57 +01:00 · 2024-03-21 15:20:57 +01:00 · 1c2f2708ff
commit 1c2f2708ff
parent 40b339395f
6 changed files with 255 additions and 124 deletions
--- a/packages/core/content-manager/server/src/history/constants.ts
+++ b/packages/core/content-manager/server/src/history/constants.ts
@ -1 +1,11 @@
 export const HISTORY_VERSION_UID = 'plugin::content-manager.history-version';
+export const FIELDS_TO_IGNORE = [
+  'createdAt',
+  'updatedAt',
+  'publishedAt',
+  'createdBy',
+  'updatedBy',
+  'locale',
+  'strapi_stage',
+  'strapi_assignee',
+];
--- 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
@ -35,6 +35,7 @@ const mockStrapi = {
    i18n: {
      service: jest.fn(() => ({
        getDefaultLocale: jest.fn().mockReturnValue('en'),
+        find: jest.fn(),
      })),
    },
  },
@ -95,136 +96,140 @@ describe('history-version service', () => {
    jest.useRealTimers();
  });

-  it('inits service only once', () => {
-    historyService.bootstrap();
-    historyService.bootstrap();
-    // @ts-expect-error - ignore
-    expect(mockStrapi.documents.use).toHaveBeenCalledTimes(1);
-  });
+  describe('boostrap', () => {
+    it('inits service only once', () => {
+      historyService.bootstrap();
+      historyService.bootstrap();
+      // @ts-expect-error - ignore
+      expect(mockStrapi.documents.use).toHaveBeenCalledTimes(1);
+    });

-  it('saves relevant document actions in history', async () => {
-    const context = {
-      action: 'create',
-      contentType: {
-        uid: 'api::article.article',
-      },
-      args: [
-        {
-          locale: 'fr',
+    it('saves relevant document actions in history', async () => {
+      const context = {
+        action: 'create',
+        contentType: {
+          uid: 'api::article.article',
        },
-      ],
-    };
+        args: [
+          {
+            locale: 'fr',
+          },
+        ],
+      };

-    const next = jest.fn((context) => ({ ...context, documentId: 'document-id' }));
-    await historyService.bootstrap();
-    // @ts-expect-error - ignore
-    const historyMiddlewareFunction = mockStrapi.documents.use.mock.calls[0][0];
+      const next = jest.fn((context) => ({ ...context, documentId: 'document-id' }));
+      await historyService.bootstrap();
+      // @ts-expect-error - ignore
+      const historyMiddlewareFunction = mockStrapi.documents.use.mock.calls[0][0];

-    // Check that we don't break the middleware chain
-    await historyMiddlewareFunction(context, next);
-    expect(next).toHaveBeenCalledWith(context);
+      // Check that we don't break the middleware chain
+      await historyMiddlewareFunction(context, next);
+      expect(next).toHaveBeenCalledWith(context);

-    // Create and update actions should be saved in history
-    expect(createMock).toHaveBeenCalled();
-    context.action = 'update';
-    await historyMiddlewareFunction(context, next);
-    expect(createMock).toHaveBeenCalledTimes(2);
+      // Create and update actions should be saved in history
+      expect(createMock).toHaveBeenCalled();
+      context.action = 'update';
+      await historyMiddlewareFunction(context, next);
+      expect(createMock).toHaveBeenCalledTimes(2);

-    // Publish and unpublish actions should be saved in history
-    createMock.mockClear();
-    await historyMiddlewareFunction(context, next);
-    context.action = 'unpublish';
-    await historyMiddlewareFunction(context, next);
-    expect(createMock).toHaveBeenCalledTimes(2);
+      // Publish and unpublish actions should be saved in history
+      createMock.mockClear();
+      await historyMiddlewareFunction(context, next);
+      context.action = 'unpublish';
+      await historyMiddlewareFunction(context, next);
+      expect(createMock).toHaveBeenCalledTimes(2);

-    // Other actions should be ignored
-    createMock.mockClear();
-    context.action = 'findOne';
-    await historyMiddlewareFunction(context, next);
-    context.action = 'delete';
-    await historyMiddlewareFunction(context, next);
-    expect(createMock).toHaveBeenCalledTimes(0);
+      // Other actions should be ignored
+      createMock.mockClear();
+      context.action = 'findOne';
+      await historyMiddlewareFunction(context, next);
+      context.action = 'delete';
+      await historyMiddlewareFunction(context, next);
+      expect(createMock).toHaveBeenCalledTimes(0);

-    // Non-api content types should be ignored
-    createMock.mockClear();
-    context.contentType.uid = 'plugin::upload.file';
-    context.action = 'create';
-    await historyMiddlewareFunction(context, next);
-    expect(createMock).toHaveBeenCalledTimes(0);
+      // Non-api content types should be ignored
+      createMock.mockClear();
+      context.contentType.uid = 'plugin::upload.file';
+      context.action = 'create';
+      await historyMiddlewareFunction(context, next);
+      expect(createMock).toHaveBeenCalledTimes(0);

-    // Don't break middleware chain even if we don't save the action in history
-    next.mockClear();
-    await historyMiddlewareFunction(context, next);
-    expect(next).toHaveBeenCalledWith(context);
-  });
+      // Don't break middleware chain even if we don't save the action in history
+      next.mockClear();
+      await historyMiddlewareFunction(context, next);
+      expect(next).toHaveBeenCalledWith(context);
+    });

-  it('creates a history version with the author', async () => {
-    jest.useFakeTimers().setSystemTime(fakeDate);
+    it('should create a cron job that runs once a day', async () => {
+      // @ts-expect-error - this is a mock
+      const mockScheduleJob = scheduleJob.mockImplementationOnce(
+        jest.fn((rule, callback) => callback())
+      );

-    const historyVersionData = {
-      contentType: 'api::article.article' as UID.ContentType,
-      data: {
-        title: 'My article',
-      },
-      locale: 'en',
-      relatedDocumentId: 'randomid',
-      schema: {
-        title: {
-          type: 'string',
-        },
-      },
-      status: 'draft' as const,
-    };
+      await historyService.bootstrap();

-    await historyService.createVersion(historyVersionData);
-    expect(createMock).toHaveBeenCalledWith({
-      data: {
-        ...historyVersionData,
-        createdBy: userId,
-        createdAt: fakeDate,
-      },
+      expect(mockScheduleJob).toHaveBeenCalledTimes(1);
+      expect(mockScheduleJob).toHaveBeenCalledWith('0 0 * * *', expect.any(Function));
    });
  });

-  it('creates a history version without any author', async () => {
-    jest.useFakeTimers().setSystemTime(fakeDate);
+  describe('createVersion', () => {
+    it('creates a history version with the author', async () => {
+      jest.useFakeTimers().setSystemTime(fakeDate);

-    const historyVersionData = {
-      contentType: 'api::article.article' as UID.ContentType,
-      data: {
-        title: 'My article',
-      },
-      locale: 'en',
-      relatedDocumentId: 'randomid',
-      schema: {
-        title: {
-          type: 'string',
+      const historyVersionData = {
+        contentType: 'api::article.article' as UID.ContentType,
+        data: {
+          title: 'My article',
        },
-      },
-      status: null,
-    };
+        locale: 'en',
+        relatedDocumentId: 'randomid',
+        schema: {
+          title: {
+            type: 'string',
+          },
+        },
+        status: 'draft' as const,
+      };

-    mockGetRequestContext.mockReturnValueOnce(null as any);
-
-    await historyService.createVersion(historyVersionData);
-    expect(createMock).toHaveBeenCalledWith({
-      data: {
-        ...historyVersionData,
-        createdBy: undefined,
-        createdAt: fakeDate,
-      },
+      await historyService.createVersion(historyVersionData);
+      expect(createMock).toHaveBeenCalledWith({
+        data: {
+          ...historyVersionData,
+          createdBy: userId,
+          createdAt: fakeDate,
+        },
+      });
    });
-  });

-  it('should create a cron job that runs once a day', async () => {
-    // @ts-expect-error - this is a mock
-    const mockScheduleJob = scheduleJob.mockImplementationOnce(
-      jest.fn((rule, callback) => callback())
-    );
+    it('creates a history version without any author', async () => {
+      jest.useFakeTimers().setSystemTime(fakeDate);

-    await historyService.bootstrap();
+      const historyVersionData = {
+        contentType: 'api::article.article' as UID.ContentType,
+        data: {
+          title: 'My article',
+        },
+        locale: 'en',
+        relatedDocumentId: 'randomid',
+        schema: {
+          title: {
+            type: 'string',
+          },
+        },
+        status: null,
+      };

-    expect(mockScheduleJob).toHaveBeenCalledTimes(1);
-    expect(mockScheduleJob).toHaveBeenCalledWith('0 0 * * *', expect.any(Function));
+      mockGetRequestContext.mockReturnValueOnce(null as any);
+
+      await historyService.createVersion(historyVersionData);
+      expect(createMock).toHaveBeenCalledWith({
+        data: {
+          ...historyVersionData,
+          createdBy: undefined,
+          createdAt: fakeDate,
+        },
+      });
+    });
  });
 });
--- a/packages/core/content-manager/server/src/history/services/tests/utils.test.ts
+++ b/packages/core/content-manager/server/src/history/services/tests/utils.test.ts
@ -0,0 +1,63 @@
+import { getSchemaAttributesDiff } from '../utils';
+
+describe('history-version service utils', () => {
+  describe('getSchemaAttributesDiff', () => {
+    it('should return a diff', () => {
+      const versionSchema = {
+        title: {
+          type: 'string',
+        },
+        someOtherField: {
+          type: 'string',
+        },
+      };
+      const contentTypeSchema = {
+        renamed: {
+          type: 'string',
+        },
+        newField: {
+          type: 'string',
+        },
+        someOtherField: {
+          type: 'string',
+        },
+      };
+
+      // @ts-expect-error ignore
+      const { added, removed } = getSchemaAttributesDiff(versionSchema, contentTypeSchema);
+
+      expect(added).toEqual({
+        renamed: {
+          type: 'string',
+        },
+        newField: {
+          type: 'string',
+        },
+      });
+      expect(removed).toEqual({
+        title: {
+          type: 'string',
+        },
+      });
+    });
+
+    it('should not return a diff', () => {
+      const versionSchema = {
+        title: {
+          type: 'string',
+        },
+      };
+      const contentTypeSchema = {
+        title: {
+          type: 'string',
+        },
+      };
+
+      // @ts-expect-error ignore
+      const { added, removed } = getSchemaAttributesDiff(versionSchema, contentTypeSchema);
+
+      expect(added).toEqual({});
+      expect(removed).toEqual({});
+    });
+  });
+});
--- a/packages/core/content-manager/server/src/history/services/history.ts
+++ b/packages/core/content-manager/server/src/history/services/history.ts
@ -3,8 +3,9 @@ import { omit, pick } from 'lodash/fp';

 import { scheduleJob } from 'node-schedule';

-import { HISTORY_VERSION_UID } from '../constants';
+import { FIELDS_TO_IGNORE, HISTORY_VERSION_UID } from '../constants';
 import type { HistoryVersions } from '../../../../shared/contracts';
+import { getSchemaAttributesDiff } from './utils';

 const DEFAULT_RETENTION_DAYS = 90;

@ -104,24 +105,13 @@ const createHistoryService = ({ strapi }: { strapi: Core.LoadedStrapi }) => {
          });
        const status = await getVersionStatus(contentTypeUid, document);

-        const fieldsToIgnore = [
-          'createdAt',
-          'updatedAt',
-          'publishedAt',
-          'createdBy',
-          'updatedBy',
-          'locale',
-          'strapi_stage',
-          'strapi_assignee',
-        ];
-
        // Prevent creating a history version for an action that wasn't actually executed
        await strapi.db.transaction(async ({ onCommit }) => {
          onCommit(() => {
            this.createVersion({
              contentType: contentTypeUid,
-              data: omit(fieldsToIgnore, document),
-              schema: omit(fieldsToIgnore, strapi.contentType(contentTypeUid).attributes),
+              data: omit(FIELDS_TO_IGNORE, document),
+              schema: omit(FIELDS_TO_IGNORE, strapi.contentType(contentTypeUid).attributes),
              relatedDocumentId: documentContext.documentId,
              locale,
              status,
@ -183,7 +173,20 @@ const createHistoryService = ({ strapi }: { strapi: Core.LoadedStrapi }) => {
        getLocaleDictionary(),
      ]);

-      const sanitizedResults = results.map((result) => ({
+      const versionsWithMeta = results.map((version) => {
+        const { added, removed } = getSchemaAttributesDiff(
+          version.schema,
+          strapi.getModel(params.contentType).attributes
+        );
+        const hasSchemaDiff = Object.keys(added).length > 0 || Object.keys(removed).length > 0;
+
+        return {
+          ...version,
+          ...(hasSchemaDiff ? { meta: { unknownAttributes: { added, removed } } } : {}),
+        };
+      });
+
+      const sanitizedResults = versionsWithMeta.map((result) => ({
        ...result,
        locale: result.locale ? localeDictionary[result.locale] : null,
        createdBy: result.createdBy
--- a/packages/core/content-manager/server/src/history/services/utils.ts
+++ b/packages/core/content-manager/server/src/history/services/utils.ts
@ -0,0 +1,44 @@
+import { difference, omit } from 'lodash/fp';
+import { Schema } from '@strapi/types';
+import { CreateHistoryVersion } from '../../../../shared/contracts/history-versions';
+import { FIELDS_TO_IGNORE } from '../constants';
+
+/**
+ * Get the difference between the version schema and the content type schema
+ * Returns the attributes with their original shape
+ */
+export const getSchemaAttributesDiff = (
+  versionSchemaAttributes: CreateHistoryVersion['schema'],
+  contentTypeSchemaAttributes: Schema.Attributes
+) => {
+  // Omit the same fields that were omitted when creating a history version
+  const sanitizedContentTypeSchemaAttributes = omit(
+    FIELDS_TO_IGNORE,
+    contentTypeSchemaAttributes
+  ) as CreateHistoryVersion['schema'];
+
+  const reduceDifferenceToAttributesObject = (
+    diffKeys: string[],
+    source: CreateHistoryVersion['schema']
+  ) => {
+    return diffKeys.reduce<CreateHistoryVersion['schema']>((previousAttributesObject, diffKey) => {
+      previousAttributesObject[diffKey] = source[diffKey];
+
+      return previousAttributesObject;
+    }, {});
+  };
+
+  const versionSchemaKeys = Object.keys(versionSchemaAttributes);
+  const contentTypeSchemaAttributesKeys = Object.keys(sanitizedContentTypeSchemaAttributes);
+  // The attribute is new if it's on the content type schema but not on the version schema
+  const uniqueToContentType = difference(contentTypeSchemaAttributesKeys, versionSchemaKeys);
+  const added = reduceDifferenceToAttributesObject(
+    uniqueToContentType,
+    sanitizedContentTypeSchemaAttributes
+  );
+  // The attribute was removed or renamed if it's on the version schema but not on the content type schema
+  const uniqueToVersion = difference(versionSchemaKeys, contentTypeSchemaAttributesKeys);
+  const removed = reduceDifferenceToAttributesObject(uniqueToVersion, versionSchemaAttributes);
+
+  return { added, removed };
+};
--- a/packages/core/content-manager/shared/contracts/history-versions.ts
+++ b/packages/core/content-manager/shared/contracts/history-versions.ts
@ -31,6 +31,12 @@ export interface HistoryVersionDataResponse extends Omit<CreateHistoryVersion, '
    email: string;
  };
  locale: Locale | null;
+  meta?: {
+    unknownAttributes?: {
+      added: Record<string, unknown>;
+      removed: Record<string, unknown>;
+    };
+  };
 }

 // Export to prevent the TS "cannot be named" error in the history service