diff --git a/assets/js/collaborative-editor/components/form/index.tsx b/assets/js/collaborative-editor/components/form/index.tsx index b60a7915ee..80ffcff538 100644 --- a/assets/js/collaborative-editor/components/form/index.tsx +++ b/assets/js/collaborative-editor/components/form/index.tsx @@ -26,6 +26,10 @@ const { useAppForm: useBaseAppForm } = createFormHook({ formComponents: {}, }); +export type useAppBaseFormType = ReturnType< + typeof createFormHook +>['useAppForm']; + /** * Enhanced useAppForm that automatically integrates collaborative * validation from Y.Doc @@ -52,7 +56,7 @@ const { useAppForm: useBaseAppForm } = createFormHook({ * const form = useAppForm({ defaultValues: { name: "" } }, `jobs.${jobId}`); */ export function useAppForm( - formOptions: Parameters[0], + formOptions: Parameters[0], errorPath?: string ) { const form = useBaseAppForm(formOptions); diff --git a/assets/js/collaborative-editor/hooks/useValidation.ts b/assets/js/collaborative-editor/hooks/useValidation.ts index 3a543429c0..a22942bf0f 100644 --- a/assets/js/collaborative-editor/hooks/useValidation.ts +++ b/assets/js/collaborative-editor/hooks/useValidation.ts @@ -1,6 +1,7 @@ import { useEffect } from 'react'; import { useWorkflowActions, useWorkflowState } from './useWorkflow'; +import type { useAppBaseFormType } from '../components/form'; /** * Simple type for TanStack Form instance @@ -44,7 +45,10 @@ const NO_ERRORS = {}; * - "jobs.abc-123" → job-specific errors * - "triggers.xyz-789" → trigger-specific errors */ -export function useValidation(form: FormInstance, errorPath?: string) { +export function useValidation( + form: ReturnType, + errorPath?: string +) { const { setClientErrors } = useWorkflowActions(); // Read stable errors from store (Immer provides referential stability) @@ -107,12 +111,14 @@ export function useValidation(form: FormInstance, errorPath?: string) { } }); + const isDirty = checkIfFormDirty(form); + // Write to store (debounced, with merge+dedupe) - setClientErrors(errorPath || 'workflow', clientErrors); + setClientErrors(errorPath || 'workflow', clientErrors, isDirty); }); return () => unsubscribe(); - }, [form, setClientErrors, errorPath]); + }, [form.state.isDirty, setClientErrors, errorPath]); // Inject collaborative errors into TanStack Form useEffect(() => { @@ -178,3 +184,18 @@ export function useValidation(form: FormInstance, errorPath?: string) { }); }, [collaborativeErrors, form]); } + +// why do we need this function instead of form.state.isDirty? +// form.state.isDirty doesn't work. it doesn't fallback to false after server changes have been merged into local +function checkIfFormDirty(form: ReturnType) { + const values = form.state.values as Record; + const defaultValues = form.options.defaultValues as Record; + return Object.entries(values).some(([name, value]) => { + // If defaultValues exist, compare the current value to the default value + if (defaultValues) { + return defaultValues[name] !== value; + } + // If no default values were defined, consider a field dirty if its value is not falsy + return !!value; + }); +} diff --git a/assets/js/collaborative-editor/stores/createWorkflowStore.ts b/assets/js/collaborative-editor/stores/createWorkflowStore.ts index 4222c9bc12..bf2f22ad5b 100644 --- a/assets/js/collaborative-editor/stores/createWorkflowStore.ts +++ b/assets/js/collaborative-editor/stores/createWorkflowStore.ts @@ -1179,7 +1179,13 @@ export const createWorkflowStore = () => { * @param errors - Field errors { fieldName: ["error1", "error2"] } * Empty array [] clears that field */ - const setClientErrors = (path: string, errors: Record) => { + const setClientErrors = ( + path: string, + errors: Record, + isEditing: boolean + ) => { + // Capture isEditing value at call time for the debounced execution + logger.debug('setClientErrors called (before debounce)', { path, errors, @@ -1187,106 +1193,93 @@ export const createWorkflowStore = () => { stack: new Error(), }); - // Clear any existing timeout for this path - const existingTimeout = debounceTimeouts.get(path); - if (existingTimeout) { - clearTimeout(existingTimeout); - logger.debug('setClientErrors cleared existing timeout', { path }); - } - // Set new debounced timeout - const timeoutId = setTimeout(() => { - logger.debug('setClientErrors executing (after debounce)', { - path, - errors, - }); + logger.debug('setClientErrors executing (after debounce)', { + path, + errors, + }); - if (!ydoc) { - logger.warn('Cannot set client errors: Y.Doc not connected'); - return; - } + if (!ydoc) { + logger.warn('Cannot set client errors: Y.Doc not connected'); + return; + } - const errorsMap = ydoc.getMap('errors'); - const parts = path.split('.'); - - // 1. Read current errors from Y.Doc (outside transaction) - const currentErrors = (() => { - if (parts.length === 1 || !path) { - // Top-level: "workflow" - const entityKey = path || 'workflow'; - const errors = errorsMap.get( - entityKey as 'workflow' | 'jobs' | 'triggers' | 'edges' - ) as Record | undefined; - return errors ?? {}; - } else if (parts.length === 2) { - // Entity-level: "jobs.abc-123" - const entityType = parts[0]; - const entityId = parts[1]; - - if (!entityId || !entityType) return {}; - - // Validate entity type (runtime check for path parsing) - if ( - entityType !== 'jobs' && - entityType !== 'triggers' && - entityType !== 'edges' - ) { - return {}; - } + const errorsMap = ydoc.getMap('errors'); + const parts = path.split('.'); + + // 1. Read current errors from Y.Doc (outside transaction) + const currentErrors = (() => { + if (parts.length === 1 || !path) { + // Top-level: "workflow" + const entityKey = path || 'workflow'; + const errors = errorsMap.get( + entityKey as 'workflow' | 'jobs' | 'triggers' | 'edges' + ) as Record | undefined; + return errors ?? {}; + } else if (parts.length === 2) { + // Entity-level: "jobs.abc-123" + const entityType = parts[0]; + const entityId = parts[1]; - const entityErrors = errorsMap.get(entityType); - // Type assertion needed because Y.Map.get returns unknown - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - const typedEntityErrors = entityErrors as - | Record> - | undefined; + if (!entityId || !entityType) return {}; - return typedEntityErrors?.[entityId] ?? {}; + // Validate entity type (runtime check for path parsing) + if ( + entityType !== 'jobs' && + entityType !== 'triggers' && + entityType !== 'edges' + ) { + return {}; } - return {}; - })(); - logger.debug('setClientErrors before merge', { - path, - currentErrors, - incomingErrors: errors, - }); + const entityErrors = errorsMap.get(entityType); + // Type assertion needed because Y.Map.get returns unknown + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + const typedEntityErrors = entityErrors as + | Record> + | undefined; - // 2. Use Immer to replace client errors (or clear if empty) - // Client errors REPLACE server errors for that field, not merge with them - // This ensures that when a user edits a field with server errors, - // their client validation takes precedence - const mergedErrors = produce(currentErrors, draft => { - Object.entries(errors).forEach(([fieldName, newMessages]) => { - if (newMessages.length === 0) { - // Empty array clears the field - - delete draft[fieldName]; - } else { - // Replace with client errors (deduplicate within client errors) - draft[fieldName] = Array.from(new Set(newMessages)); - } - }); - }); + return typedEntityErrors?.[entityId] ?? {}; + } + return {}; + })(); - logger.debug('setClientErrors after merge', { - path, - mergedErrors, - mergedCount: Object.keys(mergedErrors).length, - }); + logger.debug('setClientErrors before merge', { + path, + currentErrors, + incomingErrors: errors, + }); - // 3. Write to Y.Doc using setError (which checks if different before transacting) - try { - setError(path || 'workflow', mergedErrors); - } catch (error) { - logger.error('Failed to set client errors', { path, error }); - } + // 2. Use Immer to replace client errors (or clear if empty) + // Client errors REPLACE server errors for that field, not merge with them + // This ensures that when a user edits a field with server errors, + // their client validation takes precedence + const mergedErrors = isEditing + ? produce(currentErrors, draft => { + Object.entries(errors).forEach(([fieldName, newMessages]) => { + if (newMessages.length === 0) { + // Empty array clears the field + delete draft[fieldName]; + } else { + // Replace with client errors (deduplicate within client errors) + draft[fieldName] = Array.from(new Set(newMessages)); + } + }); + }) + : currentErrors; - // Clean up timeout - debounceTimeouts.delete(path); - }, 500); + logger.debug('setClientErrors after merge', { + path, + mergedErrors, + mergedCount: Object.keys(mergedErrors).length, + }); - debounceTimeouts.set(path, timeoutId); + // 3. Write to Y.Doc using setError (which checks if different before transacting) + try { + setError(path || 'workflow', mergedErrors); + } catch (error) { + logger.error('Failed to set client errors', { path, error }); + } }; // =============================================================================