fix(plugin-workflow): remove previous listeners when collection changed in config (#409)

packages/plugins/workflow/src/__tests__/triggers/collection.test.ts

@@ -0,0 +1,48 @@
+import { Application } from '@nocobase/server';
+import Database from '@nocobase/database';
+import { getApp, sleep } from '..';
+import { EXECUTION_STATUS } from '../../constants';
+
+
+
+describe('workflow > triggers > collection', () => {
+  let app: Application;
+  let db: Database;
+  let PostRepo;
+  let WorkflowModel;
+
+  beforeEach(async () => {
+    app = await getApp();
+
+    db = app.db;
+    WorkflowModel = db.getCollection('workflows').model;
+    PostRepo = db.getCollection('posts').repository;
+  });
+
+  afterEach(() => db.close());
+
+  describe('toggle', () => {
+    it('when collection change', async () => {
+      const workflow = await WorkflowModel.create({
+        enabled: true,
+        type: 'collection',
+        config: {
+          mode: 1,
+          collection: 'posts'
+        }
+      });
+
+      await workflow.update({
+        config: {
+          ...workflow.config,
+          collection: 'comments'
+        }
+      });
+
+      const post = await PostRepo.create({ values: { title: 't1' } });
+
+      const executions = await workflow.getExecutions();
+      expect(executions.length).toBe(0);
+    });
+  });
+});

packages/plugins/workflow/src/models/Workflow.ts

@@ -1,5 +1,5 @@
 import { Database, Model } from '@nocobase/database';
-import { HasManyCountAssociationsMixin, HasManyCreateAssociationMixin, HasManyGetAssociationsMixin } from 'sequelize';
+import { HasManyCountAssociationsMixin, HasManyCreateAssociationMixin, HasManyGetAssociationsMixin, Transactionable } from 'sequelize';
 
 import { EXECUTION_STATUS } from '../constants';
 import ExecutionModel from './Execution';
@@ -31,7 +31,7 @@ export default class WorkflowModel extends Model {
 
   getTransaction(options) {
     if (!this.useTransaction) {
-      return undefined;
+      return null;
     }
 
     return options.transaction && !options.transaction.finished
@@ -39,7 +39,7 @@ export default class WorkflowModel extends Model {
       : (<typeof WorkflowModel>this.constructor).database.sequelize.transaction();
   }
 
-  trigger = async (context: Object, options) => {
+  trigger = async (context: Object, options = {}) => {
     // `null` means not to trigger
     if (context === null) {
       return;
@@ -73,9 +73,11 @@ export default class WorkflowModel extends Model {
     await execution.start({ transaction });
 
     if (!this.executed) {
-      await this.update({ executed: true }, { transaction });
+      // NOTE: not to trigger afterUpdate hook here
+      await this.update({ executed: true }, { transaction, hooks: false });
     }
 
+    // @ts-ignore
     if (transaction && (!options.transaction || options.transaction.finished)) {
       await transaction.commit();
     }

packages/plugins/workflow/src/triggers/collection.ts

@@ -23,7 +23,7 @@ MODE_BITMAP_EVENTS.set(MODE_BITMAP.DESTROY, 'afterDestroy');
 
 
 function getHookId(workflow, type) {
-  return `${type}#${workflow.get('id')}`;
+  return `${type}#${workflow.id}`;
 }
 
 // async function, should return promise
@@ -43,12 +43,23 @@ function handler(this: WorkflowModel, data: Model, options) {
 }
 
 export default class CollectionTrigger implements Trigger {
+  db;
+
   events = new Map();
 
+  constructor({ app }) {
+    this.db = app.db;
+  }
+
   on(workflow: WorkflowModel) {
-    const { database } = <typeof WorkflowModel>workflow.constructor;
+    // NOTE: remove previous listener if config updated
+    const prev = workflow.previous();
+    if (prev.config) {
+      this.off({ ...workflow.get(), ...prev });
+    }
+
     const { collection, mode } = workflow.config;
-    const Collection = database.getCollection(collection);
+    const Collection = this.db.getCollection(collection);
     if (!Collection) {
       return;
     }
@@ -60,12 +71,12 @@ export default class CollectionTrigger implements Trigger {
         if (!this.events.has(name)) {
           const listener = handler.bind(workflow);
           this.events.set(name, listener);
-          database.on(event, listener);
+          this.db.on(event, listener);
         }
       } else {
         const listener = this.events.get(name);
         if (listener) {
-          database.off(event, listener);
+          this.db.off(event, listener);
           this.events.delete(name);
         }
       }
@@ -73,9 +84,8 @@ export default class CollectionTrigger implements Trigger {
   }
 
   off(workflow: WorkflowModel) {
-    const { database } = <typeof WorkflowModel>workflow.constructor;
     const { collection, mode } = workflow.config;
-    const Collection = database.getCollection(collection);
+    const Collection = this.db.getCollection(collection);
     if (!Collection) {
       return;
     }
@@ -85,7 +95,7 @@ export default class CollectionTrigger implements Trigger {
       if (mode & key) {
         const listener = this.events.get(name);
         if (listener) {
-          database.off(event, listener);
+          this.db.off(event, listener);
           this.events.delete(name);
         }
       }

packages/plugins/workflow/src/triggers/index.ts

@@ -7,7 +7,8 @@ export interface Trigger {
   off(workflow: WorkflowModel): void;
 }
 
-export default function({ triggers }) {
-  triggers.register('collection', new Collection());
-  // triggers.register('schedule', new Schedule());
+export default function(plugin) {
+  const { triggers } = plugin;
+  triggers.register('collection', new Collection(plugin));
+  // triggers.register('schedule', new Schedule(plugin));
 }