From 9e6c190af3af72f48ebf04a05efcdc43a974f744 Mon Sep 17 00:00:00 2001 From: Eyal Toledano Date: Wed, 28 May 2025 14:05:30 -0400 Subject: [PATCH] fix(move-task): Fix duplicate task creation when moving subtask to standalone task --- scripts/modules/task-manager/move-task.js | 953 +++++++++++----------- test-move-fix.js | 227 ++++++ 2 files changed, 703 insertions(+), 477 deletions(-) create mode 100644 test-move-fix.js diff --git a/scripts/modules/task-manager/move-task.js b/scripts/modules/task-manager/move-task.js index 1b90c0b3..2fde9384 100644 --- a/scripts/modules/task-manager/move-task.js +++ b/scripts/modules/task-manager/move-task.js @@ -1,7 +1,7 @@ -import path from 'path'; -import { log, readJSON, writeJSON } from '../utils.js'; -import { isTaskDependentOn } from '../task-manager.js'; -import generateTaskFiles from './generate-task-files.js'; +import path from "path"; +import { log, readJSON, writeJSON } from "../utils.js"; +import { isTaskDependentOn } from "../task-manager.js"; +import generateTaskFiles from "./generate-task-files.js"; /** * Move one or more tasks/subtasks to new positions @@ -12,39 +12,39 @@ import generateTaskFiles from './generate-task-files.js'; * @returns {Object} Result object with moved task details */ async function moveTask( - tasksPath, - sourceId, - destinationId, - generateFiles = true + tasksPath, + sourceId, + destinationId, + generateFiles = true ) { - // Check if we have comma-separated IDs (multiple moves) - const sourceIds = sourceId.split(',').map((id) => id.trim()); - const destinationIds = destinationId.split(',').map((id) => id.trim()); + // Check if we have comma-separated IDs (multiple moves) + const sourceIds = sourceId.split(",").map((id) => id.trim()); + const destinationIds = destinationId.split(",").map((id) => id.trim()); - // If multiple IDs, validate they match in count - if (sourceIds.length > 1 || destinationIds.length > 1) { - if (sourceIds.length !== destinationIds.length) { - throw new Error( - `Number of source IDs (${sourceIds.length}) must match number of destination IDs (${destinationIds.length})` - ); - } + // If multiple IDs, validate they match in count + if (sourceIds.length > 1 || destinationIds.length > 1) { + if (sourceIds.length !== destinationIds.length) { + throw new Error( + `Number of source IDs (${sourceIds.length}) must match number of destination IDs (${destinationIds.length})` + ); + } - // Perform multiple moves - return await moveMultipleTasks( - tasksPath, - sourceIds, - destinationIds, - generateFiles - ); - } + // Perform multiple moves + return await moveMultipleTasks( + tasksPath, + sourceIds, + destinationIds, + generateFiles + ); + } - // Single move - use existing logic - return await moveSingleTask( - tasksPath, - sourceId, - destinationId, - generateFiles - ); + // Single move - use existing logic + return await moveSingleTask( + tasksPath, + sourceId, + destinationId, + generateFiles + ); } /** @@ -56,44 +56,44 @@ async function moveTask( * @returns {Object} Result object with moved task details */ async function moveMultipleTasks( - tasksPath, - sourceIds, - destinationIds, - generateFiles = true + tasksPath, + sourceIds, + destinationIds, + generateFiles = true ) { - try { - log( - 'info', - `Moving multiple tasks/subtasks: ${sourceIds.join(', ')} to ${destinationIds.join(', ')}...` - ); + try { + log( + "info", + `Moving multiple tasks/subtasks: ${sourceIds.join(", ")} to ${destinationIds.join(", ")}...` + ); - const results = []; + const results = []; - // Perform moves one by one, but don't regenerate files until the end - for (let i = 0; i < sourceIds.length; i++) { - const result = await moveSingleTask( - tasksPath, - sourceIds[i], - destinationIds[i], - false - ); - results.push(result); - } + // Perform moves one by one, but don't regenerate files until the end + for (let i = 0; i < sourceIds.length; i++) { + const result = await moveSingleTask( + tasksPath, + sourceIds[i], + destinationIds[i], + false + ); + results.push(result); + } - // Generate task files once at the end if requested - if (generateFiles) { - log('info', 'Regenerating task files...'); - await generateTaskFiles(tasksPath, path.dirname(tasksPath)); - } + // Generate task files once at the end if requested + if (generateFiles) { + log("info", "Regenerating task files..."); + await generateTaskFiles(tasksPath, path.dirname(tasksPath)); + } - return { - message: `Successfully moved ${sourceIds.length} tasks/subtasks`, - moves: results - }; - } catch (error) { - log('error', `Error moving multiple tasks/subtasks: ${error.message}`); - throw error; - } + return { + message: `Successfully moved ${sourceIds.length} tasks/subtasks`, + moves: results, + }; + } catch (error) { + log("error", `Error moving multiple tasks/subtasks: ${error.message}`); + throw error; + } } /** @@ -105,237 +105,237 @@ async function moveMultipleTasks( * @returns {Object} Result object with moved task details */ async function moveSingleTask( - tasksPath, - sourceId, - destinationId, - generateFiles = true + tasksPath, + sourceId, + destinationId, + generateFiles = true ) { - try { - log('info', `Moving task/subtask ${sourceId} to ${destinationId}...`); + try { + log("info", `Moving task/subtask ${sourceId} to ${destinationId}...`); - // Read the existing tasks - const data = readJSON(tasksPath); - if (!data || !data.tasks) { - throw new Error(`Invalid or missing tasks file at ${tasksPath}`); - } + // Read the existing tasks + const data = readJSON(tasksPath); + if (!data || !data.tasks) { + throw new Error(`Invalid or missing tasks file at ${tasksPath}`); + } - // Parse source ID to determine if it's a task or subtask - const isSourceSubtask = sourceId.includes('.'); - let sourceTask, - sourceParentTask, - sourceSubtask, - sourceTaskIndex, - sourceSubtaskIndex; + // Parse source ID to determine if it's a task or subtask + const isSourceSubtask = sourceId.includes("."); + let sourceTask, + sourceParentTask, + sourceSubtask, + sourceTaskIndex, + sourceSubtaskIndex; - // Parse destination ID to determine the target - const isDestinationSubtask = destinationId.includes('.'); - let destTask, destParentTask, destSubtask, destTaskIndex, destSubtaskIndex; + // Parse destination ID to determine the target + const isDestinationSubtask = destinationId.includes("."); + let destTask, destParentTask, destSubtask, destTaskIndex, destSubtaskIndex; - // Validate source exists - if (isSourceSubtask) { - // Source is a subtask - const [parentIdStr, subtaskIdStr] = sourceId.split('.'); - const parentIdNum = parseInt(parentIdStr, 10); - const subtaskIdNum = parseInt(subtaskIdStr, 10); + // Validate source exists + if (isSourceSubtask) { + // Source is a subtask + const [parentIdStr, subtaskIdStr] = sourceId.split("."); + const parentIdNum = parseInt(parentIdStr, 10); + const subtaskIdNum = parseInt(subtaskIdStr, 10); - sourceParentTask = data.tasks.find((t) => t.id === parentIdNum); - if (!sourceParentTask) { - throw new Error(`Source parent task with ID ${parentIdNum} not found`); - } + sourceParentTask = data.tasks.find((t) => t.id === parentIdNum); + if (!sourceParentTask) { + throw new Error(`Source parent task with ID ${parentIdNum} not found`); + } - if ( - !sourceParentTask.subtasks || - sourceParentTask.subtasks.length === 0 - ) { - throw new Error(`Source parent task ${parentIdNum} has no subtasks`); - } + if ( + !sourceParentTask.subtasks || + sourceParentTask.subtasks.length === 0 + ) { + throw new Error(`Source parent task ${parentIdNum} has no subtasks`); + } - sourceSubtaskIndex = sourceParentTask.subtasks.findIndex( - (st) => st.id === subtaskIdNum - ); - if (sourceSubtaskIndex === -1) { - throw new Error(`Source subtask ${sourceId} not found`); - } + sourceSubtaskIndex = sourceParentTask.subtasks.findIndex( + (st) => st.id === subtaskIdNum + ); + if (sourceSubtaskIndex === -1) { + throw new Error(`Source subtask ${sourceId} not found`); + } - sourceSubtask = { ...sourceParentTask.subtasks[sourceSubtaskIndex] }; - } else { - // Source is a task - const sourceIdNum = parseInt(sourceId, 10); - sourceTaskIndex = data.tasks.findIndex((t) => t.id === sourceIdNum); - if (sourceTaskIndex === -1) { - throw new Error(`Source task with ID ${sourceIdNum} not found`); - } + sourceSubtask = { ...sourceParentTask.subtasks[sourceSubtaskIndex] }; + } else { + // Source is a task + const sourceIdNum = parseInt(sourceId, 10); + sourceTaskIndex = data.tasks.findIndex((t) => t.id === sourceIdNum); + if (sourceTaskIndex === -1) { + throw new Error(`Source task with ID ${sourceIdNum} not found`); + } - sourceTask = { ...data.tasks[sourceTaskIndex] }; - } + sourceTask = { ...data.tasks[sourceTaskIndex] }; + } - // Validate destination exists - if (isDestinationSubtask) { - // Destination is a subtask (target will be the parent of this subtask) - const [parentIdStr, subtaskIdStr] = destinationId.split('.'); - const parentIdNum = parseInt(parentIdStr, 10); - const subtaskIdNum = parseInt(subtaskIdStr, 10); + // Validate destination exists + if (isDestinationSubtask) { + // Destination is a subtask (target will be the parent of this subtask) + const [parentIdStr, subtaskIdStr] = destinationId.split("."); + const parentIdNum = parseInt(parentIdStr, 10); + const subtaskIdNum = parseInt(subtaskIdStr, 10); - destParentTask = data.tasks.find((t) => t.id === parentIdNum); - if (!destParentTask) { - throw new Error( - `Destination parent task with ID ${parentIdNum} not found` - ); - } + destParentTask = data.tasks.find((t) => t.id === parentIdNum); + if (!destParentTask) { + throw new Error( + `Destination parent task with ID ${parentIdNum} not found` + ); + } - // Initialize subtasks array if it doesn't exist - if (!destParentTask.subtasks) { - destParentTask.subtasks = []; - } + // Initialize subtasks array if it doesn't exist + if (!destParentTask.subtasks) { + destParentTask.subtasks = []; + } - // If there are existing subtasks, try to find the specific destination subtask - if (destParentTask.subtasks.length > 0) { - destSubtaskIndex = destParentTask.subtasks.findIndex( - (st) => st.id === subtaskIdNum - ); - if (destSubtaskIndex !== -1) { - destSubtask = destParentTask.subtasks[destSubtaskIndex]; - } else { - // Subtask doesn't exist, we'll insert at the end - destSubtaskIndex = destParentTask.subtasks.length - 1; - } - } else { - // No existing subtasks, this will be the first one - destSubtaskIndex = -1; // Will insert at position 0 - } - } else { - // Destination is a task - const destIdNum = parseInt(destinationId, 10); - destTaskIndex = data.tasks.findIndex((t) => t.id === destIdNum); + // If there are existing subtasks, try to find the specific destination subtask + if (destParentTask.subtasks.length > 0) { + destSubtaskIndex = destParentTask.subtasks.findIndex( + (st) => st.id === subtaskIdNum + ); + if (destSubtaskIndex !== -1) { + destSubtask = destParentTask.subtasks[destSubtaskIndex]; + } else { + // Subtask doesn't exist, we'll insert at the end + destSubtaskIndex = destParentTask.subtasks.length - 1; + } + } else { + // No existing subtasks, this will be the first one + destSubtaskIndex = -1; // Will insert at position 0 + } + } else { + // Destination is a task + const destIdNum = parseInt(destinationId, 10); + destTaskIndex = data.tasks.findIndex((t) => t.id === destIdNum); - if (destTaskIndex === -1) { - // Create placeholder for destination if it doesn't exist - log('info', `Creating placeholder for destination task ${destIdNum}`); - const newTask = { - id: destIdNum, - title: `Task ${destIdNum}`, - description: '', - status: 'pending', - priority: 'medium', - details: '', - testStrategy: '' - }; + if (destTaskIndex === -1) { + // Create placeholder for destination if it doesn't exist + log("info", `Creating placeholder for destination task ${destIdNum}`); + const newTask = { + id: destIdNum, + title: `Task ${destIdNum}`, + description: "", + status: "pending", + priority: "medium", + details: "", + testStrategy: "", + }; - // Find correct position to insert the new task - let insertIndex = 0; - while ( - insertIndex < data.tasks.length && - data.tasks[insertIndex].id < destIdNum - ) { - insertIndex++; - } + // Find correct position to insert the new task + let insertIndex = 0; + while ( + insertIndex < data.tasks.length && + data.tasks[insertIndex].id < destIdNum + ) { + insertIndex++; + } - // Insert the new task at the appropriate position - data.tasks.splice(insertIndex, 0, newTask); - destTaskIndex = insertIndex; - destTask = data.tasks[destTaskIndex]; - } else { - destTask = data.tasks[destTaskIndex]; - } - } + // Insert the new task at the appropriate position + data.tasks.splice(insertIndex, 0, newTask); + destTaskIndex = insertIndex; + destTask = data.tasks[destTaskIndex]; + } else { + destTask = data.tasks[destTaskIndex]; + } + } - // Validate that we aren't trying to move a task to itself - if (sourceId === destinationId) { - throw new Error('Cannot move a task/subtask to itself'); - } + // Validate that we aren't trying to move a task to itself + if (sourceId === destinationId) { + throw new Error("Cannot move a task/subtask to itself"); + } - // Prevent moving a parent to its own subtask - if (!isSourceSubtask && isDestinationSubtask) { - const destParentId = parseInt(destinationId.split('.')[0], 10); - if (parseInt(sourceId, 10) === destParentId) { - throw new Error('Cannot move a parent task to one of its own subtasks'); - } - } + // Prevent moving a parent to its own subtask + if (!isSourceSubtask && isDestinationSubtask) { + const destParentId = parseInt(destinationId.split(".")[0], 10); + if (parseInt(sourceId, 10) === destParentId) { + throw new Error("Cannot move a parent task to one of its own subtasks"); + } + } - // Check for circular dependency when moving tasks - if (!isSourceSubtask && !isDestinationSubtask) { - const sourceIdNum = parseInt(sourceId, 10); - const destIdNum = parseInt(destinationId, 10); + // Check for circular dependency when moving tasks + if (!isSourceSubtask && !isDestinationSubtask) { + const sourceIdNum = parseInt(sourceId, 10); + const destIdNum = parseInt(destinationId, 10); - // Check if destination is dependent on source - if (isTaskDependentOn(data.tasks, destTask, sourceIdNum)) { - throw new Error( - `Cannot move task ${sourceId} to task ${destinationId} as it would create a circular dependency` - ); - } - } + // Check if destination is dependent on source + if (isTaskDependentOn(data.tasks, destTask, sourceIdNum)) { + throw new Error( + `Cannot move task ${sourceId} to task ${destinationId} as it would create a circular dependency` + ); + } + } - let movedTask; + let movedTask; - // Handle different move scenarios - if (!isSourceSubtask && !isDestinationSubtask) { - // Case: Moving task to task position - // Always treat this as a task replacement/move to new ID - // The destination task will be replaced by the source task - movedTask = moveTaskToNewId( - data, - sourceTask, - sourceTaskIndex, - destTask, - destTaskIndex - ); - } else if (!isSourceSubtask && isDestinationSubtask) { - // Case 2: Move standalone task to become a subtask at a specific position - movedTask = moveTaskToSubtaskPosition( - data, - sourceTask, - sourceTaskIndex, - destParentTask, - destSubtaskIndex - ); - } else if (isSourceSubtask && !isDestinationSubtask) { - // Case 3: Move subtask to become a standalone task - movedTask = moveSubtaskToTask( - data, - sourceSubtask, - sourceParentTask, - sourceSubtaskIndex, - destTask - ); - } else if (isSourceSubtask && isDestinationSubtask) { - // Case 4: Move subtask to another parent or position - // First check if it's the same parent - const sourceParentId = parseInt(sourceId.split('.')[0], 10); - const destParentId = parseInt(destinationId.split('.')[0], 10); + // Handle different move scenarios + if (!isSourceSubtask && !isDestinationSubtask) { + // Case: Moving task to task position + // Always treat this as a task replacement/move to new ID + // The destination task will be replaced by the source task + movedTask = moveTaskToNewId( + data, + sourceTask, + sourceTaskIndex, + destTask, + destTaskIndex + ); + } else if (!isSourceSubtask && isDestinationSubtask) { + // Case 2: Move standalone task to become a subtask at a specific position + movedTask = moveTaskToSubtaskPosition( + data, + sourceTask, + sourceTaskIndex, + destParentTask, + destSubtaskIndex + ); + } else if (isSourceSubtask && !isDestinationSubtask) { + // Case 3: Move subtask to become a standalone task + movedTask = moveSubtaskToTask( + data, + sourceSubtask, + sourceParentTask, + sourceSubtaskIndex, + destTask + ); + } else if (isSourceSubtask && isDestinationSubtask) { + // Case 4: Move subtask to another parent or position + // First check if it's the same parent + const sourceParentId = parseInt(sourceId.split(".")[0], 10); + const destParentId = parseInt(destinationId.split(".")[0], 10); - if (sourceParentId === destParentId) { - // Case 4a: Move subtask within the same parent (reordering) - movedTask = reorderSubtask( - sourceParentTask, - sourceSubtaskIndex, - destSubtaskIndex - ); - } else { - // Case 4b: Move subtask to a different parent - movedTask = moveSubtaskToAnotherParent( - sourceSubtask, - sourceParentTask, - sourceSubtaskIndex, - destParentTask, - destSubtaskIndex - ); - } - } + if (sourceParentId === destParentId) { + // Case 4a: Move subtask within the same parent (reordering) + movedTask = reorderSubtask( + sourceParentTask, + sourceSubtaskIndex, + destSubtaskIndex + ); + } else { + // Case 4b: Move subtask to a different parent + movedTask = moveSubtaskToAnotherParent( + sourceSubtask, + sourceParentTask, + sourceSubtaskIndex, + destParentTask, + destSubtaskIndex + ); + } + } - // Write the updated tasks back to the file - writeJSON(tasksPath, data); + // Write the updated tasks back to the file + writeJSON(tasksPath, data); - // Generate task files if requested - if (generateFiles) { - log('info', 'Regenerating task files...'); - await generateTaskFiles(tasksPath, path.dirname(tasksPath)); - } + // Generate task files if requested + if (generateFiles) { + log("info", "Regenerating task files..."); + await generateTaskFiles(tasksPath, path.dirname(tasksPath)); + } - return movedTask; - } catch (error) { - log('error', `Error moving task/subtask: ${error.message}`); - throw error; - } + return movedTask; + } catch (error) { + log("error", `Error moving task/subtask: ${error.message}`); + throw error; + } } /** @@ -347,37 +347,37 @@ async function moveSingleTask( * @returns {Object} Moved task object */ function moveTaskToTask(data, sourceTask, sourceTaskIndex, destTask) { - // Initialize subtasks array if it doesn't exist - if (!destTask.subtasks) { - destTask.subtasks = []; - } + // Initialize subtasks array if it doesn't exist + if (!destTask.subtasks) { + destTask.subtasks = []; + } - // Find the highest subtask ID to determine the next ID - const highestSubtaskId = - destTask.subtasks.length > 0 - ? Math.max(...destTask.subtasks.map((st) => st.id)) - : 0; - const newSubtaskId = highestSubtaskId + 1; + // Find the highest subtask ID to determine the next ID + const highestSubtaskId = + destTask.subtasks.length > 0 + ? Math.max(...destTask.subtasks.map((st) => st.id)) + : 0; + const newSubtaskId = highestSubtaskId + 1; - // Create the new subtask from the source task - const newSubtask = { - ...sourceTask, - id: newSubtaskId, - parentTaskId: destTask.id - }; + // Create the new subtask from the source task + const newSubtask = { + ...sourceTask, + id: newSubtaskId, + parentTaskId: destTask.id, + }; - // Add to destination's subtasks - destTask.subtasks.push(newSubtask); + // Add to destination's subtasks + destTask.subtasks.push(newSubtask); - // Remove the original task from the tasks array - data.tasks.splice(sourceTaskIndex, 1); + // Remove the original task from the tasks array + data.tasks.splice(sourceTaskIndex, 1); - log( - 'info', - `Moved task ${sourceTask.id} to become subtask ${destTask.id}.${newSubtaskId}` - ); + log( + "info", + `Moved task ${sourceTask.id} to become subtask ${destTask.id}.${newSubtaskId}` + ); - return newSubtask; + return newSubtask; } /** @@ -390,46 +390,46 @@ function moveTaskToTask(data, sourceTask, sourceTaskIndex, destTask) { * @returns {Object} Moved task object */ function moveTaskToSubtaskPosition( - data, - sourceTask, - sourceTaskIndex, - destParentTask, - destSubtaskIndex + data, + sourceTask, + sourceTaskIndex, + destParentTask, + destSubtaskIndex ) { - // Initialize subtasks array if it doesn't exist - if (!destParentTask.subtasks) { - destParentTask.subtasks = []; - } + // Initialize subtasks array if it doesn't exist + if (!destParentTask.subtasks) { + destParentTask.subtasks = []; + } - // Find the highest subtask ID to determine the next ID - const highestSubtaskId = - destParentTask.subtasks.length > 0 - ? Math.max(...destParentTask.subtasks.map((st) => st.id)) - : 0; - const newSubtaskId = highestSubtaskId + 1; + // Find the highest subtask ID to determine the next ID + const highestSubtaskId = + destParentTask.subtasks.length > 0 + ? Math.max(...destParentTask.subtasks.map((st) => st.id)) + : 0; + const newSubtaskId = highestSubtaskId + 1; - // Create the new subtask from the source task - const newSubtask = { - ...sourceTask, - id: newSubtaskId, - parentTaskId: destParentTask.id - }; + // Create the new subtask from the source task + const newSubtask = { + ...sourceTask, + id: newSubtaskId, + parentTaskId: destParentTask.id, + }; - // Insert at specific position - // If destSubtaskIndex is -1, insert at the beginning (position 0) - // Otherwise, insert after the specified subtask - const insertPosition = destSubtaskIndex === -1 ? 0 : destSubtaskIndex + 1; - destParentTask.subtasks.splice(insertPosition, 0, newSubtask); + // Insert at specific position + // If destSubtaskIndex is -1, insert at the beginning (position 0) + // Otherwise, insert after the specified subtask + const insertPosition = destSubtaskIndex === -1 ? 0 : destSubtaskIndex + 1; + destParentTask.subtasks.splice(insertPosition, 0, newSubtask); - // Remove the original task from the tasks array - data.tasks.splice(sourceTaskIndex, 1); + // Remove the original task from the tasks array + data.tasks.splice(sourceTaskIndex, 1); - log( - 'info', - `Moved task ${sourceTask.id} to become subtask ${destParentTask.id}.${newSubtaskId}` - ); + log( + "info", + `Moved task ${sourceTask.id} to become subtask ${destParentTask.id}.${newSubtaskId}` + ); - return newSubtask; + return newSubtask; } /** @@ -438,56 +438,55 @@ function moveTaskToSubtaskPosition( * @param {Object} sourceSubtask - Source subtask to move * @param {Object} sourceParentTask - Parent task of the source subtask * @param {number} sourceSubtaskIndex - Index of source subtask in parent's subtasks - * @param {Object} destTask - Destination task (for position reference) + * @param {Object} destTask - Destination task (will be replaced) * @returns {Object} Moved task object */ function moveSubtaskToTask( - data, - sourceSubtask, - sourceParentTask, - sourceSubtaskIndex, - destTask + data, + sourceSubtask, + sourceParentTask, + sourceSubtaskIndex, + destTask ) { - // Find the highest task ID to determine the next ID - const highestId = Math.max(...data.tasks.map((t) => t.id)); - const newTaskId = highestId + 1; + // Use the destination task's ID instead of generating a new one + const newTaskId = destTask.id; - // Create the new task from the subtask - const newTask = { - ...sourceSubtask, - id: newTaskId, - priority: sourceParentTask.priority || 'medium' // Inherit priority from parent - }; - delete newTask.parentTaskId; + // Create the new task from the subtask, using the destination task's ID + const newTask = { + ...sourceSubtask, + id: newTaskId, + priority: sourceParentTask.priority || "medium", // Inherit priority from parent + }; + delete newTask.parentTaskId; - // Add the parent task as a dependency if not already present - if (!newTask.dependencies) { - newTask.dependencies = []; - } - if (!newTask.dependencies.includes(sourceParentTask.id)) { - newTask.dependencies.push(sourceParentTask.id); - } + // Add the parent task as a dependency if not already present + if (!newTask.dependencies) { + newTask.dependencies = []; + } + if (!newTask.dependencies.includes(sourceParentTask.id)) { + newTask.dependencies.push(sourceParentTask.id); + } - // Find the destination index to insert the new task - const destTaskIndex = data.tasks.findIndex((t) => t.id === destTask.id); + // Find the destination index to replace the destination task + const destTaskIndex = data.tasks.findIndex((t) => t.id === destTask.id); - // Insert the new task after the destination task - data.tasks.splice(destTaskIndex + 1, 0, newTask); + // Replace the destination task with the new task + data.tasks[destTaskIndex] = newTask; - // Remove the subtask from the parent - sourceParentTask.subtasks.splice(sourceSubtaskIndex, 1); + // Remove the subtask from the parent + sourceParentTask.subtasks.splice(sourceSubtaskIndex, 1); - // If parent has no more subtasks, remove the subtasks array - if (sourceParentTask.subtasks.length === 0) { - delete sourceParentTask.subtasks; - } + // If parent has no more subtasks, remove the subtasks array + if (sourceParentTask.subtasks.length === 0) { + delete sourceParentTask.subtasks; + } - log( - 'info', - `Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become task ${newTaskId}` - ); + log( + "info", + `Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become task ${newTaskId}` + ); - return newTask; + return newTask; } /** @@ -498,23 +497,23 @@ function moveSubtaskToTask( * @returns {Object} Moved subtask object */ function reorderSubtask(parentTask, sourceIndex, destIndex) { - // Get the subtask to move - const subtask = parentTask.subtasks[sourceIndex]; + // Get the subtask to move + const subtask = parentTask.subtasks[sourceIndex]; - // Remove the subtask from its current position - parentTask.subtasks.splice(sourceIndex, 1); + // Remove the subtask from its current position + parentTask.subtasks.splice(sourceIndex, 1); - // Insert the subtask at the new position - // If destIndex was after sourceIndex, it's now one less because we removed an item - const adjustedDestIndex = sourceIndex < destIndex ? destIndex - 1 : destIndex; - parentTask.subtasks.splice(adjustedDestIndex, 0, subtask); + // Insert the subtask at the new position + // If destIndex was after sourceIndex, it's now one less because we removed an item + const adjustedDestIndex = sourceIndex < destIndex ? destIndex - 1 : destIndex; + parentTask.subtasks.splice(adjustedDestIndex, 0, subtask); - log( - 'info', - `Reordered subtask ${parentTask.id}.${subtask.id} within parent task ${parentTask.id}` - ); + log( + "info", + `Reordered subtask ${parentTask.id}.${subtask.id} within parent task ${parentTask.id}` + ); - return subtask; + return subtask; } /** @@ -527,54 +526,54 @@ function reorderSubtask(parentTask, sourceIndex, destIndex) { * @returns {Object} Moved subtask object */ function moveSubtaskToAnotherParent( - sourceSubtask, - sourceParentTask, - sourceSubtaskIndex, - destParentTask, - destSubtaskIndex + sourceSubtask, + sourceParentTask, + sourceSubtaskIndex, + destParentTask, + destSubtaskIndex ) { - // Find the highest subtask ID in the destination parent - const highestSubtaskId = - destParentTask.subtasks.length > 0 - ? Math.max(...destParentTask.subtasks.map((st) => st.id)) - : 0; - const newSubtaskId = highestSubtaskId + 1; + // Find the highest subtask ID in the destination parent + const highestSubtaskId = + destParentTask.subtasks.length > 0 + ? Math.max(...destParentTask.subtasks.map((st) => st.id)) + : 0; + const newSubtaskId = highestSubtaskId + 1; - // Create the new subtask with updated parent reference - const newSubtask = { - ...sourceSubtask, - id: newSubtaskId, - parentTaskId: destParentTask.id - }; + // Create the new subtask with updated parent reference + const newSubtask = { + ...sourceSubtask, + id: newSubtaskId, + parentTaskId: destParentTask.id, + }; - // If the subtask depends on its original parent, keep that dependency - if (!newSubtask.dependencies) { - newSubtask.dependencies = []; - } - if (!newSubtask.dependencies.includes(sourceParentTask.id)) { - newSubtask.dependencies.push(sourceParentTask.id); - } + // If the subtask depends on its original parent, keep that dependency + if (!newSubtask.dependencies) { + newSubtask.dependencies = []; + } + if (!newSubtask.dependencies.includes(sourceParentTask.id)) { + newSubtask.dependencies.push(sourceParentTask.id); + } - // Insert at the destination position - // If destSubtaskIndex is -1, insert at the beginning (position 0) - // Otherwise, insert after the specified subtask - const insertPosition = destSubtaskIndex === -1 ? 0 : destSubtaskIndex + 1; - destParentTask.subtasks.splice(insertPosition, 0, newSubtask); + // Insert at the destination position + // If destSubtaskIndex is -1, insert at the beginning (position 0) + // Otherwise, insert after the specified subtask + const insertPosition = destSubtaskIndex === -1 ? 0 : destSubtaskIndex + 1; + destParentTask.subtasks.splice(insertPosition, 0, newSubtask); - // Remove the subtask from the original parent - sourceParentTask.subtasks.splice(sourceSubtaskIndex, 1); + // Remove the subtask from the original parent + sourceParentTask.subtasks.splice(sourceSubtaskIndex, 1); - // If original parent has no more subtasks, remove the subtasks array - if (sourceParentTask.subtasks.length === 0) { - delete sourceParentTask.subtasks; - } + // If original parent has no more subtasks, remove the subtasks array + if (sourceParentTask.subtasks.length === 0) { + delete sourceParentTask.subtasks; + } - log( - 'info', - `Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become subtask ${destParentTask.id}.${newSubtaskId}` - ); + log( + "info", + `Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become subtask ${destParentTask.id}.${newSubtaskId}` + ); - return newSubtask; + return newSubtask; } /** @@ -587,72 +586,72 @@ function moveSubtaskToAnotherParent( * @returns {Object} Moved task object */ function moveTaskToNewId( - data, - sourceTask, - sourceTaskIndex, - destTask, - destTaskIndex + data, + sourceTask, + sourceTaskIndex, + destTask, + destTaskIndex ) { - // Create a copy of the source task with the new ID - const movedTask = { - ...sourceTask, - id: destTask.id - }; + // Create a copy of the source task with the new ID + const movedTask = { + ...sourceTask, + id: destTask.id, + }; - // Get numeric IDs for comparison - const sourceIdNum = parseInt(sourceTask.id, 10); - const destIdNum = parseInt(destTask.id, 10); + // Get numeric IDs for comparison + const sourceIdNum = parseInt(sourceTask.id, 10); + const destIdNum = parseInt(destTask.id, 10); - // Handle subtasks if present - if (sourceTask.subtasks && sourceTask.subtasks.length > 0) { - // Update subtasks to reference the new parent ID if needed - movedTask.subtasks = sourceTask.subtasks.map((subtask) => ({ - ...subtask, - parentTaskId: destIdNum - })); - } + // Handle subtasks if present + if (sourceTask.subtasks && sourceTask.subtasks.length > 0) { + // Update subtasks to reference the new parent ID if needed + movedTask.subtasks = sourceTask.subtasks.map((subtask) => ({ + ...subtask, + parentTaskId: destIdNum, + })); + } - // Update any dependencies in other tasks that referenced the old source ID - data.tasks.forEach((task) => { - if (task.dependencies && task.dependencies.includes(sourceIdNum)) { - // Replace the old source ID with the new destination ID - const depIndex = task.dependencies.indexOf(sourceIdNum); - task.dependencies[depIndex] = destIdNum; - } + // Update any dependencies in other tasks that referenced the old source ID + data.tasks.forEach((task) => { + if (task.dependencies && task.dependencies.includes(sourceIdNum)) { + // Replace the old source ID with the new destination ID + const depIndex = task.dependencies.indexOf(sourceIdNum); + task.dependencies[depIndex] = destIdNum; + } - // Also check for subtask dependencies that might reference the source task - if (task.subtasks && task.subtasks.length > 0) { - task.subtasks.forEach((subtask) => { - if ( - subtask.dependencies && - subtask.dependencies.includes(sourceIdNum) - ) { - const depIndex = subtask.dependencies.indexOf(sourceIdNum); - subtask.dependencies[depIndex] = destIdNum; - } - }); - } - }); + // Also check for subtask dependencies that might reference the source task + if (task.subtasks && task.subtasks.length > 0) { + task.subtasks.forEach((subtask) => { + if ( + subtask.dependencies && + subtask.dependencies.includes(sourceIdNum) + ) { + const depIndex = subtask.dependencies.indexOf(sourceIdNum); + subtask.dependencies[depIndex] = destIdNum; + } + }); + } + }); - // Remove tasks in the correct order to avoid index shifting issues - // Always remove the higher index first to avoid shifting the lower index - if (sourceTaskIndex > destTaskIndex) { - // Remove source first (higher index), then destination - data.tasks.splice(sourceTaskIndex, 1); - data.tasks.splice(destTaskIndex, 1); - // Insert the moved task at the destination position - data.tasks.splice(destTaskIndex, 0, movedTask); - } else { - // Remove destination first (higher index), then source - data.tasks.splice(destTaskIndex, 1); - data.tasks.splice(sourceTaskIndex, 1); - // Insert the moved task at the original destination position (now shifted down by 1) - data.tasks.splice(sourceTaskIndex, 0, movedTask); - } + // Remove tasks in the correct order to avoid index shifting issues + // Always remove the higher index first to avoid shifting the lower index + if (sourceTaskIndex > destTaskIndex) { + // Remove source first (higher index), then destination + data.tasks.splice(sourceTaskIndex, 1); + data.tasks.splice(destTaskIndex, 1); + // Insert the moved task at the destination position + data.tasks.splice(destTaskIndex, 0, movedTask); + } else { + // Remove destination first (higher index), then source + data.tasks.splice(destTaskIndex, 1); + data.tasks.splice(sourceTaskIndex, 1); + // Insert the moved task at the original destination position (now shifted down by 1) + data.tasks.splice(sourceTaskIndex, 0, movedTask); + } - log('info', `Moved task ${sourceIdNum} to replace task ${destIdNum}`); + log("info", `Moved task ${sourceIdNum} to replace task ${destIdNum}`); - return movedTask; + return movedTask; } export default moveTask; diff --git a/test-move-fix.js b/test-move-fix.js new file mode 100644 index 00000000..4b2c0ede --- /dev/null +++ b/test-move-fix.js @@ -0,0 +1,227 @@ +/** + * Test script for move-task functionality + * + * This script tests various scenarios for the move-task command to ensure + * it works correctly without creating duplicate tasks or leaving orphaned data. + * + * Test scenarios covered: + * 1. Moving a subtask to become a standalone task (with specific target ID) + * 2. Moving a task to replace another task + * + * Usage: + * node test-move-fix.js # Run all tests + * + * Or import specific test functions: + * import { testMoveSubtaskToTask } from './test-move-fix.js'; + * + * This was created to verify the fix for the bug where moving subtasks + * to standalone tasks was creating duplicate entries. + */ + +import fs from "fs"; +import path from "path"; +import moveTask from "./scripts/modules/task-manager/move-task.js"; + +// Create a test tasks.json file +const testData = { + tasks: [ + { + id: 1, + title: "Parent Task", + description: "A parent task with subtasks", + status: "pending", + priority: "medium", + details: "Parent task details", + testStrategy: "Parent test strategy", + subtasks: [ + { + id: 1, + title: "Subtask 1", + description: "First subtask", + status: "pending", + details: "Subtask 1 details", + testStrategy: "Subtask 1 test strategy", + }, + { + id: 2, + title: "Subtask 2", + description: "Second subtask", + status: "pending", + details: "Subtask 2 details", + testStrategy: "Subtask 2 test strategy", + }, + ], + }, + { + id: 2, + title: "Another Task", + description: "Another standalone task", + status: "pending", + priority: "low", + details: "Another task details", + testStrategy: "Another test strategy", + }, + { + id: 3, + title: "Third Task", + description: "A third standalone task", + status: "done", + priority: "high", + details: "Third task details", + testStrategy: "Third test strategy", + }, + ], +}; + +const testFile = "./test-tasks.json"; + +function logSeparator(title) { + console.log(`\n${"=".repeat(60)}`); + console.log(` ${title}`); + console.log(`${"=".repeat(60)}`); +} + +function logTaskState(data, label) { + console.log(`\n${label}:`); + console.log( + "Tasks:", + data.tasks.map((t) => ({ id: t.id, title: t.title, status: t.status })) + ); + + data.tasks.forEach((task) => { + if (task.subtasks && task.subtasks.length > 0) { + console.log( + `Task ${task.id} subtasks:`, + task.subtasks.map((st) => ({ id: st.id, title: st.title })) + ); + } + }); +} + +async function testMoveSubtaskToTask() { + try { + logSeparator("TEST: Move Subtask to Standalone Task"); + + // Write test data + fs.writeFileSync(testFile, JSON.stringify(testData, null, 2)); + + const beforeData = JSON.parse(fs.readFileSync(testFile, "utf8")); + logTaskState(beforeData, "Before move"); + + // Move subtask 1.2 to become task 26 + console.log("\n๐Ÿ”„ Moving subtask 1.2 to task 26..."); + const result = await moveTask(testFile, "1.2", "26", false); + + const afterData = JSON.parse(fs.readFileSync(testFile, "utf8")); + logTaskState(afterData, "After move"); + + // Verify the result + const task26 = afterData.tasks.find((t) => t.id === 26); + if (task26) { + console.log("\nโœ… SUCCESS: Task 26 created with correct content:"); + console.log(" Title:", task26.title); + console.log(" Description:", task26.description); + console.log(" Details:", task26.details); + console.log(" Dependencies:", task26.dependencies); + console.log(" Priority:", task26.priority); + } else { + console.log("\nโŒ FAILED: Task 26 not found"); + } + + // Check for duplicates + const taskIds = afterData.tasks.map((t) => t.id); + const duplicates = taskIds.filter( + (id, index) => taskIds.indexOf(id) !== index + ); + if (duplicates.length > 0) { + console.log("\nโŒ FAILED: Duplicate task IDs found:", duplicates); + } else { + console.log("\nโœ… SUCCESS: No duplicate task IDs"); + } + + // Check that original subtask was removed + const task1 = afterData.tasks.find((t) => t.id === 1); + const hasSubtask2 = task1.subtasks?.some((st) => st.id === 2); + if (hasSubtask2) { + console.log("\nโŒ FAILED: Original subtask 1.2 still exists"); + } else { + console.log("\nโœ… SUCCESS: Original subtask 1.2 was removed"); + } + + return true; + } catch (error) { + console.error("\nโŒ Test failed:", error.message); + return false; + } +} + +async function testMoveTaskToTask() { + try { + logSeparator("TEST: Move Task to Replace Another Task"); + + // Reset test data + fs.writeFileSync(testFile, JSON.stringify(testData, null, 2)); + + const beforeData = JSON.parse(fs.readFileSync(testFile, "utf8")); + logTaskState(beforeData, "Before move"); + + // Move task 2 to replace task 3 + console.log("\n๐Ÿ”„ Moving task 2 to replace task 3..."); + const result = await moveTask(testFile, "2", "3", false); + + const afterData = JSON.parse(fs.readFileSync(testFile, "utf8")); + logTaskState(afterData, "After move"); + + // Verify the result + const task3 = afterData.tasks.find((t) => t.id === 3); + const task2Gone = !afterData.tasks.find((t) => t.id === 2); + + if (task3 && task3.title === "Another Task" && task2Gone) { + console.log("\nโœ… SUCCESS: Task 2 replaced task 3 correctly"); + console.log(" New Task 3 title:", task3.title); + console.log(" New Task 3 description:", task3.description); + } else { + console.log("\nโŒ FAILED: Task replacement didn't work correctly"); + } + + return true; + } catch (error) { + console.error("\nโŒ Test failed:", error.message); + return false; + } +} + +async function runAllTests() { + console.log("๐Ÿงช Running Move Task Tests"); + + const results = []; + + results.push(await testMoveSubtaskToTask()); + results.push(await testMoveTaskToTask()); + + const passed = results.filter((r) => r).length; + const total = results.length; + + logSeparator("TEST SUMMARY"); + console.log(`\n๐Ÿ“Š Results: ${passed}/${total} tests passed`); + + if (passed === total) { + console.log("๐ŸŽ‰ All tests passed!"); + } else { + console.log("โš ๏ธ Some tests failed. Check the output above."); + } + + // Clean up + if (fs.existsSync(testFile)) { + fs.unlinkSync(testFile); + console.log("\n๐Ÿงน Cleaned up test files"); + } +} + +// Run tests if this file is executed directly +if (import.meta.url === `file://${process.argv[1]}`) { + runAllTests(); +} + +// Export for use in other test files +export { testMoveSubtaskToTask, testMoveTaskToTask, runAllTests };