diff --git a/src/app/(app)/collection-confirm/page.test.tsx b/src/app/(app)/collection-confirm/page.test.tsx
index ff41cd7..aafd3ed 100644
--- a/src/app/(app)/collection-confirm/page.test.tsx
+++ b/src/app/(app)/collection-confirm/page.test.tsx
@@ -472,4 +472,64 @@ describe("CollectionConfirmPage", () => {
).toBe(true),
);
});
+
+ it("preserves a null prior cost_price when a cost-edit POST fails (does not coerce to 0)", async () => {
+ // Drive the page with one row whose cost_price is null. The
+ // /cost endpoint is forced to fail so the rollback path runs.
+ // The cell carries a `data-cost-value` attribute exposing the
+ // raw value (`"null"` vs `"0"`) — the displayed text is
+ // "R0.00" in both cases (the component uses `?? 0` for the
+ // formatted display) so we need the raw attribute to assert
+ // the rollback preserved `null` instead of stomping it to 0.
+ const nullCostRow = makeRow({ id: "row-null", cost_price: null });
+ vi.spyOn(window, "fetch").mockImplementation(async (input) => {
+ const url = fetchUrl(input);
+ if (url === "/api/v1/collection-confirm") {
+ return jsonResponse([nullCostRow]);
+ }
+ if (typeof url === "string" && url.startsWith("/api/v1/order-items/")) {
+ return jsonResponse({}, { ok: false, status: 500 });
+ }
+ return jsonResponse({});
+ });
+
+ const user = userEvent.setup();
+ render();
+
+ await waitFor(() =>
+ expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(),
+ );
+
+ // Before the edit, the cell exposes `null` as its raw value.
+ const cell = document.querySelector('[data-cost-value]');
+ expect(cell).not.toBeNull();
+ expect(cell!.getAttribute("data-cost-value")).toBe("null");
+
+ // Edit and submit — POST is mocked to fail.
+ await user.click(screen.getByTestId("cost-edit-row-null"));
+ const input = screen.getByTestId("cost-input-row-null");
+ await user.clear(input);
+ await user.type(input, "42");
+ await user.keyboard("{Enter}");
+
+ // After the rollback, the cell's raw value is back to "null" —
+ // not "0". This is the actual fix: the previous code coerced
+ // `prevCost ?? 0` and flipped the row's cost_price from null
+ // to 0 in the local state, leaving the cell to render as
+ // "R0.00" until the next load() refreshed the row from the
+ // server.
+ await waitFor(() =>
+ expect(
+ document.querySelector('[data-cost-value]')?.getAttribute("data-cost-value"),
+ ).toBe("null"),
+ );
+
+ // The cost override is also dropped — opening the confirm
+ // dialog should show "0 cost overrides applied".
+ await user.click(screen.getByTestId("po-card-confirm-PO-1"));
+ const dialog = await screen.findByRole("dialog");
+ expect(
+ within(dialog).getByText(/0 cost overrides applied/i),
+ ).toBeInTheDocument();
+ });
});
diff --git a/src/app/(app)/collection-confirm/page.tsx b/src/app/(app)/collection-confirm/page.tsx
index 0a19f84..30f70ad 100644
--- a/src/app/(app)/collection-confirm/page.tsx
+++ b/src/app/(app)/collection-confirm/page.tsx
@@ -134,10 +134,10 @@ export default function CollectionConfirmPage() {
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
toast.error(message);
- // Snap back: restore the previous cost (or null) on the row
- // and drop the override from the local map.
+ // Snap back: restore the previous cost (preserving null)
+ // on the row and drop the override from the local map.
setRows((prev) =>
- prev?.map((r) => (r.id === id ? { ...r, cost_price: prevCost ?? 0 } : r)) ?? null,
+ prev?.map((r) => (r.id === id ? { ...r, cost_price: prevCost } : r)) ?? null,
);
setCostOverrides((prev) => {
if (!(id in prev)) return prev;
diff --git a/src/components/collection-confirm/items-table.test.tsx b/src/components/collection-confirm/items-table.test.tsx
index 4430cee..a0702c6 100644
--- a/src/components/collection-confirm/items-table.test.tsx
+++ b/src/components/collection-confirm/items-table.test.tsx
@@ -100,6 +100,35 @@ describe("ItemsTable", () => {
expect(screen.queryByTestId("cost-input-row-1")).not.toBeInTheDocument();
});
+ it("does not trigger a React warning when saving a cost on Enter", async () => {
+ // React's "Cannot update a component while rendering a different
+ // component" warning is logged via console.error. Spy on it and
+ // assert no such call was made after pressing Enter on the cost
+ // input — `onCostChange` must NOT run inside the `setEdit`
+ // updater.
+ const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+ const user = userEvent.setup();
+ const onCostChange = vi.fn();
+ render();
+
+ await user.click(screen.getByTestId("cost-edit-row-1"));
+ const input = screen.getByTestId("cost-input-row-1");
+ await user.clear(input);
+ await user.type(input, "77");
+ await user.keyboard("{Enter}");
+
+ expect(onCostChange).toHaveBeenCalledWith("row-1", 77);
+ const reactWarnings = consoleSpy.mock.calls.filter((args) =>
+ args.some(
+ (arg) =>
+ typeof arg === "string" &&
+ arg.includes("Cannot update a component") &&
+ arg.includes("while rendering"),
+ ),
+ );
+ expect(reactWarnings).toHaveLength(0);
+ });
+
it("calls onCostChange on blur with the new value", async () => {
const user = userEvent.setup();
const onCostChange = vi.fn();
diff --git a/src/components/collection-confirm/items-table.tsx b/src/components/collection-confirm/items-table.tsx
index 52a0d84..d6f1a9b 100644
--- a/src/components/collection-confirm/items-table.tsx
+++ b/src/components/collection-confirm/items-table.tsx
@@ -48,19 +48,24 @@ export function ItemsTable({ rows, onCostChange }: Props) {
const cancel = () => setEdit(null);
const save = (rowId: string) => {
- setEdit((current) => {
- if (!current || current.id !== rowId) return current;
- const trimmed = current.value.trim();
- if (trimmed === "") {
- return { ...current, error: "Required" };
- }
- const parsed = Number(trimmed);
- if (Number.isNaN(parsed) || parsed < 0) {
- return { ...current, error: "Must be a non-negative number" };
- }
- onCostChange(rowId, parsed);
- return null;
- });
+ // React forbids calling parent updaters from inside a `setState`
+ // updater because the updater may run twice or not at all under
+ // concurrent rendering. Read the latest `edit` from the closure
+ // (the event handler captures the latest `save` on every render)
+ // and validate + dispatch the side effect OUTSIDE `setEdit`.
+ if (!edit || edit.id !== rowId) return;
+ const trimmed = edit.value.trim();
+ if (trimmed === "") {
+ setEdit({ ...edit, error: "Required" });
+ return;
+ }
+ const parsed = Number(trimmed);
+ if (Number.isNaN(parsed) || parsed < 0) {
+ setEdit({ ...edit, error: "Must be a non-negative number" });
+ return;
+ }
+ setEdit(null);
+ onCostChange(rowId, parsed);
};
const updateValue = (rowId: string, value: string) => {
@@ -135,7 +140,7 @@ export function ItemsTable({ rows, onCostChange }: Props) {
) : (
- R{(row.cost_price ?? 0).toFixed(2)}
+ R{(row.cost_price ?? 0).toFixed(2)}
{isStockRow ? null : (