From c0e856fdf04d9eea126ba388f0b8349437523eda Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Thu, 25 Sep 2025 23:51:42 +0200 Subject: [PATCH] chore: refactor and move updateTaskStatus business logic to storage service --- .../src/interfaces/storage.interface.ts | 30 +++- packages/tm-core/src/services/task-service.ts | 131 ++--------------- packages/tm-core/src/storage/api-storage.ts | 56 ++++++- .../src/storage/file-storage/file-storage.ts | 139 +++++++++++++++++- 4 files changed, 232 insertions(+), 124 deletions(-) diff --git a/packages/tm-core/src/interfaces/storage.interface.ts b/packages/tm-core/src/interfaces/storage.interface.ts index 4fee9893..e565a312 100644 --- a/packages/tm-core/src/interfaces/storage.interface.ts +++ b/packages/tm-core/src/interfaces/storage.interface.ts @@ -3,7 +3,7 @@ * This file defines the contract for all storage implementations */ -import type { Task, TaskMetadata } from '../types/index.js'; +import type { Task, TaskMetadata, TaskStatus } from '../types/index.js'; /** * Interface for storage operations on tasks @@ -54,6 +54,24 @@ export interface IStorage { tag?: string ): Promise; + /** + * Update task or subtask status by ID + * @param taskId - ID of the task or subtask (e.g., "1" or "1.2") + * @param newStatus - New status to set + * @param tag - Optional tag context for the task + * @returns Promise that resolves to update result with old and new status + */ + updateTaskStatus( + taskId: string, + newStatus: TaskStatus, + tag?: string + ): Promise<{ + success: boolean; + oldStatus: TaskStatus; + newStatus: TaskStatus; + taskId: string; + }>; + /** * Delete a task by ID * @param taskId - ID of the task to delete @@ -191,6 +209,16 @@ export abstract class BaseStorage implements IStorage { updates: Partial, tag?: string ): Promise; + abstract updateTaskStatus( + taskId: string, + newStatus: TaskStatus, + tag?: string + ): Promise<{ + success: boolean; + oldStatus: TaskStatus; + newStatus: TaskStatus; + taskId: string; + }>; abstract deleteTask(taskId: string, tag?: string): Promise; abstract exists(tag?: string): Promise; abstract loadMetadata(tag?: string): Promise; diff --git a/packages/tm-core/src/services/task-service.ts b/packages/tm-core/src/services/task-service.ts index 673448ff..cda791b1 100644 --- a/packages/tm-core/src/services/task-service.ts +++ b/packages/tm-core/src/services/task-service.ts @@ -446,7 +446,7 @@ export class TaskService { } /** - * Update task status + * Update task status - delegates to storage layer which handles storage-specific logic */ async updateTaskStatus( taskId: string | number, @@ -468,133 +468,22 @@ export class TaskService { // Use provided tag or get active tag const activeTag = tag || this.getActiveTag(); - const taskIdStr = String(taskId); - // Check if this is a subtask (contains a dot) - if (taskIdStr.includes('.')) { - return this.updateSubtaskStatus(taskIdStr, newStatus, activeTag); - } - - // Handle regular task update - // Get the current task to get old status (simple, direct approach) - let currentTask: Task | null; try { - // Try to get the task directly - currentTask = await this.storage.loadTask(taskIdStr, activeTag); + // Delegate to storage layer which handles the specific logic for tasks vs subtasks + return await this.storage.updateTaskStatus( + taskIdStr, + newStatus, + activeTag + ); } catch (error) { throw new TaskMasterError( - `Failed to load task ${taskIdStr}`, - ERROR_CODES.TASK_NOT_FOUND, - { taskId: taskIdStr }, + `Failed to update task status for ${taskIdStr}`, + ERROR_CODES.STORAGE_ERROR, + { taskId: taskIdStr, newStatus, tag: activeTag }, error as Error ); } - - if (!currentTask) { - throw new TaskMasterError( - `Task ${taskIdStr} not found`, - ERROR_CODES.TASK_NOT_FOUND - ); - } - - const oldStatus = currentTask.status; - - // Simple, direct update - just change the status - await this.storage.updateTask(taskIdStr, { status: newStatus }, activeTag); - - return { - success: true, - oldStatus, - newStatus, - taskId: taskIdStr - }; - } - - /** - * Update subtask status - * Handles subtask IDs in the format "parentId.subtaskId" (e.g., "21.1") - */ - private async updateSubtaskStatus( - subtaskId: string, - newStatus: TaskStatus, - activeTag: string - ): Promise<{ - success: boolean; - oldStatus: TaskStatus; - newStatus: TaskStatus; - taskId: string; - }> { - // Parse the subtask ID to get parent ID and subtask ID - const parts = subtaskId.split('.'); - if (parts.length !== 2) { - throw new TaskMasterError( - `Invalid subtask ID format: ${subtaskId}. Expected format: parentId.subtaskId`, - ERROR_CODES.VALIDATION_ERROR - ); - } - - const [parentId, subId] = parts; - const subtaskNumericId = parseInt(subId, 10); - - if (isNaN(subtaskNumericId)) { - throw new TaskMasterError( - `Invalid subtask ID: ${subId}. Subtask ID must be numeric.`, - ERROR_CODES.VALIDATION_ERROR - ); - } - - // Get the parent task - let parentTask: Task | null; - try { - parentTask = await this.storage.loadTask(parentId, activeTag); - } catch (error) { - throw new TaskMasterError( - `Failed to load parent task ${parentId}`, - ERROR_CODES.TASK_NOT_FOUND, - { parentId, subtaskId }, - error as Error - ); - } - - if (!parentTask) { - throw new TaskMasterError( - `Parent task ${parentId} not found`, - ERROR_CODES.TASK_NOT_FOUND, - { parentId, subtaskId } - ); - } - - // Find the subtask within the parent task - const subtaskIndex = parentTask.subtasks.findIndex( - (st) => st.id === subtaskNumericId - ); - - if (subtaskIndex === -1) { - throw new TaskMasterError( - `Subtask ${subtaskId} not found in parent task ${parentId}`, - ERROR_CODES.TASK_NOT_FOUND, - { parentId, subtaskId, availableSubtasks: parentTask.subtasks.map(st => st.id) } - ); - } - - const oldStatus = parentTask.subtasks[subtaskIndex].status; - - // Update the subtask status - parentTask.subtasks[subtaskIndex] = { - ...parentTask.subtasks[subtaskIndex], - status: newStatus, - updatedAt: new Date().toISOString() - }; - - // Save the updated parent task - await this.storage.updateTask(parentId, parentTask, activeTag); - - return { - success: true, - oldStatus, - newStatus, - taskId: subtaskId - }; } } diff --git a/packages/tm-core/src/storage/api-storage.ts b/packages/tm-core/src/storage/api-storage.ts index c4a475d8..2cfebb13 100644 --- a/packages/tm-core/src/storage/api-storage.ts +++ b/packages/tm-core/src/storage/api-storage.ts @@ -7,7 +7,7 @@ import type { IStorage, StorageStats } from '../interfaces/storage.interface.js'; -import type { Task, TaskMetadata, TaskTag } from '../types/index.js'; +import type { Task, TaskMetadata, TaskTag, TaskStatus } from '../types/index.js'; import { ERROR_CODES, TaskMasterError } from '../errors/task-master-error.js'; import { TaskRepository } from '../repositories/task-repository.interface.js'; import { SupabaseTaskRepository } from '../repositories/supabase-task-repository.js'; @@ -485,6 +485,60 @@ export class ApiStorage implements IStorage { } } + /** + * Update task or subtask status by ID - for API storage + */ + async updateTaskStatus( + taskId: string, + newStatus: TaskStatus, + tag?: string + ): Promise<{ + success: boolean; + oldStatus: TaskStatus; + newStatus: TaskStatus; + taskId: string; + }> { + await this.ensureInitialized(); + + try { + const existingTask = await this.retryOperation(() => + this.repository.getTask(this.projectId, taskId) + ); + + if (!existingTask) { + throw new Error(`Task ${taskId} not found`); + } + + const oldStatus = existingTask.status; + + // Update the task/subtask status + await this.retryOperation(() => + this.repository.updateTask(this.projectId, taskId, { + status: newStatus, + updatedAt: new Date().toISOString() + }) + ); + + // For subtasks, we might want to auto-adjust parent status + // but in API storage, we'd need to query for the parent and its other subtasks + // This is left as future enhancement since API storage handles relationships differently + + return { + success: true, + oldStatus, + newStatus, + taskId + }; + } catch (error) { + throw new TaskMasterError( + 'Failed to update task status via API', + ERROR_CODES.STORAGE_ERROR, + { operation: 'updateTaskStatus', taskId, newStatus, tag }, + error as Error + ); + } + } + /** * Get all available tags */ diff --git a/packages/tm-core/src/storage/file-storage/file-storage.ts b/packages/tm-core/src/storage/file-storage/file-storage.ts index 13fb27dd..9e01faa6 100644 --- a/packages/tm-core/src/storage/file-storage/file-storage.ts +++ b/packages/tm-core/src/storage/file-storage/file-storage.ts @@ -2,7 +2,7 @@ * @fileoverview Refactored file-based storage implementation for Task Master */ -import type { Task, TaskMetadata } from '../../types/index.js'; +import type { Task, TaskMetadata, TaskStatus } from '../../types/index.js'; import type { IStorage, StorageStats @@ -281,6 +281,143 @@ export class FileStorage implements IStorage { await this.saveTasks(tasks, tag); } + /** + * Update task or subtask status by ID - handles file storage logic with parent/subtask relationships + */ + async updateTaskStatus( + taskId: string, + newStatus: TaskStatus, + tag?: string + ): Promise<{ + success: boolean; + oldStatus: TaskStatus; + newStatus: TaskStatus; + taskId: string; + }> { + const tasks = await this.loadTasks(tag); + + // Check if this is a subtask (contains a dot) + if (taskId.includes('.')) { + return this.updateSubtaskStatusInFile(tasks, taskId, newStatus, tag); + } + + // Handle regular task update + const taskIndex = tasks.findIndex((t) => String(t.id) === String(taskId)); + + if (taskIndex === -1) { + throw new Error(`Task ${taskId} not found`); + } + + const oldStatus = tasks[taskIndex].status; + tasks[taskIndex] = { + ...tasks[taskIndex], + status: newStatus, + updatedAt: new Date().toISOString() + }; + + await this.saveTasks(tasks, tag); + + return { + success: true, + oldStatus, + newStatus, + taskId: String(taskId) + }; + } + + /** + * Update subtask status within file storage - handles parent status auto-adjustment + */ + private async updateSubtaskStatusInFile( + tasks: Task[], + subtaskId: string, + newStatus: TaskStatus, + tag?: string + ): Promise<{ + success: boolean; + oldStatus: TaskStatus; + newStatus: TaskStatus; + taskId: string; + }> { + // Parse the subtask ID to get parent ID and subtask ID + const parts = subtaskId.split('.'); + if (parts.length !== 2) { + throw new Error( + `Invalid subtask ID format: ${subtaskId}. Expected format: parentId.subtaskId` + ); + } + + const [parentId, subId] = parts; + const subtaskNumericId = parseInt(subId, 10); + + if (isNaN(subtaskNumericId)) { + throw new Error( + `Invalid subtask ID: ${subId}. Subtask ID must be numeric.` + ); + } + + // Find the parent task + const parentTaskIndex = tasks.findIndex( + (t) => String(t.id) === String(parentId) + ); + + if (parentTaskIndex === -1) { + throw new Error(`Parent task ${parentId} not found`); + } + + const parentTask = tasks[parentTaskIndex]; + + // Find the subtask within the parent task + const subtaskIndex = parentTask.subtasks.findIndex( + (st) => st.id === subtaskNumericId || String(st.id) === subId + ); + + if (subtaskIndex === -1) { + throw new Error( + `Subtask ${subtaskId} not found in parent task ${parentId}` + ); + } + + const oldStatus = parentTask.subtasks[subtaskIndex].status; + + // Update the subtask status + parentTask.subtasks[subtaskIndex] = { + ...parentTask.subtasks[subtaskIndex], + status: newStatus, + updatedAt: new Date().toISOString() + }; + + // Auto-adjust parent status based on subtask statuses + const subs = parentTask.subtasks; + let parentNewStatus = parentTask.status; + if (subs.length > 0) { + const allDone = subs.every((s) => (s.status || 'pending') === 'done'); + const anyInProgress = subs.some( + (s) => (s.status || 'pending') === 'in-progress' + ); + if (allDone) parentNewStatus = 'done'; + else if (anyInProgress) parentNewStatus = 'in-progress'; + } + + // Update parent task if status changed + if (parentNewStatus !== parentTask.status) { + tasks[parentTaskIndex] = { + ...parentTask, + status: parentNewStatus, + updatedAt: new Date().toISOString() + }; + } + + await this.saveTasks(tasks, tag); + + return { + success: true, + oldStatus, + newStatus, + taskId: subtaskId + }; + } + /** * Delete a task */