fix(plugin-workflow): fix destroy node bug (#2553)

packages/plugins/workflow/src/server/__tests__/actions/nodes.test.ts

@@ -0,0 +1,84 @@
+import { MockServer } from '@nocobase/test';
+import Database from '@nocobase/database';
+import { getApp, sleep } from '..';
+
+describe('workflow > actions > workflows', () => {
+  let app: MockServer;
+  let agent;
+  let db: Database;
+  let PostModel;
+  let PostRepo;
+  let WorkflowModel;
+
+  beforeEach(async () => {
+    app = await getApp();
+    agent = app.agent();
+    db = app.db;
+    WorkflowModel = db.getCollection('workflows').model;
+    PostModel = db.getCollection('posts').model;
+    PostRepo = db.getCollection('posts').repository;
+  });
+
+  afterEach(() => app.destroy());
+
+  describe('destroy', () => {
+    it('node in executed workflow could not be destroyed', async () => {
+      const workflow = await WorkflowModel.create({
+        enabled: true,
+        type: 'collection',
+        config: {
+          mode: 1,
+          collection: 'posts',
+        },
+      });
+
+      const n1 = await workflow.createNode({
+        type: 'echo',
+      });
+
+      await PostRepo.create({});
+
+      await sleep(500);
+
+      const { status } = await agent.resource('flow_nodes').destroy({
+        filterByTk: n1.id,
+      });
+
+      expect(status).toBe(400);
+    });
+
+    it('cascading destroy all nodes in sub-branches', async () => {
+      const workflow = await WorkflowModel.create({
+        enabled: true,
+        type: 'collection',
+        config: {
+          mode: 1,
+          collection: 'posts',
+        },
+      });
+
+      const n1 = await workflow.createNode({
+        type: 'echo',
+      });
+
+      const n2 = await workflow.createNode({
+        type: 'echo',
+        branchIndex: 0,
+        upstreamId: n1.id,
+      });
+
+      const n3 = await workflow.createNode({
+        type: 'echo',
+        branchIndex: 0,
+        upstreamId: n2.id,
+      });
+
+      await agent.resource('flow_nodes').destroy({
+        filterByTk: n1.id,
+      });
+
+      const nodes = await workflow.getNodes();
+      expect(nodes.length).toBe(0);
+    });
+  });
+});

packages/plugins/workflow/src/server/actions/index.ts

@@ -17,10 +17,10 @@ export default function ({ app }) {
     ...make('workflows', workflows),
     ...make('workflows.nodes', {
       create: nodes.create,
-      destroy: nodes.destroy,
     }),
     ...make('flow_nodes', {
       update: nodes.update,
+      destroy: nodes.destroy,
     }),
     ...make('executions', executions),
   });

packages/plugins/workflow/src/server/actions/nodes.ts

@@ -1,5 +1,5 @@
 import { Context, utils } from '@nocobase/actions';
-import { MultipleRelationRepository, Op } from '@nocobase/database';
+import { MultipleRelationRepository, Op, Repository } from '@nocobase/database';
 import type { WorkflowModel } from '../types';
 
 export async function create(context: Context, next) {
@@ -109,22 +109,20 @@ function searchBranchDownstreams(nodes, from) {
 
 export async function destroy(context: Context, next) {
   const { db } = context;
-  const repository = utils.getRepositoryFromParams(context) as MultipleRelationRepository;
+  const repository = utils.getRepositoryFromParams(context) as Repository;
   const { filterByTk } = context.action.params;
 
-  context.body = await db.sequelize.transaction(async (transaction) => {
-    const workflow = (await repository.getSourceModel(transaction)) as WorkflowModel;
-    if (workflow.executed) {
-      context.throw(400, 'Nodes in executed workflow could not be deleted');
-    }
+  const fields = ['id', 'upstreamId', 'downstreamId', 'branchIndex'];
+  const instance = await repository.findOne({
+    filterByTk,
+    fields: [...fields, 'workflowId'],
+    appends: ['upstream', 'downstream', 'workflow'],
+  });
+  if (instance.workflow.executed) {
+    context.throw(400, 'Nodes in executed workflow could not be deleted');
+  }
 
-    const fields = ['id', 'upstreamId', 'downstreamId', 'branchIndex'];
-    const instance = await repository.findOne({
-      filterByTk,
-      fields: [...fields, 'workflowId'],
-      appends: ['upstream', 'downstream'],
-      transaction,
-    });
+  await db.sequelize.transaction(async (transaction) => {
     const { upstream, downstream } = instance.get();
 
     if (upstream && upstream.downstreamId === instance.id) {
@@ -159,7 +157,7 @@ export async function destroy(context: Context, next) {
       nodesMap.set(item.id, item);
     });
     // overwrite
-    nodesMap.set(instance.id, instance);
+    // nodesMap.set(instance.id, instance);
     // make linked list
     nodes.forEach((item) => {
       if (item.upstreamId) {
@@ -170,16 +168,16 @@ export async function destroy(context: Context, next) {
       }
     });
 
-    const branchNodes = searchBranchNodes(nodes, instance);
+    const branchNodes = searchBranchNodes(nodes, nodesMap.get(instance.id));
 
     await repository.destroy({
       filterByTk: [instance.id, ...branchNodes.map((item) => item.id)],
       transaction,
     });
-
-    return instance;
   });
 
+  context.body = instance;
+
   await next();
 }