diff --git a/docs/optimistic-milestone-mutations.md b/docs/optimistic-milestone-mutations.md new file mode 100644 index 00000000..96c11a5c --- /dev/null +++ b/docs/optimistic-milestone-mutations.md @@ -0,0 +1,85 @@ +# Optimistic milestone mutations + +Issue #1097 makes milestone edits feel immediate while preserving the +repository as the source of truth. The behavior is shared by the milestones +page and contract detail page through +`useOptimisticMilestoneMutation`. + +## Mutation lifecycle + +Each create, update, or delete follows the same sequence: + +1. Capture the current list as a rollback snapshot. +2. Update the local list synchronously, before persistence returns. +3. Persist through the repository's version-aware operation. +4. Reconcile the successful object with its canonical version. +5. Restore the exact prior list on failure and return a typed failure code. + +The hook keeps an internal ref synchronized with queued state updates. This is +important for rapid edits: React may batch multiple event handlers before a +render, so the second patch must be based on the first optimistic object rather +than the last committed prop value. + +## Version and stale-write behavior + +Updates read the stored milestone version before writing. The repository +rejects an incoming version older than the stored version and returns a stable +stale result. The hook maps that result to `STALE_VERSION`, restores the local +snapshot, and provides a retry-safe message without exposing storage details. + +Successful updates write the incremented version back into local state. A +successful create receives version one. The canonical reconciliation is a +separate state write after persistence, so the UI cannot remain on an +unversioned optimistic object. + +The hook does not merge server conflicts or queue offline writes. Those are +explicitly outside this issue. A stale result is surfaced so the user can +reload and intentionally resolve the conflict. + +## Typed outcomes + +`OptimisticResult` uses a discriminated `ok` field. Failed operations also +carry one of these stable codes: + +| Code | Meaning | UI behavior | +| ------------------------- | ---------------------------------------- | ----------------------------- | +| `STALE_VERSION` | Another session wrote a newer object | Roll back and request reload | +| `PERSISTENCE_FAILED` | Repository could not save the mutation | Roll back and offer retry | +| `MILESTONE_NOT_FOUND` | Update target is absent from local state | Keep list unchanged | +| `DELETE_TARGET_NOT_FOUND` | Delete found no matching records | Restore list and offer reload | + +The pages translate these results into existing toast messages. The row only +announces a successful save when its callback does not explicitly return +`false`; this prevents an error from being announced as a success. Failure +messages use the existing polite live region and remain accessible to screen +readers. + +## Unrelated updates and immutability + +Optimistic state updates create new arrays and only replace the targeted +milestone. Untouched milestone objects retain their identity. Rollback restores +the pre-mutation array exactly, including fields that were not displayed in the +board. This protects unrelated changes from accidental field loss and makes a +failed edit visually indistinguishable from the pre-edit state. + +The ref-based queue handling is deliberately local to the hook. Parent-owned +state remains the public source of truth, and a later parent refresh replaces +the ref before the next mutation. No global store or cross-contract conflict +merging is introduced. + +## Test coverage + +Unit tests cover immediate create/update/delete state writes, canonical version +reconciliation, exact rollback, stale failures, typed error codes, missing +targets, immutability, and two rapid edits before a render. Existing component +tests cover the polite live-region structure and inline edit behavior. + +The focused hook test command is: + +```text +npm test -- --runInBand src/hooks/__tests__/useOptimisticMilestoneMutation.test.ts +``` + +The full application test, lint, and build commands remain the final CI gate. +This change deliberately keeps server conflict merging and offline queueing out +of the client mutation contract. diff --git a/src/app/contracts/[id]/page.tsx b/src/app/contracts/[id]/page.tsx index ddc3abf2..2836b746 100644 --- a/src/app/contracts/[id]/page.tsx +++ b/src/app/contracts/[id]/page.tsx @@ -1,358 +1,388 @@ -'use client'; - -import { use, useCallback, useEffect, useRef, useState } from 'react'; -import Link from 'next/link'; -import { notFound } from 'next/navigation'; -import Breadcrumbs from '@/components/Breadcrumbs'; -import ContractSummary from '@/components/ContractSummary'; -import MilestonesList from '@/components/MilestonesList'; -import ActionPanel from '@/components/ActionPanel'; -import ContractProgress from '@/components/ContractProgress'; -import { ContractProgressSkeleton } from '@/components/ContractProgressSkeleton'; -import { ContractSummarySkeleton } from '@/components/ContractSummarySkeleton'; -import { MilestonesListSkeleton } from '@/components/MilestonesListSkeleton'; -import ContractStatusAnnouncer from '@/components/ContractStatusAnnouncer'; -import SafeBoundary from '@/components/SafeBoundary'; -import { resolveContractData, ContractData } from '@/lib/contractResolver'; -import { useToast } from '@/components/toast/toast-provider'; -import { useCopyToClipboard } from '@/hooks/useCopyToClipboard'; -import { - listMilestonesByContract, - updateMilestone, -} from '@/lib/repository'; -import { isValidContractId } from '@/lib/validateContractId'; -import { useOptimisticContractStatus, type BuildPersistedContract } from '@/hooks/useOptimisticContractStatus'; -import type { Milestone } from '@/types/domain'; - -/** - * Merges the contract's resolved milestones with any milestones persisted in - * the repository under the same `contractId`, de-duplicating by `id`. - * - * Persisted records take precedence over resolver records that share an id, - * since the repository holds the most recently edited state. - * - * @param baseMilestones - Milestones returned by `resolveContractData`. - * @param contractId - The contract id to filter persisted milestones by. - * @returns The merged, de-duplicated milestone list for this contract. - */ -function mergeContractMilestones( - baseMilestones: Milestone[], - contractId: string, -): Milestone[] { - const merged = new Map(); - baseMilestones.forEach((milestone) => merged.set(milestone.id, milestone)); - listMilestonesByContract(contractId).forEach((milestone) => - merged.set(milestone.id, milestone), - ); - return Array.from(merged.values()); -} - -interface ContractDetailPageProps { - params: Promise<{ id: string }>; -} - -const ContractDetailPageContent = ({ id }: { id: string }) => { - const [contractData, setContractData] = useState(null); - const [milestones, setMilestones] = useState([]); - const [isLoading, setIsLoading] = useState(true); - const [errorMessage, setErrorMessage] = useState(null); - const [isPersistingStatus, setIsPersistingStatus] = useState(false); - const isMountedRef = useRef(true); - const milestonesRef = useRef(milestones); - milestonesRef.current = milestones; - const { showError, showSuccess } = useToast(); - - const { copied, copy } = useCopyToClipboard({ - delay: 2000, - onSuccess: () => { - showSuccess({ - title: 'Contract ID copied', - description: 'The contract identifier has been copied to your clipboard.', - }); - }, - onError: (err) => { - if (err instanceof Error && err.message.includes('supported')) { - showError({ - title: 'Copy not supported', - description: 'Your browser does not support clipboard access. Please copy the ID manually.', - }); - } else { - showError({ - title: 'Copy failed', - description: 'Unable to copy the contract ID to your clipboard. Please try again.', - }); - } - }, - }); - - /** - * Maps the resolved contract detail shape into the repository contract shape. - * - * The repository stores summary-friendly contract records, so the detail page - * narrows `ContractData` into the fields that persistence already expects. - * `version` is threaded through from {@link useOptimisticContractStatus} so - * the repository's stale-overwrite guard compares against the correct baseline. - */ - const buildPersistedContract: BuildPersistedContract = useCallback( - (data, status, version) => ({ - id: data.id, - contractName: data.name, - parties: data.parties, - totalValue: data.totalValue, - currency: data.currency, - status, - createdAt: data.createdAt, - updatedAt: data.updatedAt, - milestoneCount: data.milestones.length, - version, - }), - [], - ); - - const persistStatus = useOptimisticContractStatus( - contractData, - setContractData, - buildPersistedContract, - ); - - /** - * Applies a contract status transition optimistically, then persists it. - * - * The UI already reflects `nextStatus` by the time this returns (applied - * synchronously inside {@link useOptimisticContractStatus}). On failure — - * including a stale-overwrite rejection — the optimistic change is rolled - * back and a clear, specific error message is surfaced via both the inline - * `ActionPanel` banner and a dismissible toast. - * - * @param nextStatus - The status to persist to the repository. - * @param successTitle - The toast title shown after a successful write. - * @param successDescription - The toast description shown after success. - */ - const persistContractStatus = useCallback( - ( - nextStatus: ContractData['status'], - successTitle: string, - successDescription: string, - ) => { - setIsPersistingStatus(true); - setErrorMessage(null); - - const result = persistStatus(nextStatus); - - if (!result.ok) { - setErrorMessage(result.error); - showError({ - title: 'Unable to update contract', - description: result.error, - }); - setIsPersistingStatus(false); - return; - } - - setErrorMessage(null); - showSuccess({ - title: successTitle, - description: successDescription, - }); - setIsPersistingStatus(false); - }, - [persistStatus, showError, showSuccess], - ); - - useEffect(() => { - isMountedRef.current = true; - - const loadContract = async () => { - try { - setIsLoading(true); - setErrorMessage(null); - const data = await resolveContractData(id); - - if (isMountedRef.current) { - setContractData(data); - setMilestones(mergeContractMilestones(data.milestones, id)); - } - } catch (error) { - if (isMountedRef.current) { - setErrorMessage( - error instanceof Error - ? error.message - : 'Failed to load contract. Please try again.', - ); - } - } finally { - if (isMountedRef.current) { - setIsLoading(false); - } - } - }; - - loadContract(); - - return () => { - isMountedRef.current = false; - }; - }, [id]); - - /** - * Placeholder for the future milestone-submission workflow. - */ - const handleSubmitMilestone = () => { - // Replace with real milestone submission flow. - }; - - /** - * Persists the confirmed release-funds action as a completed contract. - */ - const handleReleaseFunds = useCallback(() => { - persistContractStatus( - 'Completed', - 'Funds released', - 'The contract was marked as Completed and the change was saved.', - ); - }, [persistContractStatus]); - - /** - * Persists the confirmed dispute action as a disputed contract. - */ - const handleDispute = useCallback(() => { - persistContractStatus( - 'Disputed', - 'Dispute opened', - 'The contract was marked as Disputed and the change was saved.', - ); - }, [persistContractStatus]); - - const handleViewSummary = () => { - // Replace with summary navigation. - }; - - const handleUpdateMilestone = useCallback((id: string, patch: Partial) => { - const snapshot = milestonesRef.current; - - setMilestones((current) => - current.map((item) => (item.id === id ? { ...item, ...patch } : item)), - ); - - const persisted = updateMilestone(id, patch); - - if (!persisted) { - setMilestones(snapshot); - return false; - } - - return true; - }, []); - - const status = contractData?.status || 'Active'; - - return ( -
- {contractData ? : null} -
-
-
- -
-

Contract #{id}

- -
-
- - Back to contracts - -
- -
-
- - {isLoading ? ( - - ) : contractData ? ( - - ) : null} - - - - {isLoading ? ( - - ) : contractData ? ( - - ) : null} - - - - {isLoading ? ( - - ) : contractData ? ( - - ) : null} - -
- -
- -
-
-
-
- ); -}; - -const ContractDetailPage = ({ params }: ContractDetailPageProps) => { - const { id } = use(params); - - if (!isValidContractId(id)) { - notFound(); - } - - return ; -}; - -export default ContractDetailPage; +"use client"; + +import { use, useCallback, useEffect, useRef, useState } from "react"; +import Link from "next/link"; +import { notFound } from "next/navigation"; +import Breadcrumbs from "@/components/Breadcrumbs"; +import ContractSummary from "@/components/ContractSummary"; +import MilestonesList from "@/components/MilestonesList"; +import ActionPanel from "@/components/ActionPanel"; +import ContractProgress from "@/components/ContractProgress"; +import { ContractProgressSkeleton } from "@/components/ContractProgressSkeleton"; +import { ContractSummarySkeleton } from "@/components/ContractSummarySkeleton"; +import { MilestonesListSkeleton } from "@/components/MilestonesListSkeleton"; +import ContractStatusAnnouncer from "@/components/ContractStatusAnnouncer"; +import SafeBoundary from "@/components/SafeBoundary"; +import { resolveContractData, ContractData } from "@/lib/contractResolver"; +import { useToast } from "@/components/toast/toast-provider"; +import { useCopyToClipboard } from "@/hooks/useCopyToClipboard"; +import { listMilestonesByContract } from "@/lib/repository"; +import { isValidContractId } from "@/lib/validateContractId"; +import { + useOptimisticContractStatus, + type BuildPersistedContract, +} from "@/hooks/useOptimisticContractStatus"; +import { useOptimisticMilestoneMutation } from "@/hooks/useOptimisticMilestoneMutation"; +import type { Milestone } from "@/types/domain"; + +/** + * Merges the contract's resolved milestones with any milestones persisted in + * the repository under the same `contractId`, de-duplicating by `id`. + * + * Persisted records take precedence over resolver records that share an id, + * since the repository holds the most recently edited state. + * + * @param baseMilestones - Milestones returned by `resolveContractData`. + * @param contractId - The contract id to filter persisted milestones by. + * @returns The merged, de-duplicated milestone list for this contract. + */ +function mergeContractMilestones( + baseMilestones: Milestone[], + contractId: string, +): Milestone[] { + const merged = new Map(); + baseMilestones.forEach((milestone) => merged.set(milestone.id, milestone)); + listMilestonesByContract(contractId).forEach((milestone) => + merged.set(milestone.id, milestone), + ); + return Array.from(merged.values()); +} + +interface ContractDetailPageProps { + params: Promise<{ id: string }>; +} + +const ContractDetailPageContent = ({ id }: { id: string }) => { + const [contractData, setContractData] = useState(null); + const [milestones, setMilestones] = useState([]); + const [isLoading, setIsLoading] = useState(true); + const [errorMessage, setErrorMessage] = useState(null); + const [isPersistingStatus, setIsPersistingStatus] = useState(false); + const isMountedRef = useRef(true); + const { showError, showSuccess } = useToast(); + const { optimisticUpdate } = useOptimisticMilestoneMutation( + milestones, + setMilestones, + ); + + const { copied, copy } = useCopyToClipboard({ + delay: 2000, + onSuccess: () => { + showSuccess({ + title: "Contract ID copied", + description: + "The contract identifier has been copied to your clipboard.", + }); + }, + onError: (err) => { + if (err instanceof Error && err.message.includes("supported")) { + showError({ + title: "Copy not supported", + description: + "Your browser does not support clipboard access. Please copy the ID manually.", + }); + } else { + showError({ + title: "Copy failed", + description: + "Unable to copy the contract ID to your clipboard. Please try again.", + }); + } + }, + }); + + /** + * Maps the resolved contract detail shape into the repository contract shape. + * + * The repository stores summary-friendly contract records, so the detail page + * narrows `ContractData` into the fields that persistence already expects. + * `version` is threaded through from {@link useOptimisticContractStatus} so + * the repository's stale-overwrite guard compares against the correct baseline. + */ + const buildPersistedContract: BuildPersistedContract = useCallback( + (data, status, version) => ({ + id: data.id, + contractName: data.name, + parties: data.parties, + totalValue: data.totalValue, + currency: data.currency, + status, + createdAt: data.createdAt, + updatedAt: data.updatedAt, + milestoneCount: data.milestones.length, + version, + }), + [], + ); + + const persistStatus = useOptimisticContractStatus( + contractData, + setContractData, + buildPersistedContract, + ); + + /** + * Applies a contract status transition optimistically, then persists it. + * + * The UI already reflects `nextStatus` by the time this returns (applied + * synchronously inside {@link useOptimisticContractStatus}). On failure — + * including a stale-overwrite rejection — the optimistic change is rolled + * back and a clear, specific error message is surfaced via both the inline + * `ActionPanel` banner and a dismissible toast. + * + * @param nextStatus - The status to persist to the repository. + * @param successTitle - The toast title shown after a successful write. + * @param successDescription - The toast description shown after success. + */ + const persistContractStatus = useCallback( + ( + nextStatus: ContractData["status"], + successTitle: string, + successDescription: string, + ) => { + setIsPersistingStatus(true); + setErrorMessage(null); + + const result = persistStatus(nextStatus); + + if (!result.ok) { + setErrorMessage(result.error); + showError({ + title: "Unable to update contract", + description: result.error, + }); + setIsPersistingStatus(false); + return; + } + + setErrorMessage(null); + showSuccess({ + title: successTitle, + description: successDescription, + }); + setIsPersistingStatus(false); + }, + [persistStatus, showError, showSuccess], + ); + + useEffect(() => { + isMountedRef.current = true; + + const loadContract = async () => { + try { + setIsLoading(true); + setErrorMessage(null); + const data = await resolveContractData(id); + + if (isMountedRef.current) { + setContractData(data); + setMilestones(mergeContractMilestones(data.milestones, id)); + } + } catch (error) { + if (isMountedRef.current) { + setErrorMessage( + error instanceof Error + ? error.message + : "Failed to load contract. Please try again.", + ); + } + } finally { + if (isMountedRef.current) { + setIsLoading(false); + } + } + }; + + loadContract(); + + return () => { + isMountedRef.current = false; + }; + }, [id]); + + /** + * Placeholder for the future milestone-submission workflow. + */ + const handleSubmitMilestone = () => { + // Replace with real milestone submission flow. + }; + + /** + * Persists the confirmed release-funds action as a completed contract. + */ + const handleReleaseFunds = useCallback(() => { + persistContractStatus( + "Completed", + "Funds released", + "The contract was marked as Completed and the change was saved.", + ); + }, [persistContractStatus]); + + /** + * Persists the confirmed dispute action as a disputed contract. + */ + const handleDispute = useCallback(() => { + persistContractStatus( + "Disputed", + "Dispute opened", + "The contract was marked as Disputed and the change was saved.", + ); + }, [persistContractStatus]); + + const handleViewSummary = () => { + // Replace with summary navigation. + }; + + const handleUpdateMilestone = useCallback( + (id: string, patch: Partial) => { + const result = optimisticUpdate(id, patch); + if (result.ok) return true; + showError({ + title: "Unable to update milestone", + description: result.error, + }); + return false; + }, + [optimisticUpdate, showError], + ); + + const status = contractData?.status || "Active"; + + return ( +
+ {contractData ? ( + + ) : null} +
+
+
+ +
+

+ Contract #{id} +

+ +
+
+ + Back to contracts + +
+ +
+
+ + {isLoading ? ( + + ) : contractData ? ( + + ) : null} + + + + {isLoading ? ( + + ) : contractData ? ( + + ) : null} + + + + {isLoading ? ( + + ) : contractData ? ( + + ) : null} + +
+ +
+ +
+
+
+
+ ); +}; + +const ContractDetailPage = ({ params }: ContractDetailPageProps) => { + const { id } = use(params); + + if (!isValidContractId(id)) { + notFound(); + } + + return ; +}; + +export default ContractDetailPage; diff --git a/src/app/milestones/page.tsx b/src/app/milestones/page.tsx index f6bddd20..a35896c5 100644 --- a/src/app/milestones/page.tsx +++ b/src/app/milestones/page.tsx @@ -23,6 +23,7 @@ import { downloadMilestonesICS } from '@/lib/icsExport'; import { useOfflineMilestones } from '@/hooks/useOfflineMilestones'; import { SAMPLE_MILESTONES, SAMPLE_DISMISSED_KEY } from './constants'; import type { Milestone } from '@/types/domain'; +import { useOptimisticMilestoneMutation } from '@/hooks/useOptimisticMilestoneMutation'; const UNPAGINATED_LIST_SIZE = 9999; @@ -67,9 +68,10 @@ const MilestonesContent: React.FC = () => { ); const [showForm, setShowForm] = useState(false); const { showError } = useToast(); - const offline = useOfflineMilestones(() => { - setMilestones(listMilestones()); - }); + const { optimisticCreate, optimisticUpdate } = useOptimisticMilestoneMutation( + milestones, + setMilestones, + ); useEffect(() => { setStatusFilter(getValidStatus(searchParams.get('status'))); @@ -160,52 +162,33 @@ const MilestonesContent: React.FC = () => { setShowForm(true); }, []); - const handleSubmitMilestone = useCallback( - (milestone: Milestone) => { - setShowForm(false); - const accepted = offline.mutate({ kind: 'create', milestone }); - if (accepted) { - setIsDismissed(true); - // Optimistic local update; reconciliation re-reads authoritative state - // when the change is applied online. - setMilestones((prev) => [...prev, milestone]); - } else { - showError({ - title: 'Unable to save milestone', - description: 'Your milestone could not be saved right now. Please try again.', - }); - } - }, - [offline, showError], - ); + const handleSubmitMilestone = useCallback((milestone: Milestone) => { + const result = optimisticCreate(milestone); + if (!result.ok) { + showError({ + title: 'Unable to create milestone', + description: result.error, + }); + return; + } + setShowForm(false); + setIsDismissed(true); + }, [optimisticCreate, showError]); const handleCancelForm = useCallback(() => { setShowForm(false); }, []); const handleUpdateMilestone = useCallback( (id: string, patch: Partial): boolean => { - const current = milestones.find((m) => m.id === id); - const accepted = offline.mutate({ - kind: 'update', - targetId: id, - patch, - baseVersion: current?.version, - }); - if (accepted) { - // Optimistic local update; reconciliation re-reads actual stored state - // once the change is applied. - setMilestones((prev) => - prev.map((item) => (item.id === id ? { ...item, ...patch } : item)), - ); - return true; - } + const result = optimisticUpdate(id, patch); + if (result.ok) return true; showError({ title: 'Unable to update milestone', - description: 'Your milestone could not be saved. Please try again.', + description: result.error, }); return false; }, - [milestones, offline, showError], + [optimisticUpdate, showError], ); return ( @@ -365,4 +348,4 @@ const MilestonesPage: React.FC = () => ( ); -export default MilestonesPage; \ No newline at end of file +export default MilestonesPage; diff --git a/src/components/MilestonesList.tsx b/src/components/MilestonesList.tsx index 84855ef6..1dd29be6 100644 --- a/src/components/MilestonesList.tsx +++ b/src/components/MilestonesList.tsx @@ -1,660 +1,581 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; -import { StatusType, statusColorMap, statusIconMap } from './StatusBadge'; -import MilestoneRow from './milestones/MilestoneRow'; -import { BulkActionToolbar } from './milestones/BulkActionToolbar'; -import { ConfirmDialog } from './ConfirmDialog'; -import { usePreferences } from '@/lib/preferences'; -import { isDueSoon } from '@/lib/dueSoon'; -import { findCurrencyMismatches, normalizeCurrencyCode } from '@/lib/currencyMismatch'; -import { milestoneStatusTally } from '@/lib/milestoneStatusTally'; - -export type Milestone = { - id: string; - title: string; - status: StatusType; - payout: number; - currency: string; - dueDate?: string; - /** Id of the parent `Contract` this milestone belongs to, when known. */ - contractId?: string; - /** - * Monotonically-increasing version counter used by the stale-overwrite - * guard in the persistence layer. Starts at `1` for new milestones and - * increments on each successful upsert. Callers reading from the - * repository can pass the stored version back so `upsertMilestone` can - * reject writes that would silently overwrite a newer version persisted - * by another session tab. - */ - version?: number; - createdAt?: string; - updatedAt?: string; -}; - -export const PAGE_SIZE_DEFAULT = 5; - -export type MilestonesListProps = { - milestones: Milestone[]; - contractCurrency?: string; - onUpdateMilestone?: (id: string, patch: Partial) => boolean; - pageSize?: number; - /** Callback when the selection changes. Passes an array of selected milestone ids. */ - onSelectionChange?: (selectedIds: string[]) => void; - /** Callback to export the selected milestones. */ - onBulkExport?: (selectedMilestones: Milestone[]) => void; - /** Callback to delete selected milestones. Should return the number successfully deleted. */ - onBulkDelete?: (selectedIds: string[]) => number; - /** Callback to update the status of selected milestones. Should return the number successfully updated. */ - onBulkStatusUpdate?: (selectedIds: string[], status: StatusType) => number; -}; - -export const REMINDER_WINDOW_DAYS = 7; - -const MilestonesList = ({ - milestones, - contractCurrency, - onUpdateMilestone, - pageSize = PAGE_SIZE_DEFAULT, - onSelectionChange, - onBulkExport, - onBulkDelete, - onBulkStatusUpdate, -}: MilestonesListProps) => { - const { formatAmount, preferences, updatePreference } = usePreferences(); - const [displayCount, setDisplayCount] = useState(pageSize); - const [isDensityAnnounced, setIsDensityAnnounced] = useState(false); - const [isDismissed, setIsDismissed] = useState(false); - /** - * Tracks which row is currently in inline edit mode. Mutually exclusive — - * opening one row closes any other row that was being edited so we never - * have two dirty unsaved edit states competing for focus or screen reader - * output. - */ - const [editingId, setEditingId] = useState(null); - /** - * Set of selected milestone IDs for multi-select / bulk actions. - */ - const [selectedIds, setSelectedIds] = useState>(new Set()); - /** - * Screen-reader announcement text for selection changes. - */ - const [selectionAnnouncement, setSelectionAnnouncement] = useState(''); - /** - * Whether the delete confirmation dialog is open. - */ - const [showDeleteDialog, setShowDeleteDialog] = useState(false); - /** - * Polite live-region message conveyed to assistive technologies after a - * save / save-failure. Cleared on the *next* save so repeated messages - * are always announced (screen readers intentionally skip repeat strings). - */ - const [announcement, setAnnouncement] = useState(''); - /** - * We force-bump a key on the live region right before writing the message - * so ATs re-announce identical strings ("Milestone saved.") on repeat. - */ - const [announcementNonce, setAnnouncementNonce] = useState(0); - - const listContainerRef = useRef(null); - - /** - * Roving-tabindex state for the milestone list (WAI-ARIA roving tabindex). - * Exactly one row — the "active" row — is in the tab order at a time: it - * carries tabIndex={0} while every other row carries tabIndex={-1}. Arrow - * keys move the active row, Home/End jump to the first/last row, and - * Enter/Space activate the focused row (open its inline edit form). - */ - const [activeIndex, setActiveIndex] = useState(0); - /** Mirror of `activeIndex` for use inside effects without stale closures. */ - const activeIndexRef = useRef(0); - useEffect(() => { - activeIndexRef.current = activeIndex; - }, [activeIndex]); - /** - * The last element that held focus inside the list. Lets the list-change - * sync effect distinguish "focus was in the list and its element was - * removed" (restore focus to the active row) from "focus was never in the - * list" (must NOT move focus). - */ - const lastFocusedInListRef = useRef(null); - - const isCompact = preferences.milestonesDensity === 'compact'; - - // Reset to the first page whenever the underlying list or page size - // changes (e.g. a status filter narrows the results). - useEffect(() => { - setDisplayCount(pageSize); - }, [milestones, pageSize]); - - const today = new Date(); - const visibleMilestones = milestones.slice(0, displayCount); - const hasMore = displayCount < milestones.length; - - // Track the last element focused inside the list so the sync effect below - // can tell "the focused row was removed" apart from "focus never entered - // the list" (which must not move focus). See `lastFocusedInListRef`. - useEffect(() => { - const container = listContainerRef.current; - if (!container) return; - const handleFocusIn = () => { - lastFocusedInListRef.current = document.activeElement as HTMLElement | null; - }; - const handleFocusOut = (event: FocusEvent) => { - const next = event.relatedTarget as Node | null; - // Only forget the tracked element when focus moved to a real element - // outside the list. A null relatedTarget means the focused element was - // removed or focus was lost — keep the ref so the sync effect can - // restore focus (it re-checks via `isConnected`). - if (next && !container.contains(next)) { - lastFocusedInListRef.current = null; - } - }; - container.addEventListener('focusin', handleFocusIn); - container.addEventListener('focusout', handleFocusOut); - return () => { - container.removeEventListener('focusin', handleFocusIn); - container.removeEventListener('focusout', handleFocusOut); - }; - }, []); - - // Keep the roving active index valid when the list changes (status filter, - // pagination, bulk delete) and, if the element that had focus inside the - // list was removed by the change, move focus to the clamped active row so - // keyboard users don't fall out of the list to . - useEffect(() => { - const count = visibleMilestones.length; - if (count === 0) { - setActiveIndex(0); - return; - } - setActiveIndex((prev) => Math.min(prev, count - 1)); - const lastFocused = lastFocusedInListRef.current; - if (lastFocused && !lastFocused.isConnected) { - const rows = listContainerRef.current?.querySelectorAll( - '[data-milestone-row]', - ); - const targetIndex = Math.min(activeIndexRef.current, count - 1); - rows?.[targetIndex]?.focus(); - } - }, [visibleMilestones.length]); - - const mismatchedMilestoneIds = contractCurrency - ? new Set(findCurrencyMismatches(contractCurrency, milestones)) - : new Set(); - - const mismatchedMilestones = milestones.filter((milestone) => - mismatchedMilestoneIds.has(milestone.id), - ); - - const mismatchCurrencies = Array.from( - new Set(mismatchedMilestones.map((milestone) => normalizeCurrencyCode(milestone.currency))), - ).sort(); - - const normalizedContractCurrency = contractCurrency - ? normalizeCurrencyCode(contractCurrency) - : undefined; - - const tallies = milestoneStatusTally(milestones); - - // Filter due-soon milestones: - // - Exclude terminal statuses: Paid, Completed - // - Check if due date is within REMINDER_WINDOW_DAYS - const dueSoonMilestones = milestones.filter( - (m) => - m.status !== 'Paid' && - m.status !== 'Completed' && - isDueSoon(m.dueDate, today, REMINDER_WINDOW_DAYS), - ); - - const showBanner = dueSoonMilestones.length > 0 && !isDismissed; - - const handleToggleDensity = () => { - const next: 'comfortable' | 'compact' = isCompact ? 'comfortable' : 'compact'; - updatePreference('milestonesDensity', next); - setIsDensityAnnounced(true); - }; - - const handleDismiss = () => { - setIsDismissed(true); - // Programmatically shift focus to the list container to avoid focus loss (WCAG 2.1.1) - listContainerRef.current?.focus(); - }; - - const pushAnnouncement = useCallback((message: string) => { - setAnnouncement(''); - // Bump the nonce on the wrapper span so a same-message repeat still - // announces (some SRs dedupe on identical text + key). - setAnnouncementNonce((n) => n + 1); - // Defer the actual write so React mounts a fresh text node first. - requestAnimationFrame(() => setAnnouncement(message)); - }, []); - - const handleSave = useCallback( - (id: string, patch: Partial) => { - const ok = onUpdateMilestone ? onUpdateMilestone(id, patch) : true; - if (ok) { - setEditingId(null); - // The row component also announces via `onAnnounce`. We deliberately - // re-announce here so an `onUpdateMilestone` that returns `true` - // still resolves to a "saved" status even if the row's local - // announcer was bypassed (e.g. parent owns the milestone copy). - } else { - pushAnnouncement('Failed to save milestone.'); - } - }, - [onUpdateMilestone, pushAnnouncement], - ); - - const handleCancel = useCallback(() => { - setEditingId(null); - setAnnouncement(''); - }, []); - - const prevMilestonesRef = useRef(milestones); - useEffect(() => { - if (editingId && prevMilestonesRef.current !== milestones) { - setEditingId(null); - } - prevMilestonesRef.current = milestones; - }, [milestones, editingId]); - - // -------------------------------------------------------------------------- - // Multi-select handlers - // -------------------------------------------------------------------------- - - const allSelected = milestones.length > 0 && selectedIds.size === milestones.length; - const hasSelection = selectedIds.size > 0; - - const announceSelection = useCallback((ids: Set) => { - const count = ids.size; - if (count === 0) { - requestAnimationFrame(() => setSelectionAnnouncement('Selection cleared')); - } else { - requestAnimationFrame(() => - setSelectionAnnouncement(`${count} ${count === 1 ? 'milestone' : 'milestones'} selected`), - ); - } - }, []); - - const handleToggleSelect = useCallback( - (id: string) => { - setSelectedIds((prev) => { - const next = new Set(prev); - if (next.has(id)) { - next.delete(id); - } else { - next.add(id); - } - announceSelection(next); - onSelectionChange?.(Array.from(next)); - return next; - }); - }, - [onSelectionChange, announceSelection], - ); - - const handleToggleSelectAll = useCallback(() => { - setSelectedIds((prev) => { - if (prev.size === milestones.length) { - announceSelection(new Set()); - onSelectionChange?.([]); - return new Set(); - } - const all = new Set(milestones.map((m) => m.id)); - announceSelection(all); - onSelectionChange?.(Array.from(all)); - return all; - }); - }, [milestones, onSelectionChange, announceSelection]); - - const handleClearSelection = useCallback(() => { - setSelectedIds(new Set()); - announceSelection(new Set()); - onSelectionChange?.([]); - }, [onSelectionChange, announceSelection]); - - const handleBulkExport = useCallback(() => { - const selected = milestones.filter((m) => selectedIds.has(m.id)); - onBulkExport?.(selected); - }, [milestones, selectedIds, onBulkExport]); - - const handleBulkStatusUpdate = useCallback( - (status: StatusType) => { - const ids = Array.from(selectedIds); - onBulkStatusUpdate?.(ids, status); - setSelectedIds(new Set()); - onSelectionChange?.([]); - }, - [selectedIds, onBulkStatusUpdate, onSelectionChange], - ); - - const handleDeleteConfirm = useCallback(() => { - const ids = Array.from(selectedIds); - onBulkDelete?.(ids); - setShowDeleteDialog(false); - setSelectedIds(new Set()); - onSelectionChange?.([]); - }, [selectedIds, onBulkDelete, onSelectionChange]); - - const isIndeterminate = hasSelection && !allSelected; - - // -------------------------------------------------------------------------- - // Roving-tabindex keyboard navigation - // -------------------------------------------------------------------------- - - const focusRowAtIndex = (index: number) => { - const rows = listContainerRef.current?.querySelectorAll( - '[data-milestone-row]', - ); - rows?.[index]?.focus(); - }; - - /** - * Handles the list's roving-tabindex keys via event delegation on the - * scroll region. Key events are only intercepted when the event target IS - * a milestone row element (identified via `data-milestone-row`), so inner - * controls keep their native behaviour: Space still toggles a focused - * checkbox, arrows still move the caret inside edit-form fields, and the - * region itself still scrolls with arrow keys when focused. - * - * - ArrowDown / ArrowUp -> move focus to the next / previous row - * - Home / End -> jump to the first / last row - * - Enter / Space -> activate the focused row (open edit mode) - */ - const handleListKeyDown = (event: React.KeyboardEvent) => { - const target = event.target as HTMLElement; - const row = target.closest('[data-milestone-row]'); - if (!row || row !== target) return; - - const currentIndex = Number(row.dataset.rowIndex); - const lastIndex = visibleMilestones.length - 1; - if (Number.isNaN(currentIndex) || lastIndex < 0) return; - - if (event.key === 'Enter' || event.key === ' ') { - // Activate the focused row — the same action as clicking its Edit - // button. - event.preventDefault(); - const milestone = visibleMilestones[currentIndex]; - if (milestone) setEditingId(milestone.id); - return; - } - - let nextIndex: number | null = null; - if (event.key === 'ArrowDown') { - nextIndex = Math.min(currentIndex + 1, lastIndex); - } else if (event.key === 'ArrowUp') { - nextIndex = Math.max(currentIndex - 1, 0); - } else if (event.key === 'Home') { - nextIndex = 0; - } else if (event.key === 'End') { - nextIndex = lastIndex; - } - if (nextIndex === null) return; - - // Consume the key so the page / scroll region doesn't also scroll. - event.preventDefault(); - if (nextIndex === currentIndex) return; - setActiveIndex(nextIndex); - focusRowAtIndex(nextIndex); - }; - - return ( -
-
-

- Milestones -

-
- - {milestones.length} total -
-
- - {/* aria-live region: announces density change to screen readers */} - - {isDensityAnnounced ? `Milestones density set to ${isCompact ? 'compact' : 'comfortable'}` : ''} - - - {tallies.length > 0 && ( -
- {tallies.map(({ status, count }) => ( - - - {status} - - {count} - - - ))} -
- )} - - {normalizedContractCurrency && mismatchedMilestones.length > 0 ? ( -
-

- {mismatchedMilestones.length}{' '} - {mismatchedMilestones.length === 1 ? 'milestone uses' : 'milestones use'}{' '} - {mismatchCurrencies.join(', ')} instead of {normalizedContractCurrency}. -

-
    - {mismatchedMilestones.map((milestone) => ( -
  • - {milestone.title}: {formatAmount(milestone.payout, milestone.currency)} -
    • - ))} -
-
- ) : null} - - {showBanner && ( -
-
-

- {dueSoonMilestones.length} {dueSoonMilestones.length === 1 ? 'milestone is' : 'milestones are'} due within {REMINDER_WINDOW_DAYS} days -

-
    - {dueSoonMilestones.map((m, idx) => ( -
  • - {idx > 0 && } - - {m.title} - -
    • - ))} -
-
- -
- )} - - {/* Polite live region for save / save-failure announcements. The wrapping - span's `key` (via `key={announcementNonce}`) is bumped on every - write so screen readers re-announce identical strings. Controlled - entirely from `MilestoneRow.onAnnounce` and the parent save handler. */} - - {announcement} - - - {/* Screen-reader announcement for selection changes */} - - {selectionAnnouncement} - - - {/* - Labelling (WCAG 1.3.1 / 4.1.2): - aria-labelledby references both the visible "Milestones" heading (milestones-title) and the live - count span (milestones-count) so AT users hear e.g. "Milestones, 3 total – region" rather than - a disconnected static string. This keeps the accessible name in sync with both the heading and - the rendered item count without duplicating text. - - Why the region is tabIndex={-1} (programmatically focusable only): - The milestone rows own the list's single tab stop via roving tabindex (one active row has - tabIndex={0}, every other row tabIndex={-1}), so the scroll container itself must stay out of - the natural tab order — otherwise the list would have two tab stops and "Tab enters the list at - one item" would be violated. It remains focusable programmatically so the due-soon banner - dismiss flow (WCAG 2.4.3) can move focus into the list, and arrow keys still scroll it when it - is focused. - - Why tabIndex is always applied when the list is populated: - 1. Consistency between SSR and client hydration avoids layout/hydration shifts. - 2. Testability in JSDOM where clientHeight/scrollHeight are always zero. - */} - {milestones.length > 0 && ( -
- -
- )} - - setShowDeleteDialog(true)} - /> - - setShowDeleteDialog(false)} - /> - -
0 ? 'region' : undefined} - aria-labelledby={milestones.length > 0 ? 'milestones-title milestones-count' : undefined} - tabIndex={milestones.length > 0 ? -1 : undefined} - onKeyDown={handleListKeyDown} - className={`max-h-[calc(100vh-260px)] overflow-y-auto pr-2 rounded-2xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--ring)] focus-visible:ring-offset-2 ${isCompact ? 'mt-4 space-y-2' : 'mt-6 space-y-4'}`} - > - {visibleMilestones.map((milestone, index) => ( - { - setEditingId(milestone.id); - // Interacting with a row (clicking Edit) makes it the active - // roving row so focus stays consistent after save/cancel. - setActiveIndex(index); - }} - onSave={handleSave} - onCancel={handleCancel} - onAnnounce={pushAnnouncement} - /> - ))} - {hasMore && ( -
- -
- )} -
-
- ); -}; - -export default MilestonesList; +import { useCallback, useEffect, useRef, useState } from "react"; +import { StatusType, statusColorMap, statusIconMap } from "./StatusBadge"; +import MilestoneRow from "./milestones/MilestoneRow"; +import { BulkActionToolbar } from "./milestones/BulkActionToolbar"; +import { ConfirmDialog } from "./ConfirmDialog"; +import { usePreferences } from "@/lib/preferences"; +import { isDueSoon } from "@/lib/dueSoon"; +import { + findCurrencyMismatches, + normalizeCurrencyCode, +} from "@/lib/currencyMismatch"; +import { milestoneStatusTally } from "@/lib/milestoneStatusTally"; + +export type Milestone = { + id: string; + title: string; + status: StatusType; + payout: number; + currency: string; + dueDate?: string; + /** Id of the parent `Contract` this milestone belongs to, when known. */ + contractId?: string; + /** + * Monotonically-increasing version counter used by the stale-overwrite + * guard in the persistence layer. Starts at `1` for new milestones and + * increments on each successful upsert. Callers reading from the + * repository can pass the stored version back so `upsertMilestone` can + * reject writes that would silently overwrite a newer version persisted + * by another session tab. + */ + version?: number; + createdAt?: string; + updatedAt?: string; +}; + +export const PAGE_SIZE_DEFAULT = 5; + +export type MilestonesListProps = { + milestones: Milestone[]; + contractCurrency?: string; + onUpdateMilestone?: (id: string, patch: Partial) => boolean; + pageSize?: number; + /** Callback when the selection changes. Passes an array of selected milestone ids. */ + onSelectionChange?: (selectedIds: string[]) => void; + /** Callback to export the selected milestones. */ + onBulkExport?: (selectedMilestones: Milestone[]) => void; + /** Callback to delete selected milestones. Should return the number successfully deleted. */ + onBulkDelete?: (selectedIds: string[]) => number; + /** Callback to update the status of selected milestones. Should return the number successfully updated. */ + onBulkStatusUpdate?: (selectedIds: string[], status: StatusType) => number; +}; + +export const REMINDER_WINDOW_DAYS = 7; + +const MilestonesList = ({ + milestones, + contractCurrency, + onUpdateMilestone, + pageSize = PAGE_SIZE_DEFAULT, + onSelectionChange, + onBulkExport, + onBulkDelete, + onBulkStatusUpdate, +}: MilestonesListProps) => { + const { formatAmount, preferences, updatePreference } = usePreferences(); + const [displayCount, setDisplayCount] = useState(pageSize); + const [isDensityAnnounced, setIsDensityAnnounced] = useState(false); + const [isDismissed, setIsDismissed] = useState(false); + /** + * Tracks which row is currently in inline edit mode. Mutually exclusive — + * opening one row closes any other row that was being edited so we never + * have two dirty unsaved edit states competing for focus or screen reader + * output. + */ + const [editingId, setEditingId] = useState(null); + /** + * Set of selected milestone IDs for multi-select / bulk actions. + */ + const [selectedIds, setSelectedIds] = useState>(new Set()); + /** + * Screen-reader announcement text for selection changes. + */ + const [selectionAnnouncement, setSelectionAnnouncement] = useState(""); + /** + * Whether the delete confirmation dialog is open. + */ + const [showDeleteDialog, setShowDeleteDialog] = useState(false); + /** + * Polite live-region message conveyed to assistive technologies after a + * save / save-failure. Cleared on the *next* save so repeated messages + * are always announced (screen readers intentionally skip repeat strings). + */ + const [announcement, setAnnouncement] = useState(""); + /** + * We force-bump a key on the live region right before writing the message + * so ATs re-announce identical strings ("Milestone saved.") on repeat. + */ + const [announcementNonce, setAnnouncementNonce] = useState(0); + + const listContainerRef = useRef(null); + + const isCompact = preferences.milestonesDensity === "compact"; + + // Reset to the first page whenever the underlying list or page size + // changes (e.g. a status filter narrows the results). + useEffect(() => { + setDisplayCount(pageSize); + }, [milestones, pageSize]); + + const today = new Date(); + const visibleMilestones = milestones.slice(0, displayCount); + const hasMore = displayCount < milestones.length; + + const mismatchedMilestoneIds = contractCurrency + ? new Set(findCurrencyMismatches(contractCurrency, milestones)) + : new Set(); + + const mismatchedMilestones = milestones.filter((milestone) => + mismatchedMilestoneIds.has(milestone.id), + ); + + const mismatchCurrencies = Array.from( + new Set( + mismatchedMilestones.map((milestone) => + normalizeCurrencyCode(milestone.currency), + ), + ), + ).sort(); + + const normalizedContractCurrency = contractCurrency + ? normalizeCurrencyCode(contractCurrency) + : undefined; + + const tallies = milestoneStatusTally(milestones); + + // Filter due-soon milestones: + // - Exclude terminal statuses: Paid, Completed + // - Check if due date is within REMINDER_WINDOW_DAYS + const dueSoonMilestones = milestones.filter( + (m) => + m.status !== "Paid" && + m.status !== "Completed" && + isDueSoon(m.dueDate, today, REMINDER_WINDOW_DAYS), + ); + + const showBanner = dueSoonMilestones.length > 0 && !isDismissed; + + const handleToggleDensity = () => { + const next: "comfortable" | "compact" = isCompact + ? "comfortable" + : "compact"; + updatePreference("milestonesDensity", next); + setIsDensityAnnounced(true); + }; + + const handleDismiss = () => { + setIsDismissed(true); + // Programmatically shift focus to the list container to avoid focus loss (WCAG 2.1.1) + listContainerRef.current?.focus(); + }; + + const pushAnnouncement = useCallback((message: string) => { + setAnnouncement(""); + // Bump the nonce on the wrapper span so a same-message repeat still + // announces (some SRs dedupe on identical text + key). + setAnnouncementNonce((n) => n + 1); + // Defer the actual write so React mounts a fresh text node first. + requestAnimationFrame(() => setAnnouncement(message)); + }, []); + + const handleSave = useCallback( + (id: string, patch: Partial): boolean => { + const ok = onUpdateMilestone ? onUpdateMilestone(id, patch) : true; + if (ok) { + setEditingId(null); + // The row component also announces via `onAnnounce`. We deliberately + // re-announce here so an `onUpdateMilestone` that returns `true` + // still resolves to a "saved" status even if the row's local + // announcer was bypassed (e.g. parent owns the milestone copy). + } else { + pushAnnouncement("Failed to save milestone."); + } + return ok; + }, + [onUpdateMilestone, pushAnnouncement], + ); + + const handleCancel = useCallback(() => { + setEditingId(null); + setAnnouncement(""); + }, []); + + const prevMilestonesRef = useRef(milestones); + useEffect(() => { + if (editingId && prevMilestonesRef.current !== milestones) { + setEditingId(null); + } + prevMilestonesRef.current = milestones; + }, [milestones, editingId]); + + // -------------------------------------------------------------------------- + // Multi-select handlers + // -------------------------------------------------------------------------- + + const allSelected = + milestones.length > 0 && selectedIds.size === milestones.length; + const hasSelection = selectedIds.size > 0; + + const announceSelection = useCallback((ids: Set) => { + const count = ids.size; + if (count === 0) { + requestAnimationFrame(() => + setSelectionAnnouncement("Selection cleared"), + ); + } else { + requestAnimationFrame(() => + setSelectionAnnouncement( + `${count} ${count === 1 ? "milestone" : "milestones"} selected`, + ), + ); + } + }, []); + + const handleToggleSelect = useCallback( + (id: string) => { + setSelectedIds((prev) => { + const next = new Set(prev); + if (next.has(id)) { + next.delete(id); + } else { + next.add(id); + } + announceSelection(next); + onSelectionChange?.(Array.from(next)); + return next; + }); + }, + [onSelectionChange, announceSelection], + ); + + const handleToggleSelectAll = useCallback(() => { + setSelectedIds((prev) => { + if (prev.size === milestones.length) { + announceSelection(new Set()); + onSelectionChange?.([]); + return new Set(); + } + const all = new Set(milestones.map((m) => m.id)); + announceSelection(all); + onSelectionChange?.(Array.from(all)); + return all; + }); + }, [milestones, onSelectionChange, announceSelection]); + + const handleClearSelection = useCallback(() => { + setSelectedIds(new Set()); + announceSelection(new Set()); + onSelectionChange?.([]); + }, [onSelectionChange, announceSelection]); + + const handleBulkExport = useCallback(() => { + const selected = milestones.filter((m) => selectedIds.has(m.id)); + onBulkExport?.(selected); + }, [milestones, selectedIds, onBulkExport]); + + const handleBulkStatusUpdate = useCallback( + (status: StatusType) => { + const ids = Array.from(selectedIds); + onBulkStatusUpdate?.(ids, status); + setSelectedIds(new Set()); + onSelectionChange?.([]); + }, + [selectedIds, onBulkStatusUpdate, onSelectionChange], + ); + + const handleDeleteConfirm = useCallback(() => { + const ids = Array.from(selectedIds); + onBulkDelete?.(ids); + setShowDeleteDialog(false); + setSelectedIds(new Set()); + onSelectionChange?.([]); + }, [selectedIds, onBulkDelete, onSelectionChange]); + + const isIndeterminate = hasSelection && !allSelected; + + return ( +
+
+

+ Milestones +

+
+ + + {milestones.length} total + +
+
+ + {/* aria-live region: announces density change to screen readers */} + + {isDensityAnnounced + ? `Milestones density set to ${isCompact ? "compact" : "comfortable"}` + : ""} + + + {tallies.length > 0 && ( +
+ {tallies.map(({ status, count }) => ( + + + {status} + + {count} + + + ))} +
+ )} + + {normalizedContractCurrency && mismatchedMilestones.length > 0 ? ( +
+

+ {mismatchedMilestones.length}{" "} + {mismatchedMilestones.length === 1 + ? "milestone uses" + : "milestones use"}{" "} + {mismatchCurrencies.join(", ")} instead of{" "} + {normalizedContractCurrency}. +

+
    + {mismatchedMilestones.map((milestone) => ( +
  • + {milestone.title}:{" "} + {formatAmount(milestone.payout, milestone.currency)} +
    • + ))} +
+
+ ) : null} + + {showBanner && ( +
+
+

+ {dueSoonMilestones.length}{" "} + {dueSoonMilestones.length === 1 + ? "milestone is" + : "milestones are"}{" "} + due within {REMINDER_WINDOW_DAYS} days +

+
    + {dueSoonMilestones.map((m, idx) => ( +
  • + {idx > 0 && ( + + )} + + {m.title} + +
    • + ))} +
+
+ +
+ )} + + {/* Polite live region for save / save-failure announcements. The wrapping + span's `key` (via `key={announcementNonce}`) is bumped on every + write so screen readers re-announce identical strings. Controlled + entirely from `MilestoneRow.onAnnounce` and the parent save handler. */} + + {announcement} + + + {/* Screen-reader announcement for selection changes */} + + {selectionAnnouncement} + + + {/* + Keyboard Accessibility (WCAG 2.1.1): + The scrollable container is focusable (tabIndex={0}) with role="region" so keyboard-only users + can navigate to it and scroll with arrow keys. + + Labelling (WCAG 1.3.1 / 4.1.2): + aria-labelledby references both the visible "Milestones" heading (milestones-title) and the live + count span (milestones-count) so AT users hear e.g. "Milestones, 3 total – region" rather than + a disconnected static string. This keeps the accessible name in sync with both the heading and + the rendered item count without duplicating text. + + Why tabIndex is always applied when the list is populated: + 1. Consistency between SSR and client hydration avoids layout/hydration shifts. + 2. Testability in JSDOM where clientHeight/scrollHeight are always zero. + */} + {milestones.length > 0 && ( +
+ +
+ )} + + setShowDeleteDialog(true)} + /> + + setShowDeleteDialog(false)} + /> + +
0 ? "region" : undefined} + aria-labelledby={ + milestones.length > 0 + ? "milestones-title milestones-count" + : undefined + } + tabIndex={milestones.length > 0 ? 0 : undefined} + className={`max-h-[calc(100vh-260px)] overflow-y-auto pr-2 rounded-2xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--ring)] focus-visible:ring-offset-2 ${isCompact ? "mt-4 space-y-2" : "mt-6 space-y-4"}`} + > + {visibleMilestones.map((milestone) => ( + setEditingId(milestone.id)} + onSave={handleSave} + onCancel={handleCancel} + onAnnounce={pushAnnouncement} + /> + ))} + {hasMore && ( +
+ +
+ )} +
+
+ ); +}; + +export default MilestonesList; diff --git a/src/components/milestones/MilestoneRow.tsx b/src/components/milestones/MilestoneRow.tsx index 072641d9..6b642dd2 100644 --- a/src/components/milestones/MilestoneRow.tsx +++ b/src/components/milestones/MilestoneRow.tsx @@ -1,32 +1,32 @@ -'use client'; +"use client"; -import React, { useCallback, useEffect, useRef, useState } from 'react'; -import StatusBadge from '@/components/StatusBadge'; -import type { StatusType } from '@/components/StatusBadge'; -import { FormField } from '@/components/FormField'; -import { ErrorSummary } from '@/components/ErrorSummary'; -import { usePreferences } from '@/lib/preferences'; -import { sanitizeUserText } from '@/lib/sanitizeUserText'; +import React, { useCallback, useEffect, useRef, useState } from "react"; +import StatusBadge from "@/components/StatusBadge"; +import type { StatusType } from "@/components/StatusBadge"; +import { FormField } from "@/components/FormField"; +import { ErrorSummary } from "@/components/ErrorSummary"; +import { usePreferences } from "@/lib/preferences"; +import { sanitizeUserText } from "@/lib/sanitizeUserText"; import { MAX_MILESTONE_TITLE_LENGTH, MILESTONE_EDIT_FIELD_IDS, validateMilestoneEdit, type MilestoneEditFormValues, -} from '@/lib/validateMilestoneEdit'; -import type { Milestone } from '@/components/MilestonesList'; -import { MilestoneTimestamp } from './MilestoneTimestamp'; +} from "@/lib/validateMilestoneEdit"; +import type { Milestone } from "@/components/MilestonesList"; +import { MilestoneTimestamp } from "./MilestoneTimestamp"; /** Status options exposed in the inline edit form (same set as create form). */ const EDIT_STATUS_OPTIONS: StatusType[] = [ - 'Pending', - 'Active', - 'Completed', - 'Paid', - 'Disputed', + "Pending", + "Active", + "Completed", + "Paid", + "Disputed", ]; /** Currency options exposed in the inline edit form (same set as create form). */ -const EDIT_CURRENCY_OPTIONS = ['USD', 'EUR', 'GBP', 'XLM'] as const; +const EDIT_CURRENCY_OPTIONS = ["USD", "EUR", "GBP", "XLM"] as const; export interface MilestoneRowProps { /** The milestone this row renders. */ @@ -52,7 +52,7 @@ export interface MilestoneRowProps { * (title/payout/currency trimmed, payout coerced to `number`, dueDate * normalised) so it is safe to merge directly into the milestone record. */ - onSave: (id: string, patch: Partial) => void; + onSave: (id: string, patch: Partial) => boolean | void; /** * Called when the user cancels (Cancel button, Escape key, or invalid * focus-escape attempt). Parent should flip `isEditing` to `false`. @@ -130,8 +130,10 @@ export const MilestoneRow: React.FC = ({ const [payout, setPayout] = useState(String(milestone.payout)); const [currency, setCurrency] = useState(milestone.currency); const [status, setStatus] = useState(milestone.status); - const [dueDate, setDueDate] = useState(milestone.dueDate ?? ''); - const [errors, setErrors] = useState>([]); + const [dueDate, setDueDate] = useState(milestone.dueDate ?? ""); + const [errors, setErrors] = useState< + Array<{ fieldId: string; message: string }> + >([]); // Refs for focus management. const editButtonRef = useRef(null); @@ -154,7 +156,7 @@ export const MilestoneRow: React.FC = ({ setPayout(String(milestone.payout)); setCurrency(milestone.currency); setStatus(milestone.status); - setDueDate(milestone.dueDate ?? ''); + setDueDate(milestone.dueDate ?? ""); setErrors([]); // Defer focus to the next frame so the inputs are mounted. const focusTimer = window.setTimeout(() => { @@ -177,14 +179,14 @@ export const MilestoneRow: React.FC = ({ useEffect(() => { if (!isEditing) return undefined; const handleKeyDown = (event: KeyboardEvent) => { - if (event.key !== 'Escape') return; + if (event.key !== "Escape") return; event.stopPropagation(); // Drop any unsaved validation errors. setErrors([]); onCancel(); }; - document.addEventListener('keydown', handleKeyDown); - return () => document.removeEventListener('keydown', handleKeyDown); + document.addEventListener("keydown", handleKeyDown); + return () => document.removeEventListener("keydown", handleKeyDown); }, [isEditing, onCancel]); /** @@ -236,9 +238,14 @@ export const MilestoneRow: React.FC = ({ // because the ErrorSummary itself already carries `role="alert"`. return; } - onSave(milestone.id, result.patch); - onAnnounce?.(`Milestone “${result.patch.title}” saved.`); - focusEditButton(); + // Legacy callers return void, which is treated as success. Integrated + // optimistic callers return false on rollback so a failed save is not + // announced as successful and edit mode remains available for retry. + const persisted = onSave(milestone.id, result.patch); + if (persisted !== false) { + onAnnounce?.(`Milestone “${result.patch.title}” saved.`); + focusEditButton(); + } }, [buildPatch, milestone.id, onSave, onAnnounce, focusEditButton]); /** @@ -273,8 +280,8 @@ export const MilestoneRow: React.FC = ({ tabIndex={tabIndex} className={`rounded-3xl border p-4 shadow-sm transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-indigo-500 focus-visible:ring-offset-2 ${ isSelected - ? 'border-indigo-300 bg-indigo-50' - : 'border-slate-200 bg-slate-50' + ? "border-indigo-300 bg-indigo-50" + : "border-slate-200 bg-slate-50" }`} >
@@ -285,54 +292,56 @@ export const MilestoneRow: React.FC = ({ checked={isSelected} onChange={handleToggle} onKeyDown={(e) => { - if (e.key === 'Enter') { + if (e.key === "Enter") { e.preventDefault(); handleToggle(); } }} - aria-label={`${isSelected ? 'Deselect' : 'Select'} ${milestone.title}`} - tabIndex={tabIndex} + aria-label={`${isSelected ? "Deselect" : "Select"} ${milestone.title}`} className="h-4 w-4 rounded border-slate-300 text-indigo-600 focus:ring-indigo-500" />
)}
-
-

{milestone.title}

-
- Due {milestone.dueDate ?? 'TBD'} - - +
+

+ {milestone.title} +

+
+ Due {milestone.dueDate ?? "TBD"} + + +
-
-
- - -
+
+ +
-
-

Payout

-

- {formatAmount(milestone.payout, milestone.currency)} -

-
+
+
+

Payout

+

+ {formatAmount(milestone.payout, milestone.currency)} +

+
); } diff --git a/src/hooks/__tests__/useOptimisticMilestoneMutation.test.ts b/src/hooks/__tests__/useOptimisticMilestoneMutation.test.ts index e3e0b126..c46af20c 100644 --- a/src/hooks/__tests__/useOptimisticMilestoneMutation.test.ts +++ b/src/hooks/__tests__/useOptimisticMilestoneMutation.test.ts @@ -1,10 +1,10 @@ -import { renderHook, act } from '@testing-library/react'; -import { useOptimisticMilestoneMutation } from '../useOptimisticMilestoneMutation'; -import * as repository from '@/lib/repository'; -import type { Milestone } from '@/components/MilestonesList'; +import { renderHook, act } from "@testing-library/react"; +import { useOptimisticMilestoneMutation } from "../useOptimisticMilestoneMutation"; +import * as repository from "@/lib/repository"; +import type { Milestone } from "@/components/MilestonesList"; -jest.mock('@/lib/repository', () => ({ - ...jest.requireActual('@/lib/repository'), +jest.mock("@/lib/repository", () => ({ + ...jest.requireActual("@/lib/repository"), upsertMilestone: jest.fn(), getMilestoneVersion: jest.fn(), deleteMilestones: jest.fn(), @@ -16,20 +16,20 @@ const mockedDeleteMilestones = jest.mocked(repository.deleteMilestones); const baseMilestones: Milestone[] = [ { - id: 'ms-1', - title: 'Project Kickoff', - status: 'Pending', + id: "ms-1", + title: "Project Kickoff", + status: "Pending", payout: 2500, - currency: 'USD', - dueDate: '2026-03-15', + currency: "USD", + dueDate: "2026-03-15", }, { - id: 'ms-2', - title: 'UI Design', - status: 'Completed', + id: "ms-2", + title: "UI Design", + status: "Completed", payout: 3500, - currency: 'USD', - dueDate: '2026-04-01', + currency: "USD", + dueDate: "2026-04-01", }, ]; @@ -37,21 +37,21 @@ const baseMilestones: Milestone[] = [ // optimisticCreate // ============================================================================= -describe('useOptimisticMilestoneMutation — optimisticCreate', () => { +describe("useOptimisticMilestoneMutation — optimisticCreate", () => { const newMilestone: Milestone = { - id: 'ms-new', - title: 'New Sprint', - status: 'Pending', + id: "ms-new", + title: "New Sprint", + status: "Pending", payout: 1500, - currency: 'USD', - dueDate: '2026-05-01', + currency: "USD", + dueDate: "2026-05-01", }; beforeEach(() => { jest.clearAllMocks(); }); - it('applies the new milestone optimistically before persistence', () => { + it("applies the new milestone optimistically before persistence", () => { mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); const setMilestones = jest.fn(); @@ -69,7 +69,7 @@ describe('useOptimisticMilestoneMutation — optimisticCreate', () => { expect(mockedUpsertMilestone).toHaveBeenCalledWith(newMilestone); }); - it('returns { ok: true } on successful persistence', () => { + it("returns { ok: true } on successful persistence", () => { mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); const setMilestones = jest.fn(); @@ -85,7 +85,7 @@ describe('useOptimisticMilestoneMutation — optimisticCreate', () => { expect(outcome).toEqual({ ok: true }); }); - it('rolls back the optimistic milestone when persistence fails', () => { + it("rolls back the optimistic milestone when persistence fails", () => { mockedUpsertMilestone.mockReturnValue({ success: false, stale: false }); const setMilestones = jest.fn(); @@ -100,8 +100,9 @@ describe('useOptimisticMilestoneMutation — optimisticCreate', () => { expect(outcome).toEqual({ ok: false, + code: "PERSISTENCE_FAILED", stale: false, - error: 'The milestone could not be saved. Please try again.', + error: "The milestone could not be saved. Please try again.", }); // Must have been called twice: optimistic add + rollback @@ -111,7 +112,7 @@ describe('useOptimisticMilestoneMutation — optimisticCreate', () => { expect(setMilestones.mock.calls[1][0]).toEqual(baseMilestones); }); - it('rolls back and returns stale:true when a stale overwrite is detected', () => { + it("rolls back and returns stale:true when a stale overwrite is detected", () => { mockedUpsertMilestone.mockReturnValue({ success: false, stale: true }); const setMilestones = jest.fn(); @@ -126,23 +127,178 @@ describe('useOptimisticMilestoneMutation — optimisticCreate', () => { expect(outcome).toEqual({ ok: false, + code: "STALE_VERSION", stale: true, - error: 'This milestone was updated in another session. Please reload and try again.', + error: + "This milestone was updated in another session. Please reload and try again.", }); }); }); +// ============================================================================= +// reconciliation, race protection, and typed failure outcomes +// ============================================================================= + +describe("useOptimisticMilestoneMutation — reconciliation and races", () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedGetMilestoneVersion.mockReturnValue(0); + }); + + it("reconciles a successful update with the repository version", () => { + mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); + const setMilestones = jest.fn(); + const { result } = renderHook(() => + useOptimisticMilestoneMutation(baseMilestones, setMilestones), + ); + + act(() => { + result.current.optimisticUpdate("ms-1", { title: "Canonical title" }); + }); + + expect(setMilestones).toHaveBeenCalledTimes(2); + const canonicalUpdater = setMilestones.mock.calls[1][0]; + const canonical = canonicalUpdater(baseMilestones); + expect(canonical[0]).toEqual( + expect.objectContaining({ title: "Canonical title", version: 1 }), + ); + }); + + it("keeps both fields when two edits arrive before React renders again", () => { + mockedGetMilestoneVersion.mockReturnValueOnce(0).mockReturnValueOnce(1); + mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); + const setMilestones = jest.fn(); + const { result } = renderHook(() => + useOptimisticMilestoneMutation(baseMilestones, setMilestones), + ); + + act(() => { + result.current.optimisticUpdate("ms-1", { title: "First local edit" }); + result.current.optimisticUpdate("ms-1", { status: "Completed" }); + }); + + expect(mockedUpsertMilestone).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + id: "ms-1", + title: "First local edit", + status: "Completed", + version: 1, + }), + ); + const secondOptimisticUpdater = setMilestones.mock.calls[2][0]; + const secondState = secondOptimisticUpdater(baseMilestones); + expect(secondState[0]).toEqual( + expect.objectContaining({ + title: "First local edit", + status: "Completed", + version: 2, + }), + ); + }); + + it("returns a typed stale-version outcome and rolls back the edit", () => { + mockedUpsertMilestone.mockReturnValue({ success: false, stale: true }); + const setMilestones = jest.fn(); + const { result } = renderHook(() => + useOptimisticMilestoneMutation(baseMilestones, setMilestones), + ); + + let outcome: ReturnType | undefined; + act(() => { + outcome = result.current.optimisticUpdate("ms-1", { + title: "Stale edit", + }); + }); + + expect(outcome).toEqual({ + ok: false, + code: "STALE_VERSION", + stale: true, + error: + "This milestone was updated in another session. Please reload and try again.", + }); + expect(setMilestones.mock.calls[1][0]).toEqual(baseMilestones); + }); + + it("does not mutate the original list while optimistically updating", () => { + mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); + const setMilestones = jest.fn(); + const { result } = renderHook(() => + useOptimisticMilestoneMutation(baseMilestones, setMilestones), + ); + + act(() => { + result.current.optimisticUpdate("ms-1", { payout: 9999 }); + }); + + expect(baseMilestones[0]).toEqual( + expect.objectContaining({ payout: 2500, title: "Project Kickoff" }), + ); + const optimisticUpdater = setMilestones.mock.calls[0][0]; + const optimistic = optimisticUpdater(baseMilestones); + expect(optimistic).not.toBe(baseMilestones); + expect(optimistic[1]).toBe(baseMilestones[1]); + }); + + it("reconciles a successful create with the initial canonical version", () => { + mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); + const setMilestones = jest.fn(); + const { result } = renderHook(() => + useOptimisticMilestoneMutation(baseMilestones, setMilestones), + ); + const created: Milestone = { + id: "ms-3", + title: "Canonical create", + status: "Pending", + payout: 100, + currency: "USD", + }; + + act(() => { + result.current.optimisticCreate(created); + }); + + const canonicalUpdater = setMilestones.mock.calls[1][0]; + const canonical = canonicalUpdater(baseMilestones); + expect(canonical[2]).toEqual( + expect.objectContaining({ id: "ms-3", version: 1 }), + ); + }); + + it("returns a typed delete failure for an unrelated missing target", () => { + mockedDeleteMilestones.mockReturnValue(0); + const setMilestones = jest.fn(); + const { result } = renderHook(() => + useOptimisticMilestoneMutation(baseMilestones, setMilestones), + ); + + let outcome: ReturnType | undefined; + act(() => { + outcome = result.current.optimisticDelete(["unrelated-id"]); + }); + + expect(outcome).toEqual({ + ok: false, + code: "DELETE_TARGET_NOT_FOUND", + stale: false, + error: "No milestones were found to delete. Please reload and try again.", + }); + expect(setMilestones.mock.calls[1][0]).toEqual(baseMilestones); + }); +}); + // ============================================================================= // optimisticUpdate // ============================================================================= -describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { +describe("useOptimisticMilestoneMutation — optimisticUpdate", () => { beforeEach(() => { jest.clearAllMocks(); mockedGetMilestoneVersion.mockReturnValue(0); }); - it('applies the patch optimistically before persistence', () => { + it("applies the patch optimistically before persistence", () => { mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); const setMilestones = jest.fn(); @@ -151,25 +307,37 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { ); act(() => { - result.current.optimisticUpdate('ms-1', { status: 'Completed', payout: 3000 }); + result.current.optimisticUpdate("ms-1", { + status: "Completed", + payout: 3000, + }); }); // Must have applied the patch to state const stateUpdater = setMilestones.mock.calls[0][0]; const updatedState = stateUpdater(baseMilestones); expect(updatedState[0]).toEqual( - expect.objectContaining({ id: 'ms-1', status: 'Completed', payout: 3000 }), + expect.objectContaining({ + id: "ms-1", + status: "Completed", + payout: 3000, + }), ); expect(updatedState[1]).toEqual(baseMilestones[1]); // unchanged // Must have called upsertMilestone with the patched milestone + version - expect(mockedGetMilestoneVersion).toHaveBeenCalledWith('ms-1'); + expect(mockedGetMilestoneVersion).toHaveBeenCalledWith("ms-1"); expect(mockedUpsertMilestone).toHaveBeenCalledWith( - expect.objectContaining({ id: 'ms-1', status: 'Completed', payout: 3000, version: 0 }), + expect.objectContaining({ + id: "ms-1", + status: "Completed", + payout: 3000, + version: 0, + }), ); }); - it('returns { ok: true } on successful persistence', () => { + it("returns { ok: true } on successful persistence", () => { mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); const setMilestones = jest.fn(); @@ -179,13 +347,15 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { let outcome: ReturnType | undefined; act(() => { - outcome = result.current.optimisticUpdate('ms-1', { status: 'Completed' }); + outcome = result.current.optimisticUpdate("ms-1", { + status: "Completed", + }); }); expect(outcome).toEqual({ ok: true }); }); - it('rolls back the optimistic update when persistence fails', () => { + it("rolls back the optimistic update when persistence fails", () => { mockedUpsertMilestone.mockReturnValue({ success: false, stale: false }); const setMilestones = jest.fn(); @@ -194,7 +364,7 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { ); act(() => { - result.current.optimisticUpdate('ms-1', { status: 'Completed' }); + result.current.optimisticUpdate("ms-1", { status: "Completed" }); }); // optimistic update + rollback @@ -204,7 +374,7 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { expect(setMilestones.mock.calls[1][0]).toEqual(baseMilestones); }); - it('rolls back and returns stale:true when a stale overwrite is detected', () => { + it("rolls back and returns stale:true when a stale overwrite is detected", () => { mockedUpsertMilestone.mockReturnValue({ success: false, stale: true }); const setMilestones = jest.fn(); @@ -214,17 +384,21 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { let outcome: ReturnType | undefined; act(() => { - outcome = result.current.optimisticUpdate('ms-1', { status: 'Completed' }); + outcome = result.current.optimisticUpdate("ms-1", { + status: "Completed", + }); }); expect(outcome).toEqual({ ok: false, + code: "STALE_VERSION", stale: true, - error: 'This milestone was updated in another session. Please reload and try again.', + error: + "This milestone was updated in another session. Please reload and try again.", }); }); - it('rolls back and returns error when the milestone id is not found in state', () => { + it("rolls back and returns error when the milestone id is not found in state", () => { const setMilestones = jest.fn(); const { result } = renderHook(() => useOptimisticMilestoneMutation(baseMilestones, setMilestones), @@ -232,20 +406,24 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { let outcome: ReturnType | undefined; act(() => { - outcome = result.current.optimisticUpdate('nonexistent-id', { status: 'Completed' }); + outcome = result.current.optimisticUpdate("nonexistent-id", { + status: "Completed", + }); }); expect(outcome).toEqual({ ok: false, + code: "MILESTONE_NOT_FOUND", stale: false, - error: 'Milestone not found in the current list. Please reload and try again.', + error: + "Milestone not found in the current list. Please reload and try again.", }); - // Should have been rolled back - expect(setMilestones).toHaveBeenCalledTimes(2); + // No optimistic state is applied when the target is absent. + expect(setMilestones).toHaveBeenCalledTimes(1); }); - it('passes the correct stored version to upsertMilestone', () => { + it("passes the correct stored version to upsertMilestone", () => { mockedGetMilestoneVersion.mockReturnValue(3); mockedUpsertMilestone.mockReturnValue({ success: true, stale: false }); @@ -255,12 +433,12 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { ); act(() => { - result.current.optimisticUpdate('ms-1', { status: 'Completed' }); + result.current.optimisticUpdate("ms-1", { status: "Completed" }); }); - expect(mockedGetMilestoneVersion).toHaveBeenCalledWith('ms-1'); + expect(mockedGetMilestoneVersion).toHaveBeenCalledWith("ms-1"); expect(mockedUpsertMilestone).toHaveBeenCalledWith( - expect.objectContaining({ id: 'ms-1', version: 3 }), + expect.objectContaining({ id: "ms-1", version: 3 }), ); }); }); @@ -269,12 +447,12 @@ describe('useOptimisticMilestoneMutation — optimisticUpdate', () => { // optimisticDelete // ============================================================================= -describe('useOptimisticMilestoneMutation — optimisticDelete', () => { +describe("useOptimisticMilestoneMutation — optimisticDelete", () => { beforeEach(() => { jest.clearAllMocks(); }); - it('removes milestones optimistically before persistence', () => { + it("removes milestones optimistically before persistence", () => { mockedDeleteMilestones.mockReturnValue(1); const setMilestones = jest.fn(); @@ -283,20 +461,20 @@ describe('useOptimisticMilestoneMutation — optimisticDelete', () => { ); act(() => { - result.current.optimisticDelete(['ms-1']); + result.current.optimisticDelete(["ms-1"]); }); // Must have filtered out the milestone from state const stateUpdater = setMilestones.mock.calls[0][0]; const updatedState = stateUpdater(baseMilestones); expect(updatedState).toHaveLength(1); - expect(updatedState[0].id).toBe('ms-2'); + expect(updatedState[0].id).toBe("ms-2"); // Must have called deleteMilestones - expect(mockedDeleteMilestones).toHaveBeenCalledWith(['ms-1']); + expect(mockedDeleteMilestones).toHaveBeenCalledWith(["ms-1"]); }); - it('returns { ok: true } on successful deletion', () => { + it("returns { ok: true } on successful deletion", () => { mockedDeleteMilestones.mockReturnValue(1); const setMilestones = jest.fn(); @@ -306,13 +484,13 @@ describe('useOptimisticMilestoneMutation — optimisticDelete', () => { let outcome: ReturnType | undefined; act(() => { - outcome = result.current.optimisticDelete(['ms-1']); + outcome = result.current.optimisticDelete(["ms-1"]); }); expect(outcome).toEqual({ ok: true }); }); - it('rolls back when no milestones were actually deleted', () => { + it("rolls back when no milestones were actually deleted", () => { mockedDeleteMilestones.mockReturnValue(0); const setMilestones = jest.fn(); @@ -321,7 +499,7 @@ describe('useOptimisticMilestoneMutation — optimisticDelete', () => { ); act(() => { - result.current.optimisticDelete(['nonexistent-id']); + result.current.optimisticDelete(["nonexistent-id"]); }); // optimistic delete + rollback diff --git a/src/hooks/useOptimisticMilestoneMutation.ts b/src/hooks/useOptimisticMilestoneMutation.ts index 59d1c13a..7396e93a 100644 --- a/src/hooks/useOptimisticMilestoneMutation.ts +++ b/src/hooks/useOptimisticMilestoneMutation.ts @@ -1,15 +1,30 @@ -'use client'; +"use client"; -import { useCallback, useRef } from 'react'; -import { upsertMilestone, getMilestoneVersion, deleteMilestones } from '@/lib/repository'; -import type { Milestone } from '@/components/MilestonesList'; +import { useCallback, useRef } from "react"; +import { + upsertMilestone, + getMilestoneVersion, + deleteMilestones, +} from "@/lib/repository"; +import type { Milestone } from "@/components/MilestonesList"; /** * Result returned by optimistic mutation operations. */ +export type OptimisticErrorCode = + | "STALE_VERSION" + | "PERSISTENCE_FAILED" + | "MILESTONE_NOT_FOUND" + | "DELETE_TARGET_NOT_FOUND"; + export type OptimisticResult = | { ok: true } - | { ok: false; stale: boolean; error: string }; + | { + ok: false; + code: OptimisticErrorCode; + stale: boolean; + error: string; + }; /** * A hook that applies milestone mutations (create, update, delete) optimistically @@ -56,6 +71,24 @@ export function useOptimisticMilestoneMutation( const milestonesRef = useRef(milestones); milestonesRef.current = milestones; + // Keep the ref in lockstep with the queued React update. React may batch two + // rapid edits before rendering again; reading the ref here prevents the + // second edit from being built from stale props and dropping the first edit. + const commitMilestones = useCallback( + (next: Milestone[]) => { + milestonesRef.current = next; + setMilestones(() => next); + }, + [setMilestones], + ); + const restoreMilestones = useCallback( + (snapshot: Milestone[]) => { + milestonesRef.current = snapshot; + setMilestones(snapshot); + }, + [setMilestones], + ); + // --------------------------------------------------------------------------- // Optimistic create // --------------------------------------------------------------------------- @@ -63,34 +96,44 @@ export function useOptimisticMilestoneMutation( const optimisticCreate = useCallback( (milestone: Milestone): OptimisticResult => { rollbackRef.current = milestonesRef.current; - setMilestones((prev) => [...prev, milestone]); + commitMilestones([...milestonesRef.current, milestone]); const result = upsertMilestone(milestone); if (!result.success) { if (rollbackRef.current) { - setMilestones(rollbackRef.current); + restoreMilestones(rollbackRef.current); } rollbackRef.current = []; return result.stale ? { ok: false, + code: "STALE_VERSION", stale: true, error: - 'This milestone was updated in another session. Please reload and try again.', + "This milestone was updated in another session. Please reload and try again.", } : { ok: false, + code: "PERSISTENCE_FAILED", stale: false, - error: - 'The milestone could not be saved. Please try again.', + error: "The milestone could not be saved. Please try again.", }; } + // Reconcile the optimistic object with the repository's canonical + // version, which is incremented by every successful upsert. + commitMilestones( + milestonesRef.current.map((item) => + item.id === milestone.id + ? { ...milestone, version: (milestone.version ?? 0) + 1 } + : item, + ), + ); rollbackRef.current = []; return { ok: true }; }, - [], + [commitMilestones, restoreMilestones], ); // --------------------------------------------------------------------------- @@ -100,52 +143,67 @@ export function useOptimisticMilestoneMutation( const optimisticUpdate = useCallback( (id: string, patch: Partial): OptimisticResult => { rollbackRef.current = milestonesRef.current; - setMilestones((prev) => - prev.map((m) => (m.id === id ? { ...m, ...patch } : m)), - ); - - const version = getMilestoneVersion(id); const existing = milestonesRef.current.find((m) => m.id === id); if (!existing) { // Milestone not found in current state – roll back and warn. if (rollbackRef.current) { - setMilestones(rollbackRef.current); + restoreMilestones(rollbackRef.current); } rollbackRef.current = []; return { ok: false, + code: "MILESTONE_NOT_FOUND", stale: false, - error: 'Milestone not found in the current list. Please reload and try again.', + error: + "Milestone not found in the current list. Please reload and try again.", }; } + const version = getMilestoneVersion(id); + const optimisticMilestone: Milestone = { + ...existing, + ...patch, + version: version + 1, + }; + commitMilestones( + milestonesRef.current.map((item) => + item.id === id ? optimisticMilestone : item, + ), + ); + const updatedMilestone: Milestone = { ...existing, ...patch, version }; const result = upsertMilestone(updatedMilestone); if (!result.success) { if (rollbackRef.current) { - setMilestones(rollbackRef.current); + restoreMilestones(rollbackRef.current); } rollbackRef.current = []; return result.stale ? { ok: false, + code: "STALE_VERSION", stale: true, error: - 'This milestone was updated in another session. Please reload and try again.', + "This milestone was updated in another session. Please reload and try again.", } : { ok: false, + code: "PERSISTENCE_FAILED", stale: false, - error: - 'The milestone could not be saved. Please try again.', + error: "The milestone could not be saved. Please try again.", }; } + commitMilestones( + milestonesRef.current.map((item) => + item.id === id ? { ...updatedMilestone, version: version + 1 } : item, + ), + ); rollbackRef.current = []; return { ok: true }; }, - [], + [commitMilestones, restoreMilestones], ); // --------------------------------------------------------------------------- @@ -155,27 +213,33 @@ export function useOptimisticMilestoneMutation( const optimisticDelete = useCallback( (ids: string[]): OptimisticResult => { rollbackRef.current = milestonesRef.current; - setMilestones((prev) => prev.filter((m) => !ids.includes(m.id))); + commitMilestones( + milestonesRef.current.filter( + (milestone) => !ids.includes(milestone.id), + ), + ); const removed = deleteMilestones(ids); if (removed === 0 && ids.length > 0) { // Nothing was actually deleted — roll back. if (rollbackRef.current) { - setMilestones(rollbackRef.current); + restoreMilestones(rollbackRef.current); } rollbackRef.current = []; return { ok: false, + code: "DELETE_TARGET_NOT_FOUND", stale: false, - error: 'No milestones were found to delete. Please reload and try again.', + error: + "No milestones were found to delete. Please reload and try again.", }; } rollbackRef.current = []; return { ok: true }; }, - [], + [commitMilestones, restoreMilestones], ); return { optimisticCreate, optimisticUpdate, optimisticDelete };