fix(move-task): Fix duplicate task creation when moving subtask to standalone task

This commit is contained in:
Eyal Toledano
2025-05-28 14:05:30 -04:00
parent ab64437ad2
commit 9e6c190af3
2 changed files with 703 additions and 477 deletions

View File

@@ -1,7 +1,7 @@
import path from 'path'; import path from "path";
import { log, readJSON, writeJSON } from '../utils.js'; import { log, readJSON, writeJSON } from "../utils.js";
import { isTaskDependentOn } from '../task-manager.js'; import { isTaskDependentOn } from "../task-manager.js";
import generateTaskFiles from './generate-task-files.js'; import generateTaskFiles from "./generate-task-files.js";
/** /**
* Move one or more tasks/subtasks to new positions * Move one or more tasks/subtasks to new positions
@@ -18,8 +18,8 @@ async function moveTask(
generateFiles = true generateFiles = true
) { ) {
// Check if we have comma-separated IDs (multiple moves) // Check if we have comma-separated IDs (multiple moves)
const sourceIds = sourceId.split(',').map((id) => id.trim()); const sourceIds = sourceId.split(",").map((id) => id.trim());
const destinationIds = destinationId.split(',').map((id) => id.trim()); const destinationIds = destinationId.split(",").map((id) => id.trim());
// If multiple IDs, validate they match in count // If multiple IDs, validate they match in count
if (sourceIds.length > 1 || destinationIds.length > 1) { if (sourceIds.length > 1 || destinationIds.length > 1) {
@@ -63,8 +63,8 @@ async function moveMultipleTasks(
) { ) {
try { try {
log( log(
'info', "info",
`Moving multiple tasks/subtasks: ${sourceIds.join(', ')} to ${destinationIds.join(', ')}...` `Moving multiple tasks/subtasks: ${sourceIds.join(", ")} to ${destinationIds.join(", ")}...`
); );
const results = []; const results = [];
@@ -82,16 +82,16 @@ async function moveMultipleTasks(
// Generate task files once at the end if requested // Generate task files once at the end if requested
if (generateFiles) { if (generateFiles) {
log('info', 'Regenerating task files...'); log("info", "Regenerating task files...");
await generateTaskFiles(tasksPath, path.dirname(tasksPath)); await generateTaskFiles(tasksPath, path.dirname(tasksPath));
} }
return { return {
message: `Successfully moved ${sourceIds.length} tasks/subtasks`, message: `Successfully moved ${sourceIds.length} tasks/subtasks`,
moves: results moves: results,
}; };
} catch (error) { } catch (error) {
log('error', `Error moving multiple tasks/subtasks: ${error.message}`); log("error", `Error moving multiple tasks/subtasks: ${error.message}`);
throw error; throw error;
} }
} }
@@ -111,7 +111,7 @@ async function moveSingleTask(
generateFiles = true generateFiles = true
) { ) {
try { try {
log('info', `Moving task/subtask ${sourceId} to ${destinationId}...`); log("info", `Moving task/subtask ${sourceId} to ${destinationId}...`);
// Read the existing tasks // Read the existing tasks
const data = readJSON(tasksPath); const data = readJSON(tasksPath);
@@ -120,7 +120,7 @@ async function moveSingleTask(
} }
// Parse source ID to determine if it's a task or subtask // Parse source ID to determine if it's a task or subtask
const isSourceSubtask = sourceId.includes('.'); const isSourceSubtask = sourceId.includes(".");
let sourceTask, let sourceTask,
sourceParentTask, sourceParentTask,
sourceSubtask, sourceSubtask,
@@ -128,13 +128,13 @@ async function moveSingleTask(
sourceSubtaskIndex; sourceSubtaskIndex;
// Parse destination ID to determine the target // Parse destination ID to determine the target
const isDestinationSubtask = destinationId.includes('.'); const isDestinationSubtask = destinationId.includes(".");
let destTask, destParentTask, destSubtask, destTaskIndex, destSubtaskIndex; let destTask, destParentTask, destSubtask, destTaskIndex, destSubtaskIndex;
// Validate source exists // Validate source exists
if (isSourceSubtask) { if (isSourceSubtask) {
// Source is a subtask // Source is a subtask
const [parentIdStr, subtaskIdStr] = sourceId.split('.'); const [parentIdStr, subtaskIdStr] = sourceId.split(".");
const parentIdNum = parseInt(parentIdStr, 10); const parentIdNum = parseInt(parentIdStr, 10);
const subtaskIdNum = parseInt(subtaskIdStr, 10); const subtaskIdNum = parseInt(subtaskIdStr, 10);
@@ -172,7 +172,7 @@ async function moveSingleTask(
// Validate destination exists // Validate destination exists
if (isDestinationSubtask) { if (isDestinationSubtask) {
// Destination is a subtask (target will be the parent of this subtask) // Destination is a subtask (target will be the parent of this subtask)
const [parentIdStr, subtaskIdStr] = destinationId.split('.'); const [parentIdStr, subtaskIdStr] = destinationId.split(".");
const parentIdNum = parseInt(parentIdStr, 10); const parentIdNum = parseInt(parentIdStr, 10);
const subtaskIdNum = parseInt(subtaskIdStr, 10); const subtaskIdNum = parseInt(subtaskIdStr, 10);
@@ -210,15 +210,15 @@ async function moveSingleTask(
if (destTaskIndex === -1) { if (destTaskIndex === -1) {
// Create placeholder for destination if it doesn't exist // Create placeholder for destination if it doesn't exist
log('info', `Creating placeholder for destination task ${destIdNum}`); log("info", `Creating placeholder for destination task ${destIdNum}`);
const newTask = { const newTask = {
id: destIdNum, id: destIdNum,
title: `Task ${destIdNum}`, title: `Task ${destIdNum}`,
description: '', description: "",
status: 'pending', status: "pending",
priority: 'medium', priority: "medium",
details: '', details: "",
testStrategy: '' testStrategy: "",
}; };
// Find correct position to insert the new task // Find correct position to insert the new task
@@ -241,14 +241,14 @@ async function moveSingleTask(
// Validate that we aren't trying to move a task to itself // Validate that we aren't trying to move a task to itself
if (sourceId === destinationId) { if (sourceId === destinationId) {
throw new Error('Cannot move a task/subtask to itself'); throw new Error("Cannot move a task/subtask to itself");
} }
// Prevent moving a parent to its own subtask // Prevent moving a parent to its own subtask
if (!isSourceSubtask && isDestinationSubtask) { if (!isSourceSubtask && isDestinationSubtask) {
const destParentId = parseInt(destinationId.split('.')[0], 10); const destParentId = parseInt(destinationId.split(".")[0], 10);
if (parseInt(sourceId, 10) === destParentId) { if (parseInt(sourceId, 10) === destParentId) {
throw new Error('Cannot move a parent task to one of its own subtasks'); throw new Error("Cannot move a parent task to one of its own subtasks");
} }
} }
@@ -300,8 +300,8 @@ async function moveSingleTask(
} else if (isSourceSubtask && isDestinationSubtask) { } else if (isSourceSubtask && isDestinationSubtask) {
// Case 4: Move subtask to another parent or position // Case 4: Move subtask to another parent or position
// First check if it's the same parent // First check if it's the same parent
const sourceParentId = parseInt(sourceId.split('.')[0], 10); const sourceParentId = parseInt(sourceId.split(".")[0], 10);
const destParentId = parseInt(destinationId.split('.')[0], 10); const destParentId = parseInt(destinationId.split(".")[0], 10);
if (sourceParentId === destParentId) { if (sourceParentId === destParentId) {
// Case 4a: Move subtask within the same parent (reordering) // Case 4a: Move subtask within the same parent (reordering)
@@ -327,13 +327,13 @@ async function moveSingleTask(
// Generate task files if requested // Generate task files if requested
if (generateFiles) { if (generateFiles) {
log('info', 'Regenerating task files...'); log("info", "Regenerating task files...");
await generateTaskFiles(tasksPath, path.dirname(tasksPath)); await generateTaskFiles(tasksPath, path.dirname(tasksPath));
} }
return movedTask; return movedTask;
} catch (error) { } catch (error) {
log('error', `Error moving task/subtask: ${error.message}`); log("error", `Error moving task/subtask: ${error.message}`);
throw error; throw error;
} }
} }
@@ -363,7 +363,7 @@ function moveTaskToTask(data, sourceTask, sourceTaskIndex, destTask) {
const newSubtask = { const newSubtask = {
...sourceTask, ...sourceTask,
id: newSubtaskId, id: newSubtaskId,
parentTaskId: destTask.id parentTaskId: destTask.id,
}; };
// Add to destination's subtasks // Add to destination's subtasks
@@ -373,7 +373,7 @@ function moveTaskToTask(data, sourceTask, sourceTaskIndex, destTask) {
data.tasks.splice(sourceTaskIndex, 1); data.tasks.splice(sourceTaskIndex, 1);
log( log(
'info', "info",
`Moved task ${sourceTask.id} to become subtask ${destTask.id}.${newSubtaskId}` `Moved task ${sourceTask.id} to become subtask ${destTask.id}.${newSubtaskId}`
); );
@@ -412,7 +412,7 @@ function moveTaskToSubtaskPosition(
const newSubtask = { const newSubtask = {
...sourceTask, ...sourceTask,
id: newSubtaskId, id: newSubtaskId,
parentTaskId: destParentTask.id parentTaskId: destParentTask.id,
}; };
// Insert at specific position // Insert at specific position
@@ -425,7 +425,7 @@ function moveTaskToSubtaskPosition(
data.tasks.splice(sourceTaskIndex, 1); data.tasks.splice(sourceTaskIndex, 1);
log( log(
'info', "info",
`Moved task ${sourceTask.id} to become subtask ${destParentTask.id}.${newSubtaskId}` `Moved task ${sourceTask.id} to become subtask ${destParentTask.id}.${newSubtaskId}`
); );
@@ -438,7 +438,7 @@ function moveTaskToSubtaskPosition(
* @param {Object} sourceSubtask - Source subtask to move * @param {Object} sourceSubtask - Source subtask to move
* @param {Object} sourceParentTask - Parent task of the source subtask * @param {Object} sourceParentTask - Parent task of the source subtask
* @param {number} sourceSubtaskIndex - Index of source subtask in parent's subtasks * @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 * @returns {Object} Moved task object
*/ */
function moveSubtaskToTask( function moveSubtaskToTask(
@@ -448,15 +448,14 @@ function moveSubtaskToTask(
sourceSubtaskIndex, sourceSubtaskIndex,
destTask destTask
) { ) {
// Find the highest task ID to determine the next ID // Use the destination task's ID instead of generating a new one
const highestId = Math.max(...data.tasks.map((t) => t.id)); const newTaskId = destTask.id;
const newTaskId = highestId + 1;
// Create the new task from the subtask // Create the new task from the subtask, using the destination task's ID
const newTask = { const newTask = {
...sourceSubtask, ...sourceSubtask,
id: newTaskId, id: newTaskId,
priority: sourceParentTask.priority || 'medium' // Inherit priority from parent priority: sourceParentTask.priority || "medium", // Inherit priority from parent
}; };
delete newTask.parentTaskId; delete newTask.parentTaskId;
@@ -468,11 +467,11 @@ function moveSubtaskToTask(
newTask.dependencies.push(sourceParentTask.id); newTask.dependencies.push(sourceParentTask.id);
} }
// Find the destination index to insert the new task // Find the destination index to replace the destination task
const destTaskIndex = data.tasks.findIndex((t) => t.id === destTask.id); const destTaskIndex = data.tasks.findIndex((t) => t.id === destTask.id);
// Insert the new task after the destination task // Replace the destination task with the new task
data.tasks.splice(destTaskIndex + 1, 0, newTask); data.tasks[destTaskIndex] = newTask;
// Remove the subtask from the parent // Remove the subtask from the parent
sourceParentTask.subtasks.splice(sourceSubtaskIndex, 1); sourceParentTask.subtasks.splice(sourceSubtaskIndex, 1);
@@ -483,7 +482,7 @@ function moveSubtaskToTask(
} }
log( log(
'info', "info",
`Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become task ${newTaskId}` `Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become task ${newTaskId}`
); );
@@ -510,7 +509,7 @@ function reorderSubtask(parentTask, sourceIndex, destIndex) {
parentTask.subtasks.splice(adjustedDestIndex, 0, subtask); parentTask.subtasks.splice(adjustedDestIndex, 0, subtask);
log( log(
'info', "info",
`Reordered subtask ${parentTask.id}.${subtask.id} within parent task ${parentTask.id}` `Reordered subtask ${parentTask.id}.${subtask.id} within parent task ${parentTask.id}`
); );
@@ -544,7 +543,7 @@ function moveSubtaskToAnotherParent(
const newSubtask = { const newSubtask = {
...sourceSubtask, ...sourceSubtask,
id: newSubtaskId, id: newSubtaskId,
parentTaskId: destParentTask.id parentTaskId: destParentTask.id,
}; };
// If the subtask depends on its original parent, keep that dependency // If the subtask depends on its original parent, keep that dependency
@@ -570,7 +569,7 @@ function moveSubtaskToAnotherParent(
} }
log( log(
'info', "info",
`Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become subtask ${destParentTask.id}.${newSubtaskId}` `Moved subtask ${sourceParentTask.id}.${sourceSubtask.id} to become subtask ${destParentTask.id}.${newSubtaskId}`
); );
@@ -596,7 +595,7 @@ function moveTaskToNewId(
// Create a copy of the source task with the new ID // Create a copy of the source task with the new ID
const movedTask = { const movedTask = {
...sourceTask, ...sourceTask,
id: destTask.id id: destTask.id,
}; };
// Get numeric IDs for comparison // Get numeric IDs for comparison
@@ -608,7 +607,7 @@ function moveTaskToNewId(
// Update subtasks to reference the new parent ID if needed // Update subtasks to reference the new parent ID if needed
movedTask.subtasks = sourceTask.subtasks.map((subtask) => ({ movedTask.subtasks = sourceTask.subtasks.map((subtask) => ({
...subtask, ...subtask,
parentTaskId: destIdNum parentTaskId: destIdNum,
})); }));
} }
@@ -650,7 +649,7 @@ function moveTaskToNewId(
data.tasks.splice(sourceTaskIndex, 0, movedTask); 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;
} }

227
test-move-fix.js Normal file
View File

@@ -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 };