diff --git a/packages/twenty-server/src/modules/workflow/common/query-hooks/__tests__/workflow-create-many.pre-query.hook.spec.ts b/packages/twenty-server/src/modules/workflow/common/query-hooks/__tests__/workflow-create-many.pre-query.hook.spec.ts new file mode 100644 index 0000000000..4315a49a60 --- /dev/null +++ b/packages/twenty-server/src/modules/workflow/common/query-hooks/__tests__/workflow-create-many.pre-query.hook.spec.ts @@ -0,0 +1,106 @@ +import { type CreateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; +import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; +import { WorkflowCreateManyPreQueryHook } from 'src/modules/workflow/common/query-hooks/workflow-create-many.pre-query.hook'; +import { + type WorkflowWorkspaceEntity, + WorkflowStatus, +} from 'src/modules/workflow/common/standard-objects/workflow.workspace-entity'; + +describe('WorkflowCreateManyPreQueryHook', () => { + const hook = new WorkflowCreateManyPreQueryHook(); + const authContext = {} as WorkspaceAuthContext; + const objectName = 'workflow'; + + const buildPayload = ( + data: Array>, + ): CreateManyResolverArgs => ({ + data: data as WorkflowWorkspaceEntity[], + }); + + // Regression guard: prior to this hook silently stripping `statuses`, passing + // any non-empty `statuses` array to `createManyWorkflows` made the resolver + // throw `WorkflowQueryValidationException` ("Statuses cannot be set + // manually."), which surfaced to clients as a 400 Bad Request and broke + // workflow creation in prod whenever the client forwarded a default value + // computed from the multi-select field metadata. + it('should not respond a 400 when statuses are passed in any entry of the payload (regression)', async () => { + await expect( + hook.execute( + authContext, + objectName, + buildPayload([ + { name: 'Workflow 1', statuses: [WorkflowStatus.ACTIVE] }, + { name: 'Workflow 2' }, + { name: 'Workflow 3', statuses: [WorkflowStatus.DRAFT] }, + ]), + ), + ).resolves.toBeDefined(); + }); + + it('should strip statuses from every entry that has it set', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload([ + { name: 'Workflow 1', statuses: [WorkflowStatus.ACTIVE] }, + { name: 'Workflow 2', statuses: [WorkflowStatus.DRAFT] }, + ]), + ); + + expect(result.data).toHaveLength(2); + expect(result.data[0]).not.toHaveProperty('statuses'); + expect(result.data[0].name).toBe('Workflow 1'); + expect(result.data[1]).not.toHaveProperty('statuses'); + expect(result.data[1].name).toBe('Workflow 2'); + }); + + it('should strip statuses when it is an empty array', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload([{ name: 'Workflow 1', statuses: [] }]), + ); + + expect(result.data[0]).not.toHaveProperty('statuses'); + expect(result.data[0].name).toBe('Workflow 1'); + }); + + it('should leave entries untouched when statuses is not set', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload([{ name: 'Workflow 1' }, { name: 'Workflow 2' }]), + ); + + expect(result.data[0]).not.toHaveProperty('statuses'); + expect(result.data[0].name).toBe('Workflow 1'); + expect(result.data[1]).not.toHaveProperty('statuses'); + expect(result.data[1].name).toBe('Workflow 2'); + }); + + it('should preserve other top-level payload fields (e.g. upsert)', async () => { + const result = await hook.execute(authContext, objectName, { + data: [ + { + name: 'Workflow 1', + statuses: [WorkflowStatus.ACTIVE], + } as WorkflowWorkspaceEntity, + ], + upsert: true, + }); + + expect(result.upsert).toBe(true); + expect(result.data[0]).not.toHaveProperty('statuses'); + expect(result.data[0].name).toBe('Workflow 1'); + }); + + it('should return an empty data array unchanged', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload([]), + ); + + expect(result.data).toEqual([]); + }); +}); diff --git a/packages/twenty-server/src/modules/workflow/common/query-hooks/__tests__/workflow-create-one.pre-query.hook.spec.ts b/packages/twenty-server/src/modules/workflow/common/query-hooks/__tests__/workflow-create-one.pre-query.hook.spec.ts new file mode 100644 index 0000000000..f26dff0fe2 --- /dev/null +++ b/packages/twenty-server/src/modules/workflow/common/query-hooks/__tests__/workflow-create-one.pre-query.hook.spec.ts @@ -0,0 +1,91 @@ +import { type CreateOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; +import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; +import { WorkflowCreateOnePreQueryHook } from 'src/modules/workflow/common/query-hooks/workflow-create-one.pre-query.hook'; +import { + type WorkflowWorkspaceEntity, + WorkflowStatus, +} from 'src/modules/workflow/common/standard-objects/workflow.workspace-entity'; + +describe('WorkflowCreateOnePreQueryHook', () => { + const hook = new WorkflowCreateOnePreQueryHook(); + const authContext = {} as WorkspaceAuthContext; + const objectName = 'workflow'; + + const buildPayload = ( + data: Partial, + ): CreateOneResolverArgs => ({ + data: data as WorkflowWorkspaceEntity, + }); + + // Regression guard: prior to this hook silently stripping `statuses`, passing + // any non-empty `statuses` array to `createOneWorkflow` made the resolver + // throw `WorkflowQueryValidationException` ("Statuses cannot be set + // manually."), which surfaced to clients as a 400 Bad Request and broke + // workflow creation in prod whenever the client forwarded a default value + // computed from the multi-select field metadata. + it('should not respond a 400 when statuses are passed in the payload (regression)', async () => { + await expect( + hook.execute( + authContext, + objectName, + buildPayload({ + name: 'My workflow', + statuses: [WorkflowStatus.ACTIVE, WorkflowStatus.DRAFT], + }), + ), + ).resolves.toBeDefined(); + }); + + it('should strip statuses from payload data when statuses is set', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload({ + name: 'My workflow', + statuses: [WorkflowStatus.ACTIVE], + }), + ); + + expect(result.data).not.toHaveProperty('statuses'); + expect(result.data.name).toBe('My workflow'); + }); + + it('should strip statuses from payload data when statuses is an empty array', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload({ + name: 'My workflow', + statuses: [], + }), + ); + + expect(result.data).not.toHaveProperty('statuses'); + expect(result.data.name).toBe('My workflow'); + }); + + it('should leave payload data untouched when statuses is not set', async () => { + const result = await hook.execute( + authContext, + objectName, + buildPayload({ name: 'My workflow' }), + ); + + expect(result.data).not.toHaveProperty('statuses'); + expect(result.data.name).toBe('My workflow'); + }); + + it('should preserve other top-level payload fields (e.g. upsert)', async () => { + const result = await hook.execute(authContext, objectName, { + data: { + name: 'My workflow', + statuses: [WorkflowStatus.DRAFT], + } as WorkflowWorkspaceEntity, + upsert: true, + }); + + expect(result.upsert).toBe(true); + expect(result.data).not.toHaveProperty('statuses'); + expect(result.data.name).toBe('My workflow'); + }); +}); diff --git a/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-many.pre-query.hook.ts b/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-many.pre-query.hook.ts index 8f1a564610..423704a6ca 100644 --- a/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-many.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-many.pre-query.hook.ts @@ -4,7 +4,6 @@ import { type CreateManyResolverArgs } from 'src/engine/api/graphql/workspace-re import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; import { type WorkflowWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow.workspace-entity'; -import { assertWorkflowStatusesNotSetOrEmpty } from 'src/modules/workflow/common/utils/assert-workflow-statuses-not-set-or-empty'; @WorkspaceQueryHook(`workflow.createMany`) export class WorkflowCreateManyPreQueryHook implements WorkspacePreQueryHookInstance { @@ -13,10 +12,15 @@ export class WorkflowCreateManyPreQueryHook implements WorkspacePreQueryHookInst _objectName: string, payload: CreateManyResolverArgs, ): Promise> { - payload.data.forEach((workflow) => { - assertWorkflowStatusesNotSetOrEmpty(workflow.statuses); + const sanitizedData = payload.data.map((workflow) => { + const { statuses: _statuses, ...workflowWithoutStatuses } = workflow; // silent not to break creation from view with filter + + return workflowWithoutStatuses as WorkflowWorkspaceEntity; }); - return payload; + return { + ...payload, + data: sanitizedData, + }; } } diff --git a/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-one.pre-query.hook.ts index 6049ec1d76..bc0a3df2f5 100644 --- a/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-one.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-create-one.pre-query.hook.ts @@ -4,7 +4,6 @@ import { type CreateOneResolverArgs } from 'src/engine/api/graphql/workspace-res import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; import { type WorkflowWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow.workspace-entity'; -import { assertWorkflowStatusesNotSetOrEmpty } from 'src/modules/workflow/common/utils/assert-workflow-statuses-not-set-or-empty'; @WorkspaceQueryHook(`workflow.createOne`) export class WorkflowCreateOnePreQueryHook implements WorkspacePreQueryHookInstance { @@ -13,8 +12,11 @@ export class WorkflowCreateOnePreQueryHook implements WorkspacePreQueryHookInsta _objectName: string, payload: CreateOneResolverArgs, ): Promise> { - assertWorkflowStatusesNotSetOrEmpty(payload.data.statuses); + const { statuses: _statuses, ...dataWithoutStatuses } = payload.data; // silent not to break creation from view with filter - return payload; + return { + ...payload, + data: dataWithoutStatuses as WorkflowWorkspaceEntity, + }; } } diff --git a/packages/twenty-server/src/modules/workflow/common/utils/assert-workflow-statuses-not-set-or-empty.ts b/packages/twenty-server/src/modules/workflow/common/utils/assert-workflow-statuses-not-set-or-empty.ts deleted file mode 100644 index dd4a79ca5b..0000000000 --- a/packages/twenty-server/src/modules/workflow/common/utils/assert-workflow-statuses-not-set-or-empty.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { - WorkflowQueryValidationException, - WorkflowQueryValidationExceptionCode, -} from 'src/modules/workflow/common/exceptions/workflow-query-validation.exception'; -import { type WorkflowStatus } from 'src/modules/workflow/common/standard-objects/workflow.workspace-entity'; - -export const assertWorkflowStatusesNotSetOrEmpty = ( - statuses?: WorkflowStatus[] | null, -) => { - if (statuses && statuses.length > 0) { - throw new WorkflowQueryValidationException( - 'Statuses cannot be set manually.', - WorkflowQueryValidationExceptionCode.FORBIDDEN, - ); - } -};