refactor: move SaveState into debounced-save.ts, delete action-state.ts (RD-14)
RD-12 and RD-13 moved both ActionState consumers into their machines. This left ActionState with zero users, so this ticket deletes it outright. SaveState is different. It has two four-way consumers (brief.page.ts and org-template.page.ts) that still switch on all four cases, so it survives. It moves into debounced-save.ts, next to createDebouncedSave, the only function that sets it. Both store imports merge into the existing debounced-save import line. The doc comments that named ActionState are reworded, since the type no longer exists. No store's public saveState signature changes, so brief/ui/ needs no edit. This completes the phase. Two encodings survive: RemoteData for fetched data, and each machine's own state union. SaveState stays as an explicitly justified third encoding, for a separate concern (debounced autosave status) that the other two do not cover. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,9 +1,8 @@
|
||||
import { Injectable, computed, inject, signal } from '@angular/core';
|
||||
import { Result } from '@shared/kernel/fp';
|
||||
import { createStore } from '@shared/application/store';
|
||||
import { SaveState } from '@shared/application/action-state';
|
||||
import { createHistory } from '@shared/application/history';
|
||||
import { createDebouncedSave } from '@shared/application/debounced-save';
|
||||
import { SaveState, createDebouncedSave } from '@shared/application/debounced-save';
|
||||
import { fromLoadLifecycle } from '@shared/application/remote-data';
|
||||
import {
|
||||
Brief,
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { Injectable, computed, effect, inject, signal } from '@angular/core';
|
||||
import { createStore } from '@shared/application/store';
|
||||
import { SaveState } from '@shared/application/action-state';
|
||||
import { createDebouncedSave } from '@shared/application/debounced-save';
|
||||
import { SaveState, createDebouncedSave } from '@shared/application/debounced-save';
|
||||
import { fromLoadLifecycle } from '@shared/application/remote-data';
|
||||
import { UploadAdapter, uploadContentUrl } from '@shared/infrastructure/upload.adapter';
|
||||
import { UploadShellService } from '@shared/application/upload-shell.service';
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
# RD-14 — Move `SaveState` beside its producer, delete `action-state.ts`
|
||||
|
||||
Status: done
|
||||
Source: PLAN.md 1b#2b
|
||||
|
||||
## Why
|
||||
|
||||
RD-12 and RD-13 moved both `ActionState` consumers into their machines, so **`ActionState`
|
||||
now has zero real users.** Word-anchored, it survives only in its own definition and in one
|
||||
doc-comment mention.
|
||||
|
||||
`SaveState` is different and must survive: it has two genuine consumers that keep all four
|
||||
cases (`brief.page.ts:150` and `org-template.page.ts:102` both `switch` on it, and
|
||||
`brief.page.ts:77` reads `=== 'Error'`). The original plan called for deleting both types;
|
||||
that was corrected once the consumers were read.
|
||||
|
||||
So the file's remaining job is to hold one type whose only producer lives elsewhere. Move
|
||||
`SaveState` next to `createDebouncedSave`, which is what sets it, and the file has no reason
|
||||
to exist.
|
||||
|
||||
## Read first
|
||||
|
||||
- `libs/shared/src/application/action-state.ts` — 9 lines, both types
|
||||
- `libs/shared/src/application/debounced-save.ts` — `SaveState`'s new home; note the comment
|
||||
at line 16, which names `ActionState`
|
||||
- `apps/ssp/src/app/brief/application/brief.store.ts:4,57` and
|
||||
`org-template.store.ts:3,66` — the two importers
|
||||
|
||||
## Decisions (pre-made, don't relitigate)
|
||||
|
||||
1. **Delete `ActionState` outright.** Zero users after RD-12 and RD-13. Do not deprecate it,
|
||||
do not keep a re-export.
|
||||
|
||||
2. **Move `SaveState` verbatim into `debounced-save.ts`**, keeping its doc comment. That file
|
||||
already owns the debounced-autosave concern and `createDebouncedSave` is the only thing
|
||||
that drives the state, so the type belongs beside it. Keep the four cases exactly as they
|
||||
are — `Idle | Saving | Saved | Error`.
|
||||
|
||||
3. **Delete `libs/shared/src/application/action-state.ts`.** Nothing else lives in it.
|
||||
|
||||
4. **Update the two store imports** to `@shared/application/debounced-save`. Both stores
|
||||
already import from that module for `createDebouncedSave`, so this should merge into an
|
||||
existing import line rather than adding one.
|
||||
|
||||
5. **Reword `debounced-save.ts:16`**, which currently reads "it touches that store's
|
||||
`SaveState`/`ActionState` + adapter". Drop the `ActionState` half — the type will not
|
||||
exist.
|
||||
|
||||
6. **Change no UI file and no page.** `saveState`'s public signature on both stores stays
|
||||
identical, so the three consumer sites need no edit.
|
||||
|
||||
## Files
|
||||
|
||||
- `libs/shared/src/application/debounced-save.ts` — gains `SaveState`, comment reworded
|
||||
- `libs/shared/src/application/action-state.ts` — **deleted**
|
||||
- `apps/ssp/src/app/brief/application/brief.store.ts` — import only
|
||||
- `apps/ssp/src/app/brief/application/org-template.store.ts` — import only
|
||||
|
||||
No spec files. No UI files. No machine files.
|
||||
|
||||
## Steps
|
||||
|
||||
1. Move the `SaveState` declaration and its doc comment into `debounced-save.ts`.
|
||||
2. Reword the `ActionState` mention at line 16 (decision 5).
|
||||
3. Re-point both store imports (decision 4).
|
||||
4. `git rm libs/shared/src/application/action-state.ts`.
|
||||
5. Update this ticket's `Status:` to `done` and the README's RD-14 row to `done`.
|
||||
6. Commit all of it together.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
Measured baselines, dry-run before handover.
|
||||
|
||||
```bash
|
||||
# The file is gone, and nothing imports it.
|
||||
ls libs/shared/src/application/action-state.ts # MUST be "No such file"
|
||||
git grep -l "application/action-state" -- apps libs # was 2 files -> MUST return nothing
|
||||
|
||||
# ActionState is gone entirely, word-anchored (a name containing it would defeat a bare grep).
|
||||
git grep -nw "ActionState" -- apps libs # MUST return nothing
|
||||
|
||||
# SaveState survives, in its new home, with all four cases. Anchor on the DECLARATION:
|
||||
# a bare `-w SaveState` grep already returns 1 today, from the line-16 comment.
|
||||
D=libs/shared/src/application/debounced-save.ts
|
||||
git grep -c "export type SaveState" -- $D # was 0 -> MUST be 1
|
||||
git grep -c "'Idle'\|'Saving'\|'Saved'\|'Error'" -- $D # MUST be >= 4
|
||||
|
||||
# The render seam did not move: the three consumer sites are untouched.
|
||||
git diff --name-only c599fee | grep -c "brief/ui/" || true # MUST be 0
|
||||
git grep -c "readonly saveState" -- \
|
||||
apps/ssp/src/app/brief/application/brief.store.ts \
|
||||
apps/ssp/src/app/brief/application/org-template.store.ts # still 1 each
|
||||
```
|
||||
|
||||
```bash
|
||||
npm run ci # exits 0
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
`npm run ci`. No story, no `.mdx`, no `libs/shared/src/ui/**`, so `--full` is not required.
|
||||
|
||||
`dep:check` matters here: `debounced-save.ts` is in `libs/shared/src/application`, the same
|
||||
layer `action-state.ts` was in, so no boundary changes. If `dep:check` fails, the type landed
|
||||
in the wrong layer.
|
||||
|
||||
If `dotnet test` fails with `SQLite Error 1: 'no such table: …'`, that is the stale
|
||||
`bigregister.db` artifact in this README's Troubleshooting section, unrelated to your change.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Anything under `brief/ui/` (decision 6).
|
||||
- The machines. RD-12 and RD-13 already moved the action lifecycles.
|
||||
- `NO_SUBORGS` becoming `Empty` — optional RD-34.
|
||||
- `UploadStatus`'s `type:` discriminant — optional RD-35.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Do not delete `SaveState` along with the file.** It has two four-way consumers. The
|
||||
original plan said to delete both types; reading the consumers corrected that, and this
|
||||
ticket is the corrected version.
|
||||
- **Merge into the existing `debounced-save` import** in both stores rather than adding a
|
||||
second import line from the same module — lint will not complain, but it reads badly.
|
||||
- **This is the last ticket that touches `action-state.ts`.** After it, the phase's claim
|
||||
holds: two encodings survive, `RemoteData` for fetched data and each machine's own state
|
||||
union, plus `SaveState` as an explicitly-justified third for a different concern.
|
||||
@@ -108,7 +108,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di
|
||||
| RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | done |
|
||||
| RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | done |
|
||||
| RD-13 | Same for org-template, folding `pendingPublish` in | 12 | | done |
|
||||
| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | todo |
|
||||
| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | done |
|
||||
| RD-15 | Remove 22 abandoned agent worktrees (4.7 GB) | 01 | | todo |
|
||||
| RD-16 | `parseDashboardView` returns `BigProfile`; delete `DashboardView` | 01 | | todo |
|
||||
| RD-17 | `successOf`/`successOr` sweep — 10 sites, 8 files | 01 | | todo |
|
||||
|
||||
@@ -1,9 +0,0 @@
|
||||
/** 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' };
|
||||
@@ -1,3 +1,8 @@
|
||||
/** Debounced-autosave indicator, shown in a small status line near a toolbar — a separate
|
||||
concern from a store's one-shot action lifecycle (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' };
|
||||
|
||||
export interface DebouncedSave {
|
||||
/** (Re)arm the debounce timer; no-op when `canSave()` is false. */
|
||||
schedule(): void;
|
||||
@@ -13,7 +18,7 @@ export interface DebouncedSave {
|
||||
/**
|
||||
* 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
|
||||
* (store-specific — it touches that store's SaveState + 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.
|
||||
|
||||
Reference in New Issue
Block a user