docs(dn): plan for deletion flags a person can act on
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DEls
parent
c4f2fd7f56
commit
0113d1a1fa
1 changed files with 197 additions and 0 deletions
197
docs/superpowers/plans/2026-10-09-dn-deletion-flags.md
Normal file
197
docs/superpowers/plans/2026-10-09-dn-deletion-flags.md
Normal file
|
|
@ -0,0 +1,197 @@
|
|||
# Deletion flags a person can act on — Implementation Plan
|
||||
|
||||
> **For agentic workers:** implement task by task, test first. Steps use checkbox (`- [ ]`) syntax.
|
||||
|
||||
**Goal:** Agent Foreman's deletion flags stop being a footnote at the bottom of the inbox. They become amber cards at the top. Each card opens a page that says in plain words what the record is, why it was flagged, and what it affects, with **Delete** and **Keep** buttons for the right person. Nobody needs PocketBase admin.
|
||||
|
||||
**Architecture:** `src/lib/purchasing/deletion-decisions.ts` holds all the rules: describe, block, permission, resolve. Callers:
|
||||
- a POST route performs a decision;
|
||||
- a server page shows a flag;
|
||||
- the inbox shows the strip.
|
||||
|
||||
Flags already live as `mcp_operations` rows (`tool:"request_deletion"`, `status:"committed"`, `result:{collection,recordId,reason,summary}`). See `src/lib/purchasing/deletion-requests.ts`. A decision journals a `deletion_decision` row with a full copy of the record, marks the flag rows `status:"resolved"`, and only then deletes (for Delete).
|
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-10-09-dn-explain-or-ask-design.md` §6 ("Flagged for deletion" decision) and §7. Approved by David on 2026-10-09. The requester is always shown as **"Agent Foreman"**.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- This is the **only** place app code may call `.delete(`, and only inside `resolveFlag` after the journal row is written. Agents (MCP tools) can't call it: there is no MCP tool for decisions.
|
||||
- Evidence is never deleted, from here or anywhere else: `delivery_notes`, `delivery_note_items`, `dn_batches`, `dn_batch_notes`, `dn_batch_pages`.
|
||||
- No PocketBase schema changes.
|
||||
- Every decision needs a reason (min 3 characters) and is recorded with who and when.
|
||||
- Verify each task: `npx vitest run src/lib/purchasing src/app/dashboard/financial/delivery-notes src/app/api`, `./node_modules/.bin/tsc --noEmit --project tsconfig.json`, `npx eslint <changed files>`. Check that each new test fails without its change; don't use `git stash` (shared).
|
||||
|
||||
---
|
||||
|
||||
### Task 1: The rules (`deletion-decisions.ts`)
|
||||
|
||||
**Files:** create `src/lib/purchasing/deletion-decisions.ts` and `src/lib/purchasing/deletion-decisions.test.ts`. Modify `src/lib/purchasing/deletion-requests.ts` (the requester label, and the open flag rows for a record).
|
||||
|
||||
**Interfaces (produced):**
|
||||
|
||||
```ts
|
||||
import type PocketBase from 'pocketbase';
|
||||
import type {AppUser} from '@/lib/auth/access';
|
||||
export const EVIDENCE=['delivery_notes','delivery_note_items','dn_batches','dn_batch_notes','dn_batch_pages'] as const;
|
||||
export type FlagLink={label:string;href:string};
|
||||
export type FlagDetail={collection:string;recordId:string;kind:string;headline:string;fields:Array<{label:string;value:string}>;affects:FlagLink[];reasons:string[];requestedBy:'Agent Foreman'};
|
||||
export async function describeFlag(pb:PocketBase,collection:string,recordId:string):Promise<FlagDetail|null>; // null when there is no open flag or the record is gone
|
||||
export async function deletionBlock(pb:PocketBase,collection:string,record:Record<string,unknown>):Promise<string|null>; // why Delete is not allowed, or null
|
||||
export async function canDecide(pb:PocketBase,user:AppUser,collection:string,record:Record<string,unknown>):Promise<boolean>;
|
||||
export async function resolveFlag(pb:PocketBase,user:AppUser,input:{collection:string;recordId:string;action:'delete'|'keep';reason:string}):Promise<{action:'delete'|'keep';journalId:string}>;
|
||||
export class FlagDecisionError extends Error{constructor(readonly code:string,readonly status:number,message:string){super(message);}}
|
||||
```
|
||||
|
||||
**Rules:**
|
||||
|
||||
`kind` and `headline`, in plain words:
|
||||
|
||||
| Collection | kind | headline |
|
||||
|---|---|---|
|
||||
| `supplier_document_aliases` | `Supplier alias` | `"<raw_value>" on documents is read as <supplier name>` |
|
||||
| `suppliers` | `Supplier` | `<name>` |
|
||||
| `purchase_orders` | `Purchase order` | `<purchase_order_number> · <supplier>` |
|
||||
| `purchase_order_items` | `PO line` | `<description> on <PO number>` |
|
||||
| `delivery_notes` | `Booked receipt` | `<delivery_note_number>` |
|
||||
| `dn_batch_notes` | `Delivery document` | `<doc_number>` |
|
||||
| `dn_batches` | `Upload` | `<filename or id>` |
|
||||
| `dn_batch_pages` | `Photo page` | `page <global_page>` |
|
||||
| `delivery_note_items` | `Receipt line` | `<qty_received> on <PO line description>` |
|
||||
|
||||
`fields`: 3–6 label/value pairs a person recognises (supplier, site, PO number, numbers, dates). Never the raw JSON.
|
||||
|
||||
`affects`: links (max 10).
|
||||
- **alias:** the supplier, plus delivery documents whose `supplier_name_raw` normalises (`normaliseSupplierKey` from `@/lib/dn-pipeline/resolve-entities`) to the alias's `normalised_key` → `/dashboard/financial/delivery-notes/document/<id>` labelled `<doc_number> (<site name>)`.
|
||||
- **supplier:** counts as text links to `/dashboard/financial/purchase-orders?supplier=<id>` (label `N purchase orders`), plus the documents.
|
||||
- **PO:** its page `/dashboard/financial/purchase-orders/<id>` (check this route exists: `ls src/app/dashboard/financial/purchase-orders`; use whatever detail route exists).
|
||||
- **evidence:** its document page or batch page (`/dashboard/financial/delivery-notes/inbox/<batchId>`).
|
||||
|
||||
`reasons`: every flag reason for that record. `requestedBy`: always `'Agent Foreman'`.
|
||||
|
||||
`deletionBlock` (Delete is allowed only when this returns null):
|
||||
- `supplier_document_aliases` → `null`.
|
||||
- `suppliers` → count `purchase_orders` (`supplier_ref`), `dn_batch_notes` (`supplier`), `supplier_document_aliases` (`supplier`). Any > 0 → `Still used by <a> purchase orders, <b> delivery documents and <c> aliases. Merge it into the right supplier instead.`
|
||||
- `purchase_order_items` → any `delivery_note_items` with this `purchase_order_item` → `<n> receipts were booked against this PO line.`
|
||||
- `purchase_orders` → any `delivery_notes` (`purchase_order`), `dn_batch_notes` (`purchase_order`) or `purchase_order_items` → `This PO has <n> deliveries, <m> linked documents and <k> lines.`
|
||||
- `EVIDENCE` → `This is delivery evidence and is never deleted. Keep it, or set it aside or move the receipt from its document page.`
|
||||
|
||||
`canDecide`:
|
||||
- `user.role==='admin'` → true;
|
||||
- `clock_only` → false;
|
||||
- otherwise the record's site decides:
|
||||
- site-bound records via `canAccessSite(user, site)`, where site comes from: `record.site` (purchase_orders, delivery_notes, dn_batches), `dn_batches.site` via `record.batch` (dn_batch_notes, dn_batch_pages), the PO's site (purchase_order_items), the delivery note's site (delivery_note_items);
|
||||
- `suppliers` / `supplier_document_aliases` (supplier = `record.id` / `record.supplier`) → true when the user can access the site of at least one `purchase_orders` row with that `supplier_ref`.
|
||||
|
||||
`resolveFlag`:
|
||||
1. Load the record (missing → `FlagDecisionError('record_not_found',404,'This record no longer exists.')`).
|
||||
2. Find the open flag rows: `mcp_operations` with `tool="request_deletion" && status="committed"` and `result.collection/recordId` matching. Filter in JS too, as `listDeletionRequests` does. None → `FlagDecisionError('no_open_flag',409,'This record is not flagged for deletion.')`.
|
||||
3. `canDecide` false → `FlagDecisionError('not_allowed',403,'You can only decide flags for your own sites.')`.
|
||||
4. Reason shorter than 3 characters after trim → `FlagDecisionError('reason_required',400,'Give a short reason.')`.
|
||||
5. `delete` and `deletionBlock` returns a message → `FlagDecisionError('delete_blocked',409,<message>)`.
|
||||
6. Create the journal row in `mcp_operations`:
|
||||
|
||||
```ts
|
||||
{id: operationRecordId(user.id,'deletion_decision',crypto.randomUUID()),
|
||||
credential:user.id, tool:'deletion_decision', operation_id:crypto.randomUUID(),
|
||||
payload_hash:operationPayloadHash('deletion_decision',{collection,recordId,action,reason}),
|
||||
status:'committed',
|
||||
result:{collection,recordId,action,reason,decidedBy:user.id,decidedByName:user.name??user.email??user.id,at:new Date().toISOString(),snapshot:record}}
|
||||
```
|
||||
|
||||
`operationRecordId` and `operationPayloadHash` come from `./operations`. Check the `AppUser` fields for a display name and use what exists.
|
||||
7. For `delete`: `await pb.collection(collection).delete(recordId)`. If it throws, update the journal row `status:'failed'` with the error message and rethrow `FlagDecisionError('delete_failed',409,…)`.
|
||||
8. Update every open flag row: `status:'resolved'`, `result:{...result,resolution:{action,reason,by:user.id,at}}`.
|
||||
9. Return `{action,journalId}`.
|
||||
|
||||
`deletion-requests.ts`: set `requestedBy` to `'Agent Foreman'` for every flag. Keep `credential` available as `requestedByKey` if you need it. `listDeletionRequests` already only lists `status:"committed"` rows, so resolved flags drop off by themselves. Confirm this with a test.
|
||||
|
||||
- [ ] **Step 1: Tests** (`memoryDatabase()` from `./test-support`; users are plain objects shaped like `AppUser`: `{id, role, siteIds, ...}`; read the type in `src/lib/auth/access.ts`):
|
||||
1. Admin deletes a flagged alias:
|
||||
- the alias is gone;
|
||||
- one `deletion_decision` row exists, its `result.snapshot.raw_value` is the alias text, and its `action` is `delete`;
|
||||
- the flag row is `status:'resolved'`;
|
||||
- `listDeletionRequests` is empty.
|
||||
2. Admin keeps it: the alias is still there, the flag is resolved, and the list is empty.
|
||||
3. A supplier still used by a PO → `delete_blocked` (message mentions `1 purchase orders`). Keep is still allowed.
|
||||
4. Evidence (a flagged `dn_batch_notes` row) → `delete_blocked` with the evidence message. Keep is allowed.
|
||||
5. A Site manager whose `siteIds` include the site of a PO for the alias's supplier → allowed. A Site manager of another site → `not_allowed`. A Clock user → `not_allowed`.
|
||||
6. No reason → `reason_required`. Not flagged → `no_open_flag`.
|
||||
7. `describeFlag` for the alias: `kind 'Supplier alias'`, a headline containing the raw value and the supplier name, `requestedBy 'Agent Foreman'`, and `affects` including the delivery document whose `supplier_name_raw` matches.
|
||||
- [ ] **Step 2:** run and see them fail. **Step 3:** implement. **Step 4:** verify. **Step 5:** commit `feat(dn): rules for deciding deletion flags (delete with a saved copy, keep, never evidence)`.
|
||||
|
||||
---
|
||||
|
||||
### Task 2: The decision route
|
||||
|
||||
**Files:** create `src/app/api/deletion-flags/[collection]/[recordId]/route.ts` and `route.test.ts`.
|
||||
|
||||
`POST` body `{action:'delete'|'keep', reason:string}`:
|
||||
- `requireApiUser('financial')` (return the Response if it is one);
|
||||
- validate `recordId` against `/^[a-z0-9]{15}$/i`, the collection against the allowed list in `deletion-requests.ts` (`RequestDeletionSchema` enum), and `action` (→ 400 `invalid_request`);
|
||||
- `resolveFlag(await pbAdmin(), user, {...})` → `200 {ok:true,action}`;
|
||||
- `FlagDecisionError` → `{error:code,detail:message}` with its status;
|
||||
- anything else → `409 {error:'decision_not_saved',detail:message}`.
|
||||
|
||||
Follow `src/app/api/dn-batches/[id]/notes/[noteId]/site-decision/route.ts` for style and the test mocks.
|
||||
|
||||
Tests: 401 passes through; 400 for a bad action; maps `delete_blocked` to 409 with detail; success returns `{ok:true,action:'keep'}` (mock `resolveFlag`).
|
||||
|
||||
Commit: `feat(dn): route to delete or keep a flagged record`.
|
||||
|
||||
---
|
||||
|
||||
### Task 3: The flag page
|
||||
|
||||
**Files:**
|
||||
- create `src/app/dashboard/financial/delivery-notes/flags/[collection]/[recordId]/page.tsx` (server);
|
||||
- create `flag-decision.tsx` (client), with `flag-decision.test.tsx`.
|
||||
|
||||
Page:
|
||||
- `requirePageUser('financial')`;
|
||||
- `describeFlag`; if null, show "This flag has already been decided" with a link back to the inbox;
|
||||
- load the record, `deletionBlock` and `canDecide`;
|
||||
- render, in this order:
|
||||
1. **Header:** `<kind>` badge + `<headline>`;
|
||||
2. **"Why it was flagged":** each reason, then `— Agent Foreman`;
|
||||
3. **"What it is":** the `fields` as a definition list;
|
||||
4. **"What it affects":** the `affects` links;
|
||||
5. `<FlagDecision>`.
|
||||
|
||||
Use the `Card` and `Badge` components and the amber colour (`border-2 border-amber-500`) like the review cards.
|
||||
|
||||
`FlagDecision({collection,recordId,canDecide,deleteBlock}:{collection:string;recordId:string;canDecide:boolean;deleteBlock:string|null})`:
|
||||
- a reason `<textarea>` (required);
|
||||
- **Keep** (outline) and **Delete** (destructive) buttons:
|
||||
- Delete is disabled when `deleteBlock` is set, with `deleteBlock` shown under it in amber;
|
||||
- when allowed, Delete asks `window.confirm('Delete this record? A copy is saved so it can be restored.')`;
|
||||
- both POST to the route, then `router.push('/dashboard/financial/delivery-notes/inbox')`; errors show in red;
|
||||
- when `!canDecide`, no buttons, just the text "Only an admin or a site manager of the sites involved can decide this flag."
|
||||
|
||||
Tests (static markup): renders both buttons when allowed; Delete is disabled with the block text when blocked; no buttons when not allowed.
|
||||
|
||||
Commit: `feat(dn): a page to see and decide a deletion flag`.
|
||||
|
||||
---
|
||||
|
||||
### Task 4: The inbox strip
|
||||
|
||||
**Files:** modify `src/app/dashboard/financial/delivery-notes/inbox/page.tsx`. Create `src/app/dashboard/financial/delivery-notes/inbox/attention-strip.tsx` and `attention-strip.test.tsx`.
|
||||
|
||||
- Remove the "Flagged for deletion" section at the bottom.
|
||||
- Above the board, when there are flags, render `<AttentionStrip flags={…}/>`: heading **"Needs attention"**, then one amber card per flag (`border-2 border-amber-500 rounded-xl p-3`), as a `Link` to `/dashboard/financial/delivery-notes/flags/<collection>/<recordId>`. Each card shows:
|
||||
- **"Flagged for deletion"** · `<plain kind>`;
|
||||
- `<summary>`;
|
||||
- the first reason, cut to 140 characters;
|
||||
- **"Agent Foreman"**;
|
||||
- **"Open →"**.
|
||||
- Map collection to plain kind with the same table as Task 1. Export a small `flagKind(collection)` from `deletion-decisions.ts`, or a shared const, and reuse it.
|
||||
- A non-admin only sees flags where `canDecide` is true (call it per flag; a handful of flags at most). Admins see all.
|
||||
|
||||
Tests: renders a card with the plain kind, summary, "Agent Foreman" and the link; renders nothing for an empty list.
|
||||
|
||||
Commit: `feat(dn): deletion flags open as Needs attention cards at the top of the inbox`.
|
||||
|
||||
---
|
||||
|
||||
## Done when
|
||||
Four commits, all checks green, each new test fails without its change. The Sabrix alias flag shows at the top of the inbox and opens a page with Delete and Keep.
|
||||
Loading…
Reference in a new issue