mirror of
https://github.com/coleam00/Archon.git
synced 2025-12-30 21:49:30 -05:00
fix: resolve task jumping and optimistic update issues
- Fix polling feedback loop by removing tasks from useEffect deps - Increase polling intervals to 8s (tasks) and 10s (projects) - Clean up dead code in DraggableTaskCard and TaskBoardView - Remove unused imports and debug logging - Improve task comparison logic for better polling efficiency Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Optimistic Updates Pattern (Future State)
|
||||
|
||||
**⚠️ STATUS: This is NOT currently implemented. there is a POC in project page in the FE. This document describes the desired future state for handling optimistic updates in a simple, consistent way.**
|
||||
**⚠️ STATUS:** This is not currently implemented. There is a proof‑of‑concept (POC) on the frontend Project page. This document describes the desired future state for handling optimistic updates in a simple, consistent way.
|
||||
|
||||
## Mental Model
|
||||
|
||||
@@ -9,16 +9,17 @@ Think of optimistic updates as "assuming success" - update the UI immediately fo
|
||||
## The Pattern
|
||||
|
||||
```typescript
|
||||
// 1. Save current state (for rollback)
|
||||
const previousState = currentState; // Note: Use deep clone for objects/arrays in production
|
||||
// 1. Save current state (for rollback) — take an immutable snapshot
|
||||
const previousState = structuredClone(currentState);
|
||||
|
||||
// 2. Update UI immediately
|
||||
setState(newState);
|
||||
|
||||
// 3. Call API
|
||||
try {
|
||||
await api.updateResource(newState);
|
||||
// Success - UI already updated, nothing to do
|
||||
const serverState = await api.updateResource(newState);
|
||||
// Success — use server as the source of truth
|
||||
setState(serverState);
|
||||
} catch (error) {
|
||||
// 4. Rollback on failure
|
||||
setState(previousState);
|
||||
@@ -35,24 +36,35 @@ function useOptimistic<T>(initialValue: T, updateFn: (value: T) => Promise<T>) {
|
||||
const [value, setValue] = useState(initialValue);
|
||||
const [isUpdating, setIsUpdating] = useState(false);
|
||||
const previousValueRef = useRef<T>(initialValue);
|
||||
const opSeqRef = useRef(0); // monotonically increasing op id
|
||||
const mountedRef = useRef(true); // avoid setState after unmount
|
||||
useEffect(() => () => { mountedRef.current = false; }, []);
|
||||
|
||||
const optimisticUpdate = async (newValue: T) => {
|
||||
const opId = ++opSeqRef.current;
|
||||
// Save for rollback
|
||||
previousValueRef.current = value;
|
||||
|
||||
// Update immediately
|
||||
setValue(newValue);
|
||||
setIsUpdating(true);
|
||||
if (mountedRef.current) setValue(newValue);
|
||||
if (mountedRef.current) setIsUpdating(true);
|
||||
|
||||
try {
|
||||
const result = await updateFn(newValue);
|
||||
setValue(result); // Use server response as source of truth
|
||||
// Apply only if latest op and still mounted
|
||||
if (mountedRef.current && opId === opSeqRef.current) {
|
||||
setValue(result); // Server is source of truth
|
||||
}
|
||||
} catch (error) {
|
||||
// Rollback
|
||||
setValue(previousValueRef.current);
|
||||
if (mountedRef.current && opId === opSeqRef.current) {
|
||||
setValue(previousValueRef.current);
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
setIsUpdating(false);
|
||||
if (mountedRef.current && opId === opSeqRef.current) {
|
||||
setIsUpdating(false);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@@ -82,11 +94,11 @@ const handleStatusChange = (newStatus: string) => {
|
||||
|
||||
## Key Principles
|
||||
|
||||
1. **Keep it simple** - Just save, update, and rollback
|
||||
2. **Server is truth** - Always use server response as final state
|
||||
3. **User feedback** - Show loading states and error messages
|
||||
4. **Selective usage** - Only for actions where instant feedback matters:
|
||||
- Drag and drop
|
||||
1. **Keep it simple** — save, update, roll back.
|
||||
2. **Server is the source of truth** — always use the server response as the final state.
|
||||
3. **User feedback** — show loading states and clear error messages.
|
||||
4. **Selective usage** — only where instant feedback matters:
|
||||
- Drag‑and‑drop
|
||||
- Status changes
|
||||
- Toggle switches
|
||||
- Quick edits
|
||||
@@ -95,7 +107,7 @@ const handleStatusChange = (newStatus: string) => {
|
||||
|
||||
- Don't track complex state histories
|
||||
- Don't try to merge conflicts
|
||||
- Don't use for create/delete operations (too complex to rollback cleanly)
|
||||
- Use with caution for create/delete operations. If used, generate temporary client IDs, reconcile with server‑assigned IDs, ensure idempotency, and define clear rollback/error states. Prefer non‑optimistic flows when side effects are complex.
|
||||
- Don't over-engineer with queues or reconciliation
|
||||
|
||||
## When to Implement
|
||||
@@ -130,5 +142,7 @@ The examples above are simplified for clarity. Production implementations should
|
||||
3. **Unmount safety**: Avoid setState after component unmount
|
||||
4. **Debouncing**: For rapid updates (e.g., sliders), debounce API calls
|
||||
5. **Conflict resolution**: For collaborative editing, consider operational transforms
|
||||
6. **Polling/ETag interplay**: When polling, ignore stale responses (e.g., compare opId or Last-Modified) and rely on ETag/304 to prevent flicker overriding optimistic state.
|
||||
7. **Idempotency & retries**: Use idempotency keys on write APIs so client retries (or duplicate submits) don't create duplicate effects.
|
||||
|
||||
These complexities are why we recommend starting simple and only adding optimistic updates where the UX benefit is clear.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import React from 'react';
|
||||
import React, { useId } from 'react';
|
||||
import { Trash2 } from 'lucide-react';
|
||||
|
||||
interface DeleteConfirmModalProps {
|
||||
@@ -14,30 +14,27 @@ export const DeleteConfirmModal: React.FC<DeleteConfirmModalProps> = ({
|
||||
onCancel,
|
||||
type,
|
||||
}) => {
|
||||
const getTitle = () => {
|
||||
switch (type) {
|
||||
case "project":
|
||||
return "Delete Project";
|
||||
case "task":
|
||||
return "Delete Task";
|
||||
case "client":
|
||||
return "Delete MCP Client";
|
||||
}
|
||||
const titleId = useId();
|
||||
const descId = useId();
|
||||
const TITLES: Record<DeleteConfirmModalProps['type'], string> = {
|
||||
project: "Delete Project",
|
||||
task: "Delete Task",
|
||||
client: "Delete MCP Client",
|
||||
};
|
||||
|
||||
const getMessage = () => {
|
||||
switch (type) {
|
||||
case "project":
|
||||
return `Are you sure you want to delete the "${itemName}" project? This will also delete all associated tasks and documents and cannot be undone.`;
|
||||
case "task":
|
||||
return `Are you sure you want to delete the "${itemName}" task? This action cannot be undone.`;
|
||||
case "client":
|
||||
return `Are you sure you want to delete the "${itemName}" client? This will permanently remove its configuration and cannot be undone.`;
|
||||
}
|
||||
const MESSAGES: Record<DeleteConfirmModalProps['type'], (n: string) => string> = {
|
||||
project: (n) => `Are you sure you want to delete the "${n}" project? This will also delete all associated tasks and documents and cannot be undone.`,
|
||||
task: (n) => `Are you sure you want to delete the "${n}" task? This action cannot be undone.`,
|
||||
client: (n) => `Are you sure you want to delete the "${n}" client? This will permanently remove its configuration and cannot be undone.`,
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="fixed inset-0 bg-black/50 backdrop-blur-sm flex items-center justify-center z-50">
|
||||
<div
|
||||
className="fixed inset-0 bg-black/50 backdrop-blur-sm flex items-center justify-center z-50"
|
||||
onClick={onCancel}
|
||||
onKeyDown={(e) => { if (e.key === 'Escape') onCancel(); }}
|
||||
aria-hidden={false}
|
||||
>
|
||||
<div
|
||||
className="relative p-6 rounded-md backdrop-blur-md w-full max-w-md
|
||||
bg-gradient-to-b from-white/80 to-white/60 dark:from-white/10 dark:to-black/30
|
||||
@@ -46,6 +43,11 @@ export const DeleteConfirmModal: React.FC<DeleteConfirmModalProps> = ({
|
||||
before:content-[''] before:absolute before:top-0 before:left-0 before:right-0 before:h-[2px]
|
||||
before:rounded-t-[4px] before:bg-red-500
|
||||
before:shadow-[0_0_10px_2px_rgba(239,68,68,0.4)] dark:before:shadow-[0_0_20px_5px_rgba(239,68,68,0.7)]"
|
||||
role="dialog"
|
||||
aria-modal="true"
|
||||
aria-labelledby={titleId}
|
||||
aria-describedby={descId}
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
>
|
||||
<div className="relative z-10">
|
||||
<div className="flex items-center gap-3 mb-4">
|
||||
@@ -53,8 +55,8 @@ export const DeleteConfirmModal: React.FC<DeleteConfirmModalProps> = ({
|
||||
<Trash2 className="w-6 h-6 text-red-600 dark:text-red-400" />
|
||||
</div>
|
||||
<div>
|
||||
<h3 className="text-lg font-semibold text-gray-800 dark:text-white">
|
||||
{getTitle()}
|
||||
<h3 id={titleId} className="text-lg font-semibold text-gray-800 dark:text-white">
|
||||
{TITLES[type]}
|
||||
</h3>
|
||||
<p className="text-sm text-gray-600 dark:text-gray-400">
|
||||
This action cannot be undone
|
||||
@@ -62,18 +64,21 @@ export const DeleteConfirmModal: React.FC<DeleteConfirmModalProps> = ({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<p className="text-gray-700 dark:text-gray-300 mb-6">
|
||||
{getMessage()}
|
||||
<p id={descId} className="text-gray-700 dark:text-gray-300 mb-6">
|
||||
{MESSAGES[type](itemName)}
|
||||
</p>
|
||||
|
||||
<div className="flex justify-end gap-3">
|
||||
<button
|
||||
type="button"
|
||||
onClick={onCancel}
|
||||
className="px-4 py-2 text-gray-600 dark:text-gray-400 hover:text-gray-800 dark:hover:text-gray-200 transition-colors"
|
||||
autoFocus
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={onConfirm}
|
||||
className="px-4 py-2 bg-red-600 hover:bg-red-700 text-white rounded-lg transition-colors shadow-lg shadow-red-600/25 hover:shadow-red-700/25"
|
||||
>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import React, { useRef, useState } from 'react';
|
||||
import { useDrag, useDrop } from 'react-dnd';
|
||||
import { Edit, Trash2, RefreshCw, Tag, User, Bot, Clipboard } from 'lucide-react';
|
||||
import { Edit, Trash2, RefreshCw, Tag, Clipboard } from 'lucide-react';
|
||||
import { Task } from './TaskTableView';
|
||||
import { ItemTypes, getAssigneeIcon, getAssigneeGlow, getOrderColor, getOrderGlow } from '../../lib/task-utils';
|
||||
|
||||
@@ -52,7 +52,6 @@ export const DraggableTaskCard = ({
|
||||
|
||||
if (draggedIndex === hoveredIndex) return;
|
||||
|
||||
console.log('BOARD HOVER: Moving task', draggedItem.id, 'from index', draggedIndex, 'to', hoveredIndex, 'in status', task.status);
|
||||
|
||||
// Move the task immediately for visual feedback (same pattern as table view)
|
||||
onTaskReorder(draggedItem.id, hoveredIndex, task.status);
|
||||
@@ -69,15 +68,7 @@ export const DraggableTaskCard = ({
|
||||
setIsFlipped(!isFlipped);
|
||||
};
|
||||
|
||||
// Calculate hover effects for parent-child relationships
|
||||
const getRelatedTaskIds = () => {
|
||||
const relatedIds = new Set<string>();
|
||||
|
||||
return relatedIds;
|
||||
};
|
||||
|
||||
const relatedTaskIds = getRelatedTaskIds();
|
||||
const isHighlighted = hoveredTaskId ? relatedTaskIds.has(hoveredTaskId) || hoveredTaskId === task.id : false;
|
||||
const isHighlighted = hoveredTaskId === task.id;
|
||||
const isSelected = selectedTasks?.has(task.id) || false;
|
||||
|
||||
const handleMouseEnter = () => {
|
||||
|
||||
@@ -6,7 +6,7 @@ import { Trash2 } from 'lucide-react';
|
||||
import { projectService } from '../../services/projectService';
|
||||
import { Task } from './TaskTableView'; // Import Task interface
|
||||
import { ItemTypes, getAssigneeIcon, getAssigneeGlow, getOrderColor, getOrderGlow } from '../../lib/task-utils';
|
||||
import { DraggableTaskCard, DraggableTaskCardProps } from './DraggableTaskCard'; // Import the new component and its props
|
||||
import { DraggableTaskCard } from './DraggableTaskCard';
|
||||
|
||||
interface TaskBoardViewProps {
|
||||
tasks: Task[];
|
||||
@@ -175,20 +175,13 @@ export const TaskBoardView = ({
|
||||
const tasksToDelete = tasks.filter(task => selectedTasks.has(task.id));
|
||||
|
||||
try {
|
||||
// Delete all selected tasks
|
||||
await Promise.all(
|
||||
tasksToDelete.map(task => projectService.deleteTask(task.id))
|
||||
);
|
||||
|
||||
// Clear selection
|
||||
// Use onTaskDelete for each task so parent updates state/metrics
|
||||
await Promise.all(tasksToDelete.map(task => onTaskDelete(task)));
|
||||
clearSelection();
|
||||
|
||||
showToast(`${tasksToDelete.length} tasks deleted successfully`, 'success');
|
||||
} catch (error) {
|
||||
console.error('Failed to delete tasks:', error);
|
||||
showToast('Failed to delete some tasks', 'error');
|
||||
}
|
||||
}, [selectedTasks, tasks, clearSelection, showToast]);
|
||||
}, [selectedTasks, tasks, onTaskDelete, clearSelection]);
|
||||
|
||||
// Mass status change handler
|
||||
const handleMassStatusChange = useCallback(async (newStatus: Task['status']) => {
|
||||
@@ -197,24 +190,15 @@ export const TaskBoardView = ({
|
||||
const tasksToUpdate = tasks.filter(task => selectedTasks.has(task.id));
|
||||
|
||||
try {
|
||||
// Update all selected tasks
|
||||
// Call onTaskMove so optimistic UI and counts refresh immediately; parent persists
|
||||
await Promise.all(
|
||||
tasksToUpdate.map(task =>
|
||||
projectService.updateTask(task.id, {
|
||||
status: newStatus
|
||||
})
|
||||
)
|
||||
tasksToUpdate.map(task => onTaskMove(task.id, newStatus))
|
||||
);
|
||||
|
||||
// Clear selection
|
||||
clearSelection();
|
||||
|
||||
showToast(`${tasksToUpdate.length} tasks moved to ${newStatus}`, 'success');
|
||||
} catch (error) {
|
||||
console.error('Failed to update tasks:', error);
|
||||
showToast('Failed to update some tasks', 'error');
|
||||
}
|
||||
}, [selectedTasks, tasks, clearSelection, showToast]);
|
||||
}, [selectedTasks, tasks, onTaskMove, clearSelection]);
|
||||
|
||||
// No status mapping needed - using database values directly
|
||||
|
||||
@@ -229,18 +213,15 @@ export const TaskBoardView = ({
|
||||
if (!taskToDelete) return;
|
||||
|
||||
try {
|
||||
await projectService.deleteTask(taskToDelete.id);
|
||||
// Notify parent to update tasks
|
||||
onTaskDelete(taskToDelete);
|
||||
showToast(`Task "${taskToDelete.title}" deleted successfully`, 'success');
|
||||
// Delegate deletion to parent to avoid duplicate API calls
|
||||
await onTaskDelete(taskToDelete);
|
||||
} catch (error) {
|
||||
console.error('Failed to delete task:', error);
|
||||
showToast(error instanceof Error ? error.message : 'Failed to delete task', 'error');
|
||||
} finally {
|
||||
setShowDeleteConfirm(false);
|
||||
setTaskToDelete(null);
|
||||
}
|
||||
}, [taskToDelete, onTaskDelete, showToast, setShowDeleteConfirm, setTaskToDelete, projectService]);
|
||||
}, [taskToDelete, onTaskDelete, setShowDeleteConfirm, setTaskToDelete]);
|
||||
|
||||
// Cancel deletion
|
||||
const cancelDeleteTask = useCallback(() => {
|
||||
@@ -319,7 +300,7 @@ export const TaskBoardView = ({
|
||||
onTaskMove={onTaskMove}
|
||||
onTaskView={onTaskView}
|
||||
onTaskComplete={onTaskComplete}
|
||||
onTaskDelete={onTaskDelete}
|
||||
onTaskDelete={handleDeleteTask}
|
||||
onTaskReorder={onTaskReorder}
|
||||
allTasks={tasks}
|
||||
hoveredTaskId={hoveredTaskId}
|
||||
@@ -336,7 +317,7 @@ export const TaskBoardView = ({
|
||||
onTaskMove={onTaskMove}
|
||||
onTaskView={onTaskView}
|
||||
onTaskComplete={onTaskComplete}
|
||||
onTaskDelete={onTaskDelete}
|
||||
onTaskDelete={handleDeleteTask}
|
||||
onTaskReorder={onTaskReorder}
|
||||
allTasks={tasks}
|
||||
hoveredTaskId={hoveredTaskId}
|
||||
@@ -353,7 +334,7 @@ export const TaskBoardView = ({
|
||||
onTaskMove={onTaskMove}
|
||||
onTaskView={onTaskView}
|
||||
onTaskComplete={onTaskComplete}
|
||||
onTaskDelete={onTaskDelete}
|
||||
onTaskDelete={handleDeleteTask}
|
||||
onTaskReorder={onTaskReorder}
|
||||
allTasks={tasks}
|
||||
hoveredTaskId={hoveredTaskId}
|
||||
@@ -370,7 +351,7 @@ export const TaskBoardView = ({
|
||||
onTaskMove={onTaskMove}
|
||||
onTaskView={onTaskView}
|
||||
onTaskComplete={onTaskComplete}
|
||||
onTaskDelete={onTaskDelete}
|
||||
onTaskDelete={handleDeleteTask}
|
||||
onTaskReorder={onTaskReorder}
|
||||
allTasks={tasks}
|
||||
hoveredTaskId={hoveredTaskId}
|
||||
|
||||
@@ -6,12 +6,16 @@ import { Toggle } from '../ui/Toggle';
|
||||
import { projectService } from '../../services/projectService';
|
||||
import { useToast } from '../../contexts/ToastContext';
|
||||
import { debounce } from '../../utils/debounce';
|
||||
import { calculateReorderPosition, getDefaultTaskOrder } from '../../utils/taskOrdering';
|
||||
|
||||
import type { CreateTaskRequest, UpdateTaskRequest } from '../../types/project';
|
||||
import { TaskTableView, Task } from './TaskTableView';
|
||||
import { TaskBoardView } from './TaskBoardView';
|
||||
import { EditTaskModal } from './EditTaskModal';
|
||||
|
||||
// Type for optimistic task updates with operation tracking
|
||||
type OptimisticTask = Task & { _optimisticOperationId: string };
|
||||
|
||||
|
||||
|
||||
export const TasksTab = ({
|
||||
@@ -31,7 +35,7 @@ export const TasksTab = ({
|
||||
const [projectFeatures, setProjectFeatures] = useState<any[]>([]);
|
||||
const [isLoadingFeatures, setIsLoadingFeatures] = useState(false);
|
||||
const [isSavingTask, setIsSavingTask] = useState<boolean>(false);
|
||||
const [optimisticTaskUpdates, setOptimisticTaskUpdates] = useState<Map<string, Task>>(new Map());
|
||||
const [optimisticTaskUpdates, setOptimisticTaskUpdates] = useState<Map<string, OptimisticTask>>(new Map());
|
||||
|
||||
// Initialize tasks, but preserve optimistic updates
|
||||
useEffect(() => {
|
||||
@@ -46,7 +50,7 @@ export const TasksTab = ({
|
||||
console.log(`[TasksTab] Preserving optimistic update for task ${task.id}:`, optimisticUpdate.status);
|
||||
// Clean up internal tracking field before returning
|
||||
const { _optimisticOperationId, ...cleanTask } = optimisticUpdate;
|
||||
return cleanTask; // Keep optimistic version without internal fields
|
||||
return cleanTask as Task; // Keep optimistic version without internal fields
|
||||
}
|
||||
return task; // Use polling data for non-optimistic tasks
|
||||
});
|
||||
@@ -125,7 +129,7 @@ export const TasksTab = ({
|
||||
closeModal();
|
||||
} catch (error) {
|
||||
console.error('Failed to save task:', error);
|
||||
alert(`Failed to save task: ${error instanceof Error ? error.message : 'Unknown error'}`);
|
||||
showToast(`Failed to save task: ${error instanceof Error ? error.message : 'Unknown error'}`, 'error');
|
||||
} finally {
|
||||
setIsSavingTask(false);
|
||||
}
|
||||
@@ -170,7 +174,7 @@ export const TasksTab = ({
|
||||
// Polling will eventually sync the correct state
|
||||
}
|
||||
}, 800), // Slightly reduced delay for better responsiveness
|
||||
[projectId]
|
||||
[]
|
||||
);
|
||||
|
||||
// Optimized task reordering without optimistic update conflicts
|
||||
@@ -206,41 +210,8 @@ export const TasksTab = ({
|
||||
const movingTask = statusTasks[movingTaskIndex];
|
||||
console.log('REORDER: Moving', movingTask.title, 'from', movingTaskIndex, 'to', targetIndex);
|
||||
|
||||
// Calculate new position using improved algorithm
|
||||
let newPosition: number;
|
||||
|
||||
if (targetIndex === 0) {
|
||||
// Moving to first position
|
||||
const firstTask = statusTasks[0];
|
||||
newPosition = firstTask.task_order / 2;
|
||||
} else if (targetIndex === statusTasks.length - 1) {
|
||||
// Moving to last position
|
||||
const lastTask = statusTasks[statusTasks.length - 1];
|
||||
newPosition = lastTask.task_order + 1024;
|
||||
} else {
|
||||
// Moving between two items
|
||||
let prevTask, nextTask;
|
||||
|
||||
if (targetIndex > movingTaskIndex) {
|
||||
// Moving down
|
||||
prevTask = statusTasks[targetIndex];
|
||||
nextTask = statusTasks[targetIndex + 1];
|
||||
} else {
|
||||
// Moving up
|
||||
prevTask = statusTasks[targetIndex - 1];
|
||||
nextTask = statusTasks[targetIndex];
|
||||
}
|
||||
|
||||
if (prevTask && nextTask) {
|
||||
newPosition = (prevTask.task_order + nextTask.task_order) / 2;
|
||||
} else if (prevTask) {
|
||||
newPosition = prevTask.task_order + 1024;
|
||||
} else if (nextTask) {
|
||||
newPosition = nextTask.task_order / 2;
|
||||
} else {
|
||||
newPosition = 1024; // Fallback
|
||||
}
|
||||
}
|
||||
// Calculate new position using shared ordering utility
|
||||
const newPosition = calculateReorderPosition(statusTasks, movingTaskIndex, targetIndex);
|
||||
|
||||
console.log('REORDER: New position calculated:', newPosition);
|
||||
|
||||
@@ -283,12 +254,12 @@ export const TasksTab = ({
|
||||
const newOrder = getNextOrderForStatus(newStatus);
|
||||
|
||||
// 2. Update UI immediately (optimistic update - no loader!)
|
||||
const optimisticTask = {
|
||||
const optimisticTask: OptimisticTask = {
|
||||
...movingTask,
|
||||
status: newStatus,
|
||||
task_order: newOrder,
|
||||
_optimisticOperationId: operationId // Track which operation created this
|
||||
} as Task & { _optimisticOperationId: string };
|
||||
};
|
||||
const optimisticTasks = tasks.map(task =>
|
||||
task.id === taskId ? optimisticTask : task
|
||||
);
|
||||
@@ -351,15 +322,12 @@ export const TasksTab = ({
|
||||
|
||||
const deleteTask = async (task: Task) => {
|
||||
try {
|
||||
// Delete the task
|
||||
await projectService.deleteTask(task.id);
|
||||
console.log(`[TasksTab] Task ${task.id} deleted`);
|
||||
|
||||
// Task deleted - polling will pick up changes automatically
|
||||
|
||||
updateTasks(tasks.filter(t => t.id !== task.id));
|
||||
showToast(`Task "${task.title}" deleted`, 'success');
|
||||
} catch (error) {
|
||||
console.error('Failed to delete task:', error);
|
||||
// Note: The toast notification for deletion is now handled by TaskBoardView and TaskTableView
|
||||
showToast('Failed to delete task', 'error');
|
||||
}
|
||||
};
|
||||
|
||||
@@ -419,7 +387,7 @@ export const TasksTab = ({
|
||||
|
||||
} catch (error) {
|
||||
console.error(`[TasksTab] Failed to update task ${taskId} inline:`, error);
|
||||
alert(`Failed to update task: ${error instanceof Error ? error.message : 'Unknown error'}`);
|
||||
showToast(`Failed to update task: ${error instanceof Error ? error.message : 'Unknown error'}`, 'error');
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
@@ -503,7 +471,7 @@ export const TasksTab = ({
|
||||
{/* Add Task Button with Luminous Style */}
|
||||
<button
|
||||
onClick={() => {
|
||||
const defaultOrder = getTasksForPrioritySelection('todo')[0]?.value || 1;
|
||||
const defaultOrder = getDefaultTaskOrder(tasks.filter(t => t.status === 'todo'));
|
||||
setEditingTask({
|
||||
id: '',
|
||||
title: '',
|
||||
|
||||
@@ -206,8 +206,8 @@ export function useTaskPolling(projectId: string, options?: UsePollingOptions<an
|
||||
const url = `${baseUrl}/${projectId}/tasks`;
|
||||
|
||||
return usePolling(url, {
|
||||
interval: 3000, // 3 seconds for tasks
|
||||
staleTime: 1000, // Consider data stale after 1 second
|
||||
interval: 8000, // 8 seconds for tasks
|
||||
staleTime: 2000, // Consider data stale after 2 seconds
|
||||
...options,
|
||||
});
|
||||
}
|
||||
@@ -219,8 +219,8 @@ export function useProjectPolling(options?: UsePollingOptions<any>) {
|
||||
const url = '/api/projects';
|
||||
|
||||
return usePolling(url, {
|
||||
interval: 5000, // 5 seconds for project list
|
||||
staleTime: 2000, // Consider data stale after 2 seconds
|
||||
interval: 10000, // 10 seconds for project list
|
||||
staleTime: 3000, // Consider data stale after 3 seconds
|
||||
...options,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { useState, useCallback, useRef, useEffect } from 'react';
|
||||
import { useToast } from '../contexts/ToastContext';
|
||||
|
||||
interface UseProjectMutationOptions<TData, TVariables> {
|
||||
onSuccess?: (data: TData, variables: TVariables) => void;
|
||||
@@ -26,6 +27,7 @@ export function useProjectMutation<TData = unknown, TVariables = unknown>(
|
||||
mutationFn: (variables: TVariables) => Promise<TData>,
|
||||
options: UseProjectMutationOptions<TData, TVariables> = {}
|
||||
): UseProjectMutationResult<TData, TVariables> {
|
||||
const { showToast } = useToast();
|
||||
const [isPending, setIsPending] = useState(false);
|
||||
const [isError, setIsError] = useState(false);
|
||||
const [isSuccess, setIsSuccess] = useState(false);
|
||||
@@ -72,8 +74,8 @@ export function useProjectMutation<TData = unknown, TVariables = unknown>(
|
||||
}
|
||||
|
||||
// Show success message if available
|
||||
if (successMessage && typeof window !== 'undefined') {
|
||||
console.log(successMessage);
|
||||
if (successMessage) {
|
||||
showToast(successMessage, 'success');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -92,9 +94,7 @@ export function useProjectMutation<TData = unknown, TVariables = unknown>(
|
||||
}
|
||||
|
||||
// Show error message
|
||||
if (typeof window !== 'undefined') {
|
||||
console.error(`${errorMessage}:`, error);
|
||||
}
|
||||
showToast(errorMessage, 'error');
|
||||
}
|
||||
|
||||
throw error;
|
||||
|
||||
@@ -69,7 +69,6 @@ function ProjectPage({
|
||||
const [newProjectForm, setNewProjectForm] = useState({
|
||||
title: "",
|
||||
description: "",
|
||||
color: "blue" as const,
|
||||
});
|
||||
|
||||
// State for delete confirmation modal
|
||||
@@ -79,6 +78,9 @@ function ProjectPage({
|
||||
title: string;
|
||||
} | null>(null);
|
||||
|
||||
// State for copy feedback
|
||||
const [copiedProjectId, setCopiedProjectId] = useState<string | null>(null);
|
||||
|
||||
const { showToast } = useToast();
|
||||
|
||||
// Polling hooks for real-time updates
|
||||
@@ -95,7 +97,7 @@ function ProjectPage({
|
||||
});
|
||||
|
||||
// Derive projects array from polling data - ensure it's always an array
|
||||
const projects = projectsData?.projects || [];
|
||||
const projects = Array.isArray(projectsData) ? projectsData : (projectsData?.projects || []);
|
||||
|
||||
// Poll tasks for selected project
|
||||
const {
|
||||
@@ -161,7 +163,7 @@ function ProjectPage({
|
||||
{
|
||||
successMessage: "Creating project...",
|
||||
onSuccess: (response) => {
|
||||
setNewProjectForm({ title: "", description: "", color: "blue" });
|
||||
setNewProjectForm({ title: "", description: "" });
|
||||
setIsNewProjectModalOpen(false);
|
||||
// Polling will pick up the new project
|
||||
showToast("Project created successfully!", "success");
|
||||
@@ -174,7 +176,7 @@ function ProjectPage({
|
||||
);
|
||||
|
||||
// Direct API call for immediate task loading during project switch
|
||||
const loadTasksForProject = async (projectId: string) => {
|
||||
const loadTasksForProject = useCallback(async (projectId: string) => {
|
||||
try {
|
||||
const taskData = await projectService.getTasksByProject(projectId);
|
||||
|
||||
@@ -203,7 +205,7 @@ function ProjectPage({
|
||||
error instanceof Error ? error.message : "Failed to load tasks",
|
||||
);
|
||||
}
|
||||
};
|
||||
}, []);
|
||||
|
||||
const handleProjectSelect = useCallback(async (project: Project) => {
|
||||
// Early return if already selected
|
||||
@@ -227,7 +229,7 @@ function ProjectPage({
|
||||
} finally {
|
||||
setIsSwitchingProject(false);
|
||||
}
|
||||
}, [selectedProject?.id, loadTasksForProject]);
|
||||
}, [selectedProject?.id, loadTasksForProject, showToast]);
|
||||
|
||||
// Load task counts for all projects using batch endpoint
|
||||
const loadTaskCountsForAllProjects = useCallback(
|
||||
@@ -332,7 +334,6 @@ function ProjectPage({
|
||||
|
||||
// Refresh task counts when tasks update via polling AND keep UI in sync for selected project
|
||||
useEffect(() => {
|
||||
// The polling returns an array directly, not wrapped in { tasks: [...] }
|
||||
if (tasksData && selectedProject) {
|
||||
const uiTasks: Task[] = tasksData.map((task: any) => ({
|
||||
id: task.id,
|
||||
@@ -348,37 +349,25 @@ function ProjectPage({
|
||||
task_order: task.task_order || 0,
|
||||
}));
|
||||
|
||||
// Check if tasks have actually changed before updating
|
||||
setTasks((prevTasks) => {
|
||||
// Quick check: if lengths differ, update
|
||||
if (prevTasks.length !== uiTasks.length) {
|
||||
return uiTasks;
|
||||
}
|
||||
|
||||
// Deep check: compare each task
|
||||
const hasChanges = uiTasks.some((newTask) => {
|
||||
const oldTask = prevTasks.find(t => t.id === newTask.id);
|
||||
if (!oldTask) return true; // New task
|
||||
|
||||
// Check if any field has changed
|
||||
return oldTask.title !== newTask.title ||
|
||||
oldTask.description !== newTask.description ||
|
||||
oldTask.status !== newTask.status ||
|
||||
oldTask.assignee.name !== newTask.assignee.name ||
|
||||
oldTask.feature !== newTask.feature ||
|
||||
oldTask.task_order !== newTask.task_order;
|
||||
const changed =
|
||||
tasks.length !== uiTasks.length ||
|
||||
uiTasks.some((t) => {
|
||||
const old = tasks.find((x) => x.id === t.id);
|
||||
return (
|
||||
!old ||
|
||||
old.title !== t.title ||
|
||||
old.description !== t.description ||
|
||||
old.status !== t.status ||
|
||||
old.assignee.name !== t.assignee.name ||
|
||||
old.feature !== t.feature ||
|
||||
old.task_order !== t.task_order
|
||||
);
|
||||
});
|
||||
|
||||
if (hasChanges) {
|
||||
return uiTasks;
|
||||
}
|
||||
|
||||
return prevTasks;
|
||||
});
|
||||
|
||||
// Force refresh task counts when tasks change
|
||||
const projectIds = projects.map((p) => p.id);
|
||||
debouncedLoadTaskCounts(projectIds, true);
|
||||
if (changed) {
|
||||
setTasks(uiTasks);
|
||||
const projectIds = projects.map((p) => p.id);
|
||||
debouncedLoadTaskCounts(projectIds, true);
|
||||
}
|
||||
}
|
||||
}, [tasksData, projects, selectedProject?.id]);
|
||||
|
||||
@@ -717,20 +706,26 @@ function ProjectPage({
|
||||
"Project ID copied to clipboard",
|
||||
"success",
|
||||
);
|
||||
// Visual feedback
|
||||
const button = e.currentTarget;
|
||||
const originalHTML = button.innerHTML;
|
||||
button.innerHTML =
|
||||
'<svg class="w-3 h-3 mr-1 inline" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M5 13l4 4L19 7"></path></svg>Copied!';
|
||||
// Visual feedback with React state
|
||||
setCopiedProjectId(project.id);
|
||||
setTimeout(() => {
|
||||
button.innerHTML = originalHTML;
|
||||
setCopiedProjectId(null);
|
||||
}, 2000);
|
||||
}}
|
||||
className="flex-1 flex items-center justify-center gap-1 text-xs text-gray-500 hover:text-gray-700 dark:text-gray-400 dark:hover:text-gray-200 transition-colors py-1"
|
||||
title="Copy Project ID to clipboard"
|
||||
>
|
||||
<Clipboard className="w-3 h-3" />
|
||||
<span>Copy ID</span>
|
||||
{copiedProjectId === project.id ? (
|
||||
<>
|
||||
<CheckCircle2 className="w-3 h-3" />
|
||||
<span>Copied!</span>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Clipboard className="w-3 h-3" />
|
||||
<span>Copy ID</span>
|
||||
</>
|
||||
)}
|
||||
</button>
|
||||
|
||||
{/* Delete button */}
|
||||
|
||||
104
archon-ui-main/src/utils/taskOrdering.ts
Normal file
104
archon-ui-main/src/utils/taskOrdering.ts
Normal file
@@ -0,0 +1,104 @@
|
||||
import { Task } from '../types/project';
|
||||
|
||||
export interface TaskOrderingOptions {
|
||||
position: 'first' | 'last' | 'between';
|
||||
existingTasks: Task[];
|
||||
beforeTaskOrder?: number;
|
||||
afterTaskOrder?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculate the optimal task_order value for positioning a task
|
||||
* Uses fractional ordering system for flexibility in drag-and-drop operations
|
||||
*/
|
||||
export function calculateTaskOrder(options: TaskOrderingOptions): number {
|
||||
const { position, existingTasks, beforeTaskOrder, afterTaskOrder } = options;
|
||||
|
||||
// Sort tasks by order for consistent calculations
|
||||
const sortedTasks = existingTasks.sort((a, b) => a.task_order - b.task_order);
|
||||
|
||||
switch (position) {
|
||||
case 'first':
|
||||
if (sortedTasks.length === 0) {
|
||||
return 1024; // Seed value for first task
|
||||
}
|
||||
return sortedTasks[0].task_order / 2;
|
||||
|
||||
case 'last':
|
||||
if (sortedTasks.length === 0) {
|
||||
return 1024; // Seed value for first task
|
||||
}
|
||||
return sortedTasks[sortedTasks.length - 1].task_order + 1024;
|
||||
|
||||
case 'between':
|
||||
if (beforeTaskOrder !== undefined && afterTaskOrder !== undefined) {
|
||||
return (beforeTaskOrder + afterTaskOrder) / 2;
|
||||
}
|
||||
if (beforeTaskOrder !== undefined) {
|
||||
return beforeTaskOrder + 1024;
|
||||
}
|
||||
if (afterTaskOrder !== undefined) {
|
||||
return afterTaskOrder / 2;
|
||||
}
|
||||
// Fallback
|
||||
return 1024;
|
||||
|
||||
default:
|
||||
return 1024;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculate task order for drag-and-drop reordering
|
||||
*/
|
||||
export function calculateReorderPosition(
|
||||
statusTasks: Task[],
|
||||
movingTaskIndex: number,
|
||||
targetIndex: number
|
||||
): number {
|
||||
if (targetIndex === 0) {
|
||||
// Moving to first position
|
||||
return calculateTaskOrder({
|
||||
position: 'first',
|
||||
existingTasks: statusTasks.filter((_, i) => i !== movingTaskIndex)
|
||||
});
|
||||
}
|
||||
|
||||
if (targetIndex === statusTasks.length - 1) {
|
||||
// Moving to last position
|
||||
return calculateTaskOrder({
|
||||
position: 'last',
|
||||
existingTasks: statusTasks.filter((_, i) => i !== movingTaskIndex)
|
||||
});
|
||||
}
|
||||
|
||||
// Moving between two items
|
||||
let prevTask, nextTask;
|
||||
|
||||
if (targetIndex > movingTaskIndex) {
|
||||
// Moving down
|
||||
prevTask = statusTasks[targetIndex];
|
||||
nextTask = statusTasks[targetIndex + 1];
|
||||
} else {
|
||||
// Moving up
|
||||
prevTask = statusTasks[targetIndex - 1];
|
||||
nextTask = statusTasks[targetIndex];
|
||||
}
|
||||
|
||||
return calculateTaskOrder({
|
||||
position: 'between',
|
||||
existingTasks: statusTasks,
|
||||
beforeTaskOrder: prevTask?.task_order,
|
||||
afterTaskOrder: nextTask?.task_order
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Get default task order for new tasks (always first position)
|
||||
*/
|
||||
export function getDefaultTaskOrder(existingTasks: Task[]): number {
|
||||
return calculateTaskOrder({
|
||||
position: 'first',
|
||||
existingTasks
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,107 @@
|
||||
import React from 'react';
|
||||
import { render, screen, fireEvent } from '@testing-library/react';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { DeleteConfirmModal } from '../../../src/components/common/DeleteConfirmModal';
|
||||
|
||||
describe('DeleteConfirmModal', () => {
|
||||
const defaultProps = {
|
||||
itemName: 'Test Item',
|
||||
onConfirm: vi.fn(),
|
||||
onCancel: vi.fn(),
|
||||
type: 'task' as const,
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('renders with correct title and message for task type', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
expect(screen.getByText('Delete Task')).toBeInTheDocument();
|
||||
expect(screen.getByText(/Are you sure you want to delete the "Test Item" task/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders with correct title and message for project type', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} type="project" />);
|
||||
|
||||
expect(screen.getByText('Delete Project')).toBeInTheDocument();
|
||||
expect(screen.getByText(/Are you sure you want to delete the "Test Item" project/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders with correct title and message for client type', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} type="client" />);
|
||||
|
||||
expect(screen.getByText('Delete MCP Client')).toBeInTheDocument();
|
||||
expect(screen.getByText(/Are you sure you want to delete the "Test Item" client/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('calls onConfirm when Delete button is clicked', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
fireEvent.click(screen.getByText('Delete'));
|
||||
|
||||
expect(defaultProps.onConfirm).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('calls onCancel when Cancel button is clicked', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
fireEvent.click(screen.getByText('Cancel'));
|
||||
|
||||
expect(defaultProps.onCancel).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('calls onCancel when Escape key is pressed', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' });
|
||||
|
||||
expect(defaultProps.onCancel).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('calls onCancel when backdrop is clicked', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
// Click the backdrop (the outer div)
|
||||
const backdrop = screen.getByRole('dialog').parentElement;
|
||||
fireEvent.click(backdrop!);
|
||||
|
||||
expect(defaultProps.onCancel).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not call onCancel when modal content is clicked', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
// Click the modal dialog itself
|
||||
fireEvent.click(screen.getByRole('dialog'));
|
||||
|
||||
expect(defaultProps.onCancel).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('has proper accessibility attributes', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
const dialog = screen.getByRole('dialog');
|
||||
expect(dialog).toHaveAttribute('aria-modal', 'true');
|
||||
expect(dialog).toHaveAttribute('aria-labelledby');
|
||||
expect(dialog).toHaveAttribute('aria-describedby');
|
||||
});
|
||||
|
||||
it('focuses Cancel button by default', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
const cancelButton = screen.getByText('Cancel');
|
||||
expect(cancelButton).toHaveFocus();
|
||||
});
|
||||
|
||||
it('has proper button types', () => {
|
||||
render(<DeleteConfirmModal {...defaultProps} />);
|
||||
|
||||
const cancelButton = screen.getByText('Cancel');
|
||||
const deleteButton = screen.getByText('Delete');
|
||||
|
||||
expect(cancelButton).toHaveAttribute('type', 'button');
|
||||
expect(deleteButton).toHaveAttribute('type', 'button');
|
||||
});
|
||||
});
|
||||
95
archon-ui-main/test/utils/taskOrdering.test.ts
Normal file
95
archon-ui-main/test/utils/taskOrdering.test.ts
Normal file
@@ -0,0 +1,95 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { calculateTaskOrder, calculateReorderPosition, getDefaultTaskOrder } from '../../src/utils/taskOrdering';
|
||||
import { Task } from '../../src/types/project';
|
||||
|
||||
// Mock task factory
|
||||
const createMockTask = (id: string, task_order: number): Task => ({
|
||||
id,
|
||||
title: `Task ${id}`,
|
||||
description: '',
|
||||
status: 'todo',
|
||||
assignee: { name: 'Test User', avatar: '' },
|
||||
feature: '',
|
||||
featureColor: '#3b82f6',
|
||||
task_order,
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
project_id: 'test-project'
|
||||
});
|
||||
|
||||
describe('taskOrdering utilities', () => {
|
||||
describe('calculateTaskOrder', () => {
|
||||
it('should return seed value for first task when no existing tasks', () => {
|
||||
const result = calculateTaskOrder({
|
||||
position: 'first',
|
||||
existingTasks: []
|
||||
});
|
||||
expect(result).toBe(1024);
|
||||
});
|
||||
|
||||
it('should calculate first position correctly', () => {
|
||||
const existingTasks = [createMockTask('1', 100), createMockTask('2', 200)];
|
||||
const result = calculateTaskOrder({
|
||||
position: 'first',
|
||||
existingTasks
|
||||
});
|
||||
expect(result).toBe(50); // 100 / 2
|
||||
});
|
||||
|
||||
it('should calculate last position correctly', () => {
|
||||
const existingTasks = [createMockTask('1', 100), createMockTask('2', 200)];
|
||||
const result = calculateTaskOrder({
|
||||
position: 'last',
|
||||
existingTasks
|
||||
});
|
||||
expect(result).toBe(1224); // 200 + 1024
|
||||
});
|
||||
|
||||
it('should calculate between position correctly', () => {
|
||||
const result = calculateTaskOrder({
|
||||
position: 'between',
|
||||
existingTasks: [],
|
||||
beforeTaskOrder: 100,
|
||||
afterTaskOrder: 200
|
||||
});
|
||||
expect(result).toBe(150); // (100 + 200) / 2
|
||||
});
|
||||
});
|
||||
|
||||
describe('getDefaultTaskOrder', () => {
|
||||
it('should return seed value when no existing tasks', () => {
|
||||
const result = getDefaultTaskOrder([]);
|
||||
expect(result).toBe(1024);
|
||||
});
|
||||
|
||||
it('should return first position when existing tasks present', () => {
|
||||
const existingTasks = [createMockTask('1', 100), createMockTask('2', 200)];
|
||||
const result = getDefaultTaskOrder(existingTasks);
|
||||
expect(result).toBe(50); // 100 / 2
|
||||
});
|
||||
});
|
||||
|
||||
describe('calculateReorderPosition', () => {
|
||||
const statusTasks = [
|
||||
createMockTask('1', 100),
|
||||
createMockTask('2', 200),
|
||||
createMockTask('3', 300)
|
||||
];
|
||||
|
||||
it('should calculate position for moving to first', () => {
|
||||
const result = calculateReorderPosition(statusTasks, 1, 0);
|
||||
expect(result).toBeLessThan(statusTasks[0].task_order);
|
||||
});
|
||||
|
||||
it('should calculate position for moving to last', () => {
|
||||
const result = calculateReorderPosition(statusTasks, 0, 2);
|
||||
expect(result).toBeGreaterThan(statusTasks[2].task_order);
|
||||
});
|
||||
|
||||
it('should calculate position for moving between items', () => {
|
||||
const result = calculateReorderPosition(statusTasks, 0, 1);
|
||||
expect(result).toBeGreaterThan(statusTasks[0].task_order);
|
||||
expect(result).toBeLessThan(statusTasks[2].task_order);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user