WFDASH-WEBAPP/docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md
Dave 4d95b7af8c docs: non-invoiceable extras design + plan, quantity-handling audit
Three documents behind the two features on this branch:

- specs/2026-08-30-non-invoiceable-order-extras-design.md — the design
  the implementation followed, including why the flag must be named for
  the exception (a positively-named field defaults to false and would
  mark every existing row non-billable the moment it is created).
- plans/2026-08-30-non-invoiceable-order-extras.md — the task-by-task
  plan, now fully implemented.
- specs/2026-08-30-quantity-handling-audit.md — the order_qty /
  quantity audit that produced the preceding fix, plus the partial
  collection findings that are still open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-30 18:28:44 +02:00

8.7 KiB

Non-Invoiceable Order Extras — Design

Date: 2026-08-30 Status: Approved, ready for implementation planning

The problem

We quote a line as a single sellable unit — "M20 bolt, nut and washer" — and the customer approves that one line at that one price. But the supply side doesn't work that way: the bolt comes from one supplier, the nut and washer from another. To raise the purchase orders we need three line items, not one.

Today the operator has no way to add those two extra lines. order_items rows are the only unit the PO flow understands, so a component with no row of its own can't be ordered, can't be assigned to a driver, and never shows as collected. The operator works around it off-system.

Adding the rows naively creates the opposite problem: all three would flow to the invoicing screen and QuickBooks would bill the customer for three lines against a quote they approved as one.

What we're building

A line item can be marked excluded from invoice. Such a line behaves like any other order item everywhere in the operational flow — it gets a supplier, a cost price, a purchase order, a driver assignment, and it shows as collected — but it never appears on a customer-facing invoice or delivery note.

The customer's paperwork comes out byte-identical to what it looks like today.

Decisions

These were settled during brainstorming and are not open for reinterpretation during implementation.

Question Decision
Delivery note Excluded, same as the invoice. Customer paperwork must agree.
Terminal status Auto-flips to invoiced alongside its siblings. The order closes normally.
Where lines are added Process page (/orders/[id]/process), for any line, with no pending gate.
Cost and margin Cost counts as normal. selling_price is forced to 0 and locked.
Picking slip Included. The picker must physically pull the washers.

Data model

One new field on order_items:

exclude_from_invoice : bool

Polarity is load-bearing. The flag must name the exception, not the norm. A positively-named field (bill_customer, invoiceable) defaults to false in PocketBase, which would silently mark every existing row in the database as non-billable the instant the field is created. exclude_from_invoice defaults to false = "bill this normally", so existing rows are unaffected.

Per AGENTS.md, the user adds this field via the PocketBase admin UI before any code references it. pb-exports/pb_schema-2.json is a snapshot for reference only.

No new item_status value is introduced. The existing select enum is untouched, which avoids a second manual PB admin change and avoids updating every status filter, pill, tab count and lifecycle comment in the app.

Architecture

The design rests on a property of the existing invoice pipeline that makes the whole feature small.

The invoice split

buildQbInvoicePayload (src/app/(app)/invoicing/view-model.ts:393) builds two arrays from one row list:

order_item_ids: input.rows.map((row) => row.id),   // drives the PB status write
items:          input.rows.map((row) => ({ ... })), // drives the QuickBooks lines

The n8n workflow consumes them independently. Its "Build PocketBase Invoice Update" node maps over order_item_ids and patches each row with item_status: 'invoiced' plus order_item_details.invoice. The QuickBooks invoice itself is built from items.

So the change is: excluded rows stay in order_item_ids and drop out of items. n8n flips them to invoiced with the batch and stamps them with the same invoice number for audit; QuickBooks never learns they exist.

No n8n changes are required. This is the crux of the design — verified against n8n_workfows/1 - WFDASH QUICKBOOKS INVOICE1 - WFDASH QUICKBOOKS INVOICE.json.

The delivery note split

/api/invoicing/delivery-note/route.ts has the same shape. Excluded rows stay in the status-flip loop (src/app/api/invoicing/delivery-note/route.ts:230), so they receive item_status: "delivery_note" and the DN number; they drop out of the lines: array that renders the PDF (src/app/api/invoicing/delivery-note/route.ts:362).

Picking slips need no change at all

Both picking-slip routes query PocketBase by document identity, not by the row list the operator submitted:

  • print-bundle/route.ts:88 — order_item_details.invoice.number = "…" && item_status = "invoiced"
  • delivery-note-print/route.ts:92 — delivery_note_number = "…" && item_status = "delivery_note"

Because excluded rows do receive the invoice number / DN number and do reach the matching status, they are picked up by these queries automatically. The picker sees the washers. This falls out of the design for free.

The margin gate

calculateOrderItemMarkup(cost, 0) returns -100, far below the ORDER_MARKUP_APPROVAL_THRESHOLD of 22 (src/lib/server/order-item-margin-approval-service.ts:4). Without an explicit bypass, every extra the operator adds would immediately land in approval_needed and block the order pending manager review.

This is the highest-risk part of the implementation. The bypass must be keyed on exclude_from_invoice, applied in the process page's save path before the threshold comparison.

Adding lines mid-flight

createOrderItemForOrder (src/lib/server/orders-service.ts:3939) currently throws a 409 order_locked if any line on the order is past pending. By the time the operator discovers they need a washer line, the bolt line is already po_generated, so this gate fires every time. It is removed.

The gate's original purpose — preventing a PO's pricing/supplier reference from being orphaned — does not apply here. Adding a new row alongside an existing PO doesn't mutate that PO; the new row gets its own PO when the operator processes it.

Flow

Operator opens /orders/[id]/process
  │
  ├── Adds "M20 nut" with [x] Don't invoice
  │     → selling_price forced 0, margin gate bypassed
  │     → row created with exclude_from_invoice = true, item_status = pending
  │
  ├── Normal flow: PO generated → driver assigned → collected → delivered
  │     (identical to every other line; order "Complete" needs no change)
  │
  └── Invoicing screen
        ├── row renders muted with a "Not invoiced" chip, checkbox locked
        ├── Send to QuickBooks
        │     → order_item_ids: [bolt, nut, washer]   ← all three
        │     → items:          [bolt]                 ← one line billed
        │     → n8n flips all three to `invoiced`
        └── Print bundle
              → picking slip query finds all three by invoice number
              → picker pulls all three

Order completion

Unchanged. isOrderComplete (src/lib/server/order-completion.ts:20) requires every item to be delivered; excluded items reach delivered on the normal path. markOrderInvoiced (src/lib/server/orders-service.ts:3397) requires every item to be invoiced; excluded items get there via the n8n batch flip. Neither function needs to learn about the flag.

Error handling

  • All rows in a group excluded. Block the send with a toast rather than firing a zero-line invoice at QuickBooks. Applies to both the invoice and delivery-note submit paths.
  • Operator un-ticks the only billable row in the include-picker modal, leaving only excluded rows. Same guard, same message.
  • Excluded row with a non-zero selling price (legacy or hand-edited in PB admin). The payload builder forces the exclusion regardless of price — the flag is authoritative, price is not consulted.

Testing

Unit tests, Vitest (npm run test:run):

  • view-model.test.ts — buildQbInvoicePayload keeps excluded ids in order_item_ids and omits them from items; an all-excluded row set is detectable by the caller.
  • orders-service.test.ts — createOrderItemForOrder succeeds against an order with po_generated siblings; exclude_from_invoice and the zeroed selling price persist.
  • Delivery-note route test — excluded rows receive the status flip and DN number but are absent from the rendered lines.
  • Margin-gate test — an excluded line with cost > 0 and selling 0 does not enter approval_needed.

Out of scope

  • Reporting changes. Extras eat margin because their cost is real and their selling price is zero; that is the correct number and no report needs adjusting to produce it.
  • Retrofitting existing orders that were worked around off-system.
  • Any change to the n8n QuickBooks workflow.
  • Bulk-adding extras, or templating common component sets (bolt/nut/washer as a saved group). Worth considering later if the operator finds themselves typing the same three lines repeatedly.