Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions docs/offline-milestones.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# Offline Milestones

TalentTrust is a fully client-side app: milestone data is persisted in `localStorage` via `src/lib/repository.ts` (there is no backend). The `/milestones` route supports working while the browser is **offline** by deferring mutations to a persistent queue that flushes automatically on reconnect.

This document explains the offline behavior, the queue, conflict handling, and how error codes map to the safe, user-facing messages shown to users.

---

## Behaviour overview

- **Online** → a milestone mutation (`create` / `update` / `delete` / `status`) is applied immediately to the repository, exactly as before this feature existed.
- **Offline** → the mutation is captured into a **persistent queue** (backed by `safeStorage`); the authoritative store is left untouched. The optimistic change is kept locally so the board still reflects the user's intent.
- **Reconnect** → the queue is flushed to the store **in the exact order it was recorded**, and the page re-reads authoritative repository state.

The `window.online` event and the mount are the two triggers that flush+reconcile. A module-level flush lock guarantees only one flush runs at a time, so a burst of `online` events never replays a mutation twice.

---

## Architecture

```
page.tsx
│ useOfflineMilestones(onReconcile)
│ ├── useOnlineStatus() → isOnline boolean
│ └── mutate(mutation) → online ? applyImmediate : enqueue
└── useOfflineMilestones → coordinator (offlineMilestoneCoordinator.ts)
├── applyImmediate() online path → write straight to repository
├── enqueue() offline path → offlineMilestoneQueue.enqueueMutation()
└── flushPending() reconnect → offlineMilestoneQueue.flushQueue()
+ returns reconciled flag for page to re-read
```

### Files

| File | Role |
|------|------|
| `src/lib/offlineMilestoneQueue.ts` | Persistent mutation queue: persistence, replay, flush lock, error normalization |
| `src/lib/offlineMilestoneCoordinator.ts` | Connectivity decision seam the page calls (online/offline/reconnect) |
| `src/hooks/useOfflineMilestones.ts` | React hook wiring `useOnlineStatus` to the coordinator and the page |
| `src/hooks/useOnlineStatus.ts` | Shared, hydration-safe browser online/offline detection |

---

## The queue

Every queued mutation carries a stable `id` generated at enqueue time (`createMutationId()` → `crypto.randomUUID` with a fallback).

Because `MilestoneCreationForm` derives milestone `id`s from title + timestamp, `create` is made **idempotent on replay**: it first checks whether a milestone with that id already exists before writing, eliminating duplicate creates if a flush was interrupted between applying a write and persisting its removal.

**Removal ordering:** a mutation is removed from the persistent queue *only after* its repository write succeeds. A crash mid-flush can therefore never cause a mutation to be lost or applied twice.

The queue is capped at `MAX_QUEUED_MUTATIONS` (200). Beyond that, enqueues are rejected with a stable persistence error.

---

## Conflict handling

Replaying an `update` goes through `upsertMilestone`'s version guard. If the authoritative store holds a *newer* version than the one the offline edit was based on (`baseVersion`), the write is rejected as stale. That becomes a **`MUTATION_CONFLICT`** error and is surfaced to the user — the mutation **stays in the queue** so it is never silently dropped.

When a mutation fails during replay, processing stops there (later mutations may depend on it, so ordering is preserved). The failed mutation remains queued and is reported via the flush result's `failed` errors.

---

## Error codes → user messages

Failures are normalized into `MilestoneMutationError` objects with a stable `code` and a `retryable` flag. UI callers map codes to safe, user-facing messages (never raw exceptions or stack traces). Below is the canonical mapping used across flush/coordinator messages.

### `MUTATION_CONFLICT`
- `retryable`: `false`
- **User message:** `"One of your offline changes conflicts with newer data. Review your milestones."`

### `MUTATION_REPLAY_FAILED`
- `retryable`: `true` by default; `false` when the target milestone no longer exists
- **User message:** `"Some offline changes could not be synchronized. Review the affected milestones."` (retryable) / `"This milestone no longer exists and could not be updated."` (target missing)

### `OFFLINE_QUEUE_PERSIST_FAILED`
- `retryable`: `true`
- **User message:** `"Your change could not be saved offline. Please try again."`, or `"Too many pending offline changes. Connect to the internet to continue."` when the queue cap is hit

### `OFFLINE_QUEUE_INVALID_ENTRY`
- Malformed/corrupt persisted data is **discarded safely** on load; the queue falls back to only the valid entries. Never crashes.

### `RECONCILIATION_FAILED` / `QUEUE_FLUSH_IN_PROGRESS`
- Internal markers; the flush lock turns a duplicate/concurrent flush into a harmless no-op (`{ flushed: 0 }`) rather than an error.

---

## User-facing notices

The page renders an amber banner (`data-testid="offline-status-banner"`) when any of the following is true: the browser is offline, a flush is in progress, there are pending changes, or a notice is set. Messages are intentionally brief and safe:

| Case | Message |
|------|---------|
| Offline (browser) | `"You're offline — milestone changes are saved on this device and will sync automatically when you reconnect."` |
| Flush in progress | `"Synchronizing your pending milestones…"` |
| Offline change saved to queue | `"Saved offline — will synchronize when you are back online."` |
| Flush succeeded (n > 0) | `"Your offline changes were synchronized."` |
| Flush failed with a conflict | `"One of your offline changes conflicts with newer data. Review your milestones."` |
| Flush failed (other) | `"Some offline changes could not be synchronized. Review the affected milestones."` |

Transient notices auto-clear after ~6 seconds so stale messages do not linger.
80 changes: 64 additions & 16 deletions src/app/milestones/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ import MilestoneFilter, {
type MilestoneStatusFilter,
} from '../../components/milestones/MilestoneFilter';
import { MilestoneCreationForm } from '../../components/milestones/MilestoneCreationForm';
import { listMilestones, saveMilestone, updateMilestone } from '@/lib/repository';
import { listMilestones } from '@/lib/repository';
import { getItem, setItem } from '@/lib/safeStorage';
import { useToast } from '@/components/toast/toast-provider';
import SafeBoundary from '@/components/SafeBoundary';
import { downloadMilestonesICS } from '@/lib/icsExport';
import { useOfflineMilestones } from '@/hooks/useOfflineMilestones';
import { SAMPLE_MILESTONES, SAMPLE_DISMISSED_KEY } from './constants';
import type { Milestone } from '@/types/domain';

Expand Down Expand Up @@ -66,6 +67,9 @@ const MilestonesContent: React.FC = () => {
);
const [showForm, setShowForm] = useState(false);
const { showError } = useToast();
const offline = useOfflineMilestones(() => {
setMilestones(listMilestones());
});

useEffect(() => {
setStatusFilter(getValidStatus(searchParams.get('status')));
Expand Down Expand Up @@ -156,33 +160,52 @@ const MilestonesContent: React.FC = () => {
setShowForm(true);
}, []);

const handleSubmitMilestone = useCallback((milestone: Milestone) => {
setShowForm(false);
saveMilestone(milestone);
setIsDismissed(true);
setMilestones((prev) => [...prev, milestone]);
}, []);
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 handleCancelForm = useCallback(() => {
setShowForm(false);
}, []);

const handleUpdateMilestone = useCallback(
(id: string, patch: Partial<Milestone>): boolean => {
try {
updateMilestone(id, patch);
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;
} catch {
showError({
title: 'Unable to update milestone',
description: 'Your milestone could not be saved. Please try again.',
});
return false;
}
showError({
title: 'Unable to update milestone',
description: 'Your milestone could not be saved. Please try again.',
});
return false;
},
[showError],
[milestones, offline, showError],
);

return (
Expand All @@ -191,6 +214,31 @@ const MilestonesContent: React.FC = () => {
Milestones
</h1>

{(offline.isFlushing || offline.notice || offline.pendingCount > 0) && (
<div
data-testid="offline-status-banner"
role="status"
aria-live="polite"
aria-atomic="true"
className="mb-6 rounded-2xl border border-amber-200 bg-amber-50 p-4 text-sm text-amber-900 shadow-sm dark:border-amber-500/20 dark:bg-amber-500/5 dark:text-amber-200"
>
<div className="flex items-start justify-between gap-3">
<p className="font-medium">
{!offline.isOnline
? 'You’re offline — milestone changes are saved on this device and will sync automatically when you reconnect.'
: offline.isFlushing
? 'Synchronizing your pending milestones…'
: offline.notice}
</p>
{!offline.isOnline && offline.pendingCount > 0 && (
<span className="ml-2 shrink-0 rounded-full bg-amber-100 px-2.5 py-0.5 text-xs font-semibold text-amber-900 dark:bg-amber-500/20 dark:text-amber-200">
{offline.pendingCount} pending
</span>
)}
</div>
</div>
)}

{showSampleBanner && (
<div
data-testid="sample-data-banner"
Expand Down
117 changes: 117 additions & 0 deletions src/hooks/__tests__/useOnlineStatus.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/**
* Unit tests for `useOnlineStatus` (`src/hooks/useOnlineStatus.ts`).
*
* Covers the issue's connectivity-detection requirements:
* - initial online and offline states
* - online/offline event transitions
* - event listeners cleaned up on unmount (no leaks / no duplicate listeners)
* - shared listener set, so many consumers never duplicate window listeners
* - hydration-safe fallback when `navigator.onLine` is unavailable
*/

import { renderHook, act } from '@testing-library/react';
import { useOnlineStatus, resetOnlineStatusForTests } from '../useOnlineStatus';

/** Re-sets navigator.onLine to a value and fires the matching window event. */
function setOnline(value: boolean): void {
Object.defineProperty(navigator, 'onLine', {
value,
configurable: true,
});
window.dispatchEvent(new Event(value ? 'online' : 'offline'));
}

describe('useOnlineStatus', () => {
let onLineGetter: PropertyDescriptor | undefined;

beforeEach(() => {
onLineGetter = Object.getOwnPropertyDescriptor(navigator, 'onLine');
Object.defineProperty(navigator, 'onLine', { value: true, configurable: true });
resetOnlineStatusForTests();
});

afterEach(() => {
if (onLineGetter) {
Object.defineProperty(navigator, 'onLine', onLineGetter);
}
});

it('reports the initial online state', () => {
Object.defineProperty(navigator, 'onLine', { value: true, configurable: true });
const { result } = renderHook(() => useOnlineStatus());
expect(result.current).toBe(true);
});

it('reports the initial offline state', () => {
Object.defineProperty(navigator, 'onLine', { value: false, configurable: true });
const { result } = renderHook(() => useOnlineStatus());
act(() => {});
expect(result.current).toBe(false);
});

it('falls back to online when navigator.onLine is unavailable', () => {
Object.defineProperty(navigator, 'onLine', { value: undefined, configurable: true });
const { result } = renderHook(() => useOnlineStatus());
expect(result.current).toBe(true);
});

it('transitions to offline on the window offline event', () => {
const { result } = renderHook(() => useOnlineStatus());
act(() => setOnline(false));
expect(result.current).toBe(false);
});

it('transitions back to online on the window online event', () => {
setOnline(false);
const { result } = renderHook(() => useOnlineStatus());
act(() => setOnline(true));
expect(result.current).toBe(true);
});

it('does not duplicate window listeners across multiple consumers', () => {
const addSpy = jest.spyOn(window, 'addEventListener');

const first = renderHook(() => useOnlineStatus());
const second = renderHook(() => useOnlineStatus());

// Only ONE shared listener per event type, regardless of consumer count.
const onlineCalls = addSpy.mock.calls.filter(([event]) => event === 'online').length;
const offlineCalls = addSpy.mock.calls.filter(([event]) => event === 'offline').length;
expect(onlineCalls).toBe(1);
expect(offlineCalls).toBe(1);

first.unmount();
// Unmounting one consumer must not tear down the shared listener.
act(() => setOnline(false));
expect(second.result.current).toBe(false);

second.unmount();
addSpy.mockRestore();
});

it('cleans up window listeners when the last consumer unmounts', () => {
const removeSpy = jest.spyOn(window, 'removeEventListener');

const { unmount } = renderHook(() => useOnlineStatus());
unmount();
expect(removeSpy).toHaveBeenCalledWith('online', expect.any(Function));
expect(removeSpy).toHaveBeenCalledWith('offline', expect.any(Function));

removeSpy.mockRestore();
});

it('re-syncs with the real connection state once mounted after an initial render', () => {
// Simulate a client that loaded while actually offline even though the first
// render (SSR default) is online: after mount it must reflect the real state.
Object.defineProperty(navigator, 'onLine', { value: true, configurable: true });
const { result, rerender } = renderHook(() => useOnlineStatus());
expect(result.current).toBe(true);

Object.defineProperty(navigator, 'onLine', { value: false, configurable: true });
act(() => {});
rerender();
// The change is picked up via the shared listener, not a forced re-read.
act(() => setOnline(false));
expect(result.current).toBe(false);
});
});
Loading