Merge pull request #201 from pneumaticapp/backend/template/45972__auto_cleaning_broken_links

45972 backend [ templates ] Auto-cleaning of broken links to fields in templates

Author: EBirkenfeld

frontend/src/public/components/RichEditor/RichEditor.tsx

@@ -190,6 +190,15 @@ export const RichEditor = forwardRef<
     });
   }, []);
 
+  const replaceContent = useCallback((markdown: string): void => {
+    const editor = editorRef.current;
+    if (!editor) return;
+    applyMarkdownToEditor(editor, markdown, {
+      tag: 'history-merge',
+      templateVariables: templateVariablesRef.current,
+    });
+  }, []);
+
   useImperativeHandle(
     ref,
     () => ({
@@ -204,8 +213,9 @@ export const RichEditor = forwardRef<
         return editorRef.current ?? undefined;
       },
       clearContent,
+      replaceContent,
     }),
-    [insertVariableToEditor, clearContent],
+    [insertVariableToEditor, clearContent, replaceContent],
   );
 
 

frontend/src/public/components/RichEditor/types.ts

@@ -12,6 +12,7 @@ export interface IRichEditorHandle {
   insertVariable(apiName: string, variableTitle: string, subtitle?: string): void;
   getEditor?(): LexicalEditor | undefined;
   clearContent(): void;
+  replaceContent(markdown: string): void;
 }
 
 export type TMentionData = {

frontend/src/public/components/TemplateEdit/InputWithVariables/InputWithVariables.tsx

@@ -34,7 +34,24 @@ export const InputWithVariables: React.FC<IEditorWithVariablesProps> = ({
   onChange,
 }) => {
   const editorRef = React.useRef<IRichEditorHandle>(null);
-  const formattedValue = escapeMarkdown(value);
+  const lastEmittedValue = React.useRef<string | undefined>(value);
+
+  const handleChange: IRichEditorProps['handleChange'] = React.useCallback(
+    (markdown: string) => {
+      lastEmittedValue.current = markdown;
+
+      return onChange(markdown);
+    },
+    [onChange],
+  );
+
+  React.useEffect(() => {
+    if (value !== lastEmittedValue.current) {
+      lastEmittedValue.current = value;
+      const formattedValue = escapeMarkdown(value);
+      editorRef.current?.replaceContent(formattedValue ?? '');
+    }
+  }, [value]);
 
   const handleInsertVariable = (apiName?: string) => (e: React.MouseEvent) => {
     e.stopPropagation();
@@ -52,8 +69,8 @@ export const InputWithVariables: React.FC<IEditorWithVariablesProps> = ({
       ref={editorRef}
       title={title}
       placeholder={placeholder ?? ''}
-      defaultValue={formattedValue}
-      handleChange={onChange}
+      defaultValue={escapeMarkdown(value)}
+      handleChange={handleChange}
       withToolbar={false}
       withMentions={false}
       multiline={false}

frontend/src/public/components/TemplateEdit/TaskForm/TaskDescriptionEditor.tsx

@@ -1,4 +1,4 @@
-import React, { useMemo, useRef } from 'react';
+import React, { useCallback, useEffect, useMemo, useRef } from 'react';
 import { useIntl } from 'react-intl';
 import { useSelector } from 'react-redux';
 
@@ -31,13 +31,30 @@ export function TaskDescriptionEditor({
 }: ITaskDescriptionEditorProps) {
   const { formatMessage } = useIntl();
   const editorRef = useRef<IRichEditorHandle>(null);
+  const lastEmittedValue = useRef<string | undefined>(value);
 
   const users = useSelector(getUsers);
   const mentions = useMemo(
     () => getMentionData(getNotDeletedUsers(users)),
     [users],
   );
 
+  const wrappedHandleChange = useCallback(
+    (markdown: string) => {
+      lastEmittedValue.current = markdown;
+
+      return handleChange(markdown);
+    },
+    [handleChange],
+  );
+
+  useEffect(() => {
+    if (value !== lastEmittedValue.current) {
+      lastEmittedValue.current = value;
+      editorRef.current?.replaceContent(value ?? '');
+    }
+  }, [value]);
+
   const handleInsertVariable = (apiName?: string) => (e: React.MouseEvent) => {
     e.stopPropagation();
 
@@ -62,7 +79,7 @@ export function TaskDescriptionEditor({
       title={formatMessage({ id: 'tasks.task-description-field' })}
       placeholder={formatMessage({ id: 'template.task-description-placeholder' })}
       defaultValue={value}
-      handleChange={handleChange}
+      handleChange={wrappedHandleChange}
       handleChangeChecklists={handleChangeChecklists}
       withChecklists
       mentions={mentions}

frontend/src/public/components/TemplateEdit/TemplateEdit.tsx

@@ -14,7 +14,7 @@ import { getVariables } from './TaskForm/utils/getTaskVariables';
 import { TemplateIntegrations } from './Integrations';
 import { ERoutes } from '../../constants/routes';
 import { TUserListItem } from '../../types/user';
-import { getEmptyKickoff, getNormalizedTemplateOwners, getTemplateIdFromUrl } from '../../utils/template';
+import { cleanTemplateReferences, getEmptyKickoff, getNormalizedTemplateOwners, getTemplateIdFromUrl } from '../../utils/template';
 import { checkSomeRouteIsActive, isCreateTemplate } from '../../utils/history';
 import { KickoffReduxContainer } from './KickoffRedux';
 import { moveTask } from '../../utils/workflows';
@@ -269,12 +269,16 @@ export function TemplateEdit({
       return;
     }
 
-    const newWorkflow: ITemplate = {
+    const updatedWorkflow: ITemplate = {
       ...workflow,
       [field]: value,
       isActive: false,
     };
 
+    const newWorkflow = (field === 'kickoff' || field === 'tasks')
+      ? cleanTemplateReferences(updatedWorkflow)
+      : updatedWorkflow;
+
     setTemplate(newWorkflow);
     submitDebounced();
   };

frontend/src/public/redux/template/saga.ts

@@ -54,7 +54,7 @@ import { ITemplate, ITemplateRequest, ITemplateResponse } from '../../types/temp
 import { logger } from '../../utils/logger';
 import { NotificationManager } from '../../components/UI/Notifications';
 import { updateTemplate } from '../../api/updateTemplate';
-import { getNormalizedTemplate, mapTemplateRequest } from '../../utils/template';
+import { cleanTemplateReferences, getNormalizedTemplate, mapTemplateRequest } from '../../utils/template';
 import { getErrorMessage, isPaidFeatureError } from '../../utils/getErrorMessage';
 import { insertId } from '../../utils/templates/insertId';
 import { ETemplateStatus } from '../../types/redux';
@@ -112,12 +112,15 @@ function* patchTemplateSaga({ payload: { changedFields, onSuccess, onFailed } }:
       changedFields.kickoff?.description === template.kickoff.description ? shouldDeactivateTemplate : false;
   }
 
-  const newTemplate: ITemplate = {
+  const mergedTemplate: ITemplate = {
     ...template,
     ...changedFields,
     ...(shouldDeactivateTemplate && { isActive: false }),
   };
 
+  const needsCleanup = changedFields.hasOwnProperty('tasks') || changedFields.hasOwnProperty('kickoff');
+  const newTemplate = needsCleanup ? cleanTemplateReferences(mergedTemplate) : mergedTemplate;
+
   yield put(setTemplate(newTemplate));
   yield delay(350);
   yield put(saveTemplate({ onSuccess, onFailed }));

frontend/src/public/utils/__tests__/template.test.ts

@@ -4,10 +4,13 @@ import {
   ETaskPerformerType,
   ITemplateResponse,
   ITemplate,
+  EExtraFieldType,
+  IExtraField,
 } from '../../types/template';
 import { ESubscriptionPlan } from '../../types/account';
 import { TUserListItem, EUserStatus } from '../../types/user';
-import { getNormalizedTemplate, mapTemplateRequest, getEmptyKickoff } from '../template';
+import { getNormalizedTemplate, mapTemplateRequest, getEmptyKickoff, cleanTemplateReferences } from '../template';
+import { EConditionAction, EConditionOperators, EConditionLogicOperations, TConditionRule } from '../../components/TemplateEdit/TaskForm/Conditions/types';
 
 const createMockUser = (overrides: Partial<TUserListItem> = {}): TUserListItem => ({
   id: 1,
@@ -367,5 +370,171 @@ describe('template utilities', () => {
 
       expect(result.publicSuccessUrl).toBeNull();
     });
+  describe('cleanTemplateReferences', () => {
+    it('removes invalid field references from template and task text fields but preserves system vars', () => {
+      const template = createMockTemplate({
+        wfNameTemplate: 'Name: {{valid-field}} and {{invalid-field}} and {{template-name}}',
+        kickoff: {
+          ...getEmptyKickoff(),
+          fields: [{ apiName: 'valid-field', type: EExtraFieldType.Text, name: 'Valid Field', order: 1 } as unknown as IExtraField],
+        },
+        tasks: [
+          {
+            ...createMockTemplate().tasks[0],
+            name: 'Task {{invalid-field}} and {{valid-field}}',
+            description: 'Desc {{workflow-starter}} and {{invalid-field}}',
+            number: 1,
+            fields: [{ apiName: 'valid-task-field', type: EExtraFieldType.Text, name: 'Valid Task Field', order: 1 } as unknown as IExtraField],
+          },
+          {
+            ...createMockTemplate().tasks[0],
+            name: 'Task 2 {{valid-field}} and {{valid-task-field}} and {{broken}}',
+            description: 'Desc',
+            number: 2,
+          },
+        ],
+      });
+
+      const cleaned = cleanTemplateReferences(template);
+
+      // wfNameTemplate: {{template-name}} is a WF_NAME system var, preserved; {{invalid-field}} removed
+      expect(cleaned.wfNameTemplate).toBe('Name: {{valid-field}} and  and {{template-name}}');
+      expect(cleaned.tasks[0].name).toBe('Task  and {{valid-field}}');
+      // {{workflow-starter}} is a TASK system var, preserved; {{invalid-field}} removed
+      expect(cleaned.tasks[0].description).toBe('Desc {{workflow-starter}} and ');
+      // Task 2 variables should successfully retain valid fields from kickoff and task 1
+      expect(cleaned.tasks[1].name).toBe('Task 2 {{valid-field}} and {{valid-task-field}} and ');
+    });
+
+    it('removes invalid field references from conditions', () => {
+      const template = createMockTemplate({
+        tasks: [
+          {
+            ...createMockTemplate().tasks[0],
+            conditions: [
+              {
+                apiName: 'cond-1',
+                order: 1,
+                action: EConditionAction.StartTask,
+                rules: [
+                  { field: 'valid-field', operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '1' } as unknown as TConditionRule,
+                  { field: 'invalid-field', operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '2' } as unknown as TConditionRule,
+                ],
+              },
+            ],
+          },
+        ],
+        kickoff: {
+          ...getEmptyKickoff(),
+          fields: [{ apiName: 'valid-field', type: EExtraFieldType.Text, name: 'Valid', order: 1 } as unknown as IExtraField],
+        },
+      });
+
+      const cleaned = cleanTemplateReferences(template);
+      const rules = cleaned.tasks[0].conditions[0].rules;
+      expect(rules).toHaveLength(1);
+      expect(rules[0].field).toBe('valid-field');
+    });
+
+    it('allows task name to become empty if all variables are invalid', () => {
+      const template = createMockTemplate({
+        tasks: [
+          {
+            ...createMockTemplate().tasks[0],
+            number: 2,
+            name: '{{invalid-field}}',
+          },
+        ],
+      });
+      const cleaned = cleanTemplateReferences(template);
+      expect(cleaned.tasks[0].name).toBe('');
+    });
+
+    it('safely handles empty rawPerformers array or invalid field performers and preserves valid members', () => {
+      const template = createMockTemplate({
+        kickoff: {
+          ...getEmptyKickoff(),
+          fields: [{ apiName: 'valid-user-field', type: EExtraFieldType.User, name: 'Valid User', order: 1 } as unknown as IExtraField],
+        },
+        tasks: [
+          {
+            ...createMockTemplate().tasks[0],
+            rawPerformers: [
+              { type: ETaskPerformerType.OutputUser, sourceId: 'invalid-field', label: 'Broken', apiName: '1' },
+              { type: ETaskPerformerType.OutputUser, sourceId: 'valid-user-field', label: 'Valid User', apiName: '2' },
+              { type: ETaskPerformerType.User, sourceId: '1', label: 'Regular User', apiName: '3' },
+            ],
+          },
+        ],
+      });
+      const cleaned = cleanTemplateReferences(template);
+      expect(cleaned.tasks[0].rawPerformers).toHaveLength(2);
+      expect(cleaned.tasks[0].rawPerformers.map((p) => p.apiName)).toEqual(['2', '3']);
+    });
+
+    it('removes invalid field references from rawDueDate', () => {
+      const template = createMockTemplate({
+        kickoff: {
+          ...getEmptyKickoff(),
+          fields: [{ apiName: 'valid-date-field', type: EExtraFieldType.Date, name: 'Valid Date', order: 1 } as unknown as IExtraField],
+        },
+        tasks: [
+          {
+            ...createMockTemplate().tasks[0],
+            number: 1,
+            rawDueDate: {
+              apiName: 'due-1', duration: null, durationMonths: null, rulePreposition: 'after', ruleTarget: 'field', sourceId: 'valid-date-field',
+            },
+          },
+          {
+            ...createMockTemplate().tasks[0],
+            number: 2,
+            rawDueDate: {
+              apiName: 'due-2', duration: null, durationMonths: null, rulePreposition: 'after', ruleTarget: 'field', sourceId: 'invalid-field',
+            },
+          },
+        ]
+      });
+      const cleaned = cleanTemplateReferences(template);
+      expect(cleaned.tasks[0].rawDueDate?.sourceId).toBe('valid-date-field');
+      expect(cleaned.tasks[0].rawDueDate?.ruleTarget).toBe('field');
+      expect(cleaned.tasks[1].rawDueDate?.duration).toBeNull();
+      expect(cleaned.tasks[1].rawDueDate?.durationMonths).toBeNull();
+      expect(cleaned.tasks[1].rawDueDate?.sourceId).toBeNull();
+      expect(cleaned.tasks[1].rawDueDate?.ruleTarget).toBe('task started');
+    });
+    it('preserves Start After condition rules and handles empty fields', () => {
+      const template = createMockTemplate({
+        tasks: [
+          {
+            ...createMockTemplate().tasks[0],
+            conditions: [
+              {
+                apiName: 'cond-1',
+                order: 1,
+                action: EConditionAction.StartTask,
+                rules: [
+                  { field: 'task-123', fieldType: 'task', operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '1' } as unknown as TConditionRule,
+                  { field: '', operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '2' } as unknown as TConditionRule,
+                  { field: undefined, operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '3' } as unknown as TConditionRule,
+                  { field: null, operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '4' } as unknown as TConditionRule,
+                  { field: 'invalid-field', fieldType: 'field', operator: EConditionOperators.Equal, logicOperation: EConditionLogicOperations.And, predicateApiName: '5' } as unknown as TConditionRule,
+                ],
+              },
+            ],
+          },
+        ],
+      });
+
+      const cleaned = cleanTemplateReferences(template);
+      const rules = cleaned.tasks[0].conditions[0].rules;
+      
+      expect(rules).toHaveLength(4);
+      expect(rules[0].field).toBe('task-123');
+      expect(rules[1].field).toBe('');
+      expect(rules[2].field).toBeUndefined();
+      expect(rules[3].field).toBeNull();
+    });
   });
 });
+});