From c4a94ed902f0200f4c20bcb78cd7e4f2a03f974b Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Mon, 10 Mar 2025 15:59:39 +0100 Subject: [PATCH 1/6] refactor: Do not pass response object to throw validation error --- .../backend/src/controllers/api/v1/flows/import-flow.js | 6 +----- packages/backend/src/helpers/import-flow.js | 8 +++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/controllers/api/v1/flows/import-flow.js b/packages/backend/src/controllers/api/v1/flows/import-flow.js index c64d0f9e..4fbdea54 100644 --- a/packages/backend/src/controllers/api/v1/flows/import-flow.js +++ b/packages/backend/src/controllers/api/v1/flows/import-flow.js @@ -2,11 +2,7 @@ import { renderObject } from '../../../../helpers/renderer.js'; import importFlow from '../../../../helpers/import-flow.js'; export default async function importFlowController(request, response) { - const flow = await importFlow( - request.currentUser, - flowParams(request), - response - ); + const flow = await importFlow(request.currentUser, flowParams(request)); return renderObject(response, flow, { status: 201 }); } diff --git a/packages/backend/src/helpers/import-flow.js b/packages/backend/src/helpers/import-flow.js index ae89559a..da8faa6e 100644 --- a/packages/backend/src/helpers/import-flow.js +++ b/packages/backend/src/helpers/import-flow.js @@ -1,14 +1,12 @@ import Crypto from 'crypto'; import Step from '../models/step.js'; -import { renderObjectionError } from './renderer.js'; +import { ValidationError } from 'objection'; -const importFlow = async (user, flowData, response) => { +const importFlow = async (user, flowData) => { const steps = flowData.steps || []; - // Validation: the first step must be a trigger if (!steps.length || steps[0].type !== 'trigger') { - return renderObjectionError(response, { - statusCode: 422, + throw new ValidationError({ type: 'ValidationError', data: { steps: [{ message: 'The first step must be a trigger!' }], From 75a7a068f31d2a54e92b05c177ab1aeea0d0dd9a Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Mon, 10 Mar 2025 16:01:41 +0100 Subject: [PATCH 2/6] refactor: Use import flow as static flow model method --- packages/backend/src/controllers/api/v1/flows/import-flow.js | 4 ++-- packages/backend/src/models/flow.js | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/controllers/api/v1/flows/import-flow.js b/packages/backend/src/controllers/api/v1/flows/import-flow.js index 4fbdea54..5b6a2383 100644 --- a/packages/backend/src/controllers/api/v1/flows/import-flow.js +++ b/packages/backend/src/controllers/api/v1/flows/import-flow.js @@ -1,8 +1,8 @@ import { renderObject } from '../../../../helpers/renderer.js'; -import importFlow from '../../../../helpers/import-flow.js'; +import Flow from '../../../../models/flow.js'; export default async function importFlowController(request, response) { - const flow = await importFlow(request.currentUser, flowParams(request)); + const flow = await Flow.import(request.currentUser, flowParams(request)); return renderObject(response, flow, { status: 201 }); } diff --git a/packages/backend/src/models/flow.js b/packages/backend/src/models/flow.js index bd4a8058..51a90a3a 100644 --- a/packages/backend/src/models/flow.js +++ b/packages/backend/src/models/flow.js @@ -9,6 +9,7 @@ import globalVariable from '../helpers/global-variable.js'; import logger from '../helpers/logger.js'; import Telemetry from '../helpers/telemetry/index.js'; import exportFlow from '../helpers/export-flow.js'; +import importFlow from '../helpers/import-flow.js'; import flowQueue from '../queues/flow.js'; import { REMOVE_AFTER_30_DAYS_OR_150_JOBS, @@ -39,6 +40,10 @@ class Flow extends Base { }, }; + static async import(user, flowData) { + return importFlow(user, flowData); + } + static relationMappings = () => ({ steps: { relation: Base.HasManyRelation, From ac1f6440716932cec4111ed21d94146a00eff91f Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Mon, 10 Mar 2025 16:08:52 +0100 Subject: [PATCH 3/6] feat: Implement createEmptyFlow method for user model --- .../controllers/api/v1/flows/create-flow.js | 6 +---- packages/backend/src/models/flow.js | 8 +++---- packages/backend/src/models/user.js | 8 +++++++ packages/backend/src/models/user.test.js | 22 +++++++++++++++++++ 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/controllers/api/v1/flows/create-flow.js b/packages/backend/src/controllers/api/v1/flows/create-flow.js index 39d12f33..16e6bbe3 100644 --- a/packages/backend/src/controllers/api/v1/flows/create-flow.js +++ b/packages/backend/src/controllers/api/v1/flows/create-flow.js @@ -1,11 +1,7 @@ import { renderObject } from '../../../../helpers/renderer.js'; export default async (request, response) => { - const flow = await request.currentUser.$relatedQuery('flows').insertAndFetch({ - name: 'Name your flow', - }); - - await flow.createInitialSteps(); + const flow = await request.currentUser.createEmptyFlow(); renderObject(response, flow, { status: 201 }); }; diff --git a/packages/backend/src/models/flow.js b/packages/backend/src/models/flow.js index 51a90a3a..9c05868b 100644 --- a/packages/backend/src/models/flow.js +++ b/packages/backend/src/models/flow.js @@ -40,10 +40,6 @@ class Flow extends Base { }, }; - static async import(user, flowData) { - return importFlow(user, flowData); - } - static relationMappings = () => ({ steps: { relation: Base.HasManyRelation, @@ -104,6 +100,10 @@ class Flow extends Base { }, }); + static async import(user, flowData) { + return importFlow(user, flowData); + } + static async populateStatusProperty(flows) { const referenceFlow = flows[0]; diff --git a/packages/backend/src/models/user.js b/packages/backend/src/models/user.js index cb1e2156..2596318c 100644 --- a/packages/backend/src/models/user.js +++ b/packages/backend/src/models/user.js @@ -675,6 +675,14 @@ class User extends Base { } } + async createEmptyFlow() { + const flow = await this.$relatedQuery('flows').insertAndFetch({ + name: 'Name your flow', + }); + + return await flow.createInitialSteps(); + } + async $beforeInsert(queryContext) { await super.$beforeInsert(queryContext); diff --git a/packages/backend/src/models/user.test.js b/packages/backend/src/models/user.test.js index 4eae81c0..6f1a61a4 100644 --- a/packages/backend/src/models/user.test.js +++ b/packages/backend/src/models/user.test.js @@ -1507,6 +1507,28 @@ describe('User model', () => { }); }); + describe('createEmptyFlow', () => { + it('should create a flow with default name', async () => { + const user = await createUser(); + const flow = await user.createEmptyFlow(); + + expect(flow.name).toBe('Name your flow'); + expect(flow.userId).toBe(user.id); + }); + + it('should call createInitialSteps on the created flow', async () => { + const user = await createUser(); + const createInitialStepsSpy = vi.spyOn( + Flow.prototype, + 'createInitialSteps' + ); + + await user.createEmptyFlow(); + + expect(createInitialStepsSpy).toHaveBeenCalledOnce(); + }); + }); + describe('$beforeInsert', () => { it('should call super.$beforeInsert', async () => { const superBeforeInsertSpy = vi From 8246adec031e8c4ed04cca4ff1a1830115b4d6c2 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Mon, 10 Mar 2025 16:13:52 +0100 Subject: [PATCH 4/6] fix: User model tests by returning flow explicitly --- packages/backend/src/models/user.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/models/user.js b/packages/backend/src/models/user.js index 2596318c..bcf70754 100644 --- a/packages/backend/src/models/user.js +++ b/packages/backend/src/models/user.js @@ -680,7 +680,9 @@ class User extends Base { name: 'Name your flow', }); - return await flow.createInitialSteps(); + await flow.createInitialSteps(); + + return flow; } async $beforeInsert(queryContext) { From cf80ba28eb077e39b7834394bca224e32bcf442b Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Mon, 10 Mar 2025 16:26:10 +0100 Subject: [PATCH 5/6] feat: Add create flow from template method to user model --- packages/backend/src/models/user.js | 11 ++++++++ packages/backend/src/models/user.test.js | 35 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/packages/backend/src/models/user.js b/packages/backend/src/models/user.js index bcf70754..27bbe8b2 100644 --- a/packages/backend/src/models/user.js +++ b/packages/backend/src/models/user.js @@ -22,6 +22,7 @@ import Step from './step.js'; import Subscription from './subscription.ee.js'; import Folder from './folder.js'; import UsageData from './usage-data.ee.js'; +import Template from './template.ee.js'; import Billing from '../helpers/billing/index.ee.js'; import NotAuthorizedError from '../errors/not-authorized.js'; @@ -685,6 +686,16 @@ class User extends Base { return flow; } + async createFlowFromTemplate(templateId) { + const template = await Template.query() + .findById(templateId) + .throwIfNotFound(); + + const flow = await Flow.import(this, template.flowData); + + return flow; + } + async $beforeInsert(queryContext) { await super.$beforeInsert(queryContext); diff --git a/packages/backend/src/models/user.test.js b/packages/backend/src/models/user.test.js index 6f1a61a4..7a12abb7 100644 --- a/packages/backend/src/models/user.test.js +++ b/packages/backend/src/models/user.test.js @@ -34,7 +34,9 @@ import { createExecution } from '../../test/factories/execution.js'; import { createSubscription } from '../../test/factories/subscription.js'; import { createUsageData } from '../../test/factories/usage-data.js'; import { createFolder } from '../../test/factories/folder.js'; +import { createTemplate } from '../../test/factories/template.js'; import Billing from '../helpers/billing/index.ee.js'; +import Template from './template.ee.js'; describe('User model', () => { it('tableName should return correct name', () => { @@ -1529,6 +1531,39 @@ describe('User model', () => { }); }); + describe('createFlowFromTemplate', () => { + let user, template; + + beforeEach(async () => { + user = await createUser(); + template = await createTemplate(); + }); + + it('should throw an error if template is not found', async () => { + const nonExistentTemplateId = Crypto.randomUUID(); + + await expect( + user.createFlowFromTemplate(nonExistentTemplateId) + ).rejects.toThrow('NotFoundError'); + }); + + it('should call Flow.import with the correct parameters', async () => { + vi.spyOn(Template.query(), 'findById').mockImplementation(() => ({ + throwIfNotFound: () => template, + })); + + const importSpy = vi.spyOn(Flow, 'import').mockResolvedValue({ + id: Crypto.randomUUID(), + name: template.flowData.name, + steps: [], + }); + + await user.createFlowFromTemplate(template.id); + + expect(importSpy).toHaveBeenCalledWith(user, template.flowData); + }); + }); + describe('$beforeInsert', () => { it('should call super.$beforeInsert', async () => { const superBeforeInsertSpy = vi From 9ebba9427b064789adf317ae407611d2aad0ffc1 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Mon, 10 Mar 2025 16:34:38 +0100 Subject: [PATCH 6/6] feat: Implement create flow from template id --- .../controllers/api/v1/flows/create-flow.js | 6 ++++- .../api/v1/flows/create-flow.test.js | 24 ++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/controllers/api/v1/flows/create-flow.js b/packages/backend/src/controllers/api/v1/flows/create-flow.js index 16e6bbe3..36c46ac5 100644 --- a/packages/backend/src/controllers/api/v1/flows/create-flow.js +++ b/packages/backend/src/controllers/api/v1/flows/create-flow.js @@ -1,7 +1,11 @@ import { renderObject } from '../../../../helpers/renderer.js'; export default async (request, response) => { - const flow = await request.currentUser.createEmptyFlow(); + const { templateId } = request.query; + + const flow = templateId + ? await request.currentUser.createFlowFromTemplate(templateId) + : await request.currentUser.createEmptyFlow(); renderObject(response, flow, { status: 201 }); }; diff --git a/packages/backend/src/controllers/api/v1/flows/create-flow.test.js b/packages/backend/src/controllers/api/v1/flows/create-flow.test.js index fb8f5635..2c55f8ef 100644 --- a/packages/backend/src/controllers/api/v1/flows/create-flow.test.js +++ b/packages/backend/src/controllers/api/v1/flows/create-flow.test.js @@ -4,6 +4,7 @@ import request from 'supertest'; import app from '../../../../app.js'; import createAuthTokenByUserId from '../../../../helpers/create-auth-token-by-user-id.js'; import { createUser } from '../../../../../test/factories/user.js'; +import { createTemplate } from '../../../../../test/factories/template.js'; import createFlowMock from '../../../../../test/mocks/rest/api/v1/flows/create-flow.js'; import { createPermission } from '../../../../../test/factories/permission.js'; @@ -17,7 +18,7 @@ describe('POST /api/v1/flows', () => { token = await createAuthTokenByUserId(currentUser.id); }); - it('should return created flow', async () => { + it('should create an empty flow when no templateId is provided', async () => { await createPermission({ action: 'create', subject: 'Flow', @@ -38,4 +39,25 @@ describe('POST /api/v1/flows', () => { expect(response.body).toMatchObject(expectedPayload); }); + + it('should create a flow from template when templateId is provided', async () => { + await createPermission({ + action: 'create', + subject: 'Flow', + roleId: currentUserRole.id, + conditions: ['isCreator'], + }); + + const template = await createTemplate({ + name: 'Sample template', + }); + + const response = await request(app) + .post('/api/v1/flows') + .query({ templateId: template.id }) + .set('Authorization', token) + .expect(201); + + expect(response.body.data.name).toBe(template.flowData.name); + }); });