From 0fe5bdc859eac6e9567f26fd0af57615c3f0814f Mon Sep 17 00:00:00 2001 From: Junyi Date: Tue, 22 Aug 2023 19:13:52 +0700 Subject: [PATCH] fix(plugin-workflow): fix loop scope variable parsing (#2502) * fix(plugin-workflow): fix loop scope variable parsing * fix(plugin-workflow): fix api parameter * fix(plugin-workflow): fix test cases --- .../plugins/workflow/src/server/Processor.ts | 17 ++++--- .../src/server/__tests__/Processor.test.ts | 2 +- .../__tests__/instructions/aggregate.test.ts | 2 +- .../instructions/calculation.test.ts | 2 +- .../__tests__/instructions/condition.test.ts | 2 +- .../__tests__/instructions/create.test.ts | 2 +- .../__tests__/instructions/destroy.test.ts | 2 +- .../__tests__/instructions/manual.test.ts | 45 +++++-------------- .../__tests__/instructions/query.test.ts | 2 +- .../__tests__/instructions/update.test.ts | 2 +- .../__tests__/triggers/collection.test.ts | 2 +- .../src/server/instructions/aggregate.ts | 4 +- .../src/server/instructions/calculation.ts | 2 +- .../src/server/instructions/condition.ts | 4 +- .../src/server/instructions/create.ts | 2 +- .../src/server/instructions/destroy.ts | 2 +- .../workflow/src/server/instructions/loop.ts | 6 +-- .../src/server/instructions/manual/actions.ts | 16 ++++--- .../instructions/manual/forms/update.ts | 2 +- .../src/server/instructions/manual/index.ts | 8 ++-- .../workflow/src/server/instructions/query.ts | 2 +- .../src/server/instructions/request.ts | 2 +- .../workflow/src/server/instructions/sql.ts | 4 +- .../src/server/instructions/update.ts | 2 +- 24 files changed, 58 insertions(+), 78 deletions(-) diff --git a/packages/plugins/workflow/src/server/Processor.ts b/packages/plugins/workflow/src/server/Processor.ts index 8bcf86b73..b61db15b6 100644 --- a/packages/plugins/workflow/src/server/Processor.ts +++ b/packages/plugins/workflow/src/server/Processor.ts @@ -318,7 +318,8 @@ export default class Processor { return null; } - public getScope(node?) { + public getScope(sourceNodeId: number) { + const node = this.nodesMap.get(sourceNodeId); const systemFns = {}; const scope = { execution: this.execution, @@ -329,12 +330,10 @@ export default class Processor { } const $scopes = {}; - if (node) { - for (let n = this.findBranchParentNode(node); n; n = this.findBranchParentNode(n)) { - const instruction = this.options.plugin.instructions.get(n.type); - if (typeof instruction.getScope === 'function') { - $scopes[n.id] = instruction.getScope(n, this.jobsMapByNodeId[n.id], this); - } + for (let n = this.findBranchParentNode(node); n; n = this.findBranchParentNode(n)) { + const instruction = this.options.plugin.instructions.get(n.type); + if (typeof instruction.getScope === 'function') { + $scopes[n.id] = instruction.getScope(n, this.jobsMapByNodeId[n.id], this); } } @@ -346,9 +345,9 @@ export default class Processor { }; } - public getParsedValue(value, node?, additionalScope?: object) { + public getParsedValue(value, sourceNodeId: number, additionalScope?: object) { const template = parse(value); - const scope = Object.assign(this.getScope(node), additionalScope); + const scope = Object.assign(this.getScope(sourceNodeId), additionalScope); template.parameters.forEach(({ key }) => { appendArrayColumn(scope, key); }); diff --git a/packages/plugins/workflow/src/server/__tests__/Processor.test.ts b/packages/plugins/workflow/src/server/__tests__/Processor.test.ts index e9c6724b3..f6573c5b3 100644 --- a/packages/plugins/workflow/src/server/__tests__/Processor.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/Processor.test.ts @@ -29,7 +29,7 @@ describe('workflow > Processor', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('base', () => { it('empty workflow without any nodes', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/aggregate.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/aggregate.test.ts index aba57d6fa..a83a9b153 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/aggregate.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/aggregate.test.ts @@ -31,7 +31,7 @@ describe('workflow > instructions > aggregate', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('based on collection', () => { it('count', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/calculation.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/calculation.test.ts index 47b2943c3..5561c0c5a 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/calculation.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/calculation.test.ts @@ -30,7 +30,7 @@ describe('workflow > instructions > calculation', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('math.js', () => { it('syntax error', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/condition.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/condition.test.ts index 6287865ce..3db466963 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/condition.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/condition.test.ts @@ -28,7 +28,7 @@ describe('workflow > instructions > condition', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('config.rejectOnFalse', () => {}); diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/create.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/create.test.ts index 53f6b4e1b..53274acb8 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/create.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/create.test.ts @@ -29,7 +29,7 @@ describe('workflow > instructions > create', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('create one', () => { it('params: from context', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/destroy.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/destroy.test.ts index 5a32d3df7..d51067c96 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/destroy.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/destroy.test.ts @@ -27,7 +27,7 @@ describe('workflow > instructions > destroy', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('destroy one', () => { it('params: from context', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/manual.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/manual.test.ts index af0c01ee4..50105c1a2 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/manual.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/manual.test.ts @@ -49,7 +49,7 @@ describe('workflow > instructions > manual', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('actions configuration', () => { it('no action configured', async () => { @@ -58,8 +58,7 @@ describe('workflow > instructions > manual', () => { config: { assignees: [users[0].id], forms: { - f1: { - }, + f1: {}, }, }, }); @@ -96,9 +95,7 @@ describe('workflow > instructions > manual', () => { assignees: [users[0].id], forms: { f1: { - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], }, }, }, @@ -346,9 +343,7 @@ describe('workflow > instructions > manual', () => { assignees: [users[0].id], forms: { f1: { - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], }, }, }, @@ -418,9 +413,7 @@ describe('workflow > instructions > manual', () => { assignees: [users[0].id, users[1].id], forms: { f1: { - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ] + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], }, }, }, @@ -467,9 +460,7 @@ describe('workflow > instructions > manual', () => { assignees: [users[0].id], forms: { f1: { - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], }, }, }, @@ -512,9 +503,7 @@ describe('workflow > instructions > manual', () => { mode: 1, forms: { f1: { - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], }, }, }, @@ -575,9 +564,7 @@ describe('workflow > instructions > manual', () => { mode: 1, forms: { f1: { - actions: [ - { status: JOB_STATUS.REJECTED, key: 'reject' }, - ], + actions: [{ status: JOB_STATUS.REJECTED, key: 'reject' }], }, }, }, @@ -808,9 +795,7 @@ describe('workflow > instructions > manual', () => { mode: -1, forms: { f1: { - actions: [ - { status: JOB_STATUS.REJECTED, key: 'reject' }, - ], + actions: [{ status: JOB_STATUS.REJECTED, key: 'reject' }], }, }, }, @@ -872,9 +857,7 @@ describe('workflow > instructions > manual', () => { assignees: [users[0].id, users[1].id], forms: { f1: { - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], }, }, }, @@ -1013,9 +996,7 @@ describe('workflow > instructions > manual', () => { forms: { f1: { type: 'create', - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], collection: 'comments', }, }, @@ -1129,9 +1110,7 @@ describe('workflow > instructions > manual', () => { forms: { f1: { type: 'update', - actions: [ - { status: JOB_STATUS.RESOLVED, key: 'resolve' }, - ], + actions: [{ status: JOB_STATUS.RESOLVED, key: 'resolve' }], collection: 'posts', }, }, diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/query.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/query.test.ts index e7ebf1a37..72b3ef4f3 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/query.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/query.test.ts @@ -34,7 +34,7 @@ describe('workflow > instructions > query', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('query one', () => { it('params: empty', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/update.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/update.test.ts index e3f783f13..0ef52a0cb 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/update.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/update.test.ts @@ -27,7 +27,7 @@ describe('workflow > instructions > update', () => { }); }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('update one', () => { it('params: from context', async () => { diff --git a/packages/plugins/workflow/src/server/__tests__/triggers/collection.test.ts b/packages/plugins/workflow/src/server/__tests__/triggers/collection.test.ts index fc2f942cd..56299ebca 100644 --- a/packages/plugins/workflow/src/server/__tests__/triggers/collection.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/triggers/collection.test.ts @@ -23,7 +23,7 @@ describe('workflow > triggers > collection', () => { TagRepo = db.getCollection('tags').repository; }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); describe('toggle', () => { it('when collection change', async () => { diff --git a/packages/plugins/workflow/src/server/instructions/aggregate.ts b/packages/plugins/workflow/src/server/instructions/aggregate.ts index 7917891e5..42c8d6178 100644 --- a/packages/plugins/workflow/src/server/instructions/aggregate.ts +++ b/packages/plugins/workflow/src/server/instructions/aggregate.ts @@ -15,12 +15,12 @@ const aggregators = { export default { async run(node: FlowNodeModel, input, processor: Processor) { const { aggregator, associated, collection, association = {}, params = {} } = node.config; - const options = processor.getParsedValue(params); + const options = processor.getParsedValue(params, node.id); const { database } = node.constructor; const repo = associated ? database.getRepository( `${association?.associatedCollection}.${association.name}`, - processor.getParsedValue(association?.associatedKey), + processor.getParsedValue(association?.associatedKey, node.id), ) : database.getRepository(collection); diff --git a/packages/plugins/workflow/src/server/instructions/calculation.ts b/packages/plugins/workflow/src/server/instructions/calculation.ts index 8a5e1e169..476bbc2fb 100644 --- a/packages/plugins/workflow/src/server/instructions/calculation.ts +++ b/packages/plugins/workflow/src/server/instructions/calculation.ts @@ -15,7 +15,7 @@ export default { async run(node: FlowNodeModel, prevJob, processor: Processor) { const { dynamic = false } = node.config || {}; let { engine = 'math.js', expression = '' } = node.config; - let scope = processor.getScope(node); + let scope = processor.getScope(node.id); if (dynamic) { const parsed = parse(dynamic)(scope) ?? {}; engine = parsed.engine; diff --git a/packages/plugins/workflow/src/server/instructions/condition.ts b/packages/plugins/workflow/src/server/instructions/condition.ts index eecf3d7e5..79aee9135 100644 --- a/packages/plugins/workflow/src/server/instructions/condition.ts +++ b/packages/plugins/workflow/src/server/instructions/condition.ts @@ -125,8 +125,8 @@ export default { try { result = evaluator - ? evaluator(expression, processor.getScope()) - : logicCalculate(processor.getParsedValue(calculation, node)); + ? evaluator(expression, processor.getScope(node.id)) + : logicCalculate(processor.getParsedValue(calculation, node.id)); } catch (e) { return { result: e.toString(), diff --git a/packages/plugins/workflow/src/server/instructions/create.ts b/packages/plugins/workflow/src/server/instructions/create.ts index 837a002ef..f5e7deed8 100644 --- a/packages/plugins/workflow/src/server/instructions/create.ts +++ b/packages/plugins/workflow/src/server/instructions/create.ts @@ -7,7 +7,7 @@ export default { const { collection, params: { appends = [], ...params } = {} } = node.config; const { repository, model } = (node.constructor).database.getCollection(collection); - const options = processor.getParsedValue(params, node); + const options = processor.getParsedValue(params, node.id); const created = await repository.create({ ...options, context: { diff --git a/packages/plugins/workflow/src/server/instructions/destroy.ts b/packages/plugins/workflow/src/server/instructions/destroy.ts index 4aacf3ce7..a57f78bb7 100644 --- a/packages/plugins/workflow/src/server/instructions/destroy.ts +++ b/packages/plugins/workflow/src/server/instructions/destroy.ts @@ -6,7 +6,7 @@ export default { const { collection, params = {} } = node.config; const repo = (node.constructor).database.getRepository(collection); - const options = processor.getParsedValue(params, node); + const options = processor.getParsedValue(params, node.id); const result = await repo.destroy({ ...options, context: { diff --git a/packages/plugins/workflow/src/server/instructions/loop.ts b/packages/plugins/workflow/src/server/instructions/loop.ts index a354c69d0..7b259c319 100644 --- a/packages/plugins/workflow/src/server/instructions/loop.ts +++ b/packages/plugins/workflow/src/server/instructions/loop.ts @@ -19,7 +19,7 @@ function getTargetLength(target) { export default { async run(node: FlowNodeModel, prevJob: JobModel, processor: Processor) { const [branch] = processor.getBranches(node); - const target = processor.getParsedValue(node.config.target, node); + const target = processor.getParsedValue(node.config.target, node.id); const length = getTargetLength(target); if (!branch || !length) { @@ -61,7 +61,7 @@ export default { const nextIndex = result + 1; - const target = processor.getParsedValue(loop.config.target, node); + const target = processor.getParsedValue(loop.config.target, node.id); // branchJob.status === JOB_STATUS.RESOLVED means branchJob is done, try next loop or exit as resolved if (branchJob.status > JOB_STATUS.PENDING) { job.set({ result: nextIndex }); @@ -83,7 +83,7 @@ export default { }, getScope(node, index, processor) { - const target = processor.getParsedValue(node.config.target, node); + const target = processor.getParsedValue(node.config.target, node.id); const targets = (Array.isArray(target) ? target : [target]).filter((t) => t != null); const length = getTargetLength(target); const item = typeof target === 'number' ? index : targets[index]; diff --git a/packages/plugins/workflow/src/server/instructions/manual/actions.ts b/packages/plugins/workflow/src/server/instructions/manual/actions.ts index 91f54f853..788c7cdab 100644 --- a/packages/plugins/workflow/src/server/instructions/manual/actions.ts +++ b/packages/plugins/workflow/src/server/instructions/manual/actions.ts @@ -30,7 +30,7 @@ export async function submit(context: Context, next) { } const { forms = {} } = userJob.node.config; - const [formKey] = Object.keys(values.result ?? {}).filter(key => key !== '_'); + const [formKey] = Object.keys(values.result ?? {}).filter((key) => key !== '_'); const actionKey = values.result?._; const actionItem = forms[formKey]?.actions?.find((item) => item.key === actionKey); @@ -40,7 +40,8 @@ export async function submit(context: Context, next) { userJob.job.status !== JOB_STATUS.PENDING || userJob.execution.status !== EXECUTION_STATUS.STARTED || !userJob.workflow.enabled || - !actionKey || actionItem?.status == null + !actionKey || + actionItem?.status == null ) { return context.throw(400); } @@ -50,11 +51,11 @@ export async function submit(context: Context, next) { await processor.prepare(); // NOTE: validate assignee - const assignees = processor.getParsedValue(userJob.node.config.assignees ?? []); + const assignees = processor.getParsedValue(userJob.node.config.assignees ?? [], userJob.nodeId); if (!assignees.includes(currentUser.id) || userJob.userId !== currentUser.id) { return context.throw(403); } - const presetValues = processor.getParsedValue(actionItem.values ?? {}, null, { + const presetValues = processor.getParsedValue(actionItem.values ?? {}, userJob.nodeId, { currentUser: currentUser.toJSON(), currentRecord: values.result[formKey], currentTime: new Date(), @@ -62,9 +63,10 @@ export async function submit(context: Context, next) { userJob.set({ status: actionItem.status, - result: actionItem.status > JOB_STATUS.PENDING - ? { [formKey]: Object.assign(values.result[formKey], presetValues), _: actionKey } - : Object.assign(userJob.result ?? {}, values.result), + result: + actionItem.status > JOB_STATUS.PENDING + ? { [formKey]: Object.assign(values.result[formKey], presetValues), _: actionKey } + : Object.assign(userJob.result ?? {}, values.result), }); const handler = instruction.formTypes.get(forms[formKey].type); diff --git a/packages/plugins/workflow/src/server/instructions/manual/forms/update.ts b/packages/plugins/workflow/src/server/instructions/manual/forms/update.ts index f765e149f..7952eda2f 100644 --- a/packages/plugins/workflow/src/server/instructions/manual/forms/update.ts +++ b/packages/plugins/workflow/src/server/instructions/manual/forms/update.ts @@ -10,7 +10,7 @@ export default async function (this: ManualInstruction, instance, { collection, const { _, ...form } = instance.result; const [values] = Object.values(form); await repo.update({ - filter: processor.getParsedValue(filter), + filter: processor.getParsedValue(filter, instance.nodeId), values: { ...((values as { [key: string]: any }) ?? {}), updatedBy: instance.userId, diff --git a/packages/plugins/workflow/src/server/instructions/manual/index.ts b/packages/plugins/workflow/src/server/instructions/manual/index.ts index 43be4f224..90741e48a 100644 --- a/packages/plugins/workflow/src/server/instructions/manual/index.ts +++ b/packages/plugins/workflow/src/server/instructions/manual/index.ts @@ -2,7 +2,7 @@ import actions from '@nocobase/actions'; import { HandlerType } from '@nocobase/resourcer'; import { Registry } from '@nocobase/utils'; -import Plugin from '../..'; +import Plugin, { Processor } from '../..'; import { JOB_STATUS } from '../../constants'; import { Instruction } from '..'; import jobsCollection from './collecions/jobs'; @@ -124,9 +124,9 @@ export default class implements Instruction { initFormTypes(this); } - async run(node, prevJob, processor) { + async run(node, prevJob, processor: Processor) { const { mode, ...config } = node.config as ManualConfig; - const assignees = [...new Set(processor.getParsedValue(config.assignees) || [])]; + const assignees = [...new Set(processor.getParsedValue(config.assignees, node.id) || [])]; const job = await processor.saveJob({ status: JOB_STATUS.PENDING, @@ -154,7 +154,7 @@ export default class implements Instruction { return job; } - async resume(node, job, processor) { + async resume(node, job, processor: Processor) { // NOTE: check all users jobs related if all done then continue as parallel const { assignees = [], mode } = node.config as ManualConfig; diff --git a/packages/plugins/workflow/src/server/instructions/query.ts b/packages/plugins/workflow/src/server/instructions/query.ts index fba805170..12ac357d9 100644 --- a/packages/plugins/workflow/src/server/instructions/query.ts +++ b/packages/plugins/workflow/src/server/instructions/query.ts @@ -15,7 +15,7 @@ export default { pageSize = DEFAULT_PER_PAGE, sort = [], ...options - } = processor.getParsedValue(params, node); + } = processor.getParsedValue(params, node.id); const appends = options.appends ? Array.from( options.appends.reduce((set, field) => { diff --git a/packages/plugins/workflow/src/server/instructions/request.ts b/packages/plugins/workflow/src/server/instructions/request.ts index e2bea0b30..92e9a95c6 100644 --- a/packages/plugins/workflow/src/server/instructions/request.ts +++ b/packages/plugins/workflow/src/server/instructions/request.ts @@ -52,7 +52,7 @@ export default class implements Instruction { upstreamId: prevJob?.id ?? null, }); - const config = processor.getParsedValue(node.config, node) as RequestConfig; + const config = processor.getParsedValue(node.config, node.id) as RequestConfig; // eslint-disable-next-line promise/catch-or-return request(config) diff --git a/packages/plugins/workflow/src/server/instructions/sql.ts b/packages/plugins/workflow/src/server/instructions/sql.ts index 55b67702b..1da900353 100644 --- a/packages/plugins/workflow/src/server/instructions/sql.ts +++ b/packages/plugins/workflow/src/server/instructions/sql.ts @@ -4,11 +4,11 @@ import type { FlowNodeModel } from '../types'; export default { async run(node: FlowNodeModel, input, processor: Processor) { const { sequelize } = (node.constructor).database; - const sql = processor.getParsedValue(node.config.sql ?? '', node).trim(); + const sql = processor.getParsedValue(node.config.sql ?? '', node.id).trim(); if (!sql) { return { status: JOB_STATUS.RESOLVED, - } + }; } const result = await sequelize.query(sql, { diff --git a/packages/plugins/workflow/src/server/instructions/update.ts b/packages/plugins/workflow/src/server/instructions/update.ts index 2af2ff673..a13888028 100644 --- a/packages/plugins/workflow/src/server/instructions/update.ts +++ b/packages/plugins/workflow/src/server/instructions/update.ts @@ -7,7 +7,7 @@ export default { const { collection, params = {} } = node.config; const repo = (node.constructor).database.getRepository(collection); - const options = processor.getParsedValue(params, node); + const options = processor.getParsedValue(params, node.id); const result = await repo.update({ ...options, context: {