refactor: Reactivate workflow locking (#4770)

* feat: Reenable workflow locking

Co-authored-by: freyamade <freya@n8n.io>
Co-authored-by: Csaba Tuncsik <csaba@n8n.io>
This commit is contained in:
Omar Ajoue 2022-12-06 09:25:39 +01:00 committed by GitHub
parent 915f1445c2
commit 4813da547d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 249 additions and 118 deletions

View file

@ -41,8 +41,8 @@ abstract class ResponseError extends Error {
} }
export class BadRequestError extends ResponseError { export class BadRequestError extends ResponseError {
constructor(message: string) { constructor(message: string, errorCode?: number) {
super(message, 400); super(message, 400, errorCode);
} }
} }

View file

@ -1,4 +1,3 @@
import crypto from 'crypto';
import { Length } from 'class-validator'; import { Length } from 'class-validator';
import type { import type {
@ -11,9 +10,6 @@ import type {
} from 'n8n-workflow'; } from 'n8n-workflow';
import { import {
AfterLoad,
AfterUpdate,
AfterInsert,
Column, Column,
Entity, Entity,
Index, Index,
@ -29,7 +25,6 @@ import { SharedWorkflow } from './SharedWorkflow';
import { objectRetriever, sqlite } from '../utils/transformers'; import { objectRetriever, sqlite } from '../utils/transformers';
import { AbstractEntity, jsonColumnType } from './AbstractEntity'; import { AbstractEntity, jsonColumnType } from './AbstractEntity';
import type { IWorkflowDb } from '@/Interfaces'; import type { IWorkflowDb } from '@/Interfaces';
import { alphabetizeKeys } from '@/utils';
@Entity() @Entity()
export class WorkflowEntity extends AbstractEntity implements IWorkflowDb { export class WorkflowEntity extends AbstractEntity implements IWorkflowDb {
@ -90,32 +85,8 @@ export class WorkflowEntity extends AbstractEntity implements IWorkflowDb {
}) })
pinData: ISimplifiedPinData; pinData: ISimplifiedPinData;
/** @Column({ length: 36 })
* Hash of editable workflow state. versionId: string;
*/
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');
}
} }
/** /**

View file

@ -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<void> {
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`);
}
}

View file

@ -23,6 +23,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr
import { CreateWorkflowsEditorRole1663755770894 } from './1663755770894-CreateWorkflowsEditorRole'; import { CreateWorkflowsEditorRole1663755770894 } from './1663755770894-CreateWorkflowsEditorRole';
import { CreateCredentialUsageTable1665484192213 } from './1665484192213-CreateCredentialUsageTable'; import { CreateCredentialUsageTable1665484192213 } from './1665484192213-CreateCredentialUsageTable';
import { RemoveCredentialUsageTable1665754637026 } from './1665754637026-RemoveCredentialUsageTable'; import { RemoveCredentialUsageTable1665754637026 } from './1665754637026-RemoveCredentialUsageTable';
import { AddWorkflowVersionIdColumn1669739707125 } from './1669739707125-AddWorkflowVersionIdColumn';
export const mysqlMigrations = [ export const mysqlMigrations = [
InitialMigration1588157391238, InitialMigration1588157391238,
@ -50,4 +51,5 @@ export const mysqlMigrations = [
CreateWorkflowsEditorRole1663755770894, CreateWorkflowsEditorRole1663755770894,
CreateCredentialUsageTable1665484192213, CreateCredentialUsageTable1665484192213,
RemoveCredentialUsageTable1665754637026, RemoveCredentialUsageTable1665754637026,
AddWorkflowVersionIdColumn1669739707125,
]; ];

View file

@ -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"`);
}
}

View file

@ -21,6 +21,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr
import { CreateWorkflowsEditorRole1663755770893 } from './1663755770893-CreateWorkflowsEditorRole'; import { CreateWorkflowsEditorRole1663755770893 } from './1663755770893-CreateWorkflowsEditorRole';
import { CreateCredentialUsageTable1665484192212 } from './1665484192212-CreateCredentialUsageTable'; import { CreateCredentialUsageTable1665484192212 } from './1665484192212-CreateCredentialUsageTable';
import { RemoveCredentialUsageTable1665754637025 } from './1665754637025-RemoveCredentialUsageTable'; import { RemoveCredentialUsageTable1665754637025 } from './1665754637025-RemoveCredentialUsageTable';
import { AddWorkflowVersionIdColumn1669739707126 } from './1669739707126-AddWorkflowVersionIdColumn';
export const postgresMigrations = [ export const postgresMigrations = [
InitialMigration1587669153312, InitialMigration1587669153312,
@ -46,4 +47,5 @@ export const postgresMigrations = [
CreateWorkflowsEditorRole1663755770893, CreateWorkflowsEditorRole1663755770893,
CreateCredentialUsageTable1665484192212, CreateCredentialUsageTable1665484192212,
RemoveCredentialUsageTable1665754637025, RemoveCredentialUsageTable1665754637025,
AddWorkflowVersionIdColumn1669739707126,
]; ];

View file

@ -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"`,
);
}
}

View file

@ -20,6 +20,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr
import { CreateWorkflowsEditorRole1663755770892 } from './1663755770892-CreateWorkflowsUserRole'; import { CreateWorkflowsEditorRole1663755770892 } from './1663755770892-CreateWorkflowsUserRole';
import { CreateCredentialUsageTable1665484192211 } from './1665484192211-CreateCredentialUsageTable'; import { CreateCredentialUsageTable1665484192211 } from './1665484192211-CreateCredentialUsageTable';
import { RemoveCredentialUsageTable1665754637024 } from './1665754637024-RemoveCredentialUsageTable'; import { RemoveCredentialUsageTable1665754637024 } from './1665754637024-RemoveCredentialUsageTable';
import { AddWorkflowVersionIdColumn1669739707124 } from './1669739707124-AddWorkflowVersionIdColumn';
const sqliteMigrations = [ const sqliteMigrations = [
InitialMigration1588102412422, InitialMigration1588102412422,
@ -44,6 +45,7 @@ const sqliteMigrations = [
CreateWorkflowsEditorRole1663755770892, CreateWorkflowsEditorRole1663755770892,
CreateCredentialUsageTable1665484192211, CreateCredentialUsageTable1665484192211,
RemoveCredentialUsageTable1665754637024, RemoveCredentialUsageTable1665754637024,
AddWorkflowVersionIdColumn1669739707124,
]; ];
export { sqliteMigrations }; export { sqliteMigrations };

View file

@ -1,4 +1,5 @@
import express from 'express'; import express from 'express';
import { v4 as uuid } from 'uuid';
import * as Db from '@/Db'; import * as Db from '@/Db';
import { InternalHooksManager } from '@/InternalHooksManager'; import { InternalHooksManager } from '@/InternalHooksManager';
import * as ResponseHelper from '@/ResponseHelper'; import * as ResponseHelper from '@/ResponseHelper';
@ -112,6 +113,8 @@ EEWorkflowController.post(
Object.assign(newWorkflow, req.body); Object.assign(newWorkflow, req.body);
newWorkflow.versionId = uuid();
await validateEntity(newWorkflow); await validateEntity(newWorkflow);
await externalHooks.run('workflow.create', [newWorkflow]); await externalHooks.run('workflow.create', [newWorkflow]);
@ -213,7 +216,7 @@ EEWorkflowController.patch(
'/:id(\\d+)', '/:id(\\d+)',
ResponseHelper.send(async (req: WorkflowRequest.Update) => { ResponseHelper.send(async (req: WorkflowRequest.Update) => {
const { id: workflowId } = req.params; 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 updateData = new WorkflowEntity();
const { tags, ...rest } = req.body; const { tags, ...rest } = req.body;
@ -226,7 +229,7 @@ EEWorkflowController.patch(
safeWorkflow, safeWorkflow,
workflowId, workflowId,
tags, tags,
true, forceSave,
); );
const { id, ...remainder } = updatedWorkflow; const { id, ...remainder } = updatedWorkflow;

View file

@ -1,6 +1,7 @@
/* eslint-disable no-param-reassign */ /* eslint-disable no-param-reassign */
import express from 'express'; import express from 'express';
import { v4 as uuid } from 'uuid';
import { LoggerProxy } from 'n8n-workflow'; import { LoggerProxy } from 'n8n-workflow';
import axios from 'axios'; import axios from 'axios';
@ -52,6 +53,8 @@ workflowsController.post(
Object.assign(newWorkflow, req.body); Object.assign(newWorkflow, req.body);
newWorkflow.versionId = uuid();
await validateEntity(newWorkflow); await validateEntity(newWorkflow);
await externalHooks.run('workflow.create', [newWorkflow]); await externalHooks.run('workflow.create', [newWorkflow]);

View file

@ -2,6 +2,7 @@ import { validate as jsonSchemaValidate } from 'jsonschema';
import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow'; import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow';
import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm'; import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm';
import pick from 'lodash.pick'; import pick from 'lodash.pick';
import { v4 as uuid } from 'uuid';
import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner'; import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner';
import * as Db from '@/Db'; import * as Db from '@/Db';
import { InternalHooksManager } from '@/InternalHooksManager'; import { InternalHooksManager } from '@/InternalHooksManager';
@ -172,7 +173,7 @@ export class WorkflowsService {
} }
const query: FindManyOptions<WorkflowEntity> = { const query: FindManyOptions<WorkflowEntity> = {
select: isSharingEnabled ? [...fields, 'nodes'] : fields, select: isSharingEnabled ? [...fields, 'nodes', 'versionId'] : fields,
relations, relations,
where: { where: {
id: In(sharedWorkflowIds), 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( 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.', '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 // check credentials for old format
await WorkflowHelpers.replaceInvalidCredentials(workflow); await WorkflowHelpers.replaceInvalidCredentials(workflow);
@ -280,6 +297,7 @@ export class WorkflowsService {
'settings', 'settings',
'staticData', 'staticData',
'pinData', 'pinData',
'versionId',
]), ]),
); );

View file

@ -536,11 +536,11 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () =>
}; };
const createResponse = await authAgent(owner).post('/workflows').send(workflow); 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({ const response = await authAgent(owner).patch(`/workflows/${id}`).send({
name: 'new name', name: 'new name',
hash, versionId,
}); });
expect(response.statusCode).toBe(200); 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 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) const response = await authAgent(owner)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ .send({
hash, versionId,
nodes: [ nodes: [
{ {
id: 'uuid-1234', 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 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) const response = await authAgent(member)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ .send({
hash, versionId,
nodes: [ nodes: [
{ {
id: 'uuid-1234', 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 createResponse = await authAgent(member1).post('/workflows').send(workflow);
const { id, hash } = createResponse.body.data; const { id, versionId } = createResponse.body.data;
await authAgent(member1) await authAgent(member1)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
.send({ shareWithIds: [member2.id] }); .send({ shareWithIds: [member2.id] });
const response = await authAgent(member2).patch(`/workflows/${id}`).send({ const response = await authAgent(member2).patch(`/workflows/${id}`).send({
hash, versionId,
nodes: changedNodes, nodes: changedNodes,
}); });
@ -731,14 +731,14 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () =>
}); });
describe('PATCH /workflows/:id - validate interim updates', () => { 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 owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates and shares workflow // owner creates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); 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) await authAgent(owner)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] }); .send({ shareWithIds: [member.id] });
@ -746,38 +746,36 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
// member accesses and updates workflow name // member accesses and updates workflow name
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { hash: memberHash } = memberGetResponse.body.data; const { versionId: memberVersionId } = memberGetResponse.body.data;
await authAgent(member) await authAgent(member)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ name: 'Update by member', hash: memberHash }); .send({ name: 'Update by member', versionId: memberVersionId });
// owner blocked from updating workflow nodes // owner blocked from updating workflow nodes
const updateAttemptResponse = await authAgent(owner) const updateAttemptResponse = await authAgent(owner)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ nodes: [], hash: ownerHash }); .send({ nodes: [], versionId: ownerVersionId });
expect(updateAttemptResponse.status).toBe(400); expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain( expect(updateAttemptResponse.body.code).toBe(100);
'the workflow has been changed in the meantime',
);
}); });
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 owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates, updates and shares workflow // owner creates, updates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); 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) const updateResponse = await authAgent(owner)
.patch(`/workflows/${id}`) .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) await authAgent(owner)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
@ -786,34 +784,32 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
// member accesses workflow // member accesses workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { hash: memberHash } = memberGetResponse.body.data; const { versionId: memberVersionId } = memberGetResponse.body.data;
// owner re-updates workflow // owner re-updates workflow
await authAgent(owner) await authAgent(owner)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ name: 'Owner update again', hash: ownerSecondHash }); .send({ name: 'Owner update again', versionId: ownerSecondVersionId });
// member blocked from updating workflow // member blocked from updating workflow
const updateAttemptResponse = await authAgent(member) const updateAttemptResponse = await authAgent(member)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ nodes: [], hash: memberHash }); .send({ nodes: [], versionId: memberVersionId });
expect(updateAttemptResponse.status).toBe(400); expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain( expect(updateAttemptResponse.body.code).toBe(100);
'the workflow has been changed in the meantime',
);
}); });
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 owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates and shares workflow // owner creates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); 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) await authAgent(owner)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] }); .send({ shareWithIds: [member.id] });
@ -821,34 +817,34 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
// member accesses and activates workflow // member accesses and activates workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); 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({ active: true, hash: memberHash }); await authAgent(member)
.patch(`/workflows/${id}`)
.send({ active: true, versionId: memberVersionId });
// owner blocked from activating workflow // owner blocked from activating workflow
const activationAttemptResponse = await authAgent(owner) const activationAttemptResponse = await authAgent(owner)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ active: true, hash: ownerHash }); .send({ active: true, versionId: ownerVersionId });
expect(activationAttemptResponse.status).toBe(400); expect(activationAttemptResponse.status).toBe(400);
expect(activationAttemptResponse.body.message).toContain( expect(activationAttemptResponse.body.code).toBe(100);
'the workflow has been changed in the meantime',
);
}); });
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 owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates, updates and shares workflow // owner creates, updates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); 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) const updateResponse = await authAgent(owner)
.patch(`/workflows/${id}`) .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) await authAgent(owner)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
@ -857,32 +853,32 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
// member accesses workflow // member accesses workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { hash: memberHash } = memberGetResponse.body.data; const { versionId: memberVersionId } = memberGetResponse.body.data;
// owner activates workflow // 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 // member blocked from activating workflow
const updateAttemptResponse = await authAgent(member) const updateAttemptResponse = await authAgent(member)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ active: true, hash: memberHash }); .send({ active: true, versionId: memberVersionId });
expect(updateAttemptResponse.status).toBe(400); expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain( expect(updateAttemptResponse.body.code).toBe(100);
'the workflow has been changed in the meantime',
);
}); });
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 owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates and shares workflow // owner creates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); 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) await authAgent(owner)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] }); .send({ shareWithIds: [member.id] });
@ -890,34 +886,32 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
// member accesses workflow // member accesses workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { hash: memberHash } = memberGetResponse.body.data; const { versionId: memberVersionId } = memberGetResponse.body.data;
// owner updates workflow name // owner updates workflow name
await authAgent(owner) await authAgent(owner)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ name: 'Another name', hash: ownerHash }); .send({ name: 'Another name', versionId: ownerVersionId });
// member blocked from updating workflow settings // member blocked from updating workflow settings
const updateAttemptResponse = await authAgent(member) const updateAttemptResponse = await authAgent(member)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ settings: { saveManualExecutions: true }, hash: memberHash }); .send({ settings: { saveManualExecutions: true }, versionId: memberVersionId });
expect(updateAttemptResponse.status).toBe(400); expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain( expect(updateAttemptResponse.body.code).toBe(100);
'the workflow has been changed in the meantime',
);
}); });
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 owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
// owner creates and shares workflow // owner creates and shares workflow
const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); 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) await authAgent(owner)
.put(`/workflows/${id}/share`) .put(`/workflows/${id}/share`)
.send({ shareWithIds: [member.id] }); .send({ shareWithIds: [member.id] });
@ -925,23 +919,21 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
// member accesses workflow // member accesses workflow
const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); const memberGetResponse = await authAgent(member).get(`/workflows/${id}`);
const { hash: memberHash } = memberGetResponse.body.data; const { versionId: memberVersionId } = memberGetResponse.body.data;
// owner updates workflow settings // owner updates workflow settings
await authAgent(owner) await authAgent(owner)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ settings: { saveManualExecutions: true }, hash: ownerHash }); .send({ settings: { saveManualExecutions: true }, versionId: ownerVersionId });
// member blocked from updating workflow name // member blocked from updating workflow name
const updateAttemptResponse = await authAgent(member) const updateAttemptResponse = await authAgent(member)
.patch(`/workflows/${id}`) .patch(`/workflows/${id}`)
.send({ settings: { saveManualExecutions: true }, hash: memberHash }); .send({ settings: { saveManualExecutions: true }, versionId: memberVersionId });
expect(updateAttemptResponse.status).toBe(400); expect(updateAttemptResponse.status).toBe(400);
expect(updateAttemptResponse.body.message).toContain( expect(updateAttemptResponse.body.code).toBe(100);
'the workflow has been changed in the meantime',
);
}); });
}); });

View file

@ -264,7 +264,7 @@ export interface IWorkflowData {
settings?: IWorkflowSettings; settings?: IWorkflowSettings;
tags?: string[]; tags?: string[];
pinData?: IPinData; pinData?: IPinData;
hash?: string; versionId?: string;
} }
export interface IWorkflowDataUpdate { export interface IWorkflowDataUpdate {
@ -276,7 +276,7 @@ export interface IWorkflowDataUpdate {
active?: boolean; active?: boolean;
tags?: ITag[] | string[]; // string[] when store or requested, ITag[] from API response tags?: ITag[] | string[]; // string[] when store or requested, ITag[] from API response
pinData?: IPinData; pinData?: IPinData;
hash?: string; versionId?: string;
} }
export interface IWorkflowToShare extends IWorkflowDataUpdate { export interface IWorkflowToShare extends IWorkflowDataUpdate {
@ -313,7 +313,7 @@ export interface IWorkflowDb {
pinData?: IPinData; pinData?: IPinData;
sharedWith?: Array<Partial<IUser>>; sharedWith?: Array<Partial<IUser>>;
ownedBy?: Partial<IUser>; ownedBy?: Partial<IUser>;
hash: string; versionId: string;
usedCredentials?: Array<Partial<ICredentialsDb>>; usedCredentials?: Array<Partial<ICredentialsDb>>;
} }

View file

@ -405,7 +405,7 @@ export default mixins(restApi, showMessage, executionHelpers, debounceHelper, wo
const tags = (data.tags || []) as ITag[]; const tags = (data.tags || []) as ITag[];
const tagIds = tags.map((tag) => tag.id); const tagIds = tags.map((tag) => tag.id);
this.workflowsStore.setWorkflowTagIds(tagIds || []); this.workflowsStore.setWorkflowTagIds(tagIds || []);
this.workflowsStore.setWorkflowHash(data.hash); this.workflowsStore.setWorkflowVersionId(data.versionId);
this.tagsStore.upsertTags(tags); this.tagsStore.upsertTags(tags);

View file

@ -104,7 +104,7 @@ export default mixins(
name: '', name: '',
sharedWith: [], sharedWith: [],
ownedBy: {} as IUser, ownedBy: {} as IUser,
hash: '', versionId: '',
}), }),
}, },
readonly: { readonly: {

View file

@ -625,11 +625,11 @@ export default mixins(
delete data.settings!.maxExecutionTimeout; delete data.settings!.maxExecutionTimeout;
this.isLoading = true; this.isLoading = true;
data.hash = this.workflowsStore.workflowHash; data.versionId = this.workflowsStore.workflowVersionId;
try { try {
const workflow = await this.restApi().updateWorkflow(this.$route.params.name, data); const workflow = await this.restApi().updateWorkflow(this.$route.params.name, data);
this.workflowsStore.setWorkflowHash(workflow.hash); this.workflowsStore.setWorkflowVersionId(workflow.versionId);
} catch (error) { } catch (error) {
this.$showError( this.$showError(
error, error,

View file

@ -418,7 +418,7 @@ export const workflowHelpers = mixins(
active: this.workflowsStore.isWorkflowActive, active: this.workflowsStore.isWorkflowActive,
settings: this.workflowsStore.workflow.settings, settings: this.workflowsStore.workflow.settings,
tags: this.workflowsStore.workflowTags, tags: this.workflowsStore.workflowTags,
hash: this.workflowsStore.workflow.hash, versionId: this.workflowsStore.workflow.versionId,
}; };
const workflowId = this.workflowsStore.workflowId; const workflowId = this.workflowsStore.workflowId;
@ -680,8 +680,8 @@ export const workflowHelpers = mixins(
if (isCurrentWorkflow) { if (isCurrentWorkflow) {
data = await this.getWorkflowDataToSave(); data = await this.getWorkflowDataToSave();
} else { } else {
const { hash } = await this.restApi().getWorkflow(workflowId); const { versionId } = await this.restApi().getWorkflow(workflowId);
data.hash = hash; data.versionId = versionId;
} }
if (active !== undefined) { if (active !== undefined) {
@ -689,7 +689,7 @@ export const workflowHelpers = mixins(
} }
const workflow = await this.restApi().updateWorkflow(workflowId, data); const workflow = await this.restApi().updateWorkflow(workflowId, data);
this.workflowsStore.setWorkflowHash(workflow.hash); this.workflowsStore.setWorkflowVersionId(workflow.versionId);
if (isCurrentWorkflow) { if (isCurrentWorkflow) {
this.workflowsStore.setActive(!!workflow.active); this.workflowsStore.setActive(!!workflow.active);
@ -724,10 +724,10 @@ export const workflowHelpers = mixins(
workflowDataRequest.tags = tags; workflowDataRequest.tags = tags;
} }
workflowDataRequest.hash = this.workflowsStore.workflowHash; workflowDataRequest.versionId = this.workflowsStore.workflowVersionId;
const workflowData = await this.restApi().updateWorkflow(currentWorkflow, workflowDataRequest, forceSave); const workflowData = await this.restApi().updateWorkflow(currentWorkflow, workflowDataRequest, forceSave);
this.workflowsStore.setWorkflowHash(workflowData.hash); this.workflowsStore.setWorkflowVersionId(workflowData.versionId);
if (name) { if (name) {
this.workflowsStore.setWorkflowName({newName: workflowData.name, setStateDirty: false}); this.workflowsStore.setWorkflowName({newName: workflowData.name, setStateDirty: false});
@ -747,7 +747,7 @@ export const workflowHelpers = mixins(
} catch (error) { } catch (error) {
this.uiStore.removeActiveAction('workflowSaving'); 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( const overwrite = await this.confirmMessage(
this.$locale.baseText('workflows.concurrentChanges.confirmMessage.message'), this.$locale.baseText('workflows.concurrentChanges.confirmMessage.message'),
this.$locale.baseText('workflows.concurrentChanges.confirmMessage.title'), this.$locale.baseText('workflows.concurrentChanges.confirmMessage.title'),
@ -810,7 +810,7 @@ export const workflowHelpers = mixins(
const workflowData = await this.restApi().createNewWorkflow(workflowDataRequest); const workflowData = await this.restApi().createNewWorkflow(workflowDataRequest);
this.workflowsStore.addWorkflow(workflowData); this.workflowsStore.addWorkflow(workflowData);
this.workflowsStore.setWorkflowHash(workflowData.hash); this.workflowsStore.setWorkflowVersionId(workflowData.versionId);
if (this.settingsStore.isEnterpriseFeatureEnabled(EnterpriseEditionFeature.WorkflowSharing) && this.usersStore.currentUser) { if (this.settingsStore.isEnterpriseFeatureEnabled(EnterpriseEditionFeature.WorkflowSharing) && this.usersStore.currentUser) {
this.workflowsEEStore.setWorkflowOwnedBy({ workflowId: workflowData.id, ownedBy: this.usersStore.currentUser }); this.workflowsEEStore.setWorkflowOwnedBy({ workflowId: workflowData.id, ownedBy: this.usersStore.currentUser });

View file

@ -68,7 +68,7 @@ const createEmptyWorkflow = (): IWorkflowDb => ({
settings: {}, settings: {},
tags: [], tags: [],
pinData: {}, pinData: {},
hash: '', versionId: '',
}); });
export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
@ -97,8 +97,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
workflowId(): string { workflowId(): string {
return this.workflow.id; return this.workflow.id;
}, },
workflowHash(): string | undefined { workflowVersionId(): string | undefined {
return this.workflow.hash; return this.workflow.versionId;
}, },
workflowSettings() : IWorkflowSettings { workflowSettings() : IWorkflowSettings {
if (this.workflow.settings === undefined) { if (this.workflow.settings === undefined) {
@ -287,8 +287,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
} }
}, },
setWorkflowHash(hash: string): void { setWorkflowVersionId(versionId: string): void {
this.workflow.hash = hash; this.workflow.versionId = versionId;
}, },
// replace invalid credentials in workflow // replace invalid credentials in workflow

View file

@ -834,7 +834,7 @@ export default mixins(
this.workflowsStore.setWorkflowName({ newName: data.name, setStateDirty: false }); this.workflowsStore.setWorkflowName({ newName: data.name, setStateDirty: false });
this.workflowsStore.setWorkflowSettings(data.settings || {}); this.workflowsStore.setWorkflowSettings(data.settings || {});
this.workflowsStore.setWorkflowPinData(data.pinData || {}); this.workflowsStore.setWorkflowPinData(data.pinData || {});
this.workflowsStore.setWorkflowHash(data.hash); this.workflowsStore.setWorkflowVersionId(data.versionId);
if (data.ownedBy) { if (data.ownedBy) {
this.workflowsEEStore.setWorkflowOwnedBy({ this.workflowsEEStore.setWorkflowOwnedBy({
@ -3369,6 +3369,7 @@ export default mixins(
dataPinningEventBus.$off('pin-data', this.addPinDataConnections); dataPinningEventBus.$off('pin-data', this.addPinDataConnections);
dataPinningEventBus.$off('unpin-data', this.removePinDataConnections); dataPinningEventBus.$off('unpin-data', this.removePinDataConnections);
nodeViewEventBus.$off('saveWorkflow', this.saveCurrentWorkflowExternal); nodeViewEventBus.$off('saveWorkflow', this.saveCurrentWorkflowExternal);
this.workflowsStore.setWorkflowId(PLACEHOLDER_EMPTY_WORKFLOW_ID);
}, },
destroyed() { destroyed() {
this.resetWorkspace(); this.resetWorkspace();
@ -3377,6 +3378,7 @@ export default mixins(
this.$root.$off('newWorkflow', this.newWorkflow); this.$root.$off('newWorkflow', this.newWorkflow);
this.$root.$off('importWorkflowData', this.onImportWorkflowDataEvent); this.$root.$off('importWorkflowData', this.onImportWorkflowDataEvent);
this.$root.$off('importWorkflowUrl', this.onImportWorkflowUrlEvent); this.$root.$off('importWorkflowUrl', this.onImportWorkflowUrlEvent);
this.workflowsStore.setWorkflowId(PLACEHOLDER_EMPTY_WORKFLOW_ID);
}, },
}); });
</script> </script>