refactor(fp): WP-31 — shared store helpers (dedupe brief/org-template/stamdata)
Audit "apply high-value": extract four shared helpers into shared/application/ and rewire the editor stores (behaviour unchanged, existing specs are the gate): - action-state.ts: ActionState/SaveState (were duplicated in both brief stores). - history.ts: createHistory<T> (extracted from BriefStore's WP-27 undo/redo; WP-32 reuses). - debounced-save.ts: createDebouncedSave (the 600ms timer/PendingSave dance, was 2×+). - machine-remote-data.ts: machineRemoteData (the loading/failed/loaded→RemoteData switch, 3×). Each helper has a co-located spec. Deferred DDD findings (contracts/ inconsistency, a parse* traverse combinator, the 6× Seed boilerplate) are reported in the WP file, not built. npm run ci green; 323 tests (+13 helper specs); brief/org-template/stamdata specs unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,9 @@
|
||||
/** Transient state of a one-shot action (submit/approve/publish/reset/…): one tagged
|
||||
union instead of a busy boolean + a nullable error sitting side by side. Shared by the
|
||||
editor stores (WP-31). */
|
||||
export type ActionState = { tag: 'Idle' } | { tag: 'Busy' } | { tag: 'Failed'; error: string };
|
||||
|
||||
/** Debounced-autosave indicator, shown in a small status line near a toolbar — a separate
|
||||
concern from ActionState (a stale autosave error doesn't block submit/approve), but
|
||||
tag-aligned with it for one consistent idiom. */
|
||||
export type SaveState = { tag: 'Idle' } | { tag: 'Saving' } | { tag: 'Saved' } | { tag: 'Error' };
|
||||
@@ -0,0 +1,56 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { createDebouncedSave } from './debounced-save';
|
||||
|
||||
describe('createDebouncedSave', () => {
|
||||
beforeEach(() => vi.useFakeTimers());
|
||||
afterEach(() => vi.useRealTimers());
|
||||
|
||||
it('flushes after the delay when canSave is true', async () => {
|
||||
const flush = vi.fn().mockResolvedValue(undefined);
|
||||
const d = createDebouncedSave({ delayMs: 600, canSave: () => true, flush });
|
||||
d.schedule();
|
||||
expect(d.hasPendingSave()).toBe(true);
|
||||
expect(flush).not.toHaveBeenCalled();
|
||||
await vi.advanceTimersByTimeAsync(600);
|
||||
expect(flush).toHaveBeenCalledTimes(1);
|
||||
expect(d.hasPendingSave()).toBe(false);
|
||||
});
|
||||
|
||||
it('does not schedule when canSave is false', () => {
|
||||
const flush = vi.fn().mockResolvedValue(undefined);
|
||||
const d = createDebouncedSave({ canSave: () => false, flush });
|
||||
d.schedule();
|
||||
expect(d.hasPendingSave()).toBe(false);
|
||||
});
|
||||
|
||||
it('coalesces rapid schedules into a single flush', async () => {
|
||||
const flush = vi.fn().mockResolvedValue(undefined);
|
||||
const d = createDebouncedSave({ delayMs: 100, canSave: () => true, flush });
|
||||
d.schedule();
|
||||
d.schedule();
|
||||
d.schedule();
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
expect(flush).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('flushPending runs the save immediately and clears; no-op when idle', async () => {
|
||||
const flush = vi.fn().mockResolvedValue(undefined);
|
||||
const d = createDebouncedSave({ delayMs: 600, canSave: () => true, flush });
|
||||
await d.flushPending();
|
||||
expect(flush).not.toHaveBeenCalled(); // idle
|
||||
d.schedule();
|
||||
await d.flushPending();
|
||||
expect(flush).toHaveBeenCalledTimes(1);
|
||||
expect(d.hasPendingSave()).toBe(false);
|
||||
});
|
||||
|
||||
it('cancel drops a scheduled save without running it', async () => {
|
||||
const flush = vi.fn().mockResolvedValue(undefined);
|
||||
const d = createDebouncedSave({ delayMs: 600, canSave: () => true, flush });
|
||||
d.schedule();
|
||||
d.cancel();
|
||||
expect(d.hasPendingSave()).toBe(false);
|
||||
await vi.advanceTimersByTimeAsync(600);
|
||||
expect(flush).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,49 @@
|
||||
export interface DebouncedSave {
|
||||
/** (Re)arm the debounce timer; no-op when `canSave()` is false. */
|
||||
schedule(): void;
|
||||
/** True while a scheduled save hasn't run yet — implements `PendingSave.hasPendingSave`. */
|
||||
hasPendingSave(): boolean;
|
||||
/** Run a scheduled save now and await it; no-op when nothing is scheduled. */
|
||||
flushPending(): Promise<void>;
|
||||
/** Drop a scheduled save without running it (e.g. before an authoritative transition,
|
||||
which flushes explicitly, or a reset that discards the draft). */
|
||||
cancel(): void;
|
||||
}
|
||||
|
||||
/**
|
||||
* The debounced-autosave timer shared by the editor stores (WP-31). It owns ONLY the timer
|
||||
* bookkeeping; the actual write + save-state transitions live in the caller's `flush`
|
||||
* (store-specific — it touches that store's SaveState/ActionState + adapter). The handle is
|
||||
* nulled the moment it fires, so `hasPendingSave()` means "a write is still owed". Integrates
|
||||
* with the `PendingSave` seam (pending-saves.ts): a store delegates hasPendingSave/flushPending
|
||||
* here so the CanDeactivate guard / beforeunload handler can flush a pending edit.
|
||||
*/
|
||||
export function createDebouncedSave(opts: {
|
||||
delayMs?: number;
|
||||
canSave: () => boolean;
|
||||
flush: () => Promise<void>;
|
||||
}): DebouncedSave {
|
||||
const delay = opts.delayMs ?? 600;
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
return {
|
||||
schedule() {
|
||||
if (!opts.canSave()) return;
|
||||
clearTimeout(timer);
|
||||
timer = setTimeout(() => {
|
||||
timer = undefined;
|
||||
void opts.flush();
|
||||
}, delay);
|
||||
},
|
||||
hasPendingSave: () => timer !== undefined,
|
||||
async flushPending() {
|
||||
if (timer === undefined) return;
|
||||
clearTimeout(timer);
|
||||
timer = undefined;
|
||||
await opts.flush();
|
||||
},
|
||||
cancel() {
|
||||
clearTimeout(timer);
|
||||
timer = undefined;
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,59 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { createHistory } from './history';
|
||||
|
||||
describe('createHistory', () => {
|
||||
it('starts empty; undo/redo are no-ops', () => {
|
||||
const h = createHistory<number>();
|
||||
expect(h.canUndo()).toBe(false);
|
||||
expect(h.canRedo()).toBe(false);
|
||||
expect(h.undo(1)).toBeUndefined();
|
||||
expect(h.redo(1)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('records pre-edit snapshots, then undoes and redoes through them', () => {
|
||||
const h = createHistory<string>();
|
||||
// document went a -> b (record a) -> c (record b); current is 'c'
|
||||
h.record('a');
|
||||
h.record('b');
|
||||
expect(h.canUndo()).toBe(true);
|
||||
|
||||
expect(h.undo('c')).toBe('b'); // current 'c' pushed to redo
|
||||
expect(h.canRedo()).toBe(true);
|
||||
expect(h.undo('b')).toBe('a');
|
||||
expect(h.canUndo()).toBe(false);
|
||||
|
||||
expect(h.redo('a')).toBe('b');
|
||||
expect(h.redo('b')).toBe('c');
|
||||
expect(h.canRedo()).toBe(false);
|
||||
});
|
||||
|
||||
it('record() clears the redo stack (no dead redo after a fresh edit)', () => {
|
||||
const h = createHistory<string>();
|
||||
h.record('a');
|
||||
h.undo('b'); // redo now holds 'b'
|
||||
expect(h.canRedo()).toBe(true);
|
||||
h.record('x');
|
||||
expect(h.canRedo()).toBe(false);
|
||||
});
|
||||
|
||||
it('caps the stack depth', () => {
|
||||
const h = createHistory<number>(3);
|
||||
for (let i = 0; i < 5; i++) h.record(i);
|
||||
let undos = 0;
|
||||
let cur = 99;
|
||||
while (h.canUndo()) {
|
||||
cur = h.undo(cur)!;
|
||||
undos++;
|
||||
}
|
||||
expect(undos).toBe(3);
|
||||
});
|
||||
|
||||
it('clear() empties both stacks', () => {
|
||||
const h = createHistory<number>();
|
||||
h.record(1);
|
||||
h.undo(2);
|
||||
h.clear();
|
||||
expect(h.canUndo()).toBe(false);
|
||||
expect(h.canRedo()).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,53 @@
|
||||
import { Signal, computed, signal } from '@angular/core';
|
||||
|
||||
export interface History<T> {
|
||||
readonly canUndo: Signal<boolean>;
|
||||
readonly canRedo: Signal<boolean>;
|
||||
/** Push a pre-edit snapshot onto the undo stack and drop the redo stack. */
|
||||
record(snapshot: T): void;
|
||||
/** Undo: pop the last recorded snapshot and return it (moving `current` onto the redo
|
||||
stack); returns undefined and changes nothing when there's nothing to undo. */
|
||||
undo(current: T): T | undefined;
|
||||
/** Redo: mirror of undo. */
|
||||
redo(current: T): T | undefined;
|
||||
clear(): void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generic undo/redo history over an immutable "document" value `T`. Elm-store editors
|
||||
* restore a returned snapshot by re-dispatching a `Seed`-style Msg — this helper only
|
||||
* shuffles references, it never mutates them, so the caller must hold copy-on-write state
|
||||
* (every edit produces a fresh value). Both stacks are capped so a long session can't grow
|
||||
* unbounded. Extracted from BriefStore's WP-27 undo/redo (WP-31); reused by the stamdata
|
||||
* editor (WP-32).
|
||||
*/
|
||||
export function createHistory<T>(cap = 50): History<T> {
|
||||
const past = signal<readonly T[]>([]);
|
||||
const future = signal<readonly T[]>([]);
|
||||
return {
|
||||
canUndo: computed(() => past().length > 0),
|
||||
canRedo: computed(() => future().length > 0),
|
||||
record(snapshot) {
|
||||
past.update((p) => [...p, snapshot].slice(-cap));
|
||||
future.set([]);
|
||||
},
|
||||
undo(current) {
|
||||
const p = past();
|
||||
if (p.length === 0) return undefined;
|
||||
past.set(p.slice(0, -1));
|
||||
future.update((f) => [...f, current].slice(-cap));
|
||||
return p[p.length - 1];
|
||||
},
|
||||
redo(current) {
|
||||
const f = future();
|
||||
if (f.length === 0) return undefined;
|
||||
future.set(f.slice(0, -1));
|
||||
past.update((p) => [...p, current].slice(-cap));
|
||||
return f[f.length - 1];
|
||||
},
|
||||
clear() {
|
||||
past.set([]);
|
||||
future.set([]);
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,19 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { machineRemoteData } from './machine-remote-data';
|
||||
|
||||
describe('machineRemoteData', () => {
|
||||
it('maps loading → Loading', () => {
|
||||
expect(machineRemoteData({ tag: 'loading' })).toEqual({ tag: 'Loading' });
|
||||
});
|
||||
|
||||
it('maps failed → Failure carrying an Error with the reason', () => {
|
||||
const rd = machineRemoteData({ tag: 'failed', reason: 'boom' });
|
||||
expect(rd.tag).toBe('Failure');
|
||||
if (rd.tag === 'Failure') expect(rd.error.message).toBe('boom');
|
||||
});
|
||||
|
||||
it('maps loaded → Success carrying the whole loaded state', () => {
|
||||
const loaded = { tag: 'loaded', foo: 42 } as const;
|
||||
expect(machineRemoteData(loaded)).toEqual({ tag: 'Success', value: loaded });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,24 @@
|
||||
import { RemoteData } from '@shared/application/remote-data';
|
||||
|
||||
/** The standard load-lifecycle tags an editor machine exposes. */
|
||||
export type LoadLifecycle =
|
||||
{ tag: 'loading' } | { tag: 'failed'; reason: string } | { tag: 'loaded' };
|
||||
|
||||
/**
|
||||
* Project an Elm-machine state onto `RemoteData` for the `<app-async>` seam. The machine
|
||||
* keeps owning its own domain lifecycle (draft/submitted/…); this is purely the
|
||||
* loading/failed/loaded → async mapping, which was byte-identical across BriefStore,
|
||||
* OrgTemplateStore and StamdataStore (WP-31). Wrap the call in a `computed`.
|
||||
*/
|
||||
export function machineRemoteData<S extends LoadLifecycle>(
|
||||
s: S,
|
||||
): RemoteData<Error, Extract<S, { tag: 'loaded' }>> {
|
||||
switch (s.tag) {
|
||||
case 'loading':
|
||||
return { tag: 'Loading' };
|
||||
case 'failed':
|
||||
return { tag: 'Failure', error: new Error(s.reason) };
|
||||
default: // 'loaded'
|
||||
return { tag: 'Success', value: s as Extract<S, { tag: 'loaded' }> };
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user