refactor(brief): move the action lifecycle into the machine (RD-12)
The action lifecycle (Idle | Busy | Failed) lived in an imperative store-level signal, set from ten call sites outside the reducer. The reducer could not enforce which action transitions are legal. Add `action` to `BriefState.Loaded`, driven by three new messages (ActionStarted, ActionFinished, ActionFailed) and handled in `reduce`. Replace every `actionState.set(...)` call in `brief.store.ts` with the matching `dispatch`. `BriefLoaded` resets `action` to Idle, so a fresh load clears a stale action error instead of letting it outlive the reload. `busy` and `lastError` stay as `computed`s on the store with a byte-identical public signature — they are the render seam for four components and two page templates, and the union belongs in the machine, not the components. `revealBigNummer` still sets only `Failed`, never `Busy` — an existing asymmetry, not changed here. `SaveState`, `org-template.store.ts`, and `pendingPublish` are out of scope (RD-13, RD-14). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { Injectable, computed, inject, signal } from '@angular/core';
|
||||
import { Result } from '@shared/kernel/fp';
|
||||
import { createStore } from '@shared/application/store';
|
||||
import { ActionState, SaveState } from '@shared/application/action-state';
|
||||
import { SaveState } from '@shared/application/action-state';
|
||||
import { createHistory } from '@shared/application/history';
|
||||
import { createDebouncedSave } from '@shared/application/debounced-save';
|
||||
import { fromLoadLifecycle } from '@shared/application/remote-data';
|
||||
@@ -41,11 +41,16 @@ export class BriefStore implements PendingSave {
|
||||
|
||||
readonly model = this.store.model;
|
||||
|
||||
private actionState = signal<ActionState>({ tag: 'Idle' });
|
||||
readonly busy = computed(() => this.actionState().tag === 'Busy');
|
||||
/** The one-shot action lifecycle now lives on the machine's `Loaded.action` (RD-12);
|
||||
these stay as plain `computed`s so the render seam (four `busy = input(...)`
|
||||
components, two page templates) keeps a byte-identical boolean/string API. */
|
||||
readonly busy = computed(() => {
|
||||
const s = this.model();
|
||||
return s.tag === 'Loaded' && s.action.tag === 'Busy';
|
||||
});
|
||||
readonly lastError = computed(() => {
|
||||
const s = this.actionState();
|
||||
return s.tag === 'Failed' ? s.error : null;
|
||||
const s = this.model();
|
||||
return s.tag === 'Loaded' && s.action.tag === 'Failed' ? s.action.error : null;
|
||||
});
|
||||
|
||||
/** Surfaced autosave state for the indicator + aria-live region. */
|
||||
@@ -212,7 +217,9 @@ export class BriefStore implements PendingSave {
|
||||
if (r.ok) {
|
||||
this.saveState.set({ tag: 'Saved' });
|
||||
} else {
|
||||
this.actionState.set({ tag: 'Failed', error: r.error });
|
||||
// The autosave failure legitimately surfaces in two places: the small save
|
||||
// indicator below (kept as-is) and the action error line (RD-12).
|
||||
this.store.dispatch({ tag: 'ActionFailed', error: r.error });
|
||||
this.saveState.set({ tag: 'Error' });
|
||||
}
|
||||
}
|
||||
@@ -224,19 +231,19 @@ export class BriefStore implements PendingSave {
|
||||
|
||||
/** Demo "start over": recreate the brief server-side and load the fresh view. */
|
||||
async resetDemo() {
|
||||
this.actionState.set({ tag: 'Busy' });
|
||||
this.store.dispatch({ tag: 'ActionStarted' });
|
||||
this.debouncedSave.cancel();
|
||||
const r = await this.adapter.reset();
|
||||
this.saveState.set({ tag: 'Idle' });
|
||||
if (r.ok) {
|
||||
this.actionState.set({ tag: 'Idle' });
|
||||
this.store.dispatch({ tag: 'ActionFinished' });
|
||||
this.orgTemplate.set(r.value.orgTemplate);
|
||||
this.caseContext.set(r.value.caseContext);
|
||||
this.history.clear();
|
||||
this.rejectionSnapshot.set(null);
|
||||
this.store.dispatch({ tag: 'BriefLoaded', ...r.value });
|
||||
} else {
|
||||
this.actionState.set({ tag: 'Failed', error: r.error });
|
||||
this.store.dispatch({ tag: 'ActionFailed', error: r.error });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -249,13 +256,13 @@ export class BriefStore implements PendingSave {
|
||||
letter in a new tab via `BLOB_PRESENTER.open` — see its doc comment for why the
|
||||
object URL is never revoked. */
|
||||
async previewLetter() {
|
||||
this.actionState.set({ tag: 'Busy' });
|
||||
this.store.dispatch({ tag: 'ActionStarted' });
|
||||
const r = await this.previewAdapter.preview();
|
||||
if (!r.ok) {
|
||||
this.actionState.set({ tag: 'Failed', error: r.error });
|
||||
this.store.dispatch({ tag: 'ActionFailed', error: r.error });
|
||||
return;
|
||||
}
|
||||
this.actionState.set({ tag: 'Idle' });
|
||||
this.store.dispatch({ tag: 'ActionFinished' });
|
||||
this.blobPresenter.open(r.value);
|
||||
}
|
||||
|
||||
@@ -269,7 +276,8 @@ export class BriefStore implements PendingSave {
|
||||
async revealBigNummer() {
|
||||
const r = await this.revealAdapter.reveal(true);
|
||||
if (!r.ok) {
|
||||
this.actionState.set({ tag: 'Failed', error: r.error });
|
||||
// Never sets Busy — an existing asymmetry (RD-12), not fixed here.
|
||||
this.store.dispatch({ tag: 'ActionFailed', error: r.error });
|
||||
return;
|
||||
}
|
||||
this.caseContext.update((c) => (c ? { ...c, bigNummer: r.value } : c));
|
||||
@@ -278,15 +286,15 @@ export class BriefStore implements PendingSave {
|
||||
// A transition: flush any pending save, call the server (authoritative), then mirror
|
||||
// the returned status through the pure reducer's guarded transition.
|
||||
private async transition(action: () => Promise<Result<string, BriefView>>) {
|
||||
this.actionState.set({ tag: 'Busy' });
|
||||
this.store.dispatch({ tag: 'ActionStarted' });
|
||||
this.debouncedSave.cancel();
|
||||
await this.flushSave();
|
||||
const r = await action();
|
||||
if (!r.ok) {
|
||||
this.actionState.set({ tag: 'Failed', error: r.error });
|
||||
this.store.dispatch({ tag: 'ActionFailed', error: r.error });
|
||||
return;
|
||||
}
|
||||
this.actionState.set({ tag: 'Idle' });
|
||||
this.store.dispatch({ tag: 'ActionFinished' });
|
||||
this.applyServerStatus(r.value);
|
||||
}
|
||||
|
||||
|
||||
@@ -265,6 +265,39 @@ describe('brief.machine reduce', () => {
|
||||
expect(sent.tag === 'Loaded' && sent.brief.status).toEqual({ tag: 'sent', sentAt: 't3' });
|
||||
});
|
||||
|
||||
// --- RD-12: the action lifecycle lives on `Loaded.action`, driven by three msgs ---
|
||||
|
||||
it('ActionStarted moves a loaded brief to Busy', () => {
|
||||
const s = reduce(loaded(), { tag: 'ActionStarted' });
|
||||
expect(s.tag === 'Loaded' && s.action).toEqual({ tag: 'Busy' });
|
||||
});
|
||||
|
||||
it('ActionFailed carries the error', () => {
|
||||
const s = reduce(loaded(), { tag: 'ActionFailed', error: 'niet gelukt' });
|
||||
expect(s.tag === 'Loaded' && s.action).toEqual({ tag: 'Failed', error: 'niet gelukt' });
|
||||
});
|
||||
|
||||
it('ActionFinished returns to Idle', () => {
|
||||
const busy = reduce(loaded(), { tag: 'ActionStarted' });
|
||||
const s = reduce(busy, { tag: 'ActionFinished' });
|
||||
expect(s.tag === 'Loaded' && s.action).toEqual({ tag: 'Idle' });
|
||||
});
|
||||
|
||||
it('BriefLoaded resets a stale action error to Idle', () => {
|
||||
const failed = reduce(loaded(), { tag: 'ActionFailed', error: 'niet gelukt' });
|
||||
const reloaded = reduce(failed, {
|
||||
tag: 'BriefLoaded',
|
||||
brief: briefWith({ tag: 'draft' }),
|
||||
availablePassages: lib,
|
||||
decisions,
|
||||
});
|
||||
expect(reloaded.tag === 'Loaded' && reloaded.action).toEqual({ tag: 'Idle' });
|
||||
});
|
||||
|
||||
it('an action message is a no-op when the brief is not loaded', () => {
|
||||
expect(reduce(initialLoading(), { tag: 'ActionStarted' })).toEqual(initialLoading());
|
||||
});
|
||||
|
||||
it('a status transition replaces decisions with the fresh server value', () => {
|
||||
const submitted = loaded({ tag: 'submitted', submittedBy: 'u1', submittedAt: 't' });
|
||||
const staleApprover: BriefDecisions = {
|
||||
|
||||
@@ -36,6 +36,10 @@ import { passagesForBesluit } from './besluit';
|
||||
* structurally impossible (a pasted `{{…}}` is caught by the linter as `malformed`).
|
||||
*/
|
||||
|
||||
/** The one-shot action lifecycle (submit/approve/reject/send/preview/reveal/reset),
|
||||
owned by the reducer instead of an imperative store-level signal (RD-12). */
|
||||
export type BriefActionState = { tag: 'Idle' } | { tag: 'Busy' } | { tag: 'Failed'; error: string };
|
||||
|
||||
export type BriefState =
|
||||
| { tag: 'Loading' }
|
||||
| {
|
||||
@@ -43,6 +47,7 @@ export type BriefState =
|
||||
brief: Brief;
|
||||
availablePassages: readonly LibraryPassage[];
|
||||
decisions: BriefDecisions;
|
||||
action: BriefActionState;
|
||||
}
|
||||
| { tag: 'Failed'; reason: string };
|
||||
|
||||
@@ -65,7 +70,10 @@ export type BriefMsg =
|
||||
| { tag: 'Approved'; by: string; at: string; decisions: BriefDecisions } // submitted → approved
|
||||
| { tag: 'Rejected'; by: string; at: string; comments: string; decisions: BriefDecisions } // submitted → rejected
|
||||
| { tag: 'Sent'; at: string; decisions: BriefDecisions } // approved → sent
|
||||
| { tag: 'Seed'; state: BriefState };
|
||||
| { tag: 'Seed'; state: BriefState }
|
||||
| { tag: 'ActionStarted' } // a one-shot action (submit/approve/preview/…) began
|
||||
| { tag: 'ActionFinished' } // it completed successfully
|
||||
| { tag: 'ActionFailed'; error: string }; // it failed, carrying the message to show
|
||||
|
||||
/** Edits are allowed only in these statuses; editing a rejected letter reopens it. */
|
||||
function isEditable(status: BriefStatus): boolean {
|
||||
@@ -193,6 +201,9 @@ export function reduce(s: BriefState, m: BriefMsg): BriefState {
|
||||
brief: m.brief,
|
||||
availablePassages: m.availablePassages,
|
||||
decisions: m.decisions,
|
||||
// A fresh load clears a stale action error rather than letting it outlive
|
||||
// the reload (RD-12, decision 4).
|
||||
action: { tag: 'Idle' },
|
||||
};
|
||||
case 'BriefLoadFailed':
|
||||
return { tag: 'Failed', reason: m.reason };
|
||||
@@ -260,6 +271,15 @@ export function reduce(s: BriefState, m: BriefMsg): BriefState {
|
||||
case 'Sent':
|
||||
return transition(s, 'approved', () => ({ tag: 'sent', sentAt: m.at }), m.decisions);
|
||||
|
||||
// The action lifecycle (RD-12): a no-op unless a brief is loaded, since there is
|
||||
// nothing to attach the action state to otherwise.
|
||||
case 'ActionStarted':
|
||||
return s.tag === 'Loaded' ? { ...s, action: { tag: 'Busy' } } : s;
|
||||
case 'ActionFinished':
|
||||
return s.tag === 'Loaded' ? { ...s, action: { tag: 'Idle' } } : s;
|
||||
case 'ActionFailed':
|
||||
return s.tag === 'Loaded' ? { ...s, action: { tag: 'Failed', error: m.error } } : s;
|
||||
|
||||
default:
|
||||
return assertNever(m);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user