fix: cloud sync error with customization applied to missing collectio…

…ns (#901) * fix: cloud sync error with customization applied to missing collections * test: fix unit tests * test: fix model customization test * refactor: factorize out findCollection --------- Co-authored-by: Nicolas Moreau <nicolas.moreau76@gmail.com>
ForestAdmin · Dec 21, 2023 · 60e8d1c · 60e8d1c
1 parent d5d8d21
commit 60e8d1c
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 16 deletions.
diff --git a/...ges/agent/src/services/model-customizations/actions/update-record/update-record-plugin.ts b/...ges/agent/src/services/model-customizations/actions/update-record/update-record-plugin.ts
@@ -22,7 +22,8 @@ export default class UpdateRecordActionsPlugin {
     );
 
     actions.forEach(action => {
-      const collection = datasourceCustomizer.getCollection(action.modelName);
+      const collection = datasourceCustomizer.findCollection(action.modelName);
+      if (!collection) return;
 
       collection.addAction(action.name, {
         scope: action.configuration.scope,

diff --git a/packages/agent/src/services/model-customizations/actions/webhook/webhook-plugin.ts b/packages/agent/src/services/model-customizations/actions/webhook/webhook-plugin.ts
@@ -15,7 +15,8 @@ export default class WebhookActionsPlugin {
   ) => {
     const actions = getActions<WebhookActionConfiguration>('webhook', modelCustomizations);
     actions.forEach(action => {
-      const collection = datasourceCustomizer.getCollection(action.modelName);
+      const collection = datasourceCustomizer.findCollection(action.modelName);
+      if (!collection) return;
 
       collection.addAction(action.name, {
         scope: action.configuration.scope,

diff --git a/...ent/test/services/model-customizations/actions/update-record/update-record-plugin.test.ts b/...ent/test/services/model-customizations/actions/update-record/update-record-plugin.test.ts
@@ -39,11 +39,14 @@ describe('Services > ModelCustomizations > Actions > UpdateRecordActionsPlugin',
           },
         };
 
-        const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
         const collection = {
           addAction: jest.fn(),
+          name: 'myModel',
         };
-        (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+        const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
+        jest
+          .spyOn(customizer, 'findCollection')
+          .mockReturnValue(collection as unknown as CollectionCustomizer);
 
         UpdateRecordActionsPlugin.addUpdateRecordActions(
           customizer,
@@ -55,9 +58,39 @@ describe('Services > ModelCustomizations > Actions > UpdateRecordActionsPlugin',
           scope,
           execute: expect.any(Function),
         });
-        expect(customizer.getCollection).toHaveBeenCalledWith('myModel');
+        expect(customizer.findCollection).toHaveBeenCalledWith('myModel');
       },
     );
+    it('should do nothing if the collection is missing', async () => {
+      const action = {
+        name: 'myAction',
+        type: ModelCustomizationType.action,
+        modelName: 'myOtherModel',
+        configuration: {
+          type: 'update-record',
+          scope: 'Global',
+          configuration: { fields: [{ fieldName: 'field', value: 'value' }] },
+        },
+      };
+
+      const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
+      const collection = {
+        addAction: jest.fn(),
+        name: 'myModel',
+      };
+      jest
+        .spyOn(customizer, 'findCollection')
+        .mockReturnValue(undefined as unknown as CollectionCustomizer);
+
+      UpdateRecordActionsPlugin.addUpdateRecordActions(
+        customizer,
+        undefined as unknown as CollectionCustomizer,
+        [action],
+      );
+
+      expect(collection.addAction).not.toHaveBeenCalled();
+      expect(customizer.findCollection).toHaveBeenCalledWith('myOtherModel');
+    });
 
     it('should execute the action', async () => {
       const action = {
@@ -74,8 +107,11 @@ describe('Services > ModelCustomizations > Actions > UpdateRecordActionsPlugin',
       const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
       const collection = {
         addAction: jest.fn(),
+        name: 'myModel',
       };
-      (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+      jest
+        .spyOn(customizer, 'findCollection')
+        .mockReturnValue(collection as unknown as CollectionCustomizer);
 
       UpdateRecordActionsPlugin.addUpdateRecordActions(
         customizer,
@@ -91,15 +127,19 @@ describe('Services > ModelCustomizations > Actions > UpdateRecordActionsPlugin',
 
       await execute(context);
 
+      expect(customizer.findCollection).toHaveBeenCalledWith('myModel');
       expect(executeWebhookMock).toHaveBeenCalledWith(action, context);
     });
 
     it('should not use actions that are not update records', async () => {
       const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
       const collection = {
         addAction: jest.fn(),
+        name: 'myModel',
       };
-      (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+      jest
+        .spyOn(customizer, 'findCollection')
+        .mockReturnValue(collection as unknown as CollectionCustomizer);
 
       UpdateRecordActionsPlugin.addUpdateRecordActions(
         customizer,
@@ -119,6 +159,7 @@ describe('Services > ModelCustomizations > Actions > UpdateRecordActionsPlugin',
       );
 
       expect(collection.addAction).not.toHaveBeenCalled();
+      expect(customizer.findCollection).not.toHaveBeenCalled();
     });
   });
 });
diff --git a/packages/agent/test/services/model-customizations/actions/webhook/webhook-plugin.test.ts b/packages/agent/test/services/model-customizations/actions/webhook/webhook-plugin.test.ts
@@ -42,8 +42,12 @@ describe('Services > ModelCustomizations > Actions > WebhookActionsPlugin', () =
         const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
         const collection = {
           addAction: jest.fn(),
+          name: 'myModel',
         };
-        (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+        jest
+          .spyOn(customizer, 'findCollection')
+          .mockReturnValue(collection as unknown as CollectionCustomizer);
 
         WebhookActionsPlugin.addWebhookActions(
           customizer,
@@ -55,7 +59,6 @@ describe('Services > ModelCustomizations > Actions > WebhookActionsPlugin', () =
           scope,
           execute: expect.any(Function),
         });
-        expect(customizer.getCollection).toHaveBeenCalledWith('myModel');
       },
     );
 
@@ -74,8 +77,12 @@ describe('Services > ModelCustomizations > Actions > WebhookActionsPlugin', () =
       const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
       const collection = {
         addAction: jest.fn(),
+        name: 'myModel',
       };
-      (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+      jest
+        .spyOn(customizer, 'findCollection')
+        .mockReturnValue(collection as unknown as CollectionCustomizer);
 
       WebhookActionsPlugin.addWebhookActions(
         customizer,
@@ -91,13 +98,44 @@ describe('Services > ModelCustomizations > Actions > WebhookActionsPlugin', () =
 
       expect(executeWebhookMock).toHaveBeenCalledWith(action, context);
     });
+    it('should do nothing if the collection is missing', async () => {
+      const action = {
+        name: 'myAction',
+        type: ModelCustomizationType.action,
+        modelName: 'myOtherModel',
+        configuration: {
+          type: 'webhook',
+          scope: 'Global',
+          configuration: { fields: [{ fieldName: 'field', value: 'value' }] },
+        },
+      };
+
+      const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
+      const collection = {
+        addAction: jest.fn(),
+        name: 'myModel',
+      };
+      jest.spyOn(customizer, 'findCollection').mockReturnValue(undefined);
+
+      WebhookActionsPlugin.addWebhookActions(
+        customizer,
+        undefined as unknown as CollectionCustomizer,
+        [action],
+      );
+
+      expect(collection.addAction).not.toHaveBeenCalled();
+    });
 
     it('should not use customizations that are not actions', async () => {
       const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
       const collection = {
         addAction: jest.fn(),
+        name: 'myModel',
       };
-      (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+      jest
+        .spyOn(customizer, 'findCollection')
+        .mockReturnValue(collection as unknown as CollectionCustomizer);
 
       WebhookActionsPlugin.addWebhookActions(
         customizer,
@@ -123,8 +161,12 @@ describe('Services > ModelCustomizations > Actions > WebhookActionsPlugin', () =
       const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
       const collection = {
         addAction: jest.fn(),
+        name: 'myModel',
       };
-      (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+      jest
+        .spyOn(customizer, 'findCollection')
+        .mockReturnValue(collection as unknown as CollectionCustomizer);
 
       WebhookActionsPlugin.addWebhookActions(
         customizer,

diff --git a/packages/agent/test/services/model-customizations/customization.test.ts b/packages/agent/test/services/model-customizations/customization.test.ts
@@ -54,9 +54,13 @@ describe('Services > ModelCustomizations > CustomizationPluginService', () => {
 
         const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
         const collection = {
-          addAction: jest.fn().mockReturnThis(),
+          addAction: jest.fn(),
+          name: 'myModel',
         };
-        (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+        jest
+          .spyOn(customizer, 'findCollection')
+          .mockReturnValue(collection as unknown as CollectionCustomizer);
 
         await actionCustomization.addCustomizations(
           customizer,
@@ -84,8 +88,12 @@ describe('Services > ModelCustomizations > CustomizationPluginService', () => {
         const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
         const collection = {
           addAction: jest.fn(),
+          name: 'myModel',
         };
-        (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+        jest
+          .spyOn(customizer, 'findCollection')
+          .mockReturnValue(collection as unknown as CollectionCustomizer);
 
         await actionCustomization.addCustomizations(
           customizer,
@@ -118,8 +126,12 @@ describe('Services > ModelCustomizations > CustomizationPluginService', () => {
         const customizer = dataSourceCustomizerFactory.mockAllMethods().build();
         const collection = {
           addAction: jest.fn(),
+          name: 'myModel',
         };
-        (customizer.getCollection as jest.Mock).mockReturnValue(collection);
+
+        jest
+          .spyOn(customizer, 'findCollection')
+          .mockReturnValue(collection as unknown as CollectionCustomizer);
 
         await actionCustomization.addCustomizations(
           customizer,

diff --git a/packages/datasource-customizer/src/datasource-customizer.ts b/packages/datasource-customizer/src/datasource-customizer.ts
@@ -123,6 +123,18 @@ export default class DataSourceCustomizer<S extends TSchema = TSchema> {
     return new CollectionCustomizer<S, N>(this, this.stack, name);
   }
 
+  /**
+   * Find a collection by name. Returns undefined if the collection is missing
+   * @param name name of the collection
+   */
+  findCollection(name: string): CollectionCustomizer<S> | undefined {
+    if (this.collections.find(collection => collection.name === name))
+      /**
+       * If the collection is found, we use the getCollection to apply side effects
+       */
+      return this.getCollection(name as TCollectionName<S>);
+  }
+
   /**
    * Remove collections from the exported schema (they will still be usable within the agent).
    * @param names the collections to remove

diff --git a/packages/datasource-customizer/test/datasource-customizer.test.ts b/packages/datasource-customizer/test/datasource-customizer.test.ts
@@ -241,4 +241,37 @@ describe('DataSourceCustomizer', () => {
       expect(array).toEqual([1, 2, 3, 4, 5]);
     });
   });
+
+  describe('findCollection', () => {
+    it('should return the collection if found', async () => {
+      const customizer = new DataSourceCustomizer();
+      jest
+        .spyOn(customizer, 'collections', 'get')
+        .mockReturnValue([
+          { name: 'test' },
+          { name: 'otherCollection' },
+        ] as Array<CollectionCustomizer>);
+      jest
+        .spyOn(customizer, 'getCollection')
+        .mockReturnValue({ name: 'test' } as CollectionCustomizer);
+
+      const collection = await customizer.findCollection('test');
+      expect(collection).toMatchObject({ name: 'test' });
+      expect(customizer.getCollection).toHaveBeenCalledWith('test');
+    });
+    it('should return undefined if the collection is not found', async () => {
+      const customizer = new DataSourceCustomizer();
+      jest
+        .spyOn(customizer, 'collections', 'get')
+        .mockReturnValue([
+          { name: 'test' },
+          { name: 'otherCollection' },
+        ] as Array<CollectionCustomizer>);
+      jest.spyOn(customizer, 'getCollection');
+
+      const collection = await customizer.findCollection('somethingElse');
+      expect(collection).toBeUndefined();
+      expect(customizer.getCollection).not.toHaveBeenCalled();
+    });
+  });
 });