From 1a82e92ae396a16a76b9292eaa7b37409870b827 Mon Sep 17 00:00:00 2001 From: Junyi Date: Mon, 28 Aug 2023 21:33:53 +0700 Subject: [PATCH] fix(plugin-workflow): fix destroy node bug (#2553) --- .../server/__tests__/actions/nodes.test.ts | 84 +++++++++++++++++++ .../workflow/src/server/actions/index.ts | 2 +- .../workflow/src/server/actions/nodes.ts | 34 ++++---- 3 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 packages/plugins/workflow/src/server/__tests__/actions/nodes.test.ts diff --git a/packages/plugins/workflow/src/server/__tests__/actions/nodes.test.ts b/packages/plugins/workflow/src/server/__tests__/actions/nodes.test.ts new file mode 100644 index 000000000..c7224a6aa --- /dev/null +++ b/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); + }); + }); +}); diff --git a/packages/plugins/workflow/src/server/actions/index.ts b/packages/plugins/workflow/src/server/actions/index.ts index df3c6b3f8..3f1fb5da3 100644 --- a/packages/plugins/workflow/src/server/actions/index.ts +++ b/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), }); diff --git a/packages/plugins/workflow/src/server/actions/nodes.ts b/packages/plugins/workflow/src/server/actions/nodes.ts index d9a590a15..82bb32122 100644 --- a/packages/plugins/workflow/src/server/actions/nodes.ts +++ b/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(); }