mirror of
https://github.com/BradGroux/veritas-kanban.git
synced 2026-10-06 11:47:51 +00:00
fix: make task conflicts actionable (#1315)
This commit is contained in:
Brad Groux
GitHub
parent
baf2ee19ee
commit
51d15cb8f1
7 changed files with 615 additions and 56 deletions
225
web/src/__tests__/task-conflict-recovery.test.tsx
Normal file
225
web/src/__tests__/task-conflict-recovery.test.tsx
Normal file
|
|
@ -0,0 +1,225 @@
|
|||
import { useEffect } from 'react';
|
||||
import { useQuery } from '@tanstack/react-query';
|
||||
import { notifications } from '@mantine/notifications';
|
||||
import { act, cleanup, fireEvent, screen, waitFor } from '@testing-library/react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { Task } from '@veritas-kanban/shared';
|
||||
import { TaskConflictAlert } from '@/components/task/TaskConflictAlert';
|
||||
import { Toaster } from '@/components/ui/toaster';
|
||||
import { useDebouncedSave } from '@/hooks/useDebouncedSave';
|
||||
import {
|
||||
registerOpenTaskConflictSurface,
|
||||
resetTaskConflicts,
|
||||
resolveTaskConflict,
|
||||
} from '@/hooks/useTaskConflicts';
|
||||
import { useAddComment, useUpdateTask } from '@/hooks/useTasks';
|
||||
import { createMockTask, createTestQueryClient, renderWithProviders } from './test-utils';
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
update: vi.fn(),
|
||||
addComment: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/api', () => ({
|
||||
api: {
|
||||
tasks: {
|
||||
update: mocks.update,
|
||||
addComment: mocks.addComment,
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@/hooks/useFeatureSettings', async (importOriginal) => {
|
||||
const original = await importOriginal<typeof import('@/hooks/useFeatureSettings')>();
|
||||
return {
|
||||
...original,
|
||||
useFeatureSetting: () => 0,
|
||||
};
|
||||
});
|
||||
|
||||
function conflictFor(current: Task) {
|
||||
return Object.assign(new Error('stale revision'), {
|
||||
code: 'CONFLICT',
|
||||
details: { current },
|
||||
});
|
||||
}
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((resolvePromise) => {
|
||||
resolve = resolvePromise;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
function ConflictBurstProbe() {
|
||||
const update = useUpdateTask();
|
||||
const addComment = useAddComment();
|
||||
return (
|
||||
<>
|
||||
<button onClick={() => update.mutate({ id: 'task-1', input: { title: 'Local' } })}>
|
||||
Update Alpha
|
||||
</button>
|
||||
<button onClick={() => addComment.mutate({ taskId: 'task-1', author: 'Brad', text: 'One' })}>
|
||||
Comment Alpha
|
||||
</button>
|
||||
<button onClick={() => addComment.mutate({ taskId: 'task-2', author: 'Brad', text: 'Two' })}>
|
||||
Comment Beta
|
||||
</button>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
function ConflictEditor({ initialTask }: { initialTask: Task }) {
|
||||
const taskQuery = useQuery({
|
||||
queryKey: ['tasks', initialTask.id],
|
||||
queryFn: async () => initialTask,
|
||||
initialData: initialTask,
|
||||
enabled: false,
|
||||
});
|
||||
const save = useDebouncedSave(taskQuery.data);
|
||||
|
||||
useEffect(() => registerOpenTaskConflictSurface(initialTask.id), [initialTask.id]);
|
||||
|
||||
if (!save.localTask) return null;
|
||||
|
||||
return (
|
||||
<>
|
||||
<input
|
||||
aria-label="Task title"
|
||||
value={save.localTask.title}
|
||||
onChange={(event) => save.updateField('title', event.currentTarget.value)}
|
||||
/>
|
||||
<span data-testid="dirty-state">{String(save.isDirty)}</span>
|
||||
{save.conflict ? (
|
||||
<TaskConflictAlert
|
||||
conflict={save.conflict}
|
||||
taskTitle={save.localTask.title}
|
||||
onRetry={save.retryConflict}
|
||||
onDiscard={save.discardConflict}
|
||||
onDismiss={() => resolveTaskConflict(save.localTask?.id ?? initialTask.id)}
|
||||
/>
|
||||
) : null}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
describe('task conflict recovery feature', () => {
|
||||
beforeEach(() => {
|
||||
mocks.update.mockReset();
|
||||
mocks.addComment.mockReset();
|
||||
resetTaskConflicts();
|
||||
notifications.clean();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
resetTaskConflicts();
|
||||
notifications.clean();
|
||||
});
|
||||
|
||||
it('deduplicates one task revision while keeping different tasks actionable', async () => {
|
||||
const alpha = createMockTask({ id: 'task-1', title: 'Alpha', revision: 1 });
|
||||
const beta = createMockTask({ id: 'task-2', title: 'Beta', revision: 3 });
|
||||
const currentAlpha = { ...alpha, revision: 2 };
|
||||
const currentBeta = { ...beta, revision: 4 };
|
||||
mocks.update.mockRejectedValue(conflictFor(currentAlpha));
|
||||
mocks.addComment.mockImplementation((taskId: string) =>
|
||||
Promise.reject(conflictFor(taskId === alpha.id ? currentAlpha : currentBeta))
|
||||
);
|
||||
const queryClient = createTestQueryClient();
|
||||
queryClient.setQueryData(['tasks'], [alpha, beta]);
|
||||
|
||||
renderWithProviders(
|
||||
<>
|
||||
<Toaster />
|
||||
<ConflictBurstProbe />
|
||||
</>,
|
||||
{ queryClient }
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Update Alpha' }));
|
||||
await screen.findByRole('button', { name: 'Review conflict for Alpha' });
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Comment Alpha' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getAllByText('Task updated before your change')).toHaveLength(1);
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Comment Beta' }));
|
||||
await waitFor(() => {
|
||||
expect(screen.getAllByText('Task updated before your change')).toHaveLength(2);
|
||||
});
|
||||
expect(screen.queryByText(/elsewhere/i)).toBeNull();
|
||||
expect(screen.getByRole('button', { name: 'Review conflict for Beta' })).toBeDefined();
|
||||
|
||||
const opened = vi.fn();
|
||||
window.addEventListener('open-task', opened, { once: true });
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Review conflict for Alpha' }));
|
||||
expect(opened).toHaveBeenCalledWith(expect.objectContaining({ detail: { taskId: 'task-1' } }));
|
||||
});
|
||||
|
||||
it('preserves local edits and retries them against the latest revision', async () => {
|
||||
const task = createMockTask({ id: 'task-1', title: 'Original', revision: 1 });
|
||||
const current = { ...task, title: 'Server title', revision: 2 };
|
||||
const saved = { ...current, title: 'Local title', revision: 3 };
|
||||
mocks.update.mockRejectedValueOnce(conflictFor(current)).mockResolvedValueOnce(saved);
|
||||
const queryClient = createTestQueryClient();
|
||||
queryClient.setQueryData(['tasks'], [task]);
|
||||
|
||||
renderWithProviders(<ConflictEditor initialTask={task} />, { queryClient });
|
||||
fireEvent.change(screen.getByLabelText('Task title'), { target: { value: 'Local title' } });
|
||||
|
||||
expect(await screen.findByText(/Your unsaved title change is still available/)).toBeDefined();
|
||||
expect((screen.getByLabelText('Task title') as HTMLInputElement).value).toBe('Local title');
|
||||
expect(screen.queryByText('Task updated before your change')).toBeNull();
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Retry preserved edits for Local title' }));
|
||||
await waitFor(() => expect(mocks.update).toHaveBeenCalledTimes(2));
|
||||
expect(mocks.update.mock.calls[1]).toEqual(['task-1', { title: 'Local title' }, 2]);
|
||||
await waitFor(() => expect(screen.queryByText('Update needs review')).toBeNull());
|
||||
expect(screen.getByTestId('dirty-state').textContent).toBe('false');
|
||||
});
|
||||
|
||||
it('discards preserved edits in favor of the authoritative task', async () => {
|
||||
const task = createMockTask({ id: 'task-1', title: 'Original', revision: 1 });
|
||||
const current = { ...task, title: 'Server title', revision: 2 };
|
||||
mocks.update.mockRejectedValueOnce(conflictFor(current));
|
||||
const queryClient = createTestQueryClient();
|
||||
queryClient.setQueryData(['tasks'], [task]);
|
||||
|
||||
renderWithProviders(<ConflictEditor initialTask={task} />, { queryClient });
|
||||
fireEvent.change(screen.getByLabelText('Task title'), { target: { value: 'Local title' } });
|
||||
await screen.findByText(/Your unsaved title change is still available/);
|
||||
|
||||
fireEvent.click(
|
||||
screen.getByRole('button', { name: 'Discard preserved edits for Local title' })
|
||||
);
|
||||
await waitFor(() => expect(screen.queryByText('Update needs review')).toBeNull());
|
||||
expect((screen.getByLabelText('Task title') as HTMLInputElement).value).toBe('Server title');
|
||||
expect(screen.getByTestId('dirty-state').textContent).toBe('false');
|
||||
});
|
||||
|
||||
it('serializes rapid edits and sends the newest value with the updated revision', async () => {
|
||||
const task = createMockTask({ id: 'task-1', title: 'Original', revision: 1 });
|
||||
const firstSave = deferred<Task>();
|
||||
mocks.update
|
||||
.mockReturnValueOnce(firstSave.promise)
|
||||
.mockResolvedValueOnce({ ...task, title: 'Second edit', revision: 3 });
|
||||
const queryClient = createTestQueryClient();
|
||||
queryClient.setQueryData(['tasks'], [task]);
|
||||
|
||||
renderWithProviders(<ConflictEditor initialTask={task} />, { queryClient });
|
||||
fireEvent.change(screen.getByLabelText('Task title'), { target: { value: 'First edit' } });
|
||||
await waitFor(() => expect(mocks.update).toHaveBeenCalledTimes(1));
|
||||
|
||||
fireEvent.change(screen.getByLabelText('Task title'), { target: { value: 'Second edit' } });
|
||||
expect(mocks.update).toHaveBeenCalledTimes(1);
|
||||
|
||||
act(() => {
|
||||
firstSave.resolve({ ...task, title: 'First edit', revision: 2 });
|
||||
});
|
||||
await waitFor(() => expect(mocks.update).toHaveBeenCalledTimes(2));
|
||||
expect(mocks.update.mock.calls[1]).toEqual(['task-1', { title: 'Second edit' }, 2]);
|
||||
});
|
||||
});
|
||||
71
web/src/components/task/TaskConflictAlert.tsx
Normal file
71
web/src/components/task/TaskConflictAlert.tsx
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
import { Alert, Button, Group, Stack, Text } from '@mantine/core';
|
||||
import { AlertTriangle } from 'lucide-react';
|
||||
import type { TaskConflict } from '@/hooks/useTaskConflicts';
|
||||
|
||||
interface TaskConflictAlertProps {
|
||||
conflict: TaskConflict;
|
||||
taskTitle: string;
|
||||
onRetry: () => void;
|
||||
onDiscard: () => void;
|
||||
onDismiss: () => void;
|
||||
}
|
||||
|
||||
export function TaskConflictAlert({
|
||||
conflict,
|
||||
taskTitle,
|
||||
onRetry,
|
||||
onDiscard,
|
||||
onDismiss,
|
||||
}: TaskConflictAlertProps) {
|
||||
return (
|
||||
<Alert
|
||||
color="orange"
|
||||
icon={<AlertTriangle className="h-4 w-4" />}
|
||||
title="Update needs review"
|
||||
className="mx-4 mt-3 flex-shrink-0 sm:mx-6"
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
>
|
||||
<Stack gap="xs">
|
||||
<Text size="sm">
|
||||
{conflict.localEditsPreserved
|
||||
? `The latest version is loaded. Your unsaved ${conflict.dirtyFields.join(', ')} ${conflict.dirtyFields.length === 1 ? 'change is' : 'changes are'} still available.`
|
||||
: `The latest version is loaded. Review it before retrying your ${conflict.operation}.`}
|
||||
</Text>
|
||||
<Group gap="xs">
|
||||
{conflict.localEditsPreserved ? (
|
||||
<>
|
||||
<Button
|
||||
size="xs"
|
||||
color="orange"
|
||||
onClick={onRetry}
|
||||
aria-label={`Retry preserved edits for ${taskTitle}`}
|
||||
>
|
||||
Retry edits
|
||||
</Button>
|
||||
<Button
|
||||
size="xs"
|
||||
variant="subtle"
|
||||
color="gray"
|
||||
onClick={onDiscard}
|
||||
aria-label={`Discard preserved edits for ${taskTitle}`}
|
||||
>
|
||||
Discard edits
|
||||
</Button>
|
||||
</>
|
||||
) : (
|
||||
<Button
|
||||
size="xs"
|
||||
variant="subtle"
|
||||
color="gray"
|
||||
onClick={onDismiss}
|
||||
aria-label={`Dismiss conflict for ${taskTitle}`}
|
||||
>
|
||||
Dismiss
|
||||
</Button>
|
||||
)}
|
||||
</Group>
|
||||
</Stack>
|
||||
</Alert>
|
||||
);
|
||||
}
|
||||
|
|
@ -14,7 +14,9 @@ import {
|
|||
import { useTaskTypes, getTypeIcon } from '@/hooks/useTaskTypes';
|
||||
import { useFeatureSettings } from '@/hooks/useFeatureSettings';
|
||||
import { useDebouncedSave } from '@/hooks/useDebouncedSave';
|
||||
import { registerOpenTaskConflictSurface, resolveTaskConflict } from '@/hooks/useTaskConflicts';
|
||||
import { ChatPanel } from '@/components/chat/ChatPanel';
|
||||
import { TaskConflictAlert } from './TaskConflictAlert';
|
||||
import { ApplyTemplateDialog } from './ApplyTemplateDialog';
|
||||
import { WorkflowSection } from './WorkflowSection';
|
||||
import { shouldDefaultTaskDetailToWork } from './TaskWorkView';
|
||||
|
|
@ -61,7 +63,8 @@ export function TaskDetailPanel({
|
|||
const taskSettings = featureSettings.tasks;
|
||||
const agentSettings = featureSettings.agents;
|
||||
const canUseLocalAgentControls = clientAllowsLocalAgentControls(authContext);
|
||||
const { localTask, updateField, isDirty } = useDebouncedSave(task);
|
||||
const { localTask, updateField, isDirty, isSaving, conflict, retryConflict, discardConflict } =
|
||||
useDebouncedSave(task);
|
||||
const [activeTab, setActiveTab] = useState<TaskDetailTabId>('details');
|
||||
const [previewOpen, setPreviewOpen] = useState(false);
|
||||
const [applyTemplateOpen, setApplyTemplateOpen] = useState(false);
|
||||
|
|
@ -73,6 +76,12 @@ export function TaskDetailPanel({
|
|||
const addObservation = useAddObservation();
|
||||
const deleteObservation = useDeleteObservation();
|
||||
const nestedOverlayOpen = previewOpen || applyTemplateOpen || taskChatOpen || workflowOpen;
|
||||
const activeTaskId = localTask?.id;
|
||||
|
||||
useEffect(() => {
|
||||
if (!open || !activeTaskId) return;
|
||||
return registerOpenTaskConflictSurface(activeTaskId);
|
||||
}, [activeTaskId, open]);
|
||||
|
||||
useEffect(() => {
|
||||
const handleKeyDown = (e: KeyboardEvent) => {
|
||||
|
|
@ -86,7 +95,6 @@ export function TaskDetailPanel({
|
|||
|
||||
const isCodeTask = localTask?.type === 'code';
|
||||
const hasWorktree = !!localTask?.git?.worktreePath;
|
||||
const activeTaskId = localTask?.id;
|
||||
const defaultTab = localTask && shouldDefaultTaskDetailToWork(localTask) ? 'work' : 'details';
|
||||
const tabAvailabilityContext = useMemo(
|
||||
() => ({
|
||||
|
|
@ -230,7 +238,7 @@ export function TaskDetailPanel({
|
|||
)}
|
||||
{!readOnly && isDirty && (
|
||||
<Text size="xs" c="yellow.5">
|
||||
Saving...
|
||||
{isSaving ? 'Saving...' : 'Unsaved changes'}
|
||||
</Text>
|
||||
)}
|
||||
<ActionIcon
|
||||
|
|
@ -263,6 +271,16 @@ export function TaskDetailPanel({
|
|||
</Drawer.Title>
|
||||
</header>
|
||||
|
||||
{conflict && (
|
||||
<TaskConflictAlert
|
||||
conflict={conflict}
|
||||
taskTitle={localTask.title}
|
||||
onRetry={retryConflict}
|
||||
onDiscard={discardConflict}
|
||||
onDismiss={() => resolveTaskConflict(localTask.id)}
|
||||
/>
|
||||
)}
|
||||
|
||||
{/* Action buttons above tabs */}
|
||||
<SimpleGrid
|
||||
cols={{ base: 2, sm: 3 }}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,11 @@ import { useState, useEffect, useCallback, useRef } from 'react';
|
|||
import { isRevisionConflict, useUpdateTask } from './useTasks';
|
||||
import { useToast } from '@/hooks/useToast';
|
||||
import { useFeatureSetting } from '@/hooks/useFeatureSettings';
|
||||
import {
|
||||
markTaskConflictEditsPreserved,
|
||||
resolveTaskConflict,
|
||||
useTaskConflict,
|
||||
} from '@/hooks/useTaskConflicts';
|
||||
import type { Task } from '@veritas-kanban/shared';
|
||||
|
||||
export function useDebouncedSave(task: Task | null) {
|
||||
|
|
@ -10,12 +15,18 @@ export function useDebouncedSave(task: Task | null) {
|
|||
const autoSaveDelayMs = useFeatureSetting('tasks', 'autoSaveDelayMs');
|
||||
const [localTask, setLocalTask] = useState<Task | null>(task);
|
||||
const [changedFields, setChangedFields] = useState<Set<keyof Task>>(new Set());
|
||||
const [isSaving, setIsSaving] = useState(false);
|
||||
const conflict = useTaskConflict(task?.id);
|
||||
const changedFieldsRef = useRef(changedFields);
|
||||
const localTaskRef = useRef(localTask);
|
||||
const fieldVersionsRef = useRef(new Map<keyof Task, number>());
|
||||
const saveInFlightRef = useRef(false);
|
||||
const mutateRef = useRef(updateTask.mutate);
|
||||
const toastRef = useRef(toast);
|
||||
|
||||
// Keep refs current without triggering effects
|
||||
changedFieldsRef.current = changedFields;
|
||||
localTaskRef.current = localTask;
|
||||
mutateRef.current = updateTask.mutate;
|
||||
toastRef.current = toast;
|
||||
|
||||
|
|
@ -25,6 +36,7 @@ export function useDebouncedSave(task: Task | null) {
|
|||
if (!task) {
|
||||
setLocalTask(null);
|
||||
setChangedFields(new Set());
|
||||
fieldVersionsRef.current.clear();
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -45,59 +57,107 @@ export function useDebouncedSave(task: Task | null) {
|
|||
}
|
||||
}, [task]);
|
||||
|
||||
const savePendingChanges = useCallback(() => {
|
||||
const taskToSave = localTaskRef.current;
|
||||
const fieldsToSave = new Set(changedFieldsRef.current);
|
||||
if (!taskToSave || fieldsToSave.size === 0 || saveInFlightRef.current) return;
|
||||
|
||||
const input: Record<string, unknown> = {};
|
||||
const savedVersions = new Map<keyof Task, number>();
|
||||
fieldsToSave.forEach((field) => {
|
||||
input[field] = taskToSave[field];
|
||||
savedVersions.set(field, fieldVersionsRef.current.get(field) ?? 0);
|
||||
});
|
||||
|
||||
saveInFlightRef.current = true;
|
||||
setIsSaving(true);
|
||||
mutateRef.current(
|
||||
{
|
||||
id: taskToSave.id,
|
||||
input,
|
||||
},
|
||||
{
|
||||
onSuccess: () => {
|
||||
saveInFlightRef.current = false;
|
||||
setIsSaving(false);
|
||||
resolveTaskConflict(taskToSave.id, 'task update');
|
||||
setChangedFields((previous) => {
|
||||
const remaining = new Set(previous);
|
||||
fieldsToSave.forEach((field) => {
|
||||
if (fieldVersionsRef.current.get(field) === savedVersions.get(field)) {
|
||||
remaining.delete(field);
|
||||
}
|
||||
});
|
||||
return remaining;
|
||||
});
|
||||
},
|
||||
onError: (error) => {
|
||||
saveInFlightRef.current = false;
|
||||
setIsSaving(false);
|
||||
if (isRevisionConflict(error)) {
|
||||
markTaskConflictEditsPreserved(
|
||||
taskToSave.id,
|
||||
Array.from(fieldsToSave, (field) => String(field))
|
||||
);
|
||||
return;
|
||||
}
|
||||
toastRef.current({
|
||||
variant: 'destructive',
|
||||
title: 'Failed to save changes',
|
||||
description:
|
||||
typeof (error as { message?: unknown }).message === 'string'
|
||||
? (error as { message: string }).message
|
||||
: 'Please try again',
|
||||
});
|
||||
},
|
||||
}
|
||||
);
|
||||
}, []);
|
||||
|
||||
// Debounced save — only send fields that were actually changed
|
||||
useEffect(() => {
|
||||
if (changedFields.size === 0 || !localTask) return;
|
||||
if (changedFields.size === 0 || !localTask || conflict || saveInFlightRef.current) {
|
||||
return;
|
||||
}
|
||||
|
||||
const timeout = setTimeout(() => {
|
||||
const input: Record<string, unknown> = {};
|
||||
// Snapshot the fields being saved so we only clear those on success
|
||||
const fieldsToClear = new Set(changedFields);
|
||||
fieldsToClear.forEach((field) => {
|
||||
input[field] = localTask[field];
|
||||
});
|
||||
|
||||
mutateRef.current(
|
||||
{
|
||||
id: localTask.id,
|
||||
input,
|
||||
},
|
||||
{
|
||||
onSuccess: () => {
|
||||
// Only clear the fields we actually saved, not any new edits that
|
||||
// may have occurred while the mutation was in flight
|
||||
setChangedFields((prev) => {
|
||||
const remaining = new Set(prev);
|
||||
fieldsToClear.forEach((f) => remaining.delete(f));
|
||||
return remaining;
|
||||
});
|
||||
},
|
||||
onError: (error) => {
|
||||
if (isRevisionConflict(error)) {
|
||||
return;
|
||||
}
|
||||
toastRef.current({
|
||||
variant: 'destructive',
|
||||
title: 'Failed to save changes',
|
||||
description:
|
||||
typeof (error as { message?: unknown }).message === 'string'
|
||||
? (error as { message: string }).message
|
||||
: 'Please try again',
|
||||
});
|
||||
},
|
||||
}
|
||||
);
|
||||
savePendingChanges();
|
||||
}, autoSaveDelayMs);
|
||||
|
||||
return () => clearTimeout(timeout);
|
||||
}, [localTask, changedFields, autoSaveDelayMs]);
|
||||
}, [localTask, changedFields, autoSaveDelayMs, conflict, savePendingChanges]);
|
||||
|
||||
const updateField = useCallback(<K extends keyof Task>(field: K, value: Task[K]) => {
|
||||
fieldVersionsRef.current.set(field, (fieldVersionsRef.current.get(field) ?? 0) + 1);
|
||||
setLocalTask((prev) => (prev ? { ...prev, [field]: value } : null));
|
||||
setChangedFields((prev) => new Set(prev).add(field));
|
||||
}, []);
|
||||
|
||||
const retryConflict = useCallback(() => {
|
||||
const taskId = localTaskRef.current?.id;
|
||||
if (!taskId) return;
|
||||
resolveTaskConflict(taskId, 'task update');
|
||||
savePendingChanges();
|
||||
}, [savePendingChanges]);
|
||||
|
||||
const discardConflict = useCallback(() => {
|
||||
const taskId = localTaskRef.current?.id;
|
||||
if (!taskId) return;
|
||||
fieldVersionsRef.current.clear();
|
||||
setChangedFields(new Set());
|
||||
setLocalTask(conflict?.currentTask ?? task);
|
||||
resolveTaskConflict(taskId, 'task update');
|
||||
}, [conflict?.currentTask, task]);
|
||||
|
||||
const isDirty = changedFields.size > 0;
|
||||
|
||||
return { localTask, updateField, isDirty };
|
||||
return {
|
||||
localTask,
|
||||
updateField,
|
||||
isDirty,
|
||||
isSaving,
|
||||
conflict,
|
||||
retryConflict,
|
||||
discardConflict,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
126
web/src/hooks/useTaskConflicts.ts
Normal file
126
web/src/hooks/useTaskConflicts.ts
Normal file
|
|
@ -0,0 +1,126 @@
|
|||
import { useCallback, useSyncExternalStore } from 'react';
|
||||
import type { Task } from '@veritas-kanban/shared';
|
||||
import { dismissToast } from './useToast';
|
||||
|
||||
export type TaskConflictOperation = 'task update' | 'comment change';
|
||||
|
||||
export interface TaskConflict {
|
||||
identity: string;
|
||||
taskId: string;
|
||||
taskTitle: string;
|
||||
currentRevision?: number;
|
||||
currentTask?: Task;
|
||||
operation: TaskConflictOperation;
|
||||
localEditsPreserved: boolean;
|
||||
dirtyFields: string[];
|
||||
}
|
||||
|
||||
interface RecordTaskConflictInput {
|
||||
taskId: string;
|
||||
taskTitle: string;
|
||||
currentTask?: Task;
|
||||
operation: TaskConflictOperation;
|
||||
}
|
||||
|
||||
const conflicts = new Map<string, TaskConflict>();
|
||||
const listeners = new Set<() => void>();
|
||||
const openTaskSurfaces = new Set<string>();
|
||||
|
||||
function emitChange(): void {
|
||||
listeners.forEach((listener) => listener());
|
||||
}
|
||||
|
||||
export function taskConflictToastId(taskId: string): string {
|
||||
return `task-conflict:${taskId}`;
|
||||
}
|
||||
|
||||
export function recordTaskConflict(input: RecordTaskConflictInput): {
|
||||
conflict: TaskConflict;
|
||||
isNewIdentity: boolean;
|
||||
hasOpenSurface: boolean;
|
||||
} {
|
||||
const currentRevision =
|
||||
typeof input.currentTask?.revision === 'number' ? input.currentTask.revision : undefined;
|
||||
const identity = `${input.taskId}:${currentRevision ?? 'unknown'}`;
|
||||
const existing = conflicts.get(input.taskId);
|
||||
|
||||
if (existing?.identity === identity) {
|
||||
return {
|
||||
conflict: existing,
|
||||
isNewIdentity: false,
|
||||
hasOpenSurface: openTaskSurfaces.has(input.taskId),
|
||||
};
|
||||
}
|
||||
|
||||
const conflict: TaskConflict = {
|
||||
identity,
|
||||
taskId: input.taskId,
|
||||
taskTitle: input.taskTitle,
|
||||
currentRevision,
|
||||
currentTask: input.currentTask,
|
||||
operation: input.operation,
|
||||
localEditsPreserved: false,
|
||||
dirtyFields: [],
|
||||
};
|
||||
conflicts.set(input.taskId, conflict);
|
||||
emitChange();
|
||||
|
||||
return {
|
||||
conflict,
|
||||
isNewIdentity: true,
|
||||
hasOpenSurface: openTaskSurfaces.has(input.taskId),
|
||||
};
|
||||
}
|
||||
|
||||
export function markTaskConflictEditsPreserved(taskId: string, dirtyFields: string[]): void {
|
||||
const conflict = conflicts.get(taskId);
|
||||
if (!conflict) return;
|
||||
|
||||
const nextFields = Array.from(new Set(dirtyFields)).sort();
|
||||
conflicts.set(taskId, {
|
||||
...conflict,
|
||||
operation: 'task update',
|
||||
localEditsPreserved: true,
|
||||
dirtyFields: nextFields,
|
||||
});
|
||||
emitChange();
|
||||
}
|
||||
|
||||
export function resolveTaskConflict(taskId: string, operation?: TaskConflictOperation): void {
|
||||
const conflict = conflicts.get(taskId);
|
||||
if (operation && conflict?.operation !== operation) return;
|
||||
|
||||
if (conflicts.delete(taskId)) emitChange();
|
||||
dismissToast(taskConflictToastId(taskId));
|
||||
}
|
||||
|
||||
export function openTaskConflict(taskId: string): void {
|
||||
window.dispatchEvent(new CustomEvent('open-task', { detail: { taskId } }));
|
||||
dismissToast(taskConflictToastId(taskId));
|
||||
}
|
||||
|
||||
export function registerOpenTaskConflictSurface(taskId: string): () => void {
|
||||
openTaskSurfaces.add(taskId);
|
||||
return () => {
|
||||
openTaskSurfaces.delete(taskId);
|
||||
};
|
||||
}
|
||||
|
||||
export function useTaskConflict(taskId: string | undefined): TaskConflict | null {
|
||||
const subscribe = useCallback((listener: () => void) => {
|
||||
listeners.add(listener);
|
||||
return () => listeners.delete(listener);
|
||||
}, []);
|
||||
const getSnapshot = useCallback(
|
||||
() => (taskId ? (conflicts.get(taskId) ?? null) : null),
|
||||
[taskId]
|
||||
);
|
||||
|
||||
return useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
|
||||
}
|
||||
|
||||
export function resetTaskConflicts(): void {
|
||||
conflicts.clear();
|
||||
openTaskSurfaces.clear();
|
||||
emitChange();
|
||||
}
|
||||
|
|
@ -1,8 +1,16 @@
|
|||
import { createElement } from 'react';
|
||||
import { useQuery, useMutation, useQueryClient, QueryClient } from '@tanstack/react-query';
|
||||
import { api } from '@/lib/api';
|
||||
import { useWebSocketStatus } from '@/contexts/WebSocketContext';
|
||||
import { toast } from '@/hooks/useToast';
|
||||
import { useFeatureSettings } from '@/hooks/useFeatureSettings';
|
||||
import {
|
||||
openTaskConflict,
|
||||
recordTaskConflict,
|
||||
resolveTaskConflict,
|
||||
taskConflictToastId,
|
||||
type TaskConflictOperation,
|
||||
} from '@/hooks/useTaskConflicts';
|
||||
import {
|
||||
DEFAULT_FEATURE_SETTINGS,
|
||||
normalizeBoardColumns,
|
||||
|
|
@ -46,6 +54,13 @@ function cachedTaskRevision(queryClient: QueryClient, taskId: string): number |
|
|||
return typeof listTask?.revision === 'number' ? listTask.revision : undefined;
|
||||
}
|
||||
|
||||
function cachedTask(queryClient: QueryClient, taskId: string): Task | undefined {
|
||||
return (
|
||||
queryClient.getQueryData<Task>(['tasks', taskId]) ??
|
||||
queryClient.getQueryData<Task[]>(['tasks'])?.find((task) => task.id === taskId)
|
||||
);
|
||||
}
|
||||
|
||||
function conflictCurrentTask(error: ApiMutationError): Task | undefined {
|
||||
const details = error.details as { current?: unknown } | undefined;
|
||||
const current = details?.current;
|
||||
|
|
@ -71,7 +86,8 @@ export function isRevisionConflict(error: unknown): error is ApiMutationError {
|
|||
function handleRevisionConflict(
|
||||
queryClient: QueryClient,
|
||||
error: ApiMutationError,
|
||||
taskId: string
|
||||
taskId: string,
|
||||
operation: TaskConflictOperation = 'task update'
|
||||
): boolean {
|
||||
if (!isRevisionConflict(error)) {
|
||||
return false;
|
||||
|
|
@ -85,12 +101,34 @@ function handleRevisionConflict(
|
|||
queryClient.invalidateQueries({ queryKey: ['tasks'] });
|
||||
queryClient.invalidateQueries({ queryKey: ['tasks', taskId] });
|
||||
|
||||
toast({
|
||||
title: 'Task changed elsewhere',
|
||||
description: 'Loaded the latest task. Review your edit and save again.',
|
||||
variant: 'destructive',
|
||||
duration: 10000,
|
||||
const knownTask = current ?? cachedTask(queryClient, taskId);
|
||||
const taskTitle = knownTask?.title ?? taskId;
|
||||
const recorded = recordTaskConflict({
|
||||
taskId,
|
||||
taskTitle,
|
||||
currentTask: current,
|
||||
operation,
|
||||
});
|
||||
if (recorded.isNewIdentity && !recorded.hasOpenSurface) {
|
||||
toast({
|
||||
id: taskConflictToastId(taskId),
|
||||
title: 'Task updated before your change',
|
||||
description: `The latest version of ${taskTitle} was loaded. Review the task before trying your ${operation} again.`,
|
||||
action: createElement(
|
||||
'button',
|
||||
{
|
||||
type: 'button',
|
||||
className:
|
||||
'rounded-md border border-current px-2.5 py-1.5 text-xs font-medium hover:bg-white/10 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-current',
|
||||
'aria-label': `Review conflict for ${taskTitle}`,
|
||||
onClick: () => openTaskConflict(taskId),
|
||||
},
|
||||
'Review task'
|
||||
),
|
||||
variant: 'destructive',
|
||||
duration: 10000,
|
||||
});
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
@ -197,6 +235,7 @@ export function useUpdateTask() {
|
|||
// save response (which doesn't include timeTracking changes) would
|
||||
// overwrite the timer stop/start that happened in between.
|
||||
onSuccess: (serverTask, { input }) => {
|
||||
resolveTaskConflict(serverTask.id, 'task update');
|
||||
const mergeWithCachedTimeTracking = (cached: Task | undefined): Task => {
|
||||
if (!cached || input.timeTracking !== undefined) {
|
||||
// If this update explicitly included timeTracking, use server response as-is
|
||||
|
|
@ -623,10 +662,11 @@ export function useAddComment() {
|
|||
mutationFn: ({ taskId, author, text }: { taskId: string; author: string; text: string }) =>
|
||||
api.tasks.addComment(taskId, author, text, cachedTaskRevision(queryClient, taskId)),
|
||||
onSuccess: (task) => {
|
||||
resolveTaskConflict(task.id, 'comment change');
|
||||
patchTaskInCaches(queryClient, task);
|
||||
},
|
||||
onError: (error, { taskId }) => {
|
||||
handleRevisionConflict(queryClient, error as ApiMutationError, taskId);
|
||||
handleRevisionConflict(queryClient, error as ApiMutationError, taskId, 'comment change');
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
@ -645,10 +685,11 @@ export function useEditComment() {
|
|||
text: string;
|
||||
}) => api.tasks.editComment(taskId, commentId, text, cachedTaskRevision(queryClient, taskId)),
|
||||
onSuccess: (task) => {
|
||||
resolveTaskConflict(task.id, 'comment change');
|
||||
patchTaskInCaches(queryClient, task);
|
||||
},
|
||||
onError: (error, { taskId }) => {
|
||||
handleRevisionConflict(queryClient, error as ApiMutationError, taskId);
|
||||
handleRevisionConflict(queryClient, error as ApiMutationError, taskId, 'comment change');
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
@ -660,10 +701,11 @@ export function useDeleteComment() {
|
|||
mutationFn: ({ taskId, commentId }: { taskId: string; commentId: string }) =>
|
||||
api.tasks.deleteComment(taskId, commentId, cachedTaskRevision(queryClient, taskId)),
|
||||
onSuccess: (task) => {
|
||||
resolveTaskConflict(task.id, 'comment change');
|
||||
patchTaskInCaches(queryClient, task);
|
||||
},
|
||||
onError: (error, { taskId }) => {
|
||||
handleRevisionConflict(queryClient, error as ApiMutationError, taskId);
|
||||
handleRevisionConflict(queryClient, error as ApiMutationError, taskId, 'comment change');
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -70,6 +70,14 @@ const addToRemoveQueue = (toastId: string) => {
|
|||
export const reducer = (state: State, action: Action): State => {
|
||||
switch (action.type) {
|
||||
case 'ADD_TOAST':
|
||||
if (state.toasts.some((toast) => toast.id === action.toast.id)) {
|
||||
return {
|
||||
...state,
|
||||
toasts: state.toasts.map((toast) =>
|
||||
toast.id === action.toast.id ? { ...toast, ...action.toast } : toast
|
||||
),
|
||||
};
|
||||
}
|
||||
return {
|
||||
...state,
|
||||
toasts: [action.toast, ...state.toasts].slice(0, TOAST_LIMIT),
|
||||
|
|
@ -129,10 +137,15 @@ function dispatch(action: Action) {
|
|||
});
|
||||
}
|
||||
|
||||
type Toast = Omit<ToasterToast, 'id'>;
|
||||
type Toast = Omit<ToasterToast, 'id'> & { id?: string };
|
||||
|
||||
function toast({ ...props }: Toast) {
|
||||
const id = genId();
|
||||
function toast({ id: requestedId, ...props }: Toast) {
|
||||
const id = requestedId ?? genId();
|
||||
const pendingRemoval = toastTimeouts.get(id);
|
||||
if (pendingRemoval) {
|
||||
clearTimeout(pendingRemoval);
|
||||
toastTimeouts.delete(id);
|
||||
}
|
||||
|
||||
const update = (props: ToasterToast) =>
|
||||
dispatch({
|
||||
|
|
@ -160,6 +173,10 @@ function toast({ ...props }: Toast) {
|
|||
};
|
||||
}
|
||||
|
||||
function dismissToast(toastId: string) {
|
||||
dispatch({ type: 'DISMISS_TOAST', toastId });
|
||||
}
|
||||
|
||||
function useToast() {
|
||||
const [state, setState] = React.useState<State>(memoryState);
|
||||
|
||||
|
|
@ -180,4 +197,4 @@ function useToast() {
|
|||
};
|
||||
}
|
||||
|
||||
export { useToast, toast };
|
||||
export { useToast, toast, dismissToast };
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue