From f2710f2cdb5fd0ca458f4f2229d30d304a50c50a Mon Sep 17 00:00:00 2001 From: Junyi Date: Wed, 22 Jun 2022 23:17:30 +0800 Subject: [PATCH] fix(plugin-workflow): set current when update (#526) --- packages/plugins/workflow/src/Plugin.ts | 71 +++++---- .../plugins/workflow/src/__tests__/index.ts | 4 +- .../workflow/src/__tests__/workflow.test.ts | 139 +++++++++++++++--- .../workflow/src/collections/workflows.ts | 3 +- 4 files changed, 166 insertions(+), 51 deletions(-) diff --git a/packages/plugins/workflow/src/Plugin.ts b/packages/plugins/workflow/src/Plugin.ts index 82d6151e5..e98e5cef6 100644 --- a/packages/plugins/workflow/src/Plugin.ts +++ b/packages/plugins/workflow/src/Plugin.ts @@ -14,36 +14,53 @@ import { EXECUTION_STATUS } from './constants'; -async function setCurrent(instance: WorkflowModel, options) { - const updates: { enabled?: boolean, current?: boolean } = {}; - - if (!instance.changed('enabled') || !instance.enabled) { - return; - } - - instance.set('current', true); - updates.enabled = false; - - // NOTE: set to `null` but not `false` will not violate the unique index - updates.current = null; - const previous = await (instance.constructor).findOne({ - where: { - key: instance.key, - current: true - } - }); - - if (previous) { - await previous.update(updates, { - transaction: options.transaction - }); - } -} - export default class WorkflowPlugin extends Plugin { instructions: Registry = new Registry(); triggers: Registry = new Registry(); + onBeforeSave = async (instance: WorkflowModel, options) => { + const Model = instance.constructor; + + if (instance.enabled) { + instance.set('current', true); + } else if (!instance.current) { + const count = await Model.count({ + where: { + key: instance.key + }, + transaction: options.transaction + }); + if (!count) { + instance.set('current', true); + } + } + + if (!instance.changed('enabled') || !instance.enabled) { + return; + } + + const previous = await Model.findOne({ + where: { + key: instance.key, + current: true, + id: { + [Op.ne]: instance.id + } + }, + transaction: options.transaction + }); + + if (previous) { + // NOTE: set to `null` but not `false` will not violate the unique index + await previous.update({ enabled: false, current: null }, { + transaction: options.transaction, + hooks: false + }); + + this.toggle(previous, false); + } + }; + getName(): string { return this.getPackageName(__dirname); } @@ -59,7 +76,7 @@ export default class WorkflowPlugin extends Plugin { initTriggers(this, options.triggers); initInstructions(this, options.instructions); - db.on('workflows.beforeSave', setCurrent); + db.on('workflows.beforeSave', this.onBeforeSave); db.on('workflows.afterSave', (model: WorkflowModel) => this.toggle(model)); db.on('workflows.afterDestroy', (model: WorkflowModel) => this.toggle(model, false)); diff --git a/packages/plugins/workflow/src/__tests__/index.ts b/packages/plugins/workflow/src/__tests__/index.ts index 34b2195e1..3e6265faa 100644 --- a/packages/plugins/workflow/src/__tests__/index.ts +++ b/packages/plugins/workflow/src/__tests__/index.ts @@ -59,8 +59,8 @@ export async function getApp(options = {}): Promise { } catch (error) { console.error(error); } - // TODO: need a better life cycle event than manually trigger - await app.emitAsync('beforeStart'); + + await app.start(); return app; } diff --git a/packages/plugins/workflow/src/__tests__/workflow.test.ts b/packages/plugins/workflow/src/__tests__/workflow.test.ts index 6209a09fa..27611dcd4 100644 --- a/packages/plugins/workflow/src/__tests__/workflow.test.ts +++ b/packages/plugins/workflow/src/__tests__/workflow.test.ts @@ -21,10 +21,60 @@ describe('workflow > workflow', () => { PostRepo = db.getCollection('posts').repository; }); - afterEach(() => db.close()); + afterEach(() => app.destroy()); - describe('toggle', () => { - it('toggle after update', async () => { + describe('create', () => { + it('create with enabled', async () => { + const workflow = await WorkflowModel.create({ + enabled: true, + type: 'collection', + config: { + mode: 1, + collection: 'posts' + } + }); + + expect(workflow.current).toBe(true); + }); + + it('create with disabled', async () => { + const workflow = await WorkflowModel.create({ + type: 'collection', + config: { + mode: 1, + collection: 'posts' + } + }); + + expect(workflow.current).toBe(true); + }); + }); + + describe('update', () => { + it('toggle on', async () => { + const workflow = await WorkflowModel.create({ + enabled: false, + type: 'collection', + config: { + mode: 1, + collection: 'posts' + } + }); + expect(workflow.current).toBe(true); + + const p1 = await PostRepo.create({ values: { title: 't1' } }); + const count = await workflow.countExecutions(); + expect(count).toBe(0); + + await workflow.update({ enabled: true }); + expect(workflow.current).toBe(true); + + const p2 = await PostRepo.create({ values: { title: 't2' } }); + const executions = await workflow.getExecutions(); + expect(executions.length).toBe(1); + }); + + it('toggle off', async () => { const workflow = await WorkflowModel.create({ enabled: true, type: 'collection', @@ -35,34 +85,79 @@ describe('workflow > workflow', () => { }); expect(workflow.current).toBe(true); const p1 = await PostRepo.create({ values: { title: 't1' } }); - const executions1 = await workflow.getExecutions(); - expect(executions1.length).toBe(1); - - const w = await WorkflowModel.findOne({ - where: { - current: true - } - }); - expect(w).not.toBeNull(); - expect(w.current).toBe(true); + const c1 = await workflow.countExecutions(); + expect(c1).toBe(1); await workflow.update({ enabled: false }); + expect(workflow.current).toBe(true); + const p2 = await PostRepo.create({ values: { title: 't2' } }); + const c2 = await workflow.countExecutions(); + expect(c2).toBe(1); + }); - const executions2 = await workflow.getExecutions(); - expect(executions2.length).toBe(1); - - const workflows = await WorkflowModel.findAll({ - where: { - current: true + it('toggle off then on', async () => { + const workflow = await WorkflowModel.create({ + enabled: true, + type: 'collection', + config: { + mode: 1, + collection: 'posts' } }); - expect(workflows.length).toBe(1); + + const p1 = await PostRepo.create({ values: { title: 't1' } }); + const c1 = await workflow.countExecutions(); + expect(c1).toBe(1); + + await workflow.update({ + enabled: false + }); + expect(workflow.current).toBe(true); + + const p2 = await PostRepo.create({ values: { title: 't2' } }); + const c2 = await workflow.countExecutions(); + expect(c2).toBe(1); + + await workflow.update({ + enabled: true + }); + expect(workflow.current).toBe(true); + + const p3 = await PostRepo.create({ values: { title: 't3' } }); + const c3 = await workflow.countExecutions(); + expect(c3).toBe(2); + }); + + it('update config', async () => { + const workflow = await WorkflowModel.create({ + enabled: true, + type: 'collection', + config: { + mode: 1, + collection: 'posts' + } + }); + + const p1 = await PostRepo.create({ values: { title: 't1' } }); + const c1 = await workflow.countExecutions(); + expect(c1).toBe(1); + + await workflow.update({ + config: { + mode: 1, + collection: 'tags' + } + }); + + const p2 = await PostRepo.create({ values: { title: 't2' } }); + const c2 = await workflow.countExecutions(); + expect(c2).toBe(1); }); }); describe('revisions', () => { - it('executed count', async () => { + it('create revision', async () => { const w1 = await WorkflowModel.create({ enabled: true, type: 'collection', @@ -81,6 +176,8 @@ describe('workflow > workflow', () => { const { data: w2 } = body; expect(w2.config).toMatchObject(w1.config); expect(w2.key).toBe(w1.key); + expect(w2.current).toBeFalsy(); + expect(w2.enabled).toBe(false); const p1 = await PostRepo.create({ values: { title: 't1' } }); diff --git a/packages/plugins/workflow/src/collections/workflows.ts b/packages/plugins/workflow/src/collections/workflows.ts index 37281abdc..033fa2f0c 100644 --- a/packages/plugins/workflow/src/collections/workflows.ts +++ b/packages/plugins/workflow/src/collections/workflows.ts @@ -59,7 +59,8 @@ export default function () { }, { type: 'boolean', - name: 'current' + name: 'current', + defaultValue: null }, { type: 'hasMany',