fix(quote-sprint): propagate description edits to quote_items.description

The sprint-board pricing modal was writing operator edits only to
quote_items.quote_description, but the quote detail page renders
quote_items.description. The 'process and open' flow writes to
description directly, which is why edits appeared there but not
after saving from the sprint board.

Mirror the trimmed description into both quote_description and
description on save, matching the process page's write path.

Add three regression guards (edited description, cleared
description, untouched save) to sprint-item-pricing-modal.test.tsx.
This commit is contained in:
Dave 2026-09-08 18:45:13 +02:00
parent b9d30c8e6f
commit 56476e3ca0
2 changed files with 198 additions and 2 deletions

View file

@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, waitFor } from "@testing-library/react";
import { render, screen, waitFor, fireEvent } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { SprintItemPricingModal } from "./sprint-item-pricing-modal";
@ -20,17 +20,54 @@ import type { SprintItem } from "@/app/(app)/sprint/page";
// and fetch so the attachment list returns an empty array. The save
// flow also goes through `pb.collection().update()` — we let that
// resolve to undefined so the save handler runs end-to-end.
//
// `updateMock` is hoisted so the "save" describe block can assert
// against the payload written when the operator clicks "Save & next".
// The layout tests don't care about it but it must not interfere
// either — `vi.clearAllMocks()` in `beforeEach` resets it.
const { updateMock } = vi.hoisted(() => ({
updateMock: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("@/lib/pocketbase", () => ({
__esModule: true,
default: () => ({
collection: () => ({
getOne: vi.fn(),
update: vi.fn().mockResolvedValue(undefined),
update: updateMock,
}),
}),
}));
// Replace SupplierSearch with a plain <select> so the save tests can
// drive supplier selection without rendering the real cmdk combobox.
// Mirrors the pattern used in `order-sprint-process-modal.test.tsx`.
vi.mock("@/components/ui/supplier-search", () => ({
SupplierSearch: ({
value,
onValueChange,
suppliers,
}: {
value: string;
onValueChange: (v: string) => void;
suppliers: Array<{ id: string; suppliername?: string }>;
}) => (
<select
aria-label="Supplier"
value={value}
onChange={(e) => onValueChange(e.target.value)}
>
<option value="">Pick…</option>
{suppliers.map((s) => (
<option key={s.id} value={s.id}>
{s.suppliername ?? s.id}
</option>
))}
</select>
),
}));
// Default item-shape helper: a claimed-by-me M20 Flat Washers row
// already attached to a parent quote that has a customer expanded
// through the standard `expand.quote.expand.customer` chain. Mirrors
@ -343,4 +380,141 @@ describe("SprintItemPricingModal — layout", () => {
);
expect(togglePrecedesDescription).toBe(true);
});
});
// ─── Save flow ───────────────────────────────────────────────────────
//
// These tests exercise the operator's "edit description → Save & next"
// path. The bug they guard against: the sprint modal used to only
// write `quote_description` on save, leaving `description` untouched.
// Because the quote detail page (`/quotes/[id]`) and the process page
// (`/quotes/[id]/process`) only render `item.description`, the
// operator's edit from the sprint board was invisible on the quote
// itself — it was only visible when the PDF was regenerated. The
// working "process and open" flow updates `description` directly,
// which is why opening the quote after editing on the process page
// shows the change.
//
// The fix mirrors the operator's edited `quoteDescription` value to
// both `quote_description` AND `description` so the edit appears on
// every surface that quotes the line item.
describe("SprintItemPricingModal — save", () => {
beforeEach(() => {
updateMock.mockReset();
updateMock.mockResolvedValue(undefined);
});
it("writes both `description` and `quote_description` on save so the edit propagates to the quote detail page", async () => {
// Original RFQ wording is on `description`; `quote_description`
// is empty so the modal pre-fills the textarea with the
// customer's wording. The operator types a cleaned-up version.
renderModal();
const textarea = screen.getByTestId("sprint-quote-description");
fireEvent.change(textarea, {
target: { value: "M20 HT Nuts G8 HDG, 50mm" },
});
// Pick a supplier — the save handler refuses to proceed without
// one. The SupplierSearch component is mocked to a plain <select>
// so we can drive it from the test.
fireEvent.change(screen.getByLabelText("Supplier"), {
target: { value: "sup-1" },
});
// Set cost + selling so the row becomes "processed" and the
// gating check (`cost <= 0 && selling <= 0`) passes.
fireEvent.change(screen.getByTestId("sprint-cost-input"), {
target: { value: "2.39" },
});
fireEvent.change(screen.getByTestId("sprint-selling-input"), {
target: { value: "3.11" },
});
fireEvent.click(screen.getByTestId("sprint-save-button"));
await waitFor(() => {
expect(updateMock).toHaveBeenCalledTimes(1);
});
const [id, payload] = updateMock.mock.calls[0]!;
// PATCH goes to the right row.
expect(id).toBe("qi-1");
// Both description fields are updated — the regression guard.
// Without the fix, only `quote_description` is set and the
// operator's edit disappears on the quote detail page.
expect(payload).toMatchObject({
description: "M20 HT Nuts G8 HDG, 50mm",
quote_description: "M20 HT Nuts G8 HDG, 50mm",
});
});
it("writes empty string to both description fields when the operator clears the description", async () => {
// If the operator explicitly clears the description (e.g. they
// want a placeholder line with no description), the PATCH must
// propagate the empty value to both fields — not just clear
// `quote_description` while leaving `description` at the old
// RFQ wording. Symmetry with the populated case.
renderModal();
const textarea = screen.getByTestId("sprint-quote-description");
fireEvent.change(textarea, { target: { value: "" } });
fireEvent.change(screen.getByLabelText("Supplier"), {
target: { value: "sup-1" },
});
fireEvent.change(screen.getByTestId("sprint-cost-input"), {
target: { value: "1" },
});
fireEvent.change(screen.getByTestId("sprint-selling-input"), {
target: { value: "2" },
});
fireEvent.click(screen.getByTestId("sprint-save-button"));
await waitFor(() => {
expect(updateMock).toHaveBeenCalledTimes(1);
});
const [, payload] = updateMock.mock.calls[0]!;
expect(payload).toMatchObject({
description: "",
quote_description: "",
});
});
it("writes the existing description when the operator saves without editing the textarea", async () => {
// Pre-fill seeds the textarea with `quote_description || description`.
// When the operator hits Save without touching the field, both
// fields end up holding the original RFQ wording — `description`
// is unchanged on the row (we just wrote the same value back),
// `quote_description` now matches `description` so the PDF
// renderer keeps using the same text either way. Guards against
// an accidental regression where the mirror logic is conditional
// on "description was edited" and skips the write on no-op saves.
renderModal();
fireEvent.change(screen.getByLabelText("Supplier"), {
target: { value: "sup-1" },
});
fireEvent.change(screen.getByTestId("sprint-cost-input"), {
target: { value: "1" },
});
fireEvent.change(screen.getByTestId("sprint-selling-input"), {
target: { value: "2" },
});
fireEvent.click(screen.getByTestId("sprint-save-button"));
await waitFor(() => {
expect(updateMock).toHaveBeenCalledTimes(1);
});
const [, payload] = updateMock.mock.calls[0]!;
// The pre-fill is `description` ("M20 Flat Washers HDG") because
// `quote_description` is empty on the fixture. Both fields land
// on that same string — same value, but explicitly written.
expect(payload).toMatchObject({
description: "M20 Flat Washers HDG",
quote_description: "M20 Flat Washers HDG",
});
});
});

View file

@ -554,6 +554,28 @@ export function SprintItemPricingModal({
? Number((((selling - cost) / cost) * 100).toFixed(2))
: 0;
const itemPatch = pb.collection("quote_items").update(item.id, {
// Propagate the operator's edit to BOTH `quote_description`
// (what we put on the PDF — operator's cleaned-up text) AND
// `description` (what the quote detail page and process
// page render — see `/quotes/[id]/page.tsx` line ~2553 and
// `/quotes/[id]/process/page.tsx` textarea at line ~1257).
//
// Without mirroring to `description`, an operator who
// cleans up the description from the sprint board sees no
// change when they open the quote (the detail page only
// renders `description`, not `quote_description`); the
// working "process and open" flow updates `description`
// directly, which is why it appears to work. Mirroring
// matches that field and makes the edit visible on every
// surface that quotes line items.
//
// The two fields retain their distinct meanings
// (customer's original RFQ wording vs operator's cleaned-
// up PDF text) — the PDF render still uses
// `quote_description || description` — but the operator's
// "what we put on the quote" edit is now the authoritative
// description wherever a description is shown.
description: quoteDescription.trim(),
quote_description: quoteDescription.trim(),
quantity: Math.max(1, Number(quantity) || 1),
cost_price: round2(cost),