chore: refactor and move updateTaskStatus business logic to storage service
This commit is contained in:
@@ -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<void>;
|
||||
|
||||
/**
|
||||
* 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<Task>,
|
||||
tag?: string
|
||||
): Promise<void>;
|
||||
abstract updateTaskStatus(
|
||||
taskId: string,
|
||||
newStatus: TaskStatus,
|
||||
tag?: string
|
||||
): Promise<{
|
||||
success: boolean;
|
||||
oldStatus: TaskStatus;
|
||||
newStatus: TaskStatus;
|
||||
taskId: string;
|
||||
}>;
|
||||
abstract deleteTask(taskId: string, tag?: string): Promise<void>;
|
||||
abstract exists(tag?: string): Promise<boolean>;
|
||||
abstract loadMetadata(tag?: string): Promise<TaskMetadata | null>;
|
||||
|
||||
@@ -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
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
*/
|
||||
|
||||
@@ -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
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user