Merge pull request #212 from pneumaticapp/frontend/template/46553__checkboxes_toggle_wrong_task_in_template_editor

EBirkenfeld · web-flow · commit 112442afa843 · 2026-05-08T15:10:13.000+05:00
46553 frontend [ Templates ] Checkboxes toggle wrong task in template editor
diff --git a/frontend/src/public/components/TemplateEdit/TaskForm/TaskPerformers.tsx b/frontend/src/public/components/TemplateEdit/TaskForm/TaskPerformers.tsx
@@ -113,15 +113,15 @@ export function TaskPerformers({ task, users, variables, setCurrentTask }: ITask
     <div className={classNames(styles['task-fields-wrapper'], stylesTaskForm['content-mt16'])}>
       <div className="mb-3">
         <Checkbox
-          checkboxId="completeByAll"
+          checkboxId={`completeByAll-${task.apiName}`}
           title={formatMessage({ id: 'templates.task-require-completion-by-all' })}
           checked={task.requireCompletionByAll}
           onChange={(e) => handleRequireCompletionByAllChange(e.currentTarget.checked)}
         />
       </div>
       <div className="mb-3">
         <Checkbox
-          checkboxId="skipForStarter"
+          checkboxId={`skipForStarter-${task.apiName}`}
           title={formatMessage({ id: 'templates.task-skip-for-starter' })}
           checked={task.skipForStarter}
           onChange={(e) => setCurrentTask({ skipForStarter: e.currentTarget.checked })}
diff --git a/frontend/src/public/components/TemplateEdit/TaskForm/__tests__/TaskPerformers.test.tsx b/frontend/src/public/components/TemplateEdit/TaskForm/__tests__/TaskPerformers.test.tsx
@@ -87,14 +87,130 @@ describe('TaskPerformers', () => {
   const getSkipCheckboxCall = () => {
     const calls = (Checkbox as jest.Mock).mock.calls;
     return calls.find(
-      (c: any[]) => c[0].checkboxId === 'skipForStarter',
+      (c: any[]) => c[0].checkboxId === 'skipForStarter-task-1',
     );
   };
 
+  const getCompleteByAllCall = () => {
+    const calls = (Checkbox as jest.Mock).mock.calls;
+    return calls.find(
+      (c: any[]) => c[0].checkboxId === 'completeByAll-task-1',
+    );
+  };
+
+  const renderTwoTasks = () => {
+    const task1 = makeTask({ apiName: 'task-1' });
+    const task2 = makeTask({ apiName: 'task-2' });
+    const setCurrentTask1 = jest.fn();
+    const setCurrentTask2 = jest.fn();
+
+    render(
+      React.createElement('div', null,
+        React.createElement(TaskPerformers, {
+          task: task1,
+          tasks: [task1, task2],
+          users: [],
+          variables: [],
+          isTeamInvitesModalOpen: false,
+          setCurrentTask: setCurrentTask1,
+        }),
+        React.createElement(TaskPerformers, {
+          task: task2,
+          tasks: [task1, task2],
+          users: [],
+          variables: [],
+          isTeamInvitesModalOpen: false,
+          setCurrentTask: setCurrentTask2,
+        }),
+      ),
+    );
+
+    return { setCurrentTask1, setCurrentTask2 };
+  };
+
   beforeEach(() => {
     jest.clearAllMocks();
   });
 
+  describe('checkbox identifiers', () => {
+    it('generates unique checkboxId using task.apiName to prevent HTML id collisions', () => {
+      renderComponent({ apiName: 'custom-task-123' });
+
+      const calls = (Checkbox as jest.Mock).mock.calls;
+      const completeByAllCall = calls.find((c: any[]) => c[0].checkboxId?.startsWith('completeByAll-'));
+      const skipForStarterCall = calls.find((c: any[]) => c[0].checkboxId?.startsWith('skipForStarter-'));
+
+      expect(completeByAllCall).toBeDefined();
+      expect(skipForStarterCall).toBeDefined();
+
+      expect(completeByAllCall![0].checkboxId).toBe('completeByAll-custom-task-123');
+      expect(skipForStarterCall![0].checkboxId).toBe('skipForStarter-custom-task-123');
+    });
+
+    it('two tasks produce different checkboxIds (no DOM id collisions)', () => {
+      renderTwoTasks();
+
+      const calls = (Checkbox as jest.Mock).mock.calls;
+      const checkboxIds = calls.map((c: any[]) => c[0].checkboxId);
+
+      const completeByAllIds = checkboxIds.filter((id: string) => id?.startsWith('completeByAll-'));
+      const skipForStarterIds = checkboxIds.filter((id: string) => id?.startsWith('skipForStarter-'));
+
+      expect(completeByAllIds).toEqual(['completeByAll-task-1', 'completeByAll-task-2']);
+      expect(skipForStarterIds).toEqual(['skipForStarter-task-1', 'skipForStarter-task-2']);
+    });
+
+    it('uses checkboxId prop (not id) to ensure proper label-input binding', () => {
+      renderComponent();
+
+      const calls = (Checkbox as jest.Mock).mock.calls;
+
+      calls.forEach((c: any[]) => {
+        expect(c[0].checkboxId).toBeDefined();
+        expect(c[0].id).toBeUndefined();
+      });
+    });
+  });
+
+  describe('requireCompletionByAll checkbox', () => {
+    it('passes title with correct localization text', () => {
+      renderComponent();
+
+      const call = getCompleteByAllCall();
+
+      expect(call).toBeDefined();
+      expect(call![0].title).toBe(t('templates.task-require-completion-by-all'));
+    });
+
+    it('passes checked=true when requireCompletionByAll=true', () => {
+      renderComponent({ requireCompletionByAll: true });
+
+      const call = getCompleteByAllCall();
+
+      expect(call![0].checked).toBe(true);
+    });
+
+    it('passes checked=false when requireCompletionByAll=false', () => {
+      renderComponent({ requireCompletionByAll: false });
+
+      const call = getCompleteByAllCall();
+
+      expect(call![0].checked).toBe(false);
+    });
+
+    it('calls setCurrentTask with requireCompletionByAll on onChange', () => {
+      renderComponent({ requireCompletionByAll: false });
+
+      const call = getCompleteByAllCall();
+      const onChangeFn = call![0].onChange;
+      onChangeFn({ currentTarget: { checked: true } });
+
+      expect(mockSetCurrentTask).toHaveBeenCalledWith(
+        { requireCompletionByAll: true },
+      );
+    });
+  });
+
   describe('skipForStarter checkbox', () => {
     it('passes title with correct localization text', () => {
       renderComponent();
diff --git a/frontend/src/public/utils/__tests__/mapOutputToCompleteTask.test.ts b/frontend/src/public/utils/__tests__/mapOutputToCompleteTask.test.ts
@@ -0,0 +1,96 @@
+/// <reference types="jest" />
+import { mapOutputToCompleteTask } from '../mappers';
+import { EExtraFieldType, IExtraField } from '../../types/template';
+
+jest.mock('../dateTime', () => ({
+  getEndOfDayTsp: jest.fn((v: string) => `endOfDay(${v})`),
+  toDateString: jest.fn(),
+  toTspDate: jest.fn(),
+  toISOStringFromTsp: jest.fn(),
+  formatDateToISOInWorkflow: jest.fn(),
+  formatDateToISOInTask: jest.fn(),
+}));
+
+const makeField = (overrides: Partial<IExtraField>): IExtraField => ({
+  apiName: 'field-1',
+  name: 'Test',
+  type: EExtraFieldType.String,
+  order: 0,
+  userId: null,
+  groupId: null,
+  ...overrides,
+});
+
+describe('mapOutputToCompleteTask', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('returns empty array as-is', () => {
+    expect(mapOutputToCompleteTask([])).toEqual([]);
+  });
+
+  describe('checkbox normalization', () => {
+    it('splits a single-option string into array', () => {
+      const output = [makeField({ type: EExtraFieldType.Checkbox, value: 'New option' })];
+      const [first] = mapOutputToCompleteTask(output);
+      expect(first.value).toEqual(['New option']);
+    });
+
+    it('splits a multi-option comma-separated string into array', () => {
+      const output = [makeField({ type: EExtraFieldType.Checkbox, value: 'opt1, opt2, opt3' })];
+      const [first] = mapOutputToCompleteTask(output);
+      expect(first.value).toEqual(['opt1', 'opt2', 'opt3']);
+    });
+
+    it('keeps array value as-is', () => {
+      const output = [makeField({ type: EExtraFieldType.Checkbox, value: ['opt1', 'opt2'] })];
+      const [first] = mapOutputToCompleteTask(output);
+      expect(first.value).toEqual(['opt1', 'opt2']);
+    });
+
+    it.each([null, undefined, ''])('returns empty array for falsy value: %p', (falsy) => {
+      const output = [makeField({ type: EExtraFieldType.Checkbox, value: falsy as any })];
+      const [first] = mapOutputToCompleteTask(output);
+      expect(first.value).toEqual([]);
+    });
+
+    it('keeps empty array as empty array', () => {
+      const output = [makeField({ type: EExtraFieldType.Checkbox, value: [] })];
+      const [first] = mapOutputToCompleteTask(output);
+      expect(first.value).toEqual([]);
+    });
+
+    it('preserves other field properties', () => {
+      const output = [makeField({
+        apiName: 'cb-field',
+        name: 'My Checkbox',
+        type: EExtraFieldType.Checkbox,
+        value: 'opt',
+        isRequired: true,
+      })];
+      const [first] = mapOutputToCompleteTask(output);
+      expect(first).toEqual(expect.objectContaining({
+        apiName: 'cb-field',
+        name: 'My Checkbox',
+        type: 'checkbox',
+        value: ['opt'],
+        isRequired: true,
+      }));
+    });
+  });
+
+  it('handles mixed field types in one output array', () => {
+    const output = [
+      makeField({ apiName: 'cb', type: EExtraFieldType.Checkbox, value: 'opt1, opt2' }),
+      makeField({ apiName: 'num', type: EExtraFieldType.Number, value: '1,5' }),
+      makeField({ apiName: 'txt', type: EExtraFieldType.Text, value: 'hi' }),
+    ];
+    const result = mapOutputToCompleteTask(output);
+    expect(result).toEqual([
+      expect.objectContaining({ apiName: 'cb', value: ['opt1', 'opt2'] }),
+      expect.objectContaining({ apiName: 'num', value: '1.5' }),
+      expect.objectContaining({ apiName: 'txt', value: 'hi' }),
+    ]);
+  });
+});
diff --git a/frontend/src/public/utils/mappers.ts b/frontend/src/public/utils/mappers.ts
@@ -106,6 +106,18 @@ export const mapOutputToCompleteTask = (output: IExtraField[]): IExtraField[] =>
         value: String(item.value).replace(',', '.'),
       };
     }
+    if (item.type === 'checkbox') {
+      let checkboxValue: string[];
+      if (Array.isArray(item.value)) {
+        checkboxValue = item.value;
+      } else if (item.value) {
+        checkboxValue = (item.value as string).split(', ');
+      } else {
+        checkboxValue = [];
+      }
+
+      return { ...item, value: checkboxValue };
+    }
     return item;
   });
 };
