Fix duplicateJob to regenerate task ids and remap conditions (#2512)

rfontanarosa · gino-m · web-flow · commit 092ca9d3e6a8 · 2026-05-14T19:37:11.000+02:00
Co-authored-by: Gino Miceli &lt;228050+gino-m@users.noreply.github.com&gt;
diff --git a/web/src/app/services/job/job.service.spec.ts b/web/src/app/services/job/job.service.spec.ts
@@ -16,10 +16,18 @@
 
 import { TestBed } from '@angular/core/testing';
 import { RouterTestingModule } from '@angular/router/testing';
-import { Map } from 'immutable';
+import { List, Map } from 'immutable';
 
 import { DataCollectionStrategy, Job } from 'app/models/job.model';
 import { DataSharingType, Survey } from 'app/models/survey.model';
+import { Cardinality, MultipleChoice } from 'app/models/task/multiple-choice.model';
+import { Option } from 'app/models/task/option.model';
+import {
+  TaskCondition,
+  TaskConditionExpression,
+  TaskConditionExpressionType,
+  TaskConditionMatchType,
+} from 'app/models/task/task-condition.model';
 import { Task, TaskType } from 'app/models/task/task.model';
 import { DataStoreService } from 'app/services/data-store/data-store.service';
 import { JobService } from 'app/services/job/job.service';
@@ -90,7 +98,7 @@ describe('JobService', () => {
   describe('duplicateJob', () => {
     it('should duplicate job with new IDs', () => {
       const oldTask = new Task('task1', TaskType.TEXT, 'Label', false, 0);
-      const newTask = new Task('task1', TaskType.TEXT, 'Label', false, 0);
+      const newTask = new Task('newTask1', TaskType.TEXT, 'Label', false, 0);
       const job = new Job(
         'job1',
         0,
@@ -108,8 +116,124 @@ describe('JobService', () => {
       expect(newJob.id).toBe('job2');
       expect(newJob.name).toBe('Copy of Job 1');
       expect(newJob.color).toBe('#FFF');
-      expect(newJob.tasks?.get('task1')).toEqual(newTask);
-      expect(taskServiceSpy.duplicateTask).toHaveBeenCalledWith(oldTask, true);
+      expect(newJob.tasks?.get('newTask1')).toEqual(newTask);
+      expect(taskServiceSpy.duplicateTask).toHaveBeenCalledWith(oldTask);
+    });
+
+    it('should remap condition references to the duplicated task and option ids', () => {
+      const sourceOption = new Option('opt1', 'A', 'A', 0);
+      const sourceTask = new Task(
+        'src',
+        TaskType.MULTIPLE_CHOICE,
+        'Source',
+        false,
+        0,
+        new MultipleChoice(Cardinality.SELECT_ONE, List([sourceOption]))
+      );
+      const dependentTask = new Task(
+        'dep',
+        TaskType.TEXT,
+        'Dependent',
+        false,
+        1,
+        undefined,
+        new TaskCondition(
+          TaskConditionMatchType.MATCH_ALL,
+          List([
+            new TaskConditionExpression(
+              TaskConditionExpressionType.ONE_OF_SELECTED,
+              'src',
+              List(['opt1'])
+            ),
+          ])
+        )
+      );
+      const job = new Job(
+        'job1',
+        0,
+        '#000',
+        'Job 1',
+        Map({ src: sourceTask, dep: dependentTask }),
+        DataCollectionStrategy.MIXED
+      );
+
+      const newSourceTask = new Task(
+        'newSrc',
+        TaskType.MULTIPLE_CHOICE,
+        'Source',
+        false,
+        0,
+        new MultipleChoice(
+          Cardinality.SELECT_ONE,
+          List([new Option('newOpt1', 'A', 'A', 0)])
+        )
+      );
+      const newDependentTask = new Task(
+        'newDep',
+        TaskType.TEXT,
+        'Dependent',
+        false,
+        1,
+        undefined,
+        dependentTask.condition
+      );
+      dataStoreServiceSpy.generateId.and.returnValue('job2');
+      taskServiceSpy.duplicateTask.and.returnValues(
+        newSourceTask,
+        newDependentTask
+      );
+
+      const newJob = service.duplicateJob(job, '#FFF');
+
+      const remapped = newJob.tasks?.get('newDep');
+      const expression = remapped?.condition?.expressions.first();
+      expect(expression?.taskId).toBe('newSrc');
+      expect(expression?.optionIds.toArray()).toEqual(['newOpt1']);
+    });
+
+    it('should drop condition expressions whose source task is missing', () => {
+      const dependentTask = new Task(
+        'dep',
+        TaskType.TEXT,
+        'Dependent',
+        false,
+        0,
+        undefined,
+        new TaskCondition(
+          TaskConditionMatchType.MATCH_ALL,
+          List([
+            new TaskConditionExpression(
+              TaskConditionExpressionType.ONE_OF_SELECTED,
+              'missing',
+              List(['opt-missing'])
+            ),
+          ])
+        )
+      );
+      const job = new Job(
+        'job1',
+        0,
+        '#000',
+        'Job 1',
+        Map({ dep: dependentTask }),
+        DataCollectionStrategy.MIXED
+      );
+
+      const newDependentTask = new Task(
+        'newDep',
+        TaskType.TEXT,
+        'Dependent',
+        false,
+        0,
+        undefined,
+        dependentTask.condition
+      );
+      dataStoreServiceSpy.generateId.and.returnValue('job2');
+      taskServiceSpy.duplicateTask.and.returnValue(newDependentTask);
+
+      const newJob = service.duplicateJob(job, '#FFF');
+
+      expect(newJob.tasks?.get('newDep')?.condition?.expressions.size).toBe(0);
     });
   });
 
diff --git a/web/src/app/services/job/job.service.ts b/web/src/app/services/job/job.service.ts
@@ -20,6 +20,10 @@ import { List, Map } from 'immutable';
 import { DataCollectionStrategy, Job } from 'app/models/job.model';
 import { MultipleChoice } from 'app/models/task/multiple-choice.model';
 import { Option } from 'app/models/task/option.model';
+import {
+  TaskCondition,
+  TaskConditionExpression,
+} from 'app/models/task/task-condition.model';
 import { Task, TaskType } from 'app/models/task/task.model';
 import { DataStoreService } from 'app/services/data-store/data-store.service';
 import { Survey } from 'app/models/survey.model';
@@ -73,23 +77,73 @@ export class JobService {
   }
 
   /**
-   * Returns a new job which is an exact copy of the specified job, but with new UUIDs for all items recursively.
+   * Returns a new job which is an exact copy of the specified job, but with
+   * new UUIDs for the job, every task, and every multiple-choice option.
+   * Conditional task references (TaskConditionExpression) are remapped to
+   * the new task/option ids so dependencies survive the duplication.
    */
   duplicateJob(job: Job, color: string | undefined): Job {
+    // Pass 1: duplicate each task (with new ids) and record old → new id
+    // mappings for tasks and options.
+    const taskIdMap = new globalThis.Map<string, string>();
+    const optionIdMap = new globalThis.Map<string, string>();
+    const duplicates = (job.tasks?.toArray() ?? []).map(([, oldTask]) => {
+      const newTask = this.taskService.duplicateTask(oldTask);
+      taskIdMap.set(oldTask.id, newTask.id);
+      oldTask.multipleChoice?.options.forEach((oldOption, i) => {
+        const newOption = newTask.multipleChoice?.options.get(i);
+        if (newOption) optionIdMap.set(oldOption.id, newOption.id);
+      });
+      return newTask;
+    });
+
+    // Pass 2: remap any condition references using the id maps.
+    const tasks = Map<string, Task>(
+      duplicates.map(newTask => {
+        const remapped = newTask.condition
+          ? newTask.copyWith({
+              condition: this.remapCondition(
+                newTask.condition,
+                taskIdMap,
+                optionIdMap
+              ),
+            })
+          : newTask;
+        return [remapped.id, remapped];
+      })
+    );
+
     return job.copyWith({
       id: this.dataStoreService.generateId(),
       name: `Copy of ${job.name}`,
       color,
       index: -1,
-      tasks: Map<string, Task>(
-        job.tasks?.toArray().map(([_, task]) => {
-          const duplicateTask = this.taskService.duplicateTask(task, true);
-          return [duplicateTask.id, duplicateTask];
-        })
-      ),
+      tasks,
     });
   }
 
+  private remapCondition(
+    condition: TaskCondition,
+    taskIdMap: globalThis.Map<string, string>,
+    optionIdMap: globalThis.Map<string, string>
+  ): TaskCondition {
+    const expressions = condition.expressions
+      .map(expr => {
+        const newTaskId = taskIdMap.get(expr.taskId);
+        if (!newTaskId) return null;
+        const newOptionIds = expr.optionIds
+          .map(id => optionIdMap.get(id))
+          .filter((id): id is string => !!id);
+        return new TaskConditionExpression(
+          expr.expressionType,
+          newTaskId,
+          newOptionIds
+        );
+      })
+      .filter((expr): expr is TaskConditionExpression => !!expr);
+    return new TaskCondition(condition.matchType, expressions);
+  }
+
   /**
    * Creates and returns a new task with a generated unique identifier and a single English label.
    */
diff --git a/web/src/app/services/task/task.service.ts b/web/src/app/services/task/task.service.ts
@@ -19,6 +19,7 @@ import { List, Map } from 'immutable';
 
 import { DataCollectionStrategy, Job } from 'app/models/job.model';
 import { MultipleChoice } from 'app/models/task/multiple-choice.model';
+import { Option } from 'app/models/task/option.model';
 import { Task, TaskType } from 'app/models/task/task.model';
 import { DataStoreService } from 'app/services/data-store/data-store.service';
 
@@ -51,40 +52,40 @@ export class TaskService {
   }
 
   /**
-   * Creates a duplicate of the provided task.
-   * @param task - The task to be duplicated.
-   * @param preserveId - If true, keeps the original task ID.
-   * If false (default), generates a new unique ID for the duplicate.
-   * @returns A new Task instance with duplicated nested properties.
+   * Creates a duplicate of the provided task with a freshly generated id
+   * (and fresh option ids for multiple-choice tasks). Note: `condition`
+   * references are copied verbatim — when duplicating tasks across a job
+   * boundary, the caller is responsible for remapping them.
    */
-  duplicateTask(task: Task, preserveId: boolean = false): Task {
-    return {
-      ...task,
-      id: preserveId ? task.id : this.dataStoreService.generateId(),
-      multipleChoice: task.multipleChoice
-        ? this.duplicateMultipleChoice(task.multipleChoice, preserveId)
+  duplicateTask(task: Task): Task {
+    return new Task(
+      this.dataStoreService.generateId(),
+      task.type,
+      task.label,
+      task.required,
+      task.index,
+      task.multipleChoice
+        ? this.duplicateMultipleChoice(task.multipleChoice)
         : undefined,
-    } as Task;
+      task.condition,
+      task.addLoiTask
+    );
   }
 
-  /**
-   * Creates a duplicate of the provided multiple choice object.
-   * @param multipleChoice - The multiple choice attribute to be duplicated.
-   * @param preserveId - If true, keeps the original option IDs.
-   * If false (default), generates new unique IDs for the options.
-   * @returns A new MultipleChoice instance with duplicated options.
-   */
-  duplicateMultipleChoice(
-    multipleChoice: MultipleChoice,
-    preserveId: boolean = false
-  ): MultipleChoice {
-    return {
-      ...multipleChoice,
-      options: multipleChoice.options?.map(option => ({
-        ...option,
-        id: preserveId ? option.id : this.dataStoreService.generateId(),
-      })),
-    } as MultipleChoice;
+  duplicateMultipleChoice(multipleChoice: MultipleChoice): MultipleChoice {
+    return new MultipleChoice(
+      multipleChoice.cardinality,
+      multipleChoice.options.map(
+        option =>
+          new Option(
+            this.dataStoreService.generateId(),
+            option.code,
+            option.label,
+            option.index
+          )
+      ),
+      multipleChoice.hasOtherOption
+    );
   }
 
   addOrUpdateTasks(
