Fix workflow creation on view filtered by status (#21027)
Creating a workflow on a table with with a filter on status (eg: status is "active") failed because it added the status to createOneWorkflow (in order to have the record belonging to the view) - while createOneWorkflow throwed a 400 exception when attempting to create a workflow with a status (does not correpsond to a valid behaviour). Silently stripping status rom create workflow endpoints.
This commit is contained in:
+106
@@ -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<Partial<WorkflowWorkspaceEntity>>,
|
||||
): CreateManyResolverArgs<WorkflowWorkspaceEntity> => ({
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
+91
@@ -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<WorkflowWorkspaceEntity>,
|
||||
): CreateOneResolverArgs<WorkflowWorkspaceEntity> => ({
|
||||
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');
|
||||
});
|
||||
});
|
||||
+8
-4
@@ -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<WorkflowWorkspaceEntity>,
|
||||
): Promise<CreateManyResolverArgs<WorkflowWorkspaceEntity>> {
|
||||
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,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
+5
-3
@@ -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<WorkflowWorkspaceEntity>,
|
||||
): Promise<CreateOneResolverArgs<WorkflowWorkspaceEntity>> {
|
||||
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,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
-16
@@ -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,
|
||||
);
|
||||
}
|
||||
};
|
||||
Reference in New Issue
Block a user