- 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.
291 lines
11 KiB
TypeScript
291 lines
11 KiB
TypeScript
/**
|
||
* Service layer for `quote_items` writes.
|
||
*
|
||
* Why a service module (and not just inline PB calls in the route):
|
||
* - Per `AGENTS.md` data-layer discipline: new endpoints put
|
||
* business logic in `src/lib/server/<feature>-service.ts` and expose
|
||
* it via a thin route handler. Don't reach for `pb-helpers.ts`.
|
||
* - Multiple call sites will need to create quote_items (the existing
|
||
* AI processor, the existing `quotes.create_from_email` path, the
|
||
* upcoming "Add item" UI on the process page). Centralising the
|
||
* validation + denormalisation here keeps all three on the same
|
||
* contract.
|
||
*
|
||
* Field conventions (mirrored from the existing POST /api/v1/quote_items):
|
||
* - `description` is required and trimmed; empty/whitespace is rejected.
|
||
* - `quantity` defaults to 1; must be > 0 when provided.
|
||
* - `quote_number` is denormalised from the parent quote so downstream
|
||
* review/approve/sprint flows can read it without a join.
|
||
* - `is_processed` is forced to `false` (newly created rows are drafts).
|
||
* - `pricing_source` defaults to "manual" when not provided.
|
||
*/
|
||
|
||
import { getAdminPocketBase } from "./pocketbase-admin";
|
||
|
||
/**
|
||
* Derive `selling_price` from `cost_price * (1 + markup/100)` when the
|
||
* caller has supplied both cost and markup but omitted selling_price.
|
||
*
|
||
* Used by the PATCH, bulk_update, and re-price paths so the agent
|
||
* doesn't have to compute selling_price itself. Marked explicit in the
|
||
* hermes-skill.md workflow doc so the behaviour is documented.
|
||
*
|
||
* Pure — easy to unit-test:
|
||
*
|
||
* deriveSellingPriceIfOmitted({ cost_price: 2.39, markup: 30 })
|
||
* → { cost_price: 2.39, markup: 30, selling_price: 3.11 }
|
||
*
|
||
* deriveSellingPriceIfOmitted({ cost_price: 2.39, markup: 30, selling_price: 4.00 })
|
||
* → { cost_price: 2.39, markup: 30, selling_price: 4.00 } // explicit wins
|
||
*
|
||
* deriveSellingPriceIfOmitted({ cost_price: 0 })
|
||
* → { cost_price: 0 } // can't derive, no markup supplied
|
||
*
|
||
* deriveSellingPriceIfOmitted({ cost_price: 1, markup: NaN })
|
||
* → { cost_price: 1, markup: NaN } // non-finite markup, leave alone
|
||
*/
|
||
export function deriveSellingPriceIfOmitted(
|
||
patch: Record<string, unknown>,
|
||
): Record<string, unknown> {
|
||
if (
|
||
patch.selling_price !== undefined ||
|
||
patch.cost_price === undefined ||
|
||
patch.markup === undefined
|
||
) {
|
||
return patch;
|
||
}
|
||
const cost = Number(patch.cost_price);
|
||
const markup = Number(patch.markup);
|
||
if (!Number.isFinite(cost) || !Number.isFinite(markup)) {
|
||
return patch;
|
||
}
|
||
const selling = Math.round(cost * (1 + markup / 100) * 100) / 100;
|
||
return { ...patch, selling_price: selling };
|
||
}
|
||
|
||
export type CreateQuoteItemInput = {
|
||
quoteId: string;
|
||
description: string;
|
||
quantity?: number;
|
||
unit?: string;
|
||
sku?: string;
|
||
supplier?: string | null;
|
||
category?: string | null;
|
||
cost_price?: number;
|
||
selling_price?: number;
|
||
markup?: number;
|
||
notes?: string;
|
||
pricing_source?: string;
|
||
pricing_notes?: string;
|
||
/** 15-char PB id of the catalogue row this line was sourced from.
|
||
* Stored verbatim on `matched_price_list_id` so the catalogue tab
|
||
* can trace the line back to its origin. */
|
||
matched_price_list_id?: string;
|
||
/** Optional caller token forwarded by the route for audit. Not used
|
||
* for auth (the route uses admin PB on the server); kept here so a
|
||
* later audit-log row can stamp the actual operator. */
|
||
callerToken?: string;
|
||
};
|
||
|
||
export type CreateQuoteItemResult = {
|
||
id: string;
|
||
quote_items: Record<string, unknown>;
|
||
/** True when the create was deduped against a row created within
|
||
* the last `IDEMPOTENCY_WINDOW_MS` ms. Callers should return 200
|
||
* (not 201) and skip downstream side-effects in this case. */
|
||
idempotent?: boolean;
|
||
};
|
||
|
||
/**
|
||
* Window for `quote_items.create` idempotency. Network retries,
|
||
* double-clicks, and "AI auto-fill ran twice" all hit within this
|
||
* window; older rows aren't dedupe candidates because a deliberate
|
||
* re-add of the same line 10 minutes later is a legitimate operator
|
||
* action (e.g. editing the description and re-running the agent).
|
||
*/
|
||
export const IDEMPOTENCY_WINDOW_MS = 60_000;
|
||
|
||
export class CreateQuoteItemError extends Error {
|
||
constructor(
|
||
message: string,
|
||
public readonly status: number = 400,
|
||
) {
|
||
super(message);
|
||
this.name = "CreateQuoteItemError";
|
||
}
|
||
}
|
||
|
||
export async function createQuoteItem(
|
||
input: CreateQuoteItemInput,
|
||
): Promise<CreateQuoteItemResult> {
|
||
const quoteId = (input.quoteId || "").trim();
|
||
const description = (input.description || "").trim();
|
||
if (!quoteId) {
|
||
throw new CreateQuoteItemError("quoteId is required");
|
||
}
|
||
if (!description) {
|
||
throw new CreateQuoteItemError(
|
||
"description is required (non-empty after trim)",
|
||
);
|
||
}
|
||
|
||
let quantity = 1;
|
||
if (input.quantity !== undefined && input.quantity !== null) {
|
||
const q = Number(input.quantity);
|
||
if (!Number.isFinite(q) || q <= 0) {
|
||
throw new CreateQuoteItemError("quantity must be a finite number > 0");
|
||
}
|
||
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
|
||
// denormalisation. PB will throw a 404 on the create below if the
|
||
// relation is missing, but a clean 404 here is a better error
|
||
// surface for the UI.
|
||
let quoteNumber = "";
|
||
try {
|
||
const parent = await pb.collection("quotes").getOne(quoteId, {
|
||
fields: "id,quote_number",
|
||
});
|
||
quoteNumber = String(parent.quote_number || "");
|
||
} catch (err) {
|
||
const message = err instanceof Error ? err.message : "Unknown PB error";
|
||
// PB's "Failed to create record" or "missing relation" both map
|
||
// to 404 here.
|
||
const isMissing = /Failed to create record|not found|missing/i.test(message);
|
||
throw new CreateQuoteItemError(
|
||
`parent quote ${quoteId} not found: ${message}`,
|
||
isMissing ? 404 : 500,
|
||
);
|
||
}
|
||
|
||
// Idempotency: if a row with the same (quote_id, description, sku)
|
||
// was created within the last IDEMPOTENCY_WINDOW_MS, return it and
|
||
// skip the create. Network retries / double-clicks / "AI auto-fill
|
||
// ran twice" all land here — a duplicate is almost always a mistake.
|
||
// Older rows are NOT dedupe candidates: an operator who deliberately
|
||
// re-adds the same line 10 minutes later is doing real work.
|
||
const sku = typeof input.sku === "string" ? input.sku : "";
|
||
const existing = await findRecentDuplicateQuoteItem(
|
||
pb,
|
||
quoteId,
|
||
description,
|
||
sku,
|
||
);
|
||
if (existing) {
|
||
return {
|
||
id: String(existing.id),
|
||
quote_items: existing,
|
||
idempotent: true,
|
||
};
|
||
}
|
||
|
||
let created: Record<string, unknown>;
|
||
try {
|
||
created = await pb.collection("quote_items").create({
|
||
quote: quoteId,
|
||
quote_number: quoteNumber,
|
||
description,
|
||
quantity,
|
||
cost_price: Number(input.cost_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 : "",
|
||
supplier:
|
||
typeof input.supplier === "string" && input.supplier
|
||
? input.supplier
|
||
: null,
|
||
category:
|
||
typeof input.category === "string" && input.category
|
||
? input.category
|
||
: null,
|
||
notes: typeof input.notes === "string" ? input.notes : "",
|
||
pricing_source:
|
||
typeof input.pricing_source === "string"
|
||
? input.pricing_source
|
||
: "manual",
|
||
pricing_notes:
|
||
typeof input.pricing_notes === "string" ? input.pricing_notes : "",
|
||
matched_price_list_id:
|
||
typeof input.matched_price_list_id === "string" &&
|
||
input.matched_price_list_id
|
||
? input.matched_price_list_id
|
||
: "",
|
||
is_processed: false,
|
||
});
|
||
} catch (err) {
|
||
const message = err instanceof Error ? err.message : "Failed to create quote_item";
|
||
throw new CreateQuoteItemError(message, 500);
|
||
}
|
||
|
||
return {
|
||
id: String(created.id),
|
||
quote_items: created,
|
||
};
|
||
}
|
||
|
||
/**
|
||
* Look for a recent duplicate quote_item row — same parent quote,
|
||
* same description, same SKU — created within the last `windowMs`.
|
||
* Returns the full row (caller usually re-emits it as the create
|
||
* response) or `null` when no dedupe candidate exists.
|
||
*
|
||
* Why this exists: the AI auto-fill pipeline and the agent's
|
||
* `quote_items.create` both retry on transient errors; without a
|
||
* dedupe window a single user click or one network blip turns into
|
||
* two identical lines. Scoping the window to 60 seconds (rather
|
||
* than "ever") means a deliberate operator re-add 10 minutes later
|
||
* is still permitted.
|
||
*
|
||
* Returns `null` on PB errors (filter syntax glitch, transient
|
||
* connectivity) rather than throwing — a duplicate-prevention
|
||
* feature must not block legitimate creates.
|
||
*/
|
||
export async function findRecentDuplicateQuoteItem(
|
||
pb: Awaited<ReturnType<typeof getAdminPocketBase>>,
|
||
quoteId: string,
|
||
description: string,
|
||
sku: string,
|
||
windowMs: number = IDEMPOTENCY_WINDOW_MS,
|
||
): Promise<Record<string, unknown> | null> {
|
||
const escapedDesc = description.replace(/"/g, '\\"');
|
||
const escapedSku = sku.replace(/"/g, '\\"');
|
||
// PB datetime filters accept RFC3339 with millisecond precision.
|
||
// We subtract the window once per call so the boundary is stable
|
||
// for the duration of the create.
|
||
const since = new Date(Date.now() - windowMs).toISOString();
|
||
const filter =
|
||
`quote = "${quoteId}" && ` +
|
||
`description = "${escapedDesc}" && ` +
|
||
`sku = "${escapedSku}" && ` +
|
||
`created >= "${since}"`;
|
||
try {
|
||
const result = await pb
|
||
.collection("quote_items")
|
||
.getList(1, 1, { filter, fields: "id,description,sku,quote,created" });
|
||
return (result.items[0] as Record<string, unknown> | undefined) ?? null;
|
||
} catch {
|
||
// Don't block legitimate creates on a dedupe-lookup failure.
|
||
return null;
|
||
}
|
||
}
|