From 4813da547dd8dfbda07221f975955eb43f4e09d1 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Tue, 6 Dec 2022 09:25:39 +0100 Subject: [PATCH] refactor: Reactivate workflow locking (#4770) * feat: Reenable workflow locking Co-authored-by: freyamade Co-authored-by: Csaba Tuncsik --- packages/cli/src/ResponseHelper.ts | 4 +- .../src/databases/entities/WorkflowEntity.ts | 33 +----- ...669739707125-AddWorkflowVersionIdColumn.ts | 45 +++++++ .../src/databases/migrations/mysqldb/index.ts | 2 + ...669739707126-AddWorkflowVersionIdColumn.ts | 44 +++++++ .../databases/migrations/postgresdb/index.ts | 2 + ...669739707124-AddWorkflowVersionIdColumn.ts | 47 ++++++++ .../src/databases/migrations/sqlite/index.ts | 2 + .../src/workflows/workflows.controller.ee.ts | 7 +- .../cli/src/workflows/workflows.controller.ts | 3 + .../cli/src/workflows/workflows.services.ts | 22 +++- .../workflows.controller.ee.test.ts | 112 ++++++++---------- packages/editor-ui/src/Interface.ts | 6 +- .../ExecutionsView/ExecutionsView.vue | 2 +- .../editor-ui/src/components/WorkflowCard.vue | 2 +- .../src/components/WorkflowSettings.vue | 4 +- .../editor-ui/src/mixins/workflowHelpers.ts | 16 +-- packages/editor-ui/src/stores/workflows.ts | 10 +- packages/editor-ui/src/views/NodeView.vue | 4 +- 19 files changed, 249 insertions(+), 118 deletions(-) create mode 100644 packages/cli/src/databases/migrations/mysqldb/1669739707125-AddWorkflowVersionIdColumn.ts create mode 100644 packages/cli/src/databases/migrations/postgresdb/1669739707126-AddWorkflowVersionIdColumn.ts create mode 100644 packages/cli/src/databases/migrations/sqlite/1669739707124-AddWorkflowVersionIdColumn.ts diff --git a/packages/cli/src/ResponseHelper.ts b/packages/cli/src/ResponseHelper.ts index 01984a09f7..8ade1c3f16 100644 --- a/packages/cli/src/ResponseHelper.ts +++ b/packages/cli/src/ResponseHelper.ts @@ -41,8 +41,8 @@ abstract class ResponseError extends Error { } export class BadRequestError extends ResponseError { - constructor(message: string) { - super(message, 400); + constructor(message: string, errorCode?: number) { + super(message, 400, errorCode); } } diff --git a/packages/cli/src/databases/entities/WorkflowEntity.ts b/packages/cli/src/databases/entities/WorkflowEntity.ts index 06d47e40d1..b9ef14be32 100644 --- a/packages/cli/src/databases/entities/WorkflowEntity.ts +++ b/packages/cli/src/databases/entities/WorkflowEntity.ts @@ -1,4 +1,3 @@ -import crypto from 'crypto'; import { Length } from 'class-validator'; import type { @@ -11,9 +10,6 @@ import type { } from 'n8n-workflow'; import { - AfterLoad, - AfterUpdate, - AfterInsert, Column, Entity, Index, @@ -29,7 +25,6 @@ import { SharedWorkflow } from './SharedWorkflow'; import { objectRetriever, sqlite } from '../utils/transformers'; import { AbstractEntity, jsonColumnType } from './AbstractEntity'; import type { IWorkflowDb } from '@/Interfaces'; -import { alphabetizeKeys } from '@/utils'; @Entity() export class WorkflowEntity extends AbstractEntity implements IWorkflowDb { @@ -90,32 +85,8 @@ export class WorkflowEntity extends AbstractEntity implements IWorkflowDb { }) pinData: ISimplifiedPinData; - /** - * Hash of editable workflow state. - */ - hash: string; - - @AfterLoad() - @AfterUpdate() - @AfterInsert() - setHash(): void { - const { name, active, nodes, connections, settings, staticData, pinData } = this; - - // Workflow listing page does not request the `connections` column, so we can use this for `undefined` to avoid generating hashes for all the workflows. - if (!connections) return; - - const state = JSON.stringify({ - name, - active, - nodes: nodes ? nodes.map(alphabetizeKeys) : [], - connections, - settings, - staticData, - pinData, - }); - - this.hash = crypto.createHash('md5').update(state).digest('hex'); - } + @Column({ length: 36 }) + versionId: string; } /** diff --git a/packages/cli/src/databases/migrations/mysqldb/1669739707125-AddWorkflowVersionIdColumn.ts b/packages/cli/src/databases/migrations/mysqldb/1669739707125-AddWorkflowVersionIdColumn.ts new file mode 100644 index 0000000000..a752fc1b36 --- /dev/null +++ b/packages/cli/src/databases/migrations/mysqldb/1669739707125-AddWorkflowVersionIdColumn.ts @@ -0,0 +1,45 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; +import { logMigrationEnd, logMigrationStart } from '@db/utils/migrationHelpers'; +import config from '@/config'; +import { v4 as uuidv4 } from 'uuid'; + +export class AddWorkflowVersionIdColumn1669739707125 implements MigrationInterface { + name = 'AddWorkflowVersionIdColumn1669739707125'; + + async up(queryRunner: QueryRunner): Promise { + logMigrationStart(this.name); + + const tablePrefix = config.getEnv('database.tablePrefix'); + + await queryRunner.query( + `ALTER TABLE ${tablePrefix}workflow_entity ADD COLUMN versionId CHAR(36)`, + ); + + const workflowIds: Array<{ id: number }> = await queryRunner.query(` + SELECT id + FROM ${tablePrefix}workflow_entity + `); + + workflowIds.map(({ id }) => { + const [updateQuery, updateParams] = queryRunner.connection.driver.escapeQueryWithParameters( + ` + UPDATE ${tablePrefix}workflow_entity + SET versionId = :versionId + WHERE id = '${id}' + `, + { versionId: uuidv4() }, + {}, + ); + + return queryRunner.query(updateQuery, updateParams); + }); + + logMigrationEnd(this.name); + } + + async down(queryRunner: QueryRunner) { + const tablePrefix = config.getEnv('database.tablePrefix'); + + await queryRunner.query(`ALTER TABLE ${tablePrefix}workflow_entity DROP COLUMN versionId`); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index 09fd2e57ea..3803c8aea9 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -23,6 +23,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr import { CreateWorkflowsEditorRole1663755770894 } from './1663755770894-CreateWorkflowsEditorRole'; import { CreateCredentialUsageTable1665484192213 } from './1665484192213-CreateCredentialUsageTable'; import { RemoveCredentialUsageTable1665754637026 } from './1665754637026-RemoveCredentialUsageTable'; +import { AddWorkflowVersionIdColumn1669739707125 } from './1669739707125-AddWorkflowVersionIdColumn'; export const mysqlMigrations = [ InitialMigration1588157391238, @@ -50,4 +51,5 @@ export const mysqlMigrations = [ CreateWorkflowsEditorRole1663755770894, CreateCredentialUsageTable1665484192213, RemoveCredentialUsageTable1665754637026, + AddWorkflowVersionIdColumn1669739707125, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/1669739707126-AddWorkflowVersionIdColumn.ts b/packages/cli/src/databases/migrations/postgresdb/1669739707126-AddWorkflowVersionIdColumn.ts new file mode 100644 index 0000000000..6ba9555c8e --- /dev/null +++ b/packages/cli/src/databases/migrations/postgresdb/1669739707126-AddWorkflowVersionIdColumn.ts @@ -0,0 +1,44 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; +import { getTablePrefix, logMigrationEnd, logMigrationStart } from '@db/utils/migrationHelpers'; +import config from '@/config'; +import { v4 as uuidv4 } from 'uuid'; + +export class AddWorkflowVersionIdColumn1669739707126 implements MigrationInterface { + name = 'AddWorkflowVersionIdColumn1669739707126'; + + async up(queryRunner: QueryRunner) { + logMigrationStart(this.name); + + const tablePrefix = getTablePrefix(); + await queryRunner.query( + `ALTER TABLE ${tablePrefix}workflow_entity ADD COLUMN "versionId" CHAR(36)`, + ); + + const workflowIds: Array<{ id: number }> = await queryRunner.query(` + SELECT id + FROM ${tablePrefix}workflow_entity + `); + + workflowIds.map(({ id }) => { + const [updateQuery, updateParams] = queryRunner.connection.driver.escapeQueryWithParameters( + ` + UPDATE ${tablePrefix}workflow_entity + SET "versionId" = :versionId + WHERE id = '${id}' + `, + { versionId: uuidv4() }, + {}, + ); + + return queryRunner.query(updateQuery, updateParams); + }); + + logMigrationEnd(this.name); + } + + async down(queryRunner: QueryRunner) { + const tablePrefix = config.getEnv('database.tablePrefix'); + + await queryRunner.query(`ALTER TABLE ${tablePrefix}workflow_entity DROP COLUMN "versionId"`); + } +} diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index dfee7504f9..71b6d3df2e 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -21,6 +21,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr import { CreateWorkflowsEditorRole1663755770893 } from './1663755770893-CreateWorkflowsEditorRole'; import { CreateCredentialUsageTable1665484192212 } from './1665484192212-CreateCredentialUsageTable'; import { RemoveCredentialUsageTable1665754637025 } from './1665754637025-RemoveCredentialUsageTable'; +import { AddWorkflowVersionIdColumn1669739707126 } from './1669739707126-AddWorkflowVersionIdColumn'; export const postgresMigrations = [ InitialMigration1587669153312, @@ -46,4 +47,5 @@ export const postgresMigrations = [ CreateWorkflowsEditorRole1663755770893, CreateCredentialUsageTable1665484192212, RemoveCredentialUsageTable1665754637025, + AddWorkflowVersionIdColumn1669739707126, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/1669739707124-AddWorkflowVersionIdColumn.ts b/packages/cli/src/databases/migrations/sqlite/1669739707124-AddWorkflowVersionIdColumn.ts new file mode 100644 index 0000000000..17483e8b98 --- /dev/null +++ b/packages/cli/src/databases/migrations/sqlite/1669739707124-AddWorkflowVersionIdColumn.ts @@ -0,0 +1,47 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; +import { logMigrationEnd, logMigrationStart } from '@db/utils/migrationHelpers'; +import config from '@/config'; +import { v4 as uuidv4 } from 'uuid'; + +export class AddWorkflowVersionIdColumn1669739707124 implements MigrationInterface { + name = 'AddWorkflowVersionIdColumn1669739707124'; + + async up(queryRunner: QueryRunner) { + logMigrationStart(this.name); + + const tablePrefix = config.getEnv('database.tablePrefix'); + + await queryRunner.query( + `ALTER TABLE \`${tablePrefix}workflow_entity\` ADD COLUMN "versionId" char(36)`, + ); + + const workflowIds: Array<{ id: number }> = await queryRunner.query(` + SELECT id + FROM "${tablePrefix}workflow_entity" + `); + + workflowIds.map(({ id }) => { + const [updateQuery, updateParams] = queryRunner.connection.driver.escapeQueryWithParameters( + ` + UPDATE "${tablePrefix}workflow_entity" + SET versionId = :versionId + WHERE id = '${id}' + `, + { versionId: uuidv4() }, + {}, + ); + + return queryRunner.query(updateQuery, updateParams); + }); + + logMigrationEnd(this.name); + } + + async down(queryRunner: QueryRunner) { + const tablePrefix = config.getEnv('database.tablePrefix'); + + await queryRunner.query( + `ALTER TABLE \`${tablePrefix}workflow_entity\` DROP COLUMN "versionId"`, + ); + } +} diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index b4355a1dc0..8f081dbbb3 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -20,6 +20,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr import { CreateWorkflowsEditorRole1663755770892 } from './1663755770892-CreateWorkflowsUserRole'; import { CreateCredentialUsageTable1665484192211 } from './1665484192211-CreateCredentialUsageTable'; import { RemoveCredentialUsageTable1665754637024 } from './1665754637024-RemoveCredentialUsageTable'; +import { AddWorkflowVersionIdColumn1669739707124 } from './1669739707124-AddWorkflowVersionIdColumn'; const sqliteMigrations = [ InitialMigration1588102412422, @@ -44,6 +45,7 @@ const sqliteMigrations = [ CreateWorkflowsEditorRole1663755770892, CreateCredentialUsageTable1665484192211, RemoveCredentialUsageTable1665754637024, + AddWorkflowVersionIdColumn1669739707124, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 5403c81445..0f6d924d31 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -1,4 +1,5 @@ import express from 'express'; +import { v4 as uuid } from 'uuid'; import * as Db from '@/Db'; import { InternalHooksManager } from '@/InternalHooksManager'; import * as ResponseHelper from '@/ResponseHelper'; @@ -112,6 +113,8 @@ EEWorkflowController.post( Object.assign(newWorkflow, req.body); + newWorkflow.versionId = uuid(); + await validateEntity(newWorkflow); await externalHooks.run('workflow.create', [newWorkflow]); @@ -213,7 +216,7 @@ EEWorkflowController.patch( '/:id(\\d+)', ResponseHelper.send(async (req: WorkflowRequest.Update) => { const { id: workflowId } = req.params; - // const forceSave = req.query.forceSave === 'true'; // disabled temporarily - tests were also disabled + const forceSave = req.query.forceSave === 'true'; const updateData = new WorkflowEntity(); const { tags, ...rest } = req.body; @@ -226,7 +229,7 @@ EEWorkflowController.patch( safeWorkflow, workflowId, tags, - true, + forceSave, ); const { id, ...remainder } = updatedWorkflow; diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 0ad5bcd0dd..b909135ce3 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -1,6 +1,7 @@ /* eslint-disable no-param-reassign */ import express from 'express'; +import { v4 as uuid } from 'uuid'; import { LoggerProxy } from 'n8n-workflow'; import axios from 'axios'; @@ -52,6 +53,8 @@ workflowsController.post( Object.assign(newWorkflow, req.body); + newWorkflow.versionId = uuid(); + await validateEntity(newWorkflow); await externalHooks.run('workflow.create', [newWorkflow]); diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index c387fa4a9c..dbb2dffd23 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -2,6 +2,7 @@ import { validate as jsonSchemaValidate } from 'jsonschema'; import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow'; import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm'; import pick from 'lodash.pick'; +import { v4 as uuid } from 'uuid'; import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner'; import * as Db from '@/Db'; import { InternalHooksManager } from '@/InternalHooksManager'; @@ -172,7 +173,7 @@ export class WorkflowsService { } const query: FindManyOptions = { - select: isSharingEnabled ? [...fields, 'nodes'] : fields, + select: isSharingEnabled ? [...fields, 'nodes', 'versionId'] : fields, relations, where: { id: In(sharedWorkflowIds), @@ -220,12 +221,28 @@ export class WorkflowsService { ); } - if (!forceSave && workflow.hash !== '' && workflow.hash !== shared.workflow.hash) { + if ( + !forceSave && + workflow.versionId !== '' && + workflow.versionId !== shared.workflow.versionId + ) { throw new ResponseHelper.BadRequestError( 'Your most recent changes may be lost, because someone else just updated this workflow. Open this workflow in a new tab to see those new updates.', + 100, ); } + // Update the workflow's version + workflow.versionId = uuid(); + + LoggerProxy.verbose( + `Updating versionId for workflow ${workflowId} for user ${user.id} after saving`, + { + previousVersionId: shared.workflow.versionId, + newVersionId: workflow.versionId, + }, + ); + // check credentials for old format await WorkflowHelpers.replaceInvalidCredentials(workflow); @@ -280,6 +297,7 @@ export class WorkflowsService { 'settings', 'staticData', 'pinData', + 'versionId', ]), ); diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 7598f8e166..f928260b38 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -536,11 +536,11 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => }; const createResponse = await authAgent(owner).post('/workflows').send(workflow); - const { id, hash } = createResponse.body.data; + const { id, versionId } = createResponse.body.data; const response = await authAgent(owner).patch(`/workflows/${id}`).send({ name: 'new name', - hash, + versionId, }); expect(response.statusCode).toBe(200); @@ -574,12 +574,12 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => }; const createResponse = await authAgent(owner).post('/workflows').send(workflow); - const { id, hash } = createResponse.body.data; + const { id, versionId } = createResponse.body.data; const response = await authAgent(owner) .patch(`/workflows/${id}`) .send({ - hash, + versionId, nodes: [ { id: 'uuid-1234', @@ -630,12 +630,12 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => }; const createResponse = await authAgent(owner).post('/workflows').send(workflow); - const { id, hash } = createResponse.body.data; + const { id, versionId } = createResponse.body.data; const response = await authAgent(member) .patch(`/workflows/${id}`) .send({ - hash, + versionId, nodes: [ { id: 'uuid-1234', @@ -714,14 +714,14 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => }; const createResponse = await authAgent(member1).post('/workflows').send(workflow); - const { id, hash } = createResponse.body.data; + const { id, versionId } = createResponse.body.data; await authAgent(member1) .put(`/workflows/${id}/share`) .send({ shareWithIds: [member2.id] }); const response = await authAgent(member2).patch(`/workflows/${id}`).send({ - hash, + versionId, nodes: changedNodes, }); @@ -731,14 +731,14 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => }); describe('PATCH /workflows/:id - validate interim updates', () => { - xit('should block owner updating workflow nodes on interim update by member', async () => { + it('should block owner updating workflow nodes on interim update by member', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const member = await testDb.createUser({ globalRole: globalMemberRole }); // owner creates and shares workflow const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); - const { id, hash: ownerHash } = createResponse.body.data; + const { id, versionId: ownerVersionId } = createResponse.body.data; await authAgent(owner) .put(`/workflows/${id}/share`) .send({ shareWithIds: [member.id] }); @@ -746,38 +746,36 @@ describe('PATCH /workflows/:id - validate interim updates', () => { // member accesses and updates workflow name const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); - const { hash: memberHash } = memberGetResponse.body.data; + const { versionId: memberVersionId } = memberGetResponse.body.data; await authAgent(member) .patch(`/workflows/${id}`) - .send({ name: 'Update by member', hash: memberHash }); + .send({ name: 'Update by member', versionId: memberVersionId }); // owner blocked from updating workflow nodes const updateAttemptResponse = await authAgent(owner) .patch(`/workflows/${id}`) - .send({ nodes: [], hash: ownerHash }); + .send({ nodes: [], versionId: ownerVersionId }); expect(updateAttemptResponse.status).toBe(400); - expect(updateAttemptResponse.body.message).toContain( - 'the workflow has been changed in the meantime', - ); + expect(updateAttemptResponse.body.code).toBe(100); }); - xit('should block member updating workflow nodes on interim update by owner', async () => { + it('should block member updating workflow nodes on interim update by owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const member = await testDb.createUser({ globalRole: globalMemberRole }); // owner creates, updates and shares workflow const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); - const { id, hash: ownerFirstHash } = createResponse.body.data; + const { id, versionId: ownerFirstVersionId } = createResponse.body.data; const updateResponse = await authAgent(owner) .patch(`/workflows/${id}`) - .send({ name: 'Update by owner', hash: ownerFirstHash }); + .send({ name: 'Update by owner', versionId: ownerFirstVersionId }); - const { hash: ownerSecondHash } = updateResponse.body.data; + const { versionId: ownerSecondVersionId } = updateResponse.body.data; await authAgent(owner) .put(`/workflows/${id}/share`) @@ -786,34 +784,32 @@ describe('PATCH /workflows/:id - validate interim updates', () => { // member accesses workflow const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); - const { hash: memberHash } = memberGetResponse.body.data; + const { versionId: memberVersionId } = memberGetResponse.body.data; // owner re-updates workflow await authAgent(owner) .patch(`/workflows/${id}`) - .send({ name: 'Owner update again', hash: ownerSecondHash }); + .send({ name: 'Owner update again', versionId: ownerSecondVersionId }); // member blocked from updating workflow const updateAttemptResponse = await authAgent(member) .patch(`/workflows/${id}`) - .send({ nodes: [], hash: memberHash }); + .send({ nodes: [], versionId: memberVersionId }); expect(updateAttemptResponse.status).toBe(400); - expect(updateAttemptResponse.body.message).toContain( - 'the workflow has been changed in the meantime', - ); + expect(updateAttemptResponse.body.code).toBe(100); }); - xit('should block owner activation on interim activation by member', async () => { + it('should block owner activation on interim activation by member', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const member = await testDb.createUser({ globalRole: globalMemberRole }); // owner creates and shares workflow const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); - const { id, hash: ownerHash } = createResponse.body.data; + const { id, versionId: ownerVersionId } = createResponse.body.data; await authAgent(owner) .put(`/workflows/${id}/share`) .send({ shareWithIds: [member.id] }); @@ -821,34 +817,34 @@ describe('PATCH /workflows/:id - validate interim updates', () => { // member accesses and activates workflow const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); - const { hash: memberHash } = memberGetResponse.body.data; - await authAgent(member).patch(`/workflows/${id}`).send({ active: true, hash: memberHash }); + const { versionId: memberVersionId } = memberGetResponse.body.data; + await authAgent(member) + .patch(`/workflows/${id}`) + .send({ active: true, versionId: memberVersionId }); // owner blocked from activating workflow const activationAttemptResponse = await authAgent(owner) .patch(`/workflows/${id}`) - .send({ active: true, hash: ownerHash }); + .send({ active: true, versionId: ownerVersionId }); expect(activationAttemptResponse.status).toBe(400); - expect(activationAttemptResponse.body.message).toContain( - 'the workflow has been changed in the meantime', - ); + expect(activationAttemptResponse.body.code).toBe(100); }); - xit('should block member activation on interim activation by owner', async () => { + it('should block member activation on interim activation by owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const member = await testDb.createUser({ globalRole: globalMemberRole }); // owner creates, updates and shares workflow const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); - const { id, hash: ownerFirstHash } = createResponse.body.data; + const { id, versionId: ownerFirstVersionId } = createResponse.body.data; const updateResponse = await authAgent(owner) .patch(`/workflows/${id}`) - .send({ name: 'Update by owner', hash: ownerFirstHash }); - const { hash: ownerSecondHash } = updateResponse.body.data; + .send({ name: 'Update by owner', versionId: ownerFirstVersionId }); + const { versionId: ownerSecondVersionId } = updateResponse.body.data; await authAgent(owner) .put(`/workflows/${id}/share`) @@ -857,32 +853,32 @@ describe('PATCH /workflows/:id - validate interim updates', () => { // member accesses workflow const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); - const { hash: memberHash } = memberGetResponse.body.data; + const { versionId: memberVersionId } = memberGetResponse.body.data; // owner activates workflow - await authAgent(owner).patch(`/workflows/${id}`).send({ active: true, hash: ownerSecondHash }); + await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ active: true, versionId: ownerSecondVersionId }); // member blocked from activating workflow const updateAttemptResponse = await authAgent(member) .patch(`/workflows/${id}`) - .send({ active: true, hash: memberHash }); + .send({ active: true, versionId: memberVersionId }); expect(updateAttemptResponse.status).toBe(400); - expect(updateAttemptResponse.body.message).toContain( - 'the workflow has been changed in the meantime', - ); + expect(updateAttemptResponse.body.code).toBe(100); }); - xit('should block member updating workflow settings on interim update by owner', async () => { + it('should block member updating workflow settings on interim update by owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const member = await testDb.createUser({ globalRole: globalMemberRole }); // owner creates and shares workflow const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); - const { id, hash: ownerHash } = createResponse.body.data; + const { id, versionId: ownerVersionId } = createResponse.body.data; await authAgent(owner) .put(`/workflows/${id}/share`) .send({ shareWithIds: [member.id] }); @@ -890,34 +886,32 @@ describe('PATCH /workflows/:id - validate interim updates', () => { // member accesses workflow const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); - const { hash: memberHash } = memberGetResponse.body.data; + const { versionId: memberVersionId } = memberGetResponse.body.data; // owner updates workflow name await authAgent(owner) .patch(`/workflows/${id}`) - .send({ name: 'Another name', hash: ownerHash }); + .send({ name: 'Another name', versionId: ownerVersionId }); // member blocked from updating workflow settings const updateAttemptResponse = await authAgent(member) .patch(`/workflows/${id}`) - .send({ settings: { saveManualExecutions: true }, hash: memberHash }); + .send({ settings: { saveManualExecutions: true }, versionId: memberVersionId }); expect(updateAttemptResponse.status).toBe(400); - expect(updateAttemptResponse.body.message).toContain( - 'the workflow has been changed in the meantime', - ); + expect(updateAttemptResponse.body.code).toBe(100); }); - xit('should block member updating workflow name on interim update by owner', async () => { + it('should block member updating workflow name on interim update by owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const member = await testDb.createUser({ globalRole: globalMemberRole }); // owner creates and shares workflow const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); - const { id, hash: ownerHash } = createResponse.body.data; + const { id, versionId: ownerVersionId } = createResponse.body.data; await authAgent(owner) .put(`/workflows/${id}/share`) .send({ shareWithIds: [member.id] }); @@ -925,23 +919,21 @@ describe('PATCH /workflows/:id - validate interim updates', () => { // member accesses workflow const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); - const { hash: memberHash } = memberGetResponse.body.data; + const { versionId: memberVersionId } = memberGetResponse.body.data; // owner updates workflow settings await authAgent(owner) .patch(`/workflows/${id}`) - .send({ settings: { saveManualExecutions: true }, hash: ownerHash }); + .send({ settings: { saveManualExecutions: true }, versionId: ownerVersionId }); // member blocked from updating workflow name const updateAttemptResponse = await authAgent(member) .patch(`/workflows/${id}`) - .send({ settings: { saveManualExecutions: true }, hash: memberHash }); + .send({ settings: { saveManualExecutions: true }, versionId: memberVersionId }); expect(updateAttemptResponse.status).toBe(400); - expect(updateAttemptResponse.body.message).toContain( - 'the workflow has been changed in the meantime', - ); + expect(updateAttemptResponse.body.code).toBe(100); }); }); diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 4376bd9c5d..7ad5d44da5 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -264,7 +264,7 @@ export interface IWorkflowData { settings?: IWorkflowSettings; tags?: string[]; pinData?: IPinData; - hash?: string; + versionId?: string; } export interface IWorkflowDataUpdate { @@ -276,7 +276,7 @@ export interface IWorkflowDataUpdate { active?: boolean; tags?: ITag[] | string[]; // string[] when store or requested, ITag[] from API response pinData?: IPinData; - hash?: string; + versionId?: string; } export interface IWorkflowToShare extends IWorkflowDataUpdate { @@ -313,7 +313,7 @@ export interface IWorkflowDb { pinData?: IPinData; sharedWith?: Array>; ownedBy?: Partial; - hash: string; + versionId: string; usedCredentials?: Array>; } diff --git a/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue b/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue index 2830785b81..82f4918f79 100644 --- a/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue +++ b/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue @@ -405,7 +405,7 @@ export default mixins(restApi, showMessage, executionHelpers, debounceHelper, wo const tags = (data.tags || []) as ITag[]; const tagIds = tags.map((tag) => tag.id); this.workflowsStore.setWorkflowTagIds(tagIds || []); - this.workflowsStore.setWorkflowHash(data.hash); + this.workflowsStore.setWorkflowVersionId(data.versionId); this.tagsStore.upsertTags(tags); diff --git a/packages/editor-ui/src/components/WorkflowCard.vue b/packages/editor-ui/src/components/WorkflowCard.vue index e0c38b9374..5ae8ec100a 100644 --- a/packages/editor-ui/src/components/WorkflowCard.vue +++ b/packages/editor-ui/src/components/WorkflowCard.vue @@ -104,7 +104,7 @@ export default mixins( name: '', sharedWith: [], ownedBy: {} as IUser, - hash: '', + versionId: '', }), }, readonly: { diff --git a/packages/editor-ui/src/components/WorkflowSettings.vue b/packages/editor-ui/src/components/WorkflowSettings.vue index e0aeed6b26..076240ec33 100644 --- a/packages/editor-ui/src/components/WorkflowSettings.vue +++ b/packages/editor-ui/src/components/WorkflowSettings.vue @@ -625,11 +625,11 @@ export default mixins( delete data.settings!.maxExecutionTimeout; this.isLoading = true; - data.hash = this.workflowsStore.workflowHash; + data.versionId = this.workflowsStore.workflowVersionId; try { const workflow = await this.restApi().updateWorkflow(this.$route.params.name, data); - this.workflowsStore.setWorkflowHash(workflow.hash); + this.workflowsStore.setWorkflowVersionId(workflow.versionId); } catch (error) { this.$showError( error, diff --git a/packages/editor-ui/src/mixins/workflowHelpers.ts b/packages/editor-ui/src/mixins/workflowHelpers.ts index 41eb90c6e9..a28e42eaed 100644 --- a/packages/editor-ui/src/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/mixins/workflowHelpers.ts @@ -418,7 +418,7 @@ export const workflowHelpers = mixins( active: this.workflowsStore.isWorkflowActive, settings: this.workflowsStore.workflow.settings, tags: this.workflowsStore.workflowTags, - hash: this.workflowsStore.workflow.hash, + versionId: this.workflowsStore.workflow.versionId, }; const workflowId = this.workflowsStore.workflowId; @@ -680,8 +680,8 @@ export const workflowHelpers = mixins( if (isCurrentWorkflow) { data = await this.getWorkflowDataToSave(); } else { - const { hash } = await this.restApi().getWorkflow(workflowId); - data.hash = hash; + const { versionId } = await this.restApi().getWorkflow(workflowId); + data.versionId = versionId; } if (active !== undefined) { @@ -689,7 +689,7 @@ export const workflowHelpers = mixins( } const workflow = await this.restApi().updateWorkflow(workflowId, data); - this.workflowsStore.setWorkflowHash(workflow.hash); + this.workflowsStore.setWorkflowVersionId(workflow.versionId); if (isCurrentWorkflow) { this.workflowsStore.setActive(!!workflow.active); @@ -724,10 +724,10 @@ export const workflowHelpers = mixins( workflowDataRequest.tags = tags; } - workflowDataRequest.hash = this.workflowsStore.workflowHash; + workflowDataRequest.versionId = this.workflowsStore.workflowVersionId; const workflowData = await this.restApi().updateWorkflow(currentWorkflow, workflowDataRequest, forceSave); - this.workflowsStore.setWorkflowHash(workflowData.hash); + this.workflowsStore.setWorkflowVersionId(workflowData.versionId); if (name) { this.workflowsStore.setWorkflowName({newName: workflowData.name, setStateDirty: false}); @@ -747,7 +747,7 @@ export const workflowHelpers = mixins( } catch (error) { this.uiStore.removeActiveAction('workflowSaving'); - if (error.errorCode === 400 && error.message.startsWith('Your most recent changes may be lost')) { + if (error.errorCode === 100) { const overwrite = await this.confirmMessage( this.$locale.baseText('workflows.concurrentChanges.confirmMessage.message'), this.$locale.baseText('workflows.concurrentChanges.confirmMessage.title'), @@ -810,7 +810,7 @@ export const workflowHelpers = mixins( const workflowData = await this.restApi().createNewWorkflow(workflowDataRequest); this.workflowsStore.addWorkflow(workflowData); - this.workflowsStore.setWorkflowHash(workflowData.hash); + this.workflowsStore.setWorkflowVersionId(workflowData.versionId); if (this.settingsStore.isEnterpriseFeatureEnabled(EnterpriseEditionFeature.WorkflowSharing) && this.usersStore.currentUser) { this.workflowsEEStore.setWorkflowOwnedBy({ workflowId: workflowData.id, ownedBy: this.usersStore.currentUser }); diff --git a/packages/editor-ui/src/stores/workflows.ts b/packages/editor-ui/src/stores/workflows.ts index c55f197369..46ef5ac727 100644 --- a/packages/editor-ui/src/stores/workflows.ts +++ b/packages/editor-ui/src/stores/workflows.ts @@ -68,7 +68,7 @@ const createEmptyWorkflow = (): IWorkflowDb => ({ settings: {}, tags: [], pinData: {}, - hash: '', + versionId: '', }); export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { @@ -97,8 +97,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { workflowId(): string { return this.workflow.id; }, - workflowHash(): string | undefined { - return this.workflow.hash; + workflowVersionId(): string | undefined { + return this.workflow.versionId; }, workflowSettings() : IWorkflowSettings { if (this.workflow.settings === undefined) { @@ -287,8 +287,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { } }, - setWorkflowHash(hash: string): void { - this.workflow.hash = hash; + setWorkflowVersionId(versionId: string): void { + this.workflow.versionId = versionId; }, // replace invalid credentials in workflow diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index adf9ba1163..9d9e00b306 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -834,7 +834,7 @@ export default mixins( this.workflowsStore.setWorkflowName({ newName: data.name, setStateDirty: false }); this.workflowsStore.setWorkflowSettings(data.settings || {}); this.workflowsStore.setWorkflowPinData(data.pinData || {}); - this.workflowsStore.setWorkflowHash(data.hash); + this.workflowsStore.setWorkflowVersionId(data.versionId); if (data.ownedBy) { this.workflowsEEStore.setWorkflowOwnedBy({ @@ -3369,6 +3369,7 @@ export default mixins( dataPinningEventBus.$off('pin-data', this.addPinDataConnections); dataPinningEventBus.$off('unpin-data', this.removePinDataConnections); nodeViewEventBus.$off('saveWorkflow', this.saveCurrentWorkflowExternal); + this.workflowsStore.setWorkflowId(PLACEHOLDER_EMPTY_WORKFLOW_ID); }, destroyed() { this.resetWorkspace(); @@ -3377,6 +3378,7 @@ export default mixins( this.$root.$off('newWorkflow', this.newWorkflow); this.$root.$off('importWorkflowData', this.onImportWorkflowDataEvent); this.$root.$off('importWorkflowUrl', this.onImportWorkflowUrlEvent); + this.workflowsStore.setWorkflowId(PLACEHOLDER_EMPTY_WORKFLOW_ID); }, });