docs(buyer-tasks): implementation plan for mail-failed PO recovery
Six tasks: service field, service filter, card visual, page wiring, route smoke test, full verification. All TDD with frequent commits. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
SJCoetz
parent
fc17b46f9a
commit
83bfd294b7
1 changed files with 539 additions and 0 deletions
|
|
@ -0,0 +1,539 @@
|
|||
# Buyer Tasks: Mail-Failed PO Recovery Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Keep `mail_failed` purchase orders visible in the buyer-tasks **PO Draft** tab with a clear failure reason and a one-click **Resend** action that re-opens the shared `SendPoModal`.
|
||||
|
||||
**Architecture:** Single-line service filter change + one new field on the buyer-task group + a conditional red banner / button-label change on the card. The existing send-email pipeline already supports the `mail_failed` status (it's in `SENDABLE_STATUSES`), so no server-side send logic changes. One wiring fix: the buyer-tasks page's `SendPoModal` stub must also pass `status` and `email_error` so the modal renders the "Resend" label and the inline failure banner.
|
||||
|
||||
**Tech Stack:** Next.js (App Router), TypeScript, Vitest, React Testing Library, Tailwind, PocketBase (admin SDK), n8n (send pipeline, unchanged).
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
**Modify:**
|
||||
- `src/lib/server/buyer-tasks-service.ts` — add `emailError` to `BuyerTaskGroup`, include `mail_failed` in `po_draft` filter, fetch `email_error` from the expanded PO.
|
||||
- `src/components/buyer-tasks/buyer-task-group-card.tsx` — render mail-failed banner + Resend button + destructive class when `poStatus === "mail_failed"`.
|
||||
- `src/components/buyer-tasks/buyer-tasks-page.tsx` — pass `status` and `email_error` into the `SendPoModal` stub.
|
||||
- `src/lib/server/buyer-tasks-service.test.ts` — new tests for `mail_failed` in `po_draft` and `emailError` field.
|
||||
- `src/components/buyer-tasks/buyer-task-group-card.test.tsx` — new tests for the mail-failed visual + button label.
|
||||
- `src/app/api/v1/buyer-tasks/route.test.ts` — add a `mail_failed` smoke test for the route.
|
||||
|
||||
**No new files, no new collections, no schema changes.**
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Service — add `emailError` field to the group
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/lib/server/buyer-tasks-service.ts:9-31`
|
||||
- Test: `src/lib/server/buyer-tasks-service.test.ts`
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Add to `src/lib/server/buyer-tasks-service.test.ts` inside `describe("listBuyerTasks", ...)`:
|
||||
|
||||
```ts
|
||||
it("populates emailError from the related PO when the PO has an email_error", async () => {
|
||||
const { admin } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: {
|
||||
id: "po-1",
|
||||
status: "draft",
|
||||
po_number: "PO-26080301",
|
||||
email_error: "[webhook] Webhook returned HTTP 502",
|
||||
expand: {
|
||||
supplier: { id: "sup-1", email: "orders@steelco.example" },
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_draft");
|
||||
|
||||
expect(groups).toHaveLength(1);
|
||||
expect(groups[0].emailError).toBe("[webhook] Webhook returned HTTP 502");
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails**
|
||||
|
||||
Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "populates emailError"`
|
||||
Expected: FAIL with `TypeError: Cannot read properties of undefined (reading 'toBe')` (or similar — `groups[0].emailError` is `undefined` and the test will fail on the toBe assertion).
|
||||
|
||||
- [ ] **Step 3: Add the field to the type**
|
||||
|
||||
In `src/lib/server/buyer-tasks-service.ts`, add to `BuyerTaskGroup` (after `supplierEmail: string | null;` on line 29):
|
||||
|
||||
```ts
|
||||
/** Last send-pipeline error. Null on a clean draft or after a successful send.
|
||||
* Pulled from `purchase_orders.email_error` and truncated to 200 chars at
|
||||
* the service boundary so a runaway SMTP error doesn't blow up the card
|
||||
* layout. The full reason remains visible in the order page's send-modal
|
||||
* banner. */
|
||||
emailError: string | null;
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Populate the field in the group builder**
|
||||
|
||||
In `src/lib/server/buyer-tasks-service.ts`, inside the `group = { ... }` block (after `supplierEmail:` on line 105), add:
|
||||
|
||||
```ts
|
||||
// Truncate the SMTP error to 200 chars at the service boundary so
|
||||
// a runaway error string can't blow up the card layout. The full
|
||||
// reason is still visible in the order page's send-modal banner.
|
||||
emailError:
|
||||
(po as { email_error?: string } | undefined)?.email_error
|
||||
? String((po as { email_error?: string }).email_error).slice(0, 200)
|
||||
: null,
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run the test to verify it passes**
|
||||
|
||||
Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "populates emailError"`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/lib/server/buyer-tasks-service.ts src/lib/server/buyer-tasks-service.test.ts
|
||||
git commit -m "feat(buyer-tasks): surface email_error on buyer task groups
|
||||
|
||||
Adds an emailError field on BuyerTaskGroup so the UI can render
|
||||
the send-pipeline failure reason. The value is truncated to 200
|
||||
chars at the service boundary to keep the card layout bounded.
|
||||
"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Service — include `mail_failed` in the `po_draft` filter
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/lib/server/buyer-tasks-service.ts:78`
|
||||
- Test: `src/lib/server/buyer-tasks-service.test.ts`
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `src/lib/server/buyer-tasks-service.test.ts` inside `describe("listBuyerTasks", ...)`:
|
||||
|
||||
```ts
|
||||
it("includes mail_failed POs in the po_draft tab so the operator can retry", async () => {
|
||||
const { admin } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: {
|
||||
id: "po-1",
|
||||
status: "mail_failed",
|
||||
email_error: "[webhook] Webhook returned HTTP 502",
|
||||
},
|
||||
},
|
||||
}),
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_draft");
|
||||
|
||||
expect(groups).toHaveLength(1);
|
||||
expect(groups[0].purchaseOrderId).toBe("po-1");
|
||||
expect(groups[0].poStatus).toBe("mail_failed");
|
||||
expect(groups[0].emailError).toBe("[webhook] Webhook returned HTTP 502");
|
||||
});
|
||||
|
||||
it("excludes mail_failed POs from the po_sent tab", async () => {
|
||||
const { admin } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: {
|
||||
id: "po-1",
|
||||
status: "mail_failed",
|
||||
email_error: "[webhook] Webhook returned HTTP 502",
|
||||
},
|
||||
},
|
||||
}),
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_sent");
|
||||
|
||||
expect(groups).toHaveLength(0);
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail**
|
||||
|
||||
Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "mail_failed"`
|
||||
Expected: Both FAIL (the first because the filter excludes `mail_failed`; the second because the filter *includes* `mail_failed` in the `po_sent` item-status broad set on line 56 — wait, no: `po_sent` filter checks `po?.status` is in `["sent", "sending"]`, so `mail_failed` is currently excluded and the second test will already pass; the first is the one that needs the code change).
|
||||
|
||||
Adjust the first test run to verify only the new failure: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "includes mail_failed POs in the po_draft tab"`. Expect FAIL.
|
||||
|
||||
- [ ] **Step 3: Loosen the `po_draft` filter**
|
||||
|
||||
In `src/lib/server/buyer-tasks-service.ts`, change line 78 from:
|
||||
|
||||
```ts
|
||||
if (tab === "po_draft" && po?.status !== "draft") continue;
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```ts
|
||||
if (tab === "po_draft" && po?.status !== "draft" && po?.status !== "mail_failed") continue;
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests to verify they pass**
|
||||
|
||||
Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts -t "mail_failed"`
|
||||
Expected: Both PASS
|
||||
|
||||
- [ ] **Step 5: Run the full service test file to confirm no regressions**
|
||||
|
||||
Run: `npx vitest run src/lib/server/buyer-tasks-service.test.ts`
|
||||
Expected: All tests PASS (the existing "excludes sent/cancelled POs from po_draft tab" test should still pass because it uses `status: "sent"`, which is not in the new allowed set).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/lib/server/buyer-tasks-service.ts src/lib/server/buyer-tasks-service.test.ts
|
||||
git commit -m "fix(buyer-tasks): keep mail_failed POs visible in the PO Draft tab
|
||||
|
||||
The po_draft filter previously required po.status === 'draft', so
|
||||
a failed send (status: 'mail_failed') caused the group to vanish
|
||||
from buyer-tasks entirely. The only recovery path was the per-PO
|
||||
Resend button on the order detail page, which is friction. Now
|
||||
mail_failed groups stay in place so the operator can re-open the
|
||||
SendPoModal and retry from the same list. po_sent is unchanged
|
||||
— mail_failed must not appear there.
|
||||
"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Card — render mail-failed banner and Resend button
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/components/buyer-tasks/buyer-task-group-card.tsx`
|
||||
- Test: `src/components/buyer-tasks/buyer-task-group-card.test.tsx`
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `src/components/buyer-tasks/buyer-task-group-card.test.tsx`:
|
||||
|
||||
```tsx
|
||||
it("renders the email_error banner and 'Resend' button when poStatus is mail_failed", () => {
|
||||
render(
|
||||
<BuyerTaskGroupCard
|
||||
group={{
|
||||
...baseGroup,
|
||||
step: "po_draft",
|
||||
poStatus: "mail_failed",
|
||||
poNumber: "PO-26080301",
|
||||
emailError: "[webhook] Webhook returned HTTP 502",
|
||||
}}
|
||||
onAction={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText(/mail failed/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/webhook returned http 502/i)).toBeInTheDocument();
|
||||
// Button label changes to "Resend" when the underlying PO is mail_failed.
|
||||
const action = screen.getByTestId(`buyer-task-action-${baseGroup.groupId}-po_draft`);
|
||||
expect(action).toHaveTextContent(/resend/i);
|
||||
});
|
||||
|
||||
it("keeps the original 'Send PO' label and amber class when poStatus is draft", () => {
|
||||
render(
|
||||
<BuyerTaskGroupCard
|
||||
group={{
|
||||
...baseGroup,
|
||||
step: "po_draft",
|
||||
poStatus: "draft",
|
||||
poNumber: "PO-26080301",
|
||||
}}
|
||||
onAction={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
const action = screen.getByTestId(`buyer-task-action-${baseGroup.groupId}-po_draft`);
|
||||
expect(action).toHaveTextContent(/send po/i);
|
||||
// Amber class is the pre-existing po_draft class.
|
||||
expect(action.className).toMatch(/amber/i);
|
||||
// No failure banner on a clean draft.
|
||||
expect(screen.queryByText(/mail failed/i)).toBeNull();
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail**
|
||||
|
||||
Run: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx -t "mail_failed"`
|
||||
Expected: Both FAIL (the first because the banner and label are not yet wired; the second because the test will pass for the Send label — actually the second will already pass, since the current card always renders "Send PO" for `po_draft`. The first is the failure-driving test.)
|
||||
|
||||
Adjust the first test run to verify only the new failure: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx -t "renders the email_error banner"`. Expect FAIL.
|
||||
|
||||
- [ ] **Step 3: Add the conditional banner and button label to the card**
|
||||
|
||||
In `src/components/buyer-tasks/buyer-task-group-card.tsx`:
|
||||
|
||||
(a) At the top of the file, add an import for the warning icon. Replace the `FileText` import on line 3:
|
||||
|
||||
```tsx
|
||||
import { AlertTriangle, FileText } from "lucide-react";
|
||||
```
|
||||
|
||||
(b) Add a `data-po-status` attribute on the action button so the test can target it without depending on text. In the existing `Button` element (lines 97-105), add `data-po-status={group.poStatus ?? "none"}`:
|
||||
|
||||
```tsx
|
||||
<Button
|
||||
type="button"
|
||||
onClick={() => onAction(group)}
|
||||
disabled={disabled}
|
||||
className={cn(
|
||||
ACTION_CLASS[group.step],
|
||||
group.poStatus === "mail_failed" &&
|
||||
"bg-destructive hover:bg-destructive/90 text-destructive-foreground",
|
||||
)}
|
||||
data-testid={`buyer-task-action-${group.groupId}-${group.step}`}
|
||||
data-po-status={group.poStatus ?? "none"}
|
||||
>
|
||||
{group.poStatus === "mail_failed" ? "Resend" : ACTION_LABEL[group.step]}
|
||||
</Button>
|
||||
```
|
||||
|
||||
(c) Add the `cn` import alongside the existing imports. Update the top import block (line 1-2 area) to include `cn` if it isn't already imported:
|
||||
|
||||
```tsx
|
||||
import { cn } from "@/lib/utils";
|
||||
```
|
||||
|
||||
(d) Render the failure banner above the items list, only when `poStatus === "mail_failed"`. Inside the `<CardContent>` after the existing `<div className="flex items-start gap-3 p-3">` block (after line 106, before `<BuyerTaskItemsList>` on line 107), add:
|
||||
|
||||
```tsx
|
||||
{group.poStatus === "mail_failed" && group.emailError && (
|
||||
<div
|
||||
data-testid={`buyer-task-mail-failed-${group.groupId}`}
|
||||
role="alert"
|
||||
className="mx-3 mb-2 flex items-start gap-2 rounded-md border border-destructive/30 bg-destructive/10 px-3 py-2 text-xs text-destructive"
|
||||
>
|
||||
<AlertTriangle className="mt-0.5 h-3.5 w-3.5 shrink-0" aria-hidden="true" />
|
||||
<div className="min-w-0">
|
||||
<div className="font-semibold">Mail failed</div>
|
||||
<div className="break-words">{group.emailError}</div>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests to verify they pass**
|
||||
|
||||
Run: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx -t "mail_failed"`
|
||||
Expected: Both PASS
|
||||
|
||||
- [ ] **Step 5: Run the full card test file to confirm no regressions**
|
||||
|
||||
Run: `npx vitest run src/components/buyer-tasks/buyer-task-group-card.test.tsx`
|
||||
Expected: All tests PASS (the existing "renders the right action button per step" test should still pass — it only checks the label, not the new `data-po-status` attribute).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/components/buyer-tasks/buyer-task-group-card.tsx src/components/buyer-tasks/buyer-task-group-card.test.tsx
|
||||
git commit -m "feat(buyer-tasks): render mail-failed banner + Resend button
|
||||
|
||||
When a card's poStatus is 'mail_failed', the card now renders a
|
||||
red warning row with the email_error reason and switches the
|
||||
action button label to 'Resend' with the destructive colour. A
|
||||
new data-po-status attribute on the button lets tests target the
|
||||
state without depending on the human-readable label.
|
||||
"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Page — pass `status` and `email_error` into the `SendPoModal` stub
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/components/buyer-tasks/buyer-tasks-page.tsx:215-232`
|
||||
|
||||
- [ ] **Step 1: Inspect the current stub and confirm the change**
|
||||
|
||||
Open `src/components/buyer-tasks/buyer-tasks-page.tsx` lines 215-232. The current `purchaseOrder` stub passes only `id`, `po_number`, and `expand.supplier.email`. The `SendPoModal` reads `purchaseOrder.status` (line 426 — for the "Resend" vs "Send" label) and `purchaseOrder.email_error` (line 326 — for the failure banner). Without these, the modal currently shows "Send" instead of "Resend" and the inline banner is empty when opened from buyer-tasks.
|
||||
|
||||
- [ ] **Step 2: Update the stub**
|
||||
|
||||
In `src/components/buyer-tasks/buyer-tasks-page.tsx`, replace the existing `purchaseOrder={{...}}` prop on lines 218-226 with:
|
||||
|
||||
```tsx
|
||||
purchaseOrder={{
|
||||
id: sendPoFor.purchaseOrderId ?? "",
|
||||
po_number: sendPoFor.poNumber ?? "",
|
||||
status: sendPoFor.poStatus ?? undefined,
|
||||
email_error: sendPoFor.emailError ?? undefined,
|
||||
expand: {
|
||||
supplier: {
|
||||
email: sendPoFor.supplierEmail ?? "",
|
||||
},
|
||||
},
|
||||
} as never}
|
||||
```
|
||||
|
||||
`as never` is already there to silence the `PurchaseOrder`-shaped type check, since the modal lazy-fetches the full PO itself. The new fields ride along so the modal's banner + Resend label render correctly even before the lazy fetch completes.
|
||||
|
||||
- [ ] **Step 3: Run the full buyer-tasks page test file to confirm no regressions**
|
||||
|
||||
Run: `npx vitest run src/components/buyer-tasks/buyer-tasks-page.test.tsx`
|
||||
Expected: All tests PASS. If the test file mocks `SendPoModal` and asserts on the stub shape, update the mock to include the two new fields so the assertion still matches.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add src/components/buyer-tasks/buyer-tasks-page.tsx
|
||||
git commit -m "fix(buyer-tasks): pass status + email_error into SendPoModal stub
|
||||
|
||||
The SendPoModal renders its 'Resend' label and inline failure
|
||||
banner from purchaseOrder.status / email_error, but the buyer-tasks
|
||||
page stub didn't include either field. As a result, the modal
|
||||
showed 'Send' and an empty banner even when the underlying PO was
|
||||
mail_failed. Now the stub includes both so the operator sees the
|
||||
correct affordance.
|
||||
"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Route — smoke test for `mail_failed` group in the `po_draft` response
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/app/api/v1/buyer-tasks/route.test.ts`
|
||||
|
||||
- [ ] **Step 1: Add the smoke test**
|
||||
|
||||
Add to `src/app/api/v1/buyer-tasks/route.test.ts` inside the existing `describe("GET /api/v1/buyer-tasks", ...)` block:
|
||||
|
||||
```ts
|
||||
it("returns mail_failed groups when tab=po_draft (operator can retry from buyer-tasks)", async () => {
|
||||
vi.mocked(listBuyerTasks).mockResolvedValue([
|
||||
{
|
||||
groupId: "order-1::sup-1",
|
||||
orderId: "order-1",
|
||||
orderNumber: "ORD-001",
|
||||
supplierId: "sup-1",
|
||||
supplierName: "Steel Co",
|
||||
customerName: "J. Smith",
|
||||
companyName: "Acme",
|
||||
items: [],
|
||||
itemCount: 1,
|
||||
totalCost: 50,
|
||||
purchaseOrderId: "po-1",
|
||||
poStatus: "mail_failed",
|
||||
poNumber: "PO-26080301",
|
||||
supplierEmail: "orders@steelco.example",
|
||||
emailError: "[webhook] Webhook returned HTTP 502",
|
||||
step: "po_draft",
|
||||
},
|
||||
]);
|
||||
|
||||
const res = await GET(makeRequest("po_draft") as never);
|
||||
expect(res.status).toBe(200);
|
||||
const body = await res.json();
|
||||
expect(body.groups).toHaveLength(1);
|
||||
expect(body.groups[0].poStatus).toBe("mail_failed");
|
||||
expect(body.groups[0].emailError).toMatch(/webhook/i);
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it passes**
|
||||
|
||||
Run: `npx vitest run src/app/api/v1/buyer-tasks/route.test.ts -t "mail_failed"`
|
||||
Expected: PASS (the route is a thin pass-through; this test exists to lock the wire shape so a future refactor can't silently drop the new `emailError` field).
|
||||
|
||||
- [ ] **Step 3: Run the full route test file to confirm no regressions**
|
||||
|
||||
Run: `npx vitest run src/app/api/v1/buyer-tasks/route.test.ts`
|
||||
Expected: All tests PASS
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add src/app/api/v1/buyer-tasks/route.test.ts
|
||||
git commit -m "test(buyer-tasks): lock wire shape of mail_failed group in po_draft
|
||||
|
||||
The route is a pass-through, but the smoke test guards against a
|
||||
future refactor silently dropping the new emailError field — the
|
||||
operator's retry affordance depends on it.
|
||||
"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Full verification
|
||||
|
||||
- [ ] **Step 1: Run the full test suite**
|
||||
|
||||
Run: `npm test`
|
||||
Expected: All tests PASS. Watch for any snapshot or component test that depends on the `BuyerTaskGroup` shape — those need the new `emailError` field on their fixtures (the route test in Task 5 already includes it; check for others).
|
||||
|
||||
- [ ] **Step 2: Type-check the project**
|
||||
|
||||
Run: `npx tsc --noEmit`
|
||||
Expected: No errors. The `BuyerTaskGroup` change is in the canonical place, and the new card test + route test use the new field on their fixtures, so the compiler should be happy.
|
||||
|
||||
- [ ] **Step 3: Lint**
|
||||
|
||||
Run: `npm run lint` (or whatever the project's lint script is — check `package.json` if unsure)
|
||||
Expected: No errors. The card's new `data-po-status` attribute and the new import (`AlertTriangle`, `cn`) are the only surface changes outside the service.
|
||||
|
||||
- [ ] **Step 4: Manual smoke test in the browser**
|
||||
|
||||
1. Start the app: `npm run dev` (or whatever the project's dev script is).
|
||||
2. Navigate to `/orders/tasks`.
|
||||
3. Trigger a `mail_failed` state for a PO: easiest is to point `PO_SEND_WORKFLOW_WEBHOOK_URL` at an unreachable URL (or temporarily unset it) and click **Send PO** on a draft. The PO flips to `mail_failed` and the group should re-appear in **PO Draft** with:
|
||||
- Red banner: `⚠ Mail failed — [webhook] PO_SEND_WORKFLOW_WEBHOOK_URL is not configured. The cron pass will retry.`
|
||||
- Action button label **Resend** in destructive colour.
|
||||
4. Click **Resend**. The `SendPoModal` opens with:
|
||||
- The supplier email pre-filled.
|
||||
- "Resend PO" CTA.
|
||||
- The failure banner still visible inside the modal (so the operator can confirm the last error before retrying).
|
||||
5. Restore `PO_SEND_WORKFLOW_WEBHOOK_URL` and click the modal's **Resend PO**. The group should move to **PO Sent**.
|
||||
6. Take a screenshot for the spec, if useful.
|
||||
|
||||
- [ ] **Step 5: Commit the verification notes (no code changes)**
|
||||
|
||||
If any manual findings prompt a small follow-up (e.g. an off-by-one in the truncation, a contrast issue with the destructive colour on a dark background), file a follow-up commit. Otherwise, no commit needed for the verification step itself.
|
||||
|
||||
---
|
||||
|
||||
## Self-Review
|
||||
|
||||
**1. Spec coverage:**
|
||||
- "Filter change to include `mail_failed` in `po_draft`" → Task 2 ✓
|
||||
- "Add `emailError` field on the group" → Task 1 ✓
|
||||
- "Card visual: red banner + Resend button + destructive class when `poStatus === 'mail_failed'`" → Task 3 ✓
|
||||
- "Page wiring: pass `status` and `email_error` into `SendPoModal` stub" → Task 4 ✓
|
||||
- "Service test: `mail_failed` group in `po_draft` + `emailError` field + `po_sent` excludes `mail_failed`" → Tasks 1, 2 ✓
|
||||
- "Card test: render banner + Resend label + still renders Send PO when draft" → Task 3 ✓
|
||||
- "Route smoke test: `po_draft` response includes the new field" → Task 5 ✓
|
||||
- "Out of scope items (Failed tab, auto-retry, schema changes) not implemented" → confirmed not in plan ✓
|
||||
|
||||
**2. Placeholder scan:** No TBDs, no "fill in later", no "similar to Task N". Every code block shows the actual code.
|
||||
|
||||
**3. Type consistency:** `emailError` is added in Task 1 and used in Tasks 2-5 with the same name and shape. `poStatus` is read in Task 3 against the type union already defined in `buyer-tasks-service.ts:27` (which already includes `"mail_failed"`). `data-po-status` is only used in Task 3 and not referenced elsewhere. No drift.
|
||||
|
||||
**4. Risky bits called out:**
|
||||
- Task 2 step 2: the second test (`po_sent` excludes `mail_failed`) actually passes against the current code, since the existing `po_sent` filter on line 79 already excludes `mail_failed`. The plan acknowledges this and asks the implementer to run the failing test in isolation first.
|
||||
- Task 3 step 5: the existing "renders the right action button per step" test asserts the label, which is the only thing that could regress if the label-gate change is too broad. The plan includes a full-file re-run to catch this.
|
||||
- Task 4 step 3: if the page test file mocks `SendPoModal` and asserts on the stub shape, the new fields are required on the mock fixture. The plan flags this.
|
||||
- Task 6 step 4: the manual smoke test is the only way to confirm the modal renders correctly with the new stub fields — automated tests don't cover the modal's banner-rendering path here.
|
||||
Loading…
Reference in a new issue