feat(workflow): dedicated updateWorkflowVersionTrigger mutation + close version CRUD holes (#23207)
Prerequisite for the workflow-core soft-ref migration: every
`workflowVersion` content write must go through a dedicated,
draft-guarded server mutation so it can later be wrapped in a
transactional core mirror. This closes the generic-CRUD holes that let
writes bypass that path.
## Part A - dedicated `updateWorkflowVersionTrigger` mutation
The builder saved a version's trigger through generic
`updateOneWorkflowVersion` - the only content write not going through a
dedicated mutation. Added:
- Server: `updateWorkflowVersionTrigger(input: { workflowVersionId,
trigger })` resolver +
`WorkflowVersionStepWorkspaceService.updateWorkflowVersionTrigger`,
draft-guarded via `getValidatedDraftWorkflowVersion` then
`updateWorkflowVersionStepsAndTrigger` (reuses existing write logic).
- Front: `useUpdateWorkflowVersionTrigger` now calls the dedicated
mutation instead of `useUpdateOneRecord`.
## Part B - restrict generic `updateOneWorkflowVersion`
`validateWorkflowVersionForUpdateOne` previously allowed writing
`trigger`, `position`, `workflowId` (re-parenting),
`coreWorkflowVersionId`, and let `steps: null` slip through on a draft.
It now rejects any update that sets `steps`, `trigger`, `status`,
`workflowId`, or `coreWorkflowVersionId`, or that clears the `name`,
while still allowing a plain rename. (A name-only allowlist was tried
first but blocked legitimate renames - at the pre-hook the generic
update payload is not single-key - so it was replaced by this denylist,
verified live.)
## Part C - close the destroy/restore hole
`workflowVersion` had no `destroyOne/destroyMany/restoreOne/restoreMany`
query hooks, so a caller with object permission could hard-destroy any
version (including active) or resurrect one with no validation. Added
pre-hooks that forbid all four via the API ("Method not allowed"),
matching the existing forbidden generic mutations (`createOne`,
`deleteMany`, ...). Rationale: there is no legitimate API use for
standalone version destroy/restore - retention purging happens through
the trash-cleanup cron (internal, not hook-gated) and restore happens
through the workflow-restore cascade or create-draft-from-version.
## Tests
Integration specs that set a trigger through the generic mutation were
migrated to the new `updateWorkflowVersionTrigger` mutation (new
`update-workflow-version-trigger.util.ts`). Unit test for the front hook
updated.
## Verification
- `twenty-server` + `twenty-front` typecheck: clean.
- oxlint + oxfmt on all changed files: clean.
- `graphql.ts` regenerated for the new mutation; its types match the
server DTOs exactly. Local `graphql:generate` introspects a running
server, so it only succeeds against a server built from this branch - CI
regenerates against the PR server and verifies.
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/23207?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
---------
Co-authored-by: prastoin <paul@twenty.com>
This commit is contained in:
+11
@@ -0,0 +1,11 @@
|
||||
import { gql } from '@apollo/client';
|
||||
|
||||
export const UPDATE_WORKFLOW_VERSION_TRIGGER = gql`
|
||||
mutation UpdateWorkflowVersionTrigger(
|
||||
$input: UpdateWorkflowVersionTriggerInput!
|
||||
) {
|
||||
updateWorkflowVersionTrigger(input: $input) {
|
||||
trigger
|
||||
}
|
||||
}
|
||||
`;
|
||||
+51
-16
@@ -3,14 +3,38 @@ import { useUpdateWorkflowVersionTrigger } from '@/workflow/workflow-trigger/hoo
|
||||
import { act, renderHook } from '@testing-library/react';
|
||||
import { TRIGGER_STEP_ID } from 'twenty-shared/workflow';
|
||||
|
||||
const mockUpdateOneRecord = jest.fn();
|
||||
const mockMutate = jest.fn();
|
||||
const mockGetUpdatableWorkflowVersion = jest.fn();
|
||||
const mockGetRecordFromCache = jest.fn();
|
||||
const mockMarkStepForRecomputation = jest.fn();
|
||||
const mockEnqueueErrorSnackBar = jest.fn();
|
||||
|
||||
jest.mock('@/object-record/hooks/useUpdateOneRecord', () => ({
|
||||
useUpdateOneRecord: jest.fn(() => ({
|
||||
updateOneRecord: mockUpdateOneRecord,
|
||||
})),
|
||||
jest.mock('@/object-metadata/hooks/useApolloCoreClient', () => ({
|
||||
useApolloCoreClient: () => ({ cache: {} }),
|
||||
}));
|
||||
|
||||
jest.mock('@/object-metadata/hooks/useObjectMetadataItems', () => ({
|
||||
useObjectMetadataItems: () => ({ objectMetadataItems: [] }),
|
||||
}));
|
||||
|
||||
jest.mock('@/object-metadata/hooks/useObjectMetadataItem', () => ({
|
||||
useObjectMetadataItem: () => ({ objectMetadataItem: {} }),
|
||||
}));
|
||||
|
||||
jest.mock('@/object-record/hooks/useObjectPermissions', () => ({
|
||||
useObjectPermissions: () => ({ objectPermissionsByObjectMetadataId: {} }),
|
||||
}));
|
||||
|
||||
jest.mock('@/ui/feedback/snack-bar-manager/hooks/useSnackBar', () => ({
|
||||
useSnackBar: () => ({ enqueueErrorSnackBar: mockEnqueueErrorSnackBar }),
|
||||
}));
|
||||
|
||||
jest.mock('@/object-record/cache/hooks/useGetRecordFromCache', () => ({
|
||||
useGetRecordFromCache: () => mockGetRecordFromCache,
|
||||
}));
|
||||
|
||||
jest.mock('@/object-record/cache/utils/updateRecordFromCache', () => ({
|
||||
updateRecordFromCache: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('@/workflow/hooks/useGetUpdatableWorkflowVersionOrThrow', () => ({
|
||||
@@ -25,6 +49,10 @@ jest.mock('@/workflow/workflow-variables/hooks/useStepsOutputSchema', () => ({
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('@apollo/client/react', () => ({
|
||||
useMutation: () => [mockMutate],
|
||||
}));
|
||||
|
||||
describe('useUpdateWorkflowVersionTrigger', () => {
|
||||
const trigger: WorkflowTrigger = {
|
||||
name: 'Company created',
|
||||
@@ -38,9 +66,13 @@ describe('useUpdateWorkflowVersionTrigger', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockMutate.mockResolvedValue({
|
||||
data: { updateWorkflowVersionTrigger: { trigger } },
|
||||
});
|
||||
mockGetRecordFromCache.mockReturnValue(undefined);
|
||||
});
|
||||
|
||||
it('updates the trigger and marks it for recomputation for frontend-computed types', async () => {
|
||||
it('updates the trigger via the dedicated mutation and marks it for recomputation', async () => {
|
||||
mockGetUpdatableWorkflowVersion.mockResolvedValue('version-id');
|
||||
|
||||
const { result } = renderHook(() => useUpdateWorkflowVersionTrigger());
|
||||
@@ -50,17 +82,20 @@ describe('useUpdateWorkflowVersionTrigger', () => {
|
||||
});
|
||||
|
||||
expect(mockGetUpdatableWorkflowVersion).toHaveBeenCalled();
|
||||
expect(mockMutate).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
variables: {
|
||||
input: {
|
||||
workflowVersionId: 'version-id',
|
||||
trigger,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(mockMarkStepForRecomputation).toHaveBeenCalledWith({
|
||||
stepId: TRIGGER_STEP_ID,
|
||||
workflowVersionId: 'version-id',
|
||||
});
|
||||
expect(mockUpdateOneRecord).toHaveBeenCalledWith({
|
||||
idToUpdate: 'version-id',
|
||||
objectNameSingular: 'workflowVersion',
|
||||
updateOneRecordInput: {
|
||||
trigger,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('marks for recomputation for all trigger types', async () => {
|
||||
@@ -70,14 +105,14 @@ describe('useUpdateWorkflowVersionTrigger', () => {
|
||||
mockMarkStepForRecomputation.mockClear();
|
||||
mockGetUpdatableWorkflowVersion.mockResolvedValue('version-id');
|
||||
|
||||
const testTrigger: WorkflowTrigger = {
|
||||
const testTrigger = {
|
||||
name: `${triggerType} Trigger`,
|
||||
type: triggerType as any,
|
||||
type: triggerType,
|
||||
settings: {
|
||||
outputSchema: {},
|
||||
},
|
||||
nextStepIds: [],
|
||||
};
|
||||
} as unknown as WorkflowTrigger;
|
||||
|
||||
const { result } = renderHook(() => useUpdateWorkflowVersionTrigger());
|
||||
|
||||
|
||||
+69
-10
@@ -1,34 +1,93 @@
|
||||
import { CoreObjectNameSingular } from 'twenty-shared/types';
|
||||
import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord';
|
||||
import { useApolloCoreClient } from '@/object-metadata/hooks/useApolloCoreClient';
|
||||
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
|
||||
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems';
|
||||
import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache';
|
||||
import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordFromCache';
|
||||
import { useObjectPermissions } from '@/object-record/hooks/useObjectPermissions';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
import { UPDATE_WORKFLOW_VERSION_TRIGGER } from '@/workflow/graphql/mutations/updateWorkflowVersionTrigger';
|
||||
import { useGetUpdatableWorkflowVersionOrThrow } from '@/workflow/hooks/useGetUpdatableWorkflowVersionOrThrow';
|
||||
import { type WorkflowTrigger } from '@/workflow/types/Workflow';
|
||||
|
||||
import {
|
||||
type WorkflowTrigger,
|
||||
type WorkflowVersion,
|
||||
} from '@/workflow/types/Workflow';
|
||||
import { useStepsOutputSchema } from '@/workflow/workflow-variables/hooks/useStepsOutputSchema';
|
||||
import { useMutation } from '@apollo/client/react';
|
||||
import { CoreObjectNameSingular } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { TRIGGER_STEP_ID } from 'twenty-shared/workflow';
|
||||
import {
|
||||
type UpdateWorkflowVersionTriggerMutation,
|
||||
type UpdateWorkflowVersionTriggerMutationVariables,
|
||||
} from '~/generated/graphql';
|
||||
|
||||
export const useUpdateWorkflowVersionTrigger = () => {
|
||||
const { updateOneRecord: updateOneWorkflowVersion } = useUpdateOneRecord();
|
||||
const apolloCoreClient = useApolloCoreClient();
|
||||
const { objectMetadataItems } = useObjectMetadataItems();
|
||||
const { objectPermissionsByObjectMetadataId } = useObjectPermissions();
|
||||
const { enqueueErrorSnackBar } = useSnackBar();
|
||||
|
||||
const { getUpdatableWorkflowVersion } =
|
||||
useGetUpdatableWorkflowVersionOrThrow();
|
||||
|
||||
const { markStepForRecomputation } = useStepsOutputSchema();
|
||||
|
||||
const { objectMetadataItem } = useObjectMetadataItem({
|
||||
objectNameSingular: CoreObjectNameSingular.WorkflowVersion,
|
||||
});
|
||||
const getRecordFromCache = useGetRecordFromCache({
|
||||
objectNameSingular: CoreObjectNameSingular.WorkflowVersion,
|
||||
});
|
||||
|
||||
const [mutate] = useMutation<
|
||||
UpdateWorkflowVersionTriggerMutation,
|
||||
UpdateWorkflowVersionTriggerMutationVariables
|
||||
>(UPDATE_WORKFLOW_VERSION_TRIGGER, {
|
||||
client: apolloCoreClient,
|
||||
});
|
||||
|
||||
const updateTrigger = async (updatedTrigger: WorkflowTrigger) => {
|
||||
const workflowVersionId = await getUpdatableWorkflowVersion();
|
||||
|
||||
await updateOneWorkflowVersion({
|
||||
objectNameSingular: CoreObjectNameSingular.WorkflowVersion,
|
||||
idToUpdate: workflowVersionId,
|
||||
updateOneRecordInput: {
|
||||
trigger: updatedTrigger,
|
||||
const { data } = await mutate({
|
||||
variables: {
|
||||
input: {
|
||||
workflowVersionId,
|
||||
trigger: updatedTrigger,
|
||||
},
|
||||
},
|
||||
onError: (error) => {
|
||||
enqueueErrorSnackBar({ apolloError: error });
|
||||
},
|
||||
});
|
||||
|
||||
if (!isDefined(data?.updateWorkflowVersionTrigger)) {
|
||||
return;
|
||||
}
|
||||
|
||||
markStepForRecomputation({
|
||||
stepId: TRIGGER_STEP_ID,
|
||||
workflowVersionId,
|
||||
});
|
||||
|
||||
const cachedRecord = getRecordFromCache<WorkflowVersion>(workflowVersionId);
|
||||
if (!isDefined(cachedRecord)) {
|
||||
return;
|
||||
}
|
||||
|
||||
updateRecordFromCache({
|
||||
objectMetadataItems,
|
||||
objectMetadataItem,
|
||||
cache: apolloCoreClient.cache,
|
||||
record: {
|
||||
...cachedRecord,
|
||||
trigger: updatedTrigger,
|
||||
},
|
||||
recordGqlFields: {
|
||||
trigger: true,
|
||||
},
|
||||
objectPermissionsByObjectMetadataId,
|
||||
});
|
||||
};
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user