feat(quotes): enhance quotes.submit functionality and error handling
- Updated the `quotes.submit` tool to allow submission of quotes with unpriced items by introducing a `partial: true` flag, transitioning the quote to `partial_approval_pending` instead of `approval_pending`. - Enhanced the response structure of `submitQuote` to include `previousStatus`, `submittedAt`, and `targetStatus`, providing better context for the agent. - Improved error handling in the `sendQuoteEmail` function to surface detailed readiness issues when a PDF cannot be generated, allowing operators to address specific gaps in quote items. - Regenerated API documentation in `docs/api-mcp-layer.md` to reflect these changes, ensuring accuracy and clarity in tool descriptions. - Added comprehensive tests to validate the new functionality and error handling improvements.
This commit is contained in:
Dave
parent
e5e31a7bdc
commit
cf87a370cf
7 changed files with 314 additions and 38 deletions
|
|
@ -519,7 +519,7 @@ Grouped by resource family. The full description is in the table column; the des
|
|||
| `quotes.save_as_template` | Snapshot an existing quote as a reusable template. The original quote is not modified; a new `is_template=true` row is created. |
|
||||
| `quotes.split` | Split a quote into multiple quotes by grouping items. Body is `{ source_quote_id, groups: [{ name, item_ids: string[] }] }`. Useful for partial wins where the customer accepts some lines but not others. |
|
||||
| `quotes.partial_approve` | Mark a subset of quote items as `approved` (or `rejected`) on a quote. Body: `{ quote_id, decisions: [{ item_id, decision }] }`. Useful when the customer accepts some lines but not others. |
|
||||
| `quotes.submit` | Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote must have `selling_price > 0`;... |
|
||||
| `quotes.submit` | Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` or `partial_approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote ... |
|
||||
| `quotes.re_price` | Re-run pricing on every quote_item under a quote. Whitelist of patchable fields is the same as `bulk_update_quote_items`. Returns the per-row result list. |
|
||||
| `quotes.next_number` | Reserve the next quote number (`Q{yy}{mm}{NNN}`). Atomic counter; gaps are possible if the caller never persists the resulting draft. |
|
||||
| `quotes.lookup` | Free-text lookup over quotes for the order page's quote-picker dropdown. Substring match across quote_number, quote_reference, rfq_number, customer name, company name. Excludes templates by default. |
|
||||
|
|
|
|||
|
|
@ -149,6 +149,13 @@ These came up but need a human answer before code changes:
|
|||
|
||||
- 2026-08-15 — **`quotes.submit` shipped.** New REST + MCP tool that flips `status: new → approval_pending` once every line is priced. Closes the walk-in pipeline gap — previously the agent had to call `quotes.update({status: "approval_pending"})` directly, which had no "all items priced" gate and bypassed the audit trail. Service: `submitQuote(id, actor?)` in `quotes-service.ts` (typed `SubmitQuoteError` with `code: "quote_not_found" | "invalid_status" | "unpriced_items"`); REST: `POST /api/v1/quotes/[id]/submit` (200 + `{ quote, statusChanged }` on success, 422 with `code` + `offendingItemIds` on the gated failure); MCP: `quotes.submit({id, actor?})` entry + dispatch. Idempotent — re-submitting an already-`approval_pending` quote is a no-op (`statusChanged: false`, no PB write). The `unpriced_items` failure surfaces the offending item ids so the agent can price the gaps and retry. Optional `actor: { id, name }` lands on `quotes.submitted_by` + `quotes.submitted_at` (both are optional fields on the Quote type; PB silently drops them until ops adds the columns to the schema — see the new “submit audit fields” pending request below). 8 new unit tests for `submitQuote` (happy path, idempotent re-submit, all 6 wrong-status types, unpriced with mixed lines + moved-to-sibling exclusion, 404, empty/whitespace id, no actor, empty actor name) + 9 new REST integration tests (200/200-idempotent/200-no-actor/200-malformed-actor/404/422-invalid_status/422-unpriced_items/500/discriminator). Recipe 7 in `docs/agents/workflows.md` updated to include `submit` between `re_price` and `send_email`. `hermes-skill.md` status enum now documents the `new → approval_pending` transition via `quotes.submit` and the `quotes.send_email` rejection (`code: "invalid_status"`) when called on a `new` quote. `docs/api-mcp-layer.md` regenerated — tool count 126 → 127.
|
||||
|
||||
- 2026-08-15 — **Phase G (continued): Gielie's 4-Fail matrix closed.** Hermes re-ran the `quotes.submit` / `quotes.send_email` test matrix and reported 4 FAILs in the response payload and pre-send validation paths. All four are now fixed in the same commit batch:
|
||||
- **Test 3 — `quotes.submit` response echo.** The 200 response was a bare `{ quote, statusChanged }` so the agent had no way to confirm what target state it landed on. `submitQuote` now returns `{ quote, statusChanged, previousStatus, submittedAt, targetStatus, submittedBy? }` on every path (including idempotent re-submit + `partial_approval_pending`). The new `targetStatus` field discriminates `approval_pending` (default) vs `partial_approval_pending` (when the `partial: true` flag is passed); `previousStatus` is always populated so the agent can build a state delta without a follow-up `quotes.get` round-trip. `submittedAt` is a fresh ISO timestamp from the server, not echoed from the request. REST route signature unchanged (extra fields are additive).
|
||||
- **Test 4 — `quotes.send_email` no-PDF failure surfaces readiness gaps.** Previously the pipeline returned the opaque `no_pdf` code on any "PDF not rendered yet" path, including the cases where the quote was missing items or had unpriced / supplierless lines. The operator wanted the agent to *tell* them what to fix rather than auto-render partial PDFs. New `diagnosePdfReadiness(pb, quoteId, quote)` helper in `src/lib/server/quotes-service.ts` runs the same three gates as `quotes/[id]/pdf/route.ts` (placeholder customer, no items, items-not-ready) BEFORE the PDF-presence check, and emits a structured `SendQuoteEmailFailure` with `details.issues: [{ itemId, description, missing[] }]`. The MCP `failure` envelope now carries the per-line gaps so the agent can present them verbatim: `Read 'Black industrial bags' — missing: cost price, selling price. Read 'Hex bolt M16' — missing: no supplier.` The `no_pdf` code is preserved as a fallback when the readiness gates all pass but no PDF is on the quote. Updated `TOOL_HINTS` for `quotes.send_email` to document the new failure codes.
|
||||
- **Test 5 — `quote_items.create` auto-derive fires for real.** The `deriveSellingPriceIfOmitted` helper was already called from `quote_items.update`, `bulk_update_quote_items`, and `quotes.re_price` (Phase F), but `createQuoteItem` was reading `input.selling_price` directly — agents who passed `cost_price` + `markup` without `selling_price` got a row with `selling_price = 0`, which then tripped the `unpriced_items` gate on `quotes.submit`. The helper is now invoked in `createQuoteItem` and the persisted value comes from `derived.selling_price`, matching the REST PATCH / MCP `quote_items.update` paths. The same `deriveSellingPriceIfOmitted` is exported (10 unit tests) so callers can dry-run the derivation client-side.
|
||||
- **Test 6 — `quotes.submit` partial-approval flag.** A "partial quote" — some lines approved, others left blank because the customer only wants a subset — used to require a render-then-partial chain because the `unpriced_items` gate was unconditional. New `partial: true` flag on `quotes.submit` flips the target status to `partial_approval_pending` AND bypasses the unpriced gate. The MCP `quotes.submit` schema + description + REST route now document the flag; the tool description summarises the new target status. Idempotent re-submit of an already-`partial_approval_pending` quote is a no-op (same `statusChanged: false` semantics as the `approval_pending` path). The full `previousStatus` / `submittedAt` / `submittedBy?` / `targetStatus` echo from Test 3 carries through to the partial path.
|
||||
- **Verification:** 83/83 `quotes-service.test.ts` + 9/9 `mcp-upstream-fetch.test.ts` + 16/16 `quote-items-service.test.ts` + 110/113 `route.test.ts` (the 3 pre-existing failures on `main` are unrelated: one stale-tools-list count, two `purchase_orders.record_diff_report` 400-vs-404 mocks). `pytest scripts/mcp-doc-gen.py --check` clean (127 tools, up to date). `docs/api-mcp-layer.md` regenerated — only the `quotes.submit` description line changed (added "or `partial_approval_pending`" to the idempotency clause). No PB schema changes; no new tools added.
|
||||
|
||||
### Customers: case-insensitive email match
|
||||
- **Surfaced:** 2026-08-15 — searching by email fails when the case doesn't match (PB `=` is case-sensitive on stored values)
|
||||
- **Workaround:** use substring `~` instead of `=` (the customers service does this in the search branch but not the exact branch)
|
||||
|
|
|
|||
|
|
@ -317,10 +317,10 @@ function isAlreadyPriced(item: {
|
|||
// actually tests today. Add entries as new write tools land.
|
||||
const TOOL_HINTS: Record<string, { hint: string; nextActions: string[] }> = {
|
||||
"quotes.submit": {
|
||||
hint: "Submission can fail with 422 (unpriced_items, invalid_status) or 404 (quote_not_found). All deterministic — do NOT retry.",
|
||||
hint: "Submission can fail with 422 (unpriced_items, invalid_status) or 404 (quote_not_found). All deterministic — do NOT retry. If the quote has unpriced items but the operator wants the partial path, re-send with `partial: true` (bypasses the unpriced gate, flips to `partial_approval_pending`).",
|
||||
nextActions: [
|
||||
"Read data.upstreamBody.code — unpriced_items lists offendingItemIds; price the gaps and resend",
|
||||
"invalid_status means the quote is not in `new` or `approval_pending`; use quotes.get to inspect",
|
||||
"Read data.upstreamBody.code — unpriced_items lists offendingItemIds; either price the gaps and resend, OR resend with `partial: true` to enter the partial-approval workflow",
|
||||
"invalid_status means the quote is not in `new`; use quotes.get to inspect the current status (already-pending returns statusChanged: false, no error)",
|
||||
"quote_not_found means the id is wrong; re-resolve via quotes.list",
|
||||
],
|
||||
},
|
||||
|
|
@ -339,11 +339,13 @@ const TOOL_HINTS: Record<string, { hint: string; nextActions: string[] }> = {
|
|||
],
|
||||
},
|
||||
"quotes.send_email": {
|
||||
hint: "Send can fail with 422 (invalid_status, no recipient, needs attachment) or 500 (PDF render).",
|
||||
hint: "Send can fail with 422 (invalid_status, no_recipient, invalid_recipient, placeholder_customer, no_items, items_not_ready, no_pdf) or 404 (quote_not_found). When the PDF is missing, the failure carries `details.issues` listing every line that's missing required fields — present those to the operator verbatim, don't auto-render.",
|
||||
nextActions: [
|
||||
"If 422 invalid_status: the quote is not in approval_pending/partial_approved — call quotes.submit first",
|
||||
"If no recipient resolved: set customer.email / alternative_email or pass `to` explicitly",
|
||||
"If 500 with 'Failed to update record.': PB schema is missing a select value; ask ops to add it",
|
||||
"If 422 invalid_status: the quote is not in a resendable state — call quotes.submit first (or use quotes.retry_email if it was mail_failed)",
|
||||
"If no_recipient / invalid_recipient: set customer.email or pass `to` explicitly",
|
||||
"If placeholder_customer: the quote is still on Unknown Sender / unnamed — pick a real customer via the process page",
|
||||
"If items_not_ready: read `failure.details.issues` — that's the per-line missing-field list; ask the operator to price + supplier the gaps, or render the PDF themselves",
|
||||
"If 500 with 'Failed to update record.': PB schema is missing a select value; ask ops to add it via the PB admin",
|
||||
],
|
||||
},
|
||||
"quotes.partial_approve": {
|
||||
|
|
@ -943,7 +945,8 @@ const tools = [
|
|||
name: "quotes.send_email",
|
||||
description:
|
||||
"Front-door for the outbound email send pipeline. Validates and flips the quote to `sending`; the actual SMTP send runs asynchronously in n8n. `selectedAttachmentIds` is a list of `quote_attachments` IDs to include (the PDF is always attached separately). `attachAuthLetters` toggles the per-letterhead supplier authorization letter.\n\n" +
|
||||
"Only `id` is required. The service walks four sources for the recipient: `to` → `customer.email` → `customer.alternative_email` → `quote.email_sender`. `cc` / `bcc` default to empty. `subject` defaults to the quote's stored `email_subject` if set, else a templated \"Quote {number} — …\" fallback. `message` defaults to the quote's stored `email_body` if set, else empty.",
|
||||
"Only `id` is required. The service walks four sources for the recipient: `to` → `customer.email` → `customer.alternative_email` → `quote.email_sender`. `cc` / `bcc` default to empty. `subject` defaults to the quote's stored `email_subject` if set, else a templated \"Quote {number} — …\" fallback. `message` defaults to the quote's stored `email_body` if set, else empty.\n\n" +
|
||||
"The server does NOT auto-render the PDF when one is missing — instead it surfaces the readiness gaps (placeholder customer, no items, items-not-ready) in `failure.details.issues` so the agent can present them to the operator verbatim. Operator fixes the gaps or pre-renders the PDF themselves, then resends.",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
required: ["id"],
|
||||
|
|
@ -1049,7 +1052,7 @@ const tools = [
|
|||
{
|
||||
name: "quotes.submit",
|
||||
description:
|
||||
"Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote must have `selling_price > 0`; unpriced lines are returned as `offendingItemIds` in the 422 so the operator can fix them. Use this after walking through `quotes.create` (walk-in) or `quotes.create_from_email` (email) to push the quote into the approval queue. Body: `{ id, actor?: { id, name } }`. The `actor` is optional and lands on `quotes.submitted_by` when the PB admin has added the column. Returns: `{ quote, statusChanged: boolean }` on success; `code: \"invalid_status\" | \"unpriced_items\" | \"quote_not_found\"` on failure.",
|
||||
"Flip a `new` quote to `approval_pending` once every line is priced. Idempotent — re-submitting an already-`approval_pending` or `partial_approval_pending` quote is a no-op (returns `statusChanged: false`). Gated: every line on the quote must have `selling_price > 0` unless `partial: true` is set; unpriced lines are returned as `offendingItemIds` in the 422 so the operator can fix them. Pass `partial: true` to allow unpriced items and flip the quote to `partial_approval_pending` instead of `approval_pending` — this is the missing state-machine transition that enables the partial-approve + split workflow (closed 2026-08-15). Use this after walking through `quotes.create` (walk-in) or `quotes.create_from_email` (email) to push the quote into the approval queue. Body: `{ id, actor?: { id, name }, partial?: boolean }`. The `actor` is optional; it lands on `quotes.submitted_by` when the PB admin has added the column. Returns: `{ quote, statusChanged, previousStatus, submittedAt, submittedBy?, targetStatus }` on success; `code: \"invalid_status\" | \"unpriced_items\" | \"quote_not_found\"` on failure. `previousStatus` is the status before the call (echoed even on the no-op path); `targetStatus` is the status the call aimed for (`approval_pending` or `partial_approval_pending`).",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
required: ["id"],
|
||||
|
|
@ -1058,12 +1061,17 @@ const tools = [
|
|||
actor: {
|
||||
type: "object",
|
||||
description:
|
||||
"Optional agent / operator identity stamp. Mirrors the `actor` shape used by `order_items.confirm_collected` and `purchase_orders.record_diff_report`.",
|
||||
"Optional agent / operator identity stamp. Mirrors the `actor` shape used by `order_items.confirm_collected` and `purchase_orders.record_diff_report`. Echoed in the response as `submittedBy` and stamped on `quotes.submitted_by` when the PB admin has added the column.",
|
||||
properties: {
|
||||
id: { type: "string" },
|
||||
name: { type: "string" },
|
||||
},
|
||||
},
|
||||
partial: {
|
||||
type: "boolean",
|
||||
description:
|
||||
"When true, the quote flips to `partial_approval_pending` instead of `approval_pending`, and the unpriced-items gate is bypassed (lines with `selling_price = 0` are allowed through). Use this to enter the partial-approve + split workflow when not every item is priced yet. Defaults to false (full-approval path, strict gate).",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
@ -4231,6 +4239,9 @@ export async function POST(req: NextRequest) {
|
|||
throw new Error("Missing id for quotes.submit.");
|
||||
}
|
||||
const hintEntry = TOOL_HINTS["quotes.submit"];
|
||||
const submitBody: Record<string, unknown> = {};
|
||||
if (args.actor) submitBody.actor = args.actor;
|
||||
if (args.partial === true) submitBody.partial = true;
|
||||
const data = await fetchUpstreamJson(
|
||||
`${upstreamOrigin(req)}/api/v1/quotes/${args.id}/submit`,
|
||||
{
|
||||
|
|
@ -4238,7 +4249,7 @@ export async function POST(req: NextRequest) {
|
|||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(args.actor ? { actor: args.actor } : {}),
|
||||
body: JSON.stringify(submitBody),
|
||||
},
|
||||
"quotes.submit",
|
||||
{ hint: hintEntry?.hint, nextActions: hintEntry?.nextActions },
|
||||
|
|
|
|||
|
|
@ -40,24 +40,34 @@ export async function POST(
|
|||
) {
|
||||
const { id } = await params;
|
||||
|
||||
// Optional actor stamp. Default to the agent identity so the
|
||||
// audit trail always has a name; fall back to a sentinel if the
|
||||
// caller sent nothing.
|
||||
// Optional actor stamp + partial flag. The `actor` lands on
|
||||
// `quotes.submitted_by` (when the PB admin adds the column); the
|
||||
// `partial` flag flips the quote to `partial_approval_pending`
|
||||
// instead of `approval_pending` and lets unpriced items through
|
||||
// (closes the state-machine gap surfaced 2026-08-15 — the previous
|
||||
// path required a render-then-partial chain).
|
||||
let actor: { id: string; name: string } | undefined;
|
||||
let partial = false;
|
||||
try {
|
||||
const body = await req.json().catch(() => null);
|
||||
if (body && typeof body === "object" && body.actor && typeof body.actor === "object") {
|
||||
const a = body.actor as Record<string, unknown>;
|
||||
if (typeof a.id === "string" && typeof a.name === "string") {
|
||||
actor = { id: a.id, name: a.name };
|
||||
if (body && typeof body === "object") {
|
||||
if (body.actor && typeof body.actor === "object") {
|
||||
const a = body.actor as Record<string, unknown>;
|
||||
if (typeof a.id === "string" && typeof a.name === "string") {
|
||||
actor = { id: a.id, name: a.name };
|
||||
}
|
||||
}
|
||||
if (body.partial === true) {
|
||||
partial = true;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Body is optional — bubble up as no actor if it's malformed.
|
||||
// Body is optional — bubble up as no actor / no partial if it's
|
||||
// malformed.
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await submitQuote(id, actor);
|
||||
const result = await submitQuote(id, actor, { partial });
|
||||
return NextResponse.json(result);
|
||||
} catch (err) {
|
||||
if (err instanceof SubmitQuoteError) {
|
||||
|
|
|
|||
|
|
@ -138,6 +138,20 @@ export async function createQuoteItem(
|
|||
quantity = q;
|
||||
}
|
||||
|
||||
// F.1: when caller supplies both cost_price and markup but omits
|
||||
// selling_price, derive it here. Without this the created row lands
|
||||
// with selling_price = 0, the unpriced gate on `quotes.submit` trips,
|
||||
// and the operator has to manually PATCH each line back to life. The
|
||||
// REST PATCH path (quote_items/[id]) and the MCP `quote_items.update`
|
||||
// path both already invoke this helper; create was the outlier
|
||||
// (caught 2026-08-15 via the live test matrix — Hermes confirmed
|
||||
// `selling_price = 0` on three separate create+update sequences).
|
||||
const derived = deriveSellingPriceIfOmitted({
|
||||
cost_price: input.cost_price,
|
||||
markup: input.markup,
|
||||
selling_price: input.selling_price,
|
||||
});
|
||||
|
||||
const pb = await getAdminPocketBase();
|
||||
|
||||
// Confirm the parent quote exists and grab its quote_number for
|
||||
|
|
@ -190,7 +204,10 @@ export async function createQuoteItem(
|
|||
description,
|
||||
quantity,
|
||||
cost_price: Number(input.cost_price) || 0,
|
||||
selling_price: Number(input.selling_price) || 0,
|
||||
// Read selling_price through the derive result so the F.1
|
||||
// auto-derive (cost × (1 + markup/100)) actually fires on
|
||||
// create. See derived computation above.
|
||||
selling_price: Number(derived.selling_price) || 0,
|
||||
markup: Number(input.markup) || 0,
|
||||
sku: typeof input.sku === "string" ? input.sku : "",
|
||||
unit: typeof input.unit === "string" ? input.unit : "",
|
||||
|
|
|
|||
|
|
@ -857,6 +857,15 @@ async function seedReadyQuote(overrides: {
|
|||
customerEmail?: string | null;
|
||||
customerName?: string;
|
||||
withPdf?: boolean;
|
||||
/**
|
||||
* Whether to seed at least one ready `quote_items` row. Defaults
|
||||
* to true because `sendQuoteEmail` now runs a pre-flight readiness
|
||||
* check (`no_items` / `items_not_ready`) before the PDF-presence
|
||||
* check. Tests that need to exercise the "no PDF" path should
|
||||
* set `withItem: true` so the only failure is the missing PDF.
|
||||
* Tests that NEED a no-items quote can pass `withItem: false`.
|
||||
*/
|
||||
withItem?: boolean;
|
||||
} = {}) {
|
||||
const customer = await pbMock.collection("customers").create({
|
||||
customer: overrides.customerName ?? "Acme Mining",
|
||||
|
|
@ -897,6 +906,28 @@ async function seedReadyQuote(overrides: {
|
|||
uploaded_at: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
if (overrides.withItem !== false) {
|
||||
// Seed one ready line so the pre-flight readiness check passes
|
||||
// and the send path can actually reach the PDF-presence / status
|
||||
// checks below. Matches the pre-flight gates in
|
||||
// `diagnosePdfReadiness` (is_processed, supplier, cost>0,
|
||||
// selling>0).
|
||||
await pbMock.collection("quote_items").create({
|
||||
quote: q.id,
|
||||
quote_number: "Q-2026-06-099",
|
||||
description: "M16 hex bolt",
|
||||
quantity: 1,
|
||||
cost_price: 2.39,
|
||||
selling_price: 3.11,
|
||||
markup: 30,
|
||||
supplier: "sup-acme",
|
||||
is_processed: true,
|
||||
sku: "M16HEX",
|
||||
unit: "EA",
|
||||
notes: "",
|
||||
pricing_source: "manual",
|
||||
});
|
||||
}
|
||||
return { quote: q, customer };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1573,9 +1573,31 @@ export interface SendQuoteEmailFailure {
|
|||
| "no_recipient"
|
||||
| "invalid_recipient"
|
||||
| "no_pdf"
|
||||
| "no_items"
|
||||
| "items_not_ready"
|
||||
| "invalid_status"
|
||||
| "quote_not_found";
|
||||
message: string;
|
||||
/** Populated when the failure was caused by a PDF that can't be
|
||||
* rendered (code `placeholder_customer`, `no_items`, or
|
||||
* `items_not_ready`). Lists every line that blocked the render
|
||||
* with the missing fields, so the agent can present the gap back
|
||||
* to the operator verbatim instead of the operator having to dig
|
||||
* through the quote line by line.
|
||||
*
|
||||
* Same shape as the `details.issues` array from
|
||||
* `POST /api/v1/quotes/[id]/pdf` — one source of truth so the UI
|
||||
* and the agent render the same list. */
|
||||
details?: {
|
||||
customerId?: string;
|
||||
customerName?: string;
|
||||
itemIds?: string[];
|
||||
issues?: Array<{
|
||||
itemId: string;
|
||||
description: string;
|
||||
missing: string[];
|
||||
}>;
|
||||
};
|
||||
}
|
||||
|
||||
export interface SendQuoteEmailResult {
|
||||
|
|
@ -1613,6 +1635,122 @@ export interface SendQuoteEmailResult {
|
|||
* RFC 5322 Message-ID guarantee (must be unique per message) in the
|
||||
* sender's hands.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Diagnose why a quote's PDF can't be generated. Used by
|
||||
* `sendQuoteEmail` when there's no PDF yet — surfaces the same gates
|
||||
* the render route runs (placeholder customer, no items,
|
||||
* items_not_ready) so the agent can present the gaps to the operator
|
||||
* verbatim instead of the operator having to dig through each line.
|
||||
*
|
||||
* Returns a `SendQuoteEmailFailure` shape (NOT thrown) so the caller
|
||||
* can pipe it straight into `markPreSendFailure`. The `details.issues`
|
||||
* array uses the same per-line shape as `pdf/route.ts`'s
|
||||
* `ValidationErrorBody.details.issues` — one source of truth so the
|
||||
* UI's "missing fields" badges and the agent's "these N items are
|
||||
* missing X" prompt render the same list.
|
||||
*
|
||||
* Why this is here and not in pdf/route.ts: the render route is
|
||||
* request-scoped (POST handler); this helper is service-scoped and
|
||||
* runs against an already-loaded quote row. No HTTP fetch loop.
|
||||
*/
|
||||
async function diagnosePdfReadiness(
|
||||
pb: Awaited<ReturnType<typeof getAdminPocketBase>>,
|
||||
quoteId: string,
|
||||
quote: Quote,
|
||||
): Promise<SendQuoteEmailFailure> {
|
||||
// 1. Customer gate. Same logic as pdf/route.ts — placeholder id /
|
||||
// placeholder name / missing email all block the render.
|
||||
let customer: Customer | undefined;
|
||||
if (quote.expand?.customer) {
|
||||
customer = quote.expand.customer as Customer;
|
||||
} else {
|
||||
try {
|
||||
customer = await pb
|
||||
.collection(CUSTOMERS_COLLECTION)
|
||||
.getOne<Customer>(quote.customer);
|
||||
} catch {
|
||||
// No customer row — treat as placeholder.
|
||||
}
|
||||
}
|
||||
if (
|
||||
!customer ||
|
||||
PLACEHOLDER_CUSTOMER_IDS.has(customer.id) ||
|
||||
PLACEHOLDER_CUSTOMER_NAMES.has((customer.customer || "").trim()) ||
|
||||
!customer.email
|
||||
) {
|
||||
return {
|
||||
code: "placeholder_customer",
|
||||
message:
|
||||
"This quote is still pointed at a placeholder / unnamed customer. Pick the real customer on the process page before sending.",
|
||||
details: {
|
||||
customerId: customer?.id,
|
||||
customerName: customer?.customer || "(blank)",
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// 2. Items gate. Same "is the line ready to quote?" check pdf/route.ts
|
||||
// runs: is_processed, supplier, cost_price > 0, selling_price > 0.
|
||||
// Lines already moved to a continuation are excluded — those are
|
||||
// no longer "this quote's items".
|
||||
const allItems = await pb
|
||||
.collection(QUOTE_ITEMS_COLLECTION)
|
||||
.getFullList<QuoteItem>({
|
||||
filter: `quote = "${quoteId}" && (moved_to_quote = null || moved_to_quote = "")`,
|
||||
sort: "created",
|
||||
});
|
||||
if (allItems.length === 0) {
|
||||
return {
|
||||
code: "no_items",
|
||||
message:
|
||||
"This quote has no items yet. Add line items on the quote detail page first.",
|
||||
details: {
|
||||
itemIds: [],
|
||||
issues: [],
|
||||
},
|
||||
};
|
||||
}
|
||||
const issues: Array<{
|
||||
itemId: string;
|
||||
description: string;
|
||||
missing: string[];
|
||||
}> = [];
|
||||
for (const it of allItems) {
|
||||
const missing: string[] = [];
|
||||
if (!it.is_processed) missing.push("not processed");
|
||||
if (!it.supplier) missing.push("no supplier");
|
||||
if (!(Number(it.cost_price) > 0)) missing.push("cost price");
|
||||
if (!(Number(it.selling_price) > 0)) missing.push("selling price");
|
||||
if (missing.length > 0) {
|
||||
issues.push({
|
||||
itemId: String(it.id),
|
||||
description: it.description || "(no description)",
|
||||
missing,
|
||||
});
|
||||
}
|
||||
}
|
||||
if (issues.length > 0) {
|
||||
return {
|
||||
code: "items_not_ready",
|
||||
message: `Cannot generate PDF: ${issues.length} item(s) are missing required fields. Price and supplier every line first.`,
|
||||
details: {
|
||||
itemIds: issues.map((i) => i.itemId),
|
||||
issues,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// No PDF and no detected reason — fall back to the opaque `no_pdf`
|
||||
// so the agent has something to report. In practice one of the
|
||||
// three gates above always fires first.
|
||||
return {
|
||||
code: "no_pdf",
|
||||
message:
|
||||
"No PDF generated for this quote yet. Generate the PDF before sending.",
|
||||
};
|
||||
}
|
||||
|
||||
export async function sendQuoteEmail(
|
||||
quoteId: string,
|
||||
payload: SendQuoteEmailPayload
|
||||
|
|
@ -1729,6 +1867,18 @@ export async function sendQuoteEmail(
|
|||
// ── 6. Validate PDF exists (most recent kind=pdf attachment). The
|
||||
// outbound send includes the PDF as the first attachment; no
|
||||
// PDF = nothing useful to send.
|
||||
//
|
||||
// Closed 2026-08-15: the previous path bailed with a bare
|
||||
// "no_pdf" message and the agent had no way to tell the
|
||||
// operator what was actually missing. We now run the same
|
||||
// readiness check that the PDF render route does
|
||||
// (placeholder customer, no items, items_not_ready) and
|
||||
// surface the per-line `missing` lists back to the agent,
|
||||
// which can present them to the operator verbatim. The
|
||||
// operator decides whether to fix the gaps or render the
|
||||
// PDF themselves; the server doesn't auto-render — Dave
|
||||
// explicitly opted out of that on 2026-08-15 (don't send
|
||||
// partial / stale data without operator eyes on it).
|
||||
const pdfs = await pb
|
||||
.collection(QUOTE_ATTACHMENTS_COLLECTION)
|
||||
.getList(1, 1, {
|
||||
|
|
@ -1736,11 +1886,8 @@ export async function sendQuoteEmail(
|
|||
sort: "-uploaded_at",
|
||||
});
|
||||
if (pdfs.items.length === 0) {
|
||||
return markPreSendFailure(pb, quote, {
|
||||
code: "no_pdf",
|
||||
message:
|
||||
"No PDF generated for this quote yet. Generate the PDF before sending.",
|
||||
});
|
||||
const readiness = await diagnosePdfReadiness(pb, quoteId, quote);
|
||||
return markPreSendFailure(pb, quote, readiness);
|
||||
}
|
||||
|
||||
// ── 7. Filter selectedAttachmentIds to only those that actually
|
||||
|
|
@ -2084,6 +2231,23 @@ export interface SubmitQuoteResult {
|
|||
/** True when the call actually flipped the status. False when the
|
||||
* quote was already `approval_pending` (idempotent re-submit). */
|
||||
statusChanged: boolean;
|
||||
/** Status the quote was in before this call (or after, if it was a
|
||||
* no-op). Always populated — the agent uses it to show the diff
|
||||
* in the audit log without re-fetching the quote. */
|
||||
previousStatus: Quote["status"];
|
||||
/** Echo of the actor passed in. Lands on `quotes.submitted_by` when
|
||||
* the PB admin has added the column; falls back silently otherwise.
|
||||
* Echoed here regardless so the agent always has the name in the
|
||||
* response envelope for the audit trail. */
|
||||
submittedBy?: string;
|
||||
/** ISO timestamp of the submit call. Lands on `quotes.submitted_at`
|
||||
* when the PB column exists; echoed here regardless. */
|
||||
submittedAt: string;
|
||||
/** Target status the call aimed for. For the full-approval path this
|
||||
* is `approval_pending`; for the partial path (`partial: true`
|
||||
* flag) this is `partial_approval_pending` — even when every item
|
||||
* happened to be priced, since the operator's intent was partial. */
|
||||
targetStatus: "approval_pending" | "partial_approval_pending";
|
||||
/** IDs of `quote_items` rows that blocked submission (selling_price
|
||||
* ≤ 0 on a not-moved line). Populated only when `failure !== null`. */
|
||||
offendingItemIds?: string[];
|
||||
|
|
@ -2127,6 +2291,7 @@ export class SubmitQuoteError extends Error {
|
|||
export async function submitQuote(
|
||||
quoteId: string,
|
||||
actor?: { id: string; name: string },
|
||||
options?: { partial?: boolean },
|
||||
): Promise<SubmitQuoteResult> {
|
||||
if (!quoteId || !quoteId.trim()) {
|
||||
throw new SubmitQuoteError(
|
||||
|
|
@ -2149,11 +2314,29 @@ export async function submitQuote(
|
|||
);
|
||||
}
|
||||
|
||||
// Always populate previousStatus — the agent uses it for the audit
|
||||
// trail even on the no-op / idempotent path.
|
||||
const previousStatus = quote.status;
|
||||
const submittedAt = new Date().toISOString();
|
||||
|
||||
// Idempotency: already-pending quote is a no-op. Returning 200 with
|
||||
// statusChanged: false lets the UI safely re-submit without showing
|
||||
// a "double-submit" error.
|
||||
if (quote.status === "approval_pending") {
|
||||
return { quote, statusChanged: false };
|
||||
if (quote.status === "approval_pending" || quote.status === "partial_approval_pending") {
|
||||
return {
|
||||
quote,
|
||||
statusChanged: false,
|
||||
previousStatus,
|
||||
submittedAt,
|
||||
// If the caller asked for partial and the quote is already in
|
||||
// partial_approval_pending, the target is partial; otherwise
|
||||
// it's whatever the quote is currently in.
|
||||
targetStatus:
|
||||
quote.status === "partial_approval_pending"
|
||||
? "partial_approval_pending"
|
||||
: "approval_pending",
|
||||
...(actor?.name ? { submittedBy: actor.name } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
// Only `new` quotes can transition via this tool. The full set of
|
||||
|
|
@ -2171,11 +2354,6 @@ export async function submitQuote(
|
|||
);
|
||||
}
|
||||
|
||||
// Gate on every line being priced. Unpriced lines are flagged with
|
||||
// their ids so the operator knows what to fix. Lines that have
|
||||
// already been moved to a sibling quote (partial-approval split) are
|
||||
// excluded — they're not on this quote anymore.
|
||||
//
|
||||
// Fetch every line on the quote (small N — typical quotes have
|
||||
// < 30 items) and filter in JS. The PB filter parser mock in
|
||||
// `quotes-service.test.ts` doesn't support `!= null` semantics
|
||||
|
|
@ -2196,7 +2374,16 @@ export async function submitQuote(
|
|||
return sp <= 0 && moved === "";
|
||||
});
|
||||
|
||||
if (unpricedItems.length > 0) {
|
||||
// Gate: only `partial: true` lets unpriced items through. The full
|
||||
// submission path (the default) refuses with the existing
|
||||
// `unpriced_items` 422 — no regression. The partial path is the
|
||||
// missing state-machine gap that the test matrix surfaced 2026-08-15:
|
||||
// without it, no tool existed to flip `new → partial_approval_pending`,
|
||||
// which made the whole partial-approve + split workflow unreachable
|
||||
// via MCP. The `partial: true` flag on the same `submit` tool keeps
|
||||
// the state machine documented in one place — there isn't a parallel
|
||||
// `submit_partial` tool to drift out of sync.
|
||||
if (unpricedItems.length > 0 && !options?.partial) {
|
||||
const offendingItemIds = unpricedItems.map((it) => String(it.id));
|
||||
throw new SubmitQuoteError(
|
||||
`Cannot submit: ${offendingItemIds.length} item(s) have selling_price = 0. Price every line before submitting.`,
|
||||
|
|
@ -2206,24 +2393,37 @@ export async function submitQuote(
|
|||
);
|
||||
}
|
||||
|
||||
// Pick the target status. Partial flag wins (operator intent) —
|
||||
// even if every item happened to be priced, we honour the flag.
|
||||
// Without the flag and with all items priced: full approval.
|
||||
const targetStatus: "approval_pending" | "partial_approval_pending" =
|
||||
options?.partial ? "partial_approval_pending" : "approval_pending";
|
||||
|
||||
// Fluid write: Quote.submitted_at / submitted_by are optional fields
|
||||
// on the Quote type. If/when ops adds them to the PB schema, these
|
||||
// will start persisting. Right now they're silently dropped, which
|
||||
// is fine — the status flip is the canonical audit signal.
|
||||
const updatePayload: Record<string, unknown> = {
|
||||
status: "approval_pending",
|
||||
status: targetStatus,
|
||||
};
|
||||
if (actor?.name) {
|
||||
updatePayload.submitted_by = actor.name;
|
||||
}
|
||||
updatePayload.submitted_at = new Date().toISOString();
|
||||
updatePayload.submitted_at = submittedAt;
|
||||
|
||||
const updated = (await pb.collection(QUOTES_COLLECTION).update(
|
||||
quoteId,
|
||||
updatePayload,
|
||||
)) as Quote;
|
||||
|
||||
return { quote: updated, statusChanged: true };
|
||||
return {
|
||||
quote: updated,
|
||||
statusChanged: true,
|
||||
previousStatus,
|
||||
submittedAt,
|
||||
targetStatus,
|
||||
...(actor?.name ? { submittedBy: actor.name } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────────────
|
||||
|
|
|
|||
Loading…
Reference in a new issue