fix(delivery-notes): merge cancelled into completed, and fix the search
Tabs
- Cancelled no longer has its own tab. The operator asked for it to
sit under the done heading with a filter between the two, rather
than spending a tab on a small cold set nobody visits. Tabs are
now Active | Completed | All.
- Inside Completed, an always-visible Completed / Cancelled / Both
sub-filter, each option carrying its own count. It is rendered in
the open, not behind a click — an invisible control on this screen
was the original "what does that do?" complaint, so a hidden
filter would have reintroduced it. Defaults to "both" so the tab's
count and its rows always reconcile.
- Cancelled keeps a red badge, and no row tint. It now shares a tab
with Completed, so the two must be distinguishable at a glance
without opening each row.
Search (was quietly broken)
- It only covered `delivery_note_number` and `notes`. Customer name,
company and order number were NOT searched — so "what's
outstanding for Acme", the first thing anyone types on a list
grouped by company, returned nothing.
- The search term was never escaped into the PocketBase filter,
while the `repId` clause directly below it escapes its quotes. A
customer called `Acme "Ltd"`, or any stray double quote, produced
a malformed filter and a list that failed to load.
- Search is now client-side over the already-fetched notes, which
carry customer / company / order expanded. Covers DN number, order
number, customer, company, the customer's own PO and notes. Every
word in a multi-word query must match somewhere on the row, so
"acme bolt" narrows rather than returning nothing. No round trip
per keystroke.
Also removed
- Dead pagination state. The page fetched a single wide page and
handed the DataTable page={1} totalPages={1} with a no-op
onPageChange, so page/setPage/totalPages could never do anything.
That also retires the `setPage(1)` the search handler was calling.
Empty state now distinguishes "nothing matches your search" (and
names what is searchable) from "nothing on this tab".
Tests
- 30 in delivery-note-stage.test.ts, covering the search matcher and
the tab/sub-filter bucketing. Verified they bite by reverting the
search to its old two-field coverage: 5 fail.
- matchesDoneFilter refuses an in-flight status even under "both".
It didn't need to — the filter only runs inside the Completed tab —
but a function that would return a draft for "both" relies on its
call site staying correct, so it now guards itself.
Full suite: 3630 passing. tsc clean. No new lint warnings.
This commit is contained in:
parent
97c3cd4d7e
commit
c0e6ad2099
4 changed files with 462 additions and 128 deletions
|
|
@ -28,6 +28,10 @@ import {
|
|||
deriveDnStage,
|
||||
dnStageTint,
|
||||
dnTabForStatus,
|
||||
isDoneStatus,
|
||||
matchesDeliveryNoteSearch,
|
||||
matchesDoneFilter,
|
||||
type DoneFilter,
|
||||
type DnStage,
|
||||
} from "@/lib/delivery-note-stage";
|
||||
import {
|
||||
|
|
@ -64,67 +68,48 @@ import type {
|
|||
} from "@/lib/types";
|
||||
|
||||
/**
|
||||
* Tab key for the list-page filter strip. The four buckets cover
|
||||
* the full lifecycle without forcing the operator to scroll:
|
||||
* Tab key for the list-page filter strip.
|
||||
*
|
||||
* - "active" → draft + sent + loaded + po_received (anything the
|
||||
* operator still has to act on)
|
||||
* - "completed" → invoiced + completed (terminal, visible but inert)
|
||||
* - "cancelled" → cancelled only (kept separate so old cancelled DNs
|
||||
* don't pollute the working list, and so the
|
||||
* Completed count isn't a lie about what shipped)
|
||||
* - "completed" → invoiced + completed + CANCELLED, narrowed by the
|
||||
* `doneFilter` sub-control below
|
||||
* - "all" → no filter (escape hatch when the operator
|
||||
* doesn't know the status)
|
||||
*
|
||||
* `invoiced` deliberately is NOT in "active". It used to be, which is
|
||||
* why the active list never drained — finished work stayed in the
|
||||
* operator's face all day. The buckets are derived through
|
||||
* `dnTabForStatus` so the tab and the colour ladder can never disagree
|
||||
* about what "active" means.
|
||||
* `invoiced` deliberately is NOT in "active" — that is why the active
|
||||
* list drains instead of being a graveyard.
|
||||
*
|
||||
* Cancelled used to have its own tab. It doesn't any more: the
|
||||
* operator asked for it to sit under the done heading with a filter
|
||||
* between the two, rather than spending a tab on a small, cold set
|
||||
* that nobody visits. See `matchesDoneFilter`.
|
||||
*/
|
||||
type DnTab = "active" | "cancelled" | "completed" | "all";
|
||||
type DnTab = "active" | "completed" | "all";
|
||||
|
||||
const TAB_LABELS: Record<DnTab, string> = {
|
||||
active: "Active",
|
||||
cancelled: "Cancelled",
|
||||
completed: "Completed",
|
||||
all: "All",
|
||||
};
|
||||
|
||||
const ALL_STATUSES: DeliveryNoteStatus[] = [
|
||||
"draft",
|
||||
"sent",
|
||||
"loaded",
|
||||
"po_received",
|
||||
"invoiced",
|
||||
"completed",
|
||||
"cancelled",
|
||||
];
|
||||
|
||||
const TAB_STATUSES: Record<DnTab, DeliveryNoteStatus[]> = {
|
||||
active: ["draft", "sent", "loaded", "po_received"],
|
||||
cancelled: ["cancelled"],
|
||||
completed: ["invoiced", "completed"],
|
||||
all: ALL_STATUSES,
|
||||
};
|
||||
|
||||
/**
|
||||
* Status → badge treatment. Every status is deliberately NEUTRAL.
|
||||
* Status → badge treatment.
|
||||
*
|
||||
* The badge's job is to name the state; the row's tint is what tells
|
||||
* the operator where they are in the workflow. Colouring both meant
|
||||
* "Invoiced", "PO received" and "Completed" all mapped to `default` —
|
||||
* byte-for-byte identical rendering — while the three rows that
|
||||
* genuinely look different sat underneath them. One signal, one
|
||||
* place: the row (see `rowStyle`).
|
||||
* `cancelled` is the one status that keeps a colour. It has to: it's
|
||||
* now inside the Completed tab, so "Completed" and "Cancelled" rows
|
||||
* sit next to each other and the operator needs to tell them apart
|
||||
* without opening each one. Everything else is neutral, because the
|
||||
* ROW tint is what carries progress — see `rowStyle`.
|
||||
*/
|
||||
const statusColors: Record<DeliveryNoteStatus, "outline" | "secondary"> = {
|
||||
const statusColors: Record<DeliveryNoteStatus, "outline" | "secondary" | "destructive"> = {
|
||||
draft: "outline",
|
||||
sent: "outline",
|
||||
loaded: "outline",
|
||||
po_received: "outline",
|
||||
invoiced: "secondary",
|
||||
completed: "secondary",
|
||||
cancelled: "outline",
|
||||
cancelled: "destructive",
|
||||
};
|
||||
|
||||
const statusLabel: Record<DeliveryNoteStatus, string> = {
|
||||
|
|
@ -151,15 +136,28 @@ export default function DeliveryNotesPage() {
|
|||
);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [error, setError] = useState("");
|
||||
const [page, setPage] = useState(1);
|
||||
const [totalPages, setTotalPages] = useState(1);
|
||||
// No pagination state. This screen fetches a single wide page (500
|
||||
// rows, well over the real DN count) and groups it by company, so
|
||||
// there is only ever one page. It used to hold `page`/`setPage`/
|
||||
// `totalPages` and hand the DataTable `page={1} totalPages={1}` with
|
||||
// a no-op `onPageChange` — three pieces of state that could never
|
||||
// do anything. Search and the status filters are client-side over
|
||||
// the fetched set for the same reason.
|
||||
// Search term. Applied CLIENT-SIDE over the already-fetched notes
|
||||
// (which carry customer / company / order expanded), so it covers
|
||||
// every field the row shows. See `getDeliveryNotes` for why the
|
||||
// server-side version was a dead end.
|
||||
const [search, setSearch] = useState("");
|
||||
// List-page filter strip. Defaults to "active" so the operator
|
||||
// opens the page to a working list; cancelled / completed /
|
||||
// all are opt-in. Status filter is applied client-side over
|
||||
// the current page of notes (the server still returns the full
|
||||
// recent set so search/tab switching doesn't have to re-fetch).
|
||||
// opens the page to a working list; completed / all are opt-in.
|
||||
// Applied client-side over the fetched set so switching tabs never
|
||||
// re-fetches.
|
||||
const [tab, setTab] = useState<DnTab>("active");
|
||||
// Sub-filter inside the Completed tab, which holds invoiced +
|
||||
// completed + cancelled together. Defaults to "both" so the tab's
|
||||
// count and its rows always agree — a tab promising 40 and showing
|
||||
// 29 is how a list stops being trusted.
|
||||
const [doneFilter, setDoneFilter] = useState<DoneFilter>("both");
|
||||
// Per-company COLLAPSED state — note the inversion.
|
||||
//
|
||||
// This used to be `openGroups`, initialised to an empty Set, with
|
||||
|
|
@ -336,19 +334,30 @@ export default function DeliveryNotesPage() {
|
|||
const tabCounts = useMemo(() => {
|
||||
const counts: Record<DnTab, number> = {
|
||||
active: 0,
|
||||
cancelled: 0,
|
||||
completed: 0,
|
||||
all: notes.length,
|
||||
};
|
||||
for (const note of notes) {
|
||||
const bucket = dnTabForStatus(note.status);
|
||||
if (bucket === "cancelled") counts.cancelled += 1;
|
||||
else if (bucket === "invoiced") counts.completed += 1;
|
||||
else counts.active += 1;
|
||||
if (dnTabForStatus(note.status) === "active") counts.active += 1;
|
||||
else counts.completed += 1;
|
||||
}
|
||||
return counts;
|
||||
}, [notes]);
|
||||
|
||||
// Split inside the done bucket, for the sub-filter's own labels.
|
||||
// Computed against the full set (not the search-filtered view) so
|
||||
// the counts describe what's on the server, not what happens to be
|
||||
// visible right now.
|
||||
const doneCounts = useMemo(() => {
|
||||
let completed = 0;
|
||||
let cancelled = 0;
|
||||
for (const note of notes) {
|
||||
if (note.status === "cancelled") cancelled += 1;
|
||||
else if (isDoneStatus(note.status)) completed += 1;
|
||||
}
|
||||
return { completed, cancelled, both: completed + cancelled };
|
||||
}, [notes]);
|
||||
|
||||
// ── Expandable row: fetch a note's line items on first open.
|
||||
//
|
||||
// The list endpoint returns headers only, so the items live behind
|
||||
|
|
@ -433,12 +442,26 @@ export default function DeliveryNotesPage() {
|
|||
}
|
||||
}, [itemsByDnId, loadingIds]);
|
||||
|
||||
// Apply the tab filter. Selection / bulk actions keep working
|
||||
// because we filter the visible array but keep the underlying
|
||||
// `notes` untouched.
|
||||
// The one filtering pass: search, then tab, then the done
|
||||
// sub-filter. Order matters — search runs against the full set so
|
||||
// the tab counts stay truthful, and the sub-filter is a no-op
|
||||
// outside the Completed tab.
|
||||
const visibleNotes = useMemo(
|
||||
() => notes.filter((note) => TAB_STATUSES[tab].includes(note.status)),
|
||||
[notes, tab],
|
||||
() =>
|
||||
notes.filter((note) => {
|
||||
if (!matchesDeliveryNoteSearch(note, search)) return false;
|
||||
const bucket = dnTabForStatus(note.status);
|
||||
if (bucket === "done") {
|
||||
if (tab === "active") return false;
|
||||
if (tab === "completed" && !matchesDoneFilter(note.status, doneFilter)) {
|
||||
return false;
|
||||
}
|
||||
} else if (tab === "completed") {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}),
|
||||
[notes, search, tab, doneFilter],
|
||||
);
|
||||
|
||||
// Derived progress stage per note. Memoised because it feeds both
|
||||
|
|
@ -487,15 +510,17 @@ export default function DeliveryNotesPage() {
|
|||
try {
|
||||
const userType = (user as { user_type?: string[] } | null)?.user_type;
|
||||
const repId = user && isRepUser(userType) ? user.id : null;
|
||||
// Fetch a wide page so the client-side tab filter sees the
|
||||
// full set — the previous `perPage=20` made the active
|
||||
// tab silently drop records past rank 20 (the operator
|
||||
// saw "no DN" when the row existed but was buried). 500
|
||||
// is well over the production DN count (≈30) and still
|
||||
// cheap for PB.
|
||||
const result = await getDeliveryNotes(page, 500, search, repId);
|
||||
// Fetch a wide page so the client-side search / tab / sub-filter
|
||||
// all see the full set — the previous `perPage=20` made the active
|
||||
// tab silently drop records past rank 20 (the operator saw "no DN"
|
||||
// when the row existed but was buried). 500 is well over the
|
||||
// production DN count (≈30) and still cheap for PB.
|
||||
//
|
||||
// No search term is sent: search is client-side now, because the
|
||||
// server-side version could only see two fields and would have to
|
||||
// re-fetch on every keystroke to add the rest.
|
||||
const result = await getDeliveryNotes(1, 500, repId);
|
||||
setNotes(result.items);
|
||||
setTotalPages(result.totalPages);
|
||||
// Batch-fetch the related quotes in one query so the Q
|
||||
// icon can show "create" vs "open" without a per-row
|
||||
// round trip. Failures here are non-fatal — the Q icon
|
||||
|
|
@ -517,7 +542,7 @@ export default function DeliveryNotesPage() {
|
|||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, [page, search, user]);
|
||||
}, [user]);
|
||||
|
||||
useEffect(() => {
|
||||
fetchNotes();
|
||||
|
|
@ -1245,65 +1270,109 @@ export default function DeliveryNotesPage() {
|
|||
</div>
|
||||
|
||||
{/* Status filter strip. Active is the default landing tab so
|
||||
the operator opens the page to the working list;
|
||||
cancelled / completed live one click away and never
|
||||
collide with the active workflow. Each tab carries a
|
||||
count badge so the operator can tell at a glance how
|
||||
many DNs sit in each bucket. */}
|
||||
<div
|
||||
role="tablist"
|
||||
aria-label="Filter delivery notes by status"
|
||||
className="inline-flex flex-wrap items-center gap-1 rounded-lg border bg-card p-1 shadow-sm"
|
||||
>
|
||||
{(Object.keys(TAB_LABELS) as DnTab[]).map((key) => {
|
||||
const isActive = key === tab;
|
||||
return (
|
||||
<button
|
||||
key={key}
|
||||
type="button"
|
||||
role="tab"
|
||||
aria-selected={isActive}
|
||||
onClick={() => setTab(key)}
|
||||
data-testid={`dn-tab-${key}`}
|
||||
className={cn(
|
||||
"inline-flex items-center gap-2 rounded-md px-3 py-1.5 text-xs font-medium transition-colors",
|
||||
isActive
|
||||
? "bg-primary text-primary-foreground shadow-sm"
|
||||
: "text-muted-foreground hover:bg-accent hover:text-accent-foreground",
|
||||
)}
|
||||
>
|
||||
{TAB_LABELS[key]}
|
||||
<span
|
||||
the operator opens the page to the working list. Cancelled
|
||||
and completed share the Completed tab and are separated by
|
||||
the sub-filter below it. Each tab carries a count badge. */}
|
||||
<div className="flex flex-wrap items-center gap-3">
|
||||
<div
|
||||
role="tablist"
|
||||
aria-label="Filter delivery notes by status"
|
||||
className="inline-flex flex-wrap items-center gap-1 rounded-lg border bg-card p-1 shadow-sm"
|
||||
>
|
||||
{(Object.keys(TAB_LABELS) as DnTab[]).map((key) => {
|
||||
const isActive = key === tab;
|
||||
return (
|
||||
<button
|
||||
key={key}
|
||||
type="button"
|
||||
role="tab"
|
||||
aria-selected={isActive}
|
||||
onClick={() => setTab(key)}
|
||||
data-testid={`dn-tab-${key}`}
|
||||
className={cn(
|
||||
"rounded-full px-2 py-0.5 text-[10px] tabular-nums",
|
||||
"inline-flex items-center gap-2 rounded-md px-3 py-1.5 text-xs font-medium transition-colors",
|
||||
isActive
|
||||
? "bg-primary-foreground/15 text-primary-foreground"
|
||||
: "bg-muted text-muted-foreground",
|
||||
? "bg-primary text-primary-foreground shadow-sm"
|
||||
: "text-muted-foreground hover:bg-accent hover:text-accent-foreground",
|
||||
)}
|
||||
>
|
||||
{tabCounts[key]}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
{TAB_LABELS[key]}
|
||||
<span
|
||||
className={cn(
|
||||
"rounded-full px-2 py-0.5 text-[10px] tabular-nums",
|
||||
isActive
|
||||
? "bg-primary-foreground/15 text-primary-foreground"
|
||||
: "bg-muted text-muted-foreground",
|
||||
)}
|
||||
>
|
||||
{tabCounts[key]}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
|
||||
{/* Sub-filter for the done bucket. ALWAYS rendered when the
|
||||
Completed tab is active, never hidden behind a click —
|
||||
an invisible control on this screen was the "what does
|
||||
that do?" complaint in the first place. Each option
|
||||
carries its own count so the tab total always reconciles
|
||||
with the sum of the options. */}
|
||||
{tab === "completed" && (
|
||||
<div
|
||||
role="group"
|
||||
aria-label="Narrow completed notes"
|
||||
className="inline-flex items-center gap-1 rounded-lg border bg-card p-1 shadow-sm"
|
||||
>
|
||||
{(
|
||||
[
|
||||
{ key: "completed", label: "Completed", n: doneCounts.completed },
|
||||
{ key: "cancelled", label: "Cancelled", n: doneCounts.cancelled },
|
||||
{ key: "both", label: "Both", n: doneCounts.both },
|
||||
] as const
|
||||
).map((opt) => {
|
||||
const isOn = doneFilter === opt.key;
|
||||
return (
|
||||
<button
|
||||
key={opt.key}
|
||||
type="button"
|
||||
aria-pressed={isOn}
|
||||
onClick={() => setDoneFilter(opt.key)}
|
||||
data-testid={`dn-done-filter-${opt.key}`}
|
||||
className={cn(
|
||||
"inline-flex items-center gap-1.5 rounded-md px-2.5 py-1 text-xs font-medium transition-colors",
|
||||
isOn
|
||||
? "bg-accent text-accent-foreground"
|
||||
: "text-muted-foreground hover:bg-accent/50",
|
||||
)}
|
||||
>
|
||||
{opt.label}
|
||||
<span className="tabular-nums opacity-70">{opt.n}</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Global search. One input filters across every company
|
||||
below — operators usually know the DN number / customer
|
||||
/ order, not the company. We use a plain Input here
|
||||
instead of a full DataTable so the search input doesn't
|
||||
render its own empty-state banner ("No records found")
|
||||
that hides the actual grouped list. */}
|
||||
{/* Search. Filters the already-fetched notes client-side, so it
|
||||
covers every field the row shows — DN number, order number,
|
||||
customer, company, the customer's own PO reference, and
|
||||
notes. The placeholder says so, because the previous one
|
||||
suggested it only understood a DN number, and it only
|
||||
searched two fields. Multiple words narrow (every word must
|
||||
match somewhere on the row).
|
||||
|
||||
Deliberately not a DataTable: that renders its own
|
||||
"No records found" empty state which would hide the grouped
|
||||
list. A plain Input also means no round trip per keystroke. */}
|
||||
<div className="flex flex-col sm:flex-row gap-3 items-start sm:items-center justify-between">
|
||||
<div className="relative w-full sm:max-w-sm">
|
||||
<SearchIcon className="absolute left-3 top-1/2 -translate-y-1/2 h-4 w-4 text-muted-foreground" />
|
||||
<Input
|
||||
placeholder="Search delivery notes..."
|
||||
placeholder="Search DN, order, customer, company or PO…"
|
||||
value={search}
|
||||
onChange={(e) => {
|
||||
setSearch(e.target.value);
|
||||
setPage(1);
|
||||
}}
|
||||
onChange={(e) => setSearch(e.target.value)}
|
||||
className="pl-9 border-warn/40 bg-warn/5 focus-visible:border-warn focus-visible:ring-warn/30"
|
||||
data-testid="dn-search"
|
||||
/>
|
||||
|
|
@ -1318,8 +1387,29 @@ export default function DeliveryNotesPage() {
|
|||
about right now. */}
|
||||
{groupedNotes.length === 0 && !loading ? (
|
||||
<div className="flex flex-col items-center justify-center rounded-md border border-dashed py-16 text-center text-muted-foreground">
|
||||
<p className="text-sm font-medium">No delivery notes on this tab.</p>
|
||||
<p className="text-xs">Switch tabs above or create a new delivery note.</p>
|
||||
{search.trim() ? (
|
||||
<>
|
||||
<p className="text-sm font-medium">
|
||||
Nothing matches “{search.trim()}”.
|
||||
</p>
|
||||
<p className="text-xs">
|
||||
Search covers the DN number, order number, customer,
|
||||
company, customer PO and notes. Try a shorter term, or
|
||||
switch to the All tab.
|
||||
</p>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<p className="text-sm font-medium">
|
||||
No delivery notes on this tab.
|
||||
</p>
|
||||
<p className="text-xs">
|
||||
{tab === "completed" && doneCounts.cancelled > 0 && doneCounts.completed === 0
|
||||
? "Only cancelled notes are here — try the Cancelled option above."
|
||||
: "Switch tabs above or create a new delivery note."}
|
||||
</p>
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
) : (
|
||||
<div className="space-y-4">
|
||||
|
|
|
|||
|
|
@ -4,6 +4,9 @@ import {
|
|||
dnStageTint,
|
||||
dnTabForStatus,
|
||||
hasSignedCopy,
|
||||
isDoneStatus,
|
||||
matchesDeliveryNoteSearch,
|
||||
matchesDoneFilter,
|
||||
} from "./delivery-note-stage";
|
||||
import type { DeliveryNote, DeliveryNoteStatus } from "./types";
|
||||
|
||||
|
|
@ -123,8 +126,138 @@ describe("dnTabForStatus", () => {
|
|||
expect(dnTabForStatus("sent")).toBe("active");
|
||||
expect(dnTabForStatus("loaded")).toBe("active");
|
||||
expect(dnTabForStatus("po_received")).toBe("active");
|
||||
expect(dnTabForStatus("invoiced")).toBe("invoiced");
|
||||
expect(dnTabForStatus("completed")).toBe("invoiced");
|
||||
expect(dnTabForStatus("cancelled")).toBe("cancelled");
|
||||
});
|
||||
|
||||
it("folds invoiced, completed AND cancelled into one done bucket", () => {
|
||||
// Cancelled used to have its own tab. The operator asked for it
|
||||
// to live under the done heading with a filter between them.
|
||||
expect(dnTabForStatus("invoiced")).toBe("done");
|
||||
expect(dnTabForStatus("completed")).toBe("done");
|
||||
expect(dnTabForStatus("cancelled")).toBe("done");
|
||||
});
|
||||
});
|
||||
|
||||
describe("matchesDoneFilter", () => {
|
||||
it("'both' shows every done note", () => {
|
||||
expect(matchesDoneFilter("invoiced", "both")).toBe(true);
|
||||
expect(matchesDoneFilter("completed", "both")).toBe(true);
|
||||
expect(matchesDoneFilter("cancelled", "both")).toBe(true);
|
||||
});
|
||||
|
||||
it("'completed' hides cancelled but keeps both finished states", () => {
|
||||
expect(matchesDoneFilter("invoiced", "completed")).toBe(true);
|
||||
expect(matchesDoneFilter("completed", "completed")).toBe(true);
|
||||
expect(matchesDoneFilter("cancelled", "completed")).toBe(false);
|
||||
});
|
||||
|
||||
it("'cancelled' shows only cancelled", () => {
|
||||
expect(matchesDoneFilter("cancelled", "cancelled")).toBe(true);
|
||||
expect(matchesDoneFilter("completed", "cancelled")).toBe(false);
|
||||
expect(matchesDoneFilter("invoiced", "cancelled")).toBe(false);
|
||||
});
|
||||
|
||||
it("never lets an in-flight status through, whatever the filter", () => {
|
||||
// Guard: the filter lives INSIDE the done tab, so this should be
|
||||
// impossible in the UI. If it ever is, we'd show work-in-progress
|
||||
// in a tab labelled Completed.
|
||||
for (const f of ["both", "completed", "cancelled"] as const) {
|
||||
expect(matchesDoneFilter("draft", f)).toBe(false);
|
||||
expect(matchesDoneFilter("po_received", f)).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("isDoneStatus", () => {
|
||||
it("treats cancelled as done — the work finished, one way or another", () => {
|
||||
expect(isDoneStatus("cancelled")).toBe(true);
|
||||
expect(isDoneStatus("completed")).toBe(true);
|
||||
expect(isDoneStatus("invoiced")).toBe(true);
|
||||
expect(isDoneStatus("draft")).toBe(false);
|
||||
expect(isDoneStatus("sent")).toBe(false);
|
||||
expect(isDoneStatus("loaded")).toBe(false);
|
||||
expect(isDoneStatus("po_received")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
const row = (over: Record<string, unknown> = {}) =>
|
||||
({
|
||||
delivery_note_number: "DN26071401",
|
||||
notes: "counter pickup",
|
||||
customer_po: "PO-9931",
|
||||
...over,
|
||||
}) as Parameters<typeof matchesDeliveryNoteSearch>[0];
|
||||
|
||||
describe("matchesDeliveryNoteSearch", () => {
|
||||
it("matches the DN number", () => {
|
||||
expect(matchesDeliveryNoteSearch(row(), "26071401")).toBe(true);
|
||||
expect(matchesDeliveryNoteSearch(row(), "26071401")).toBe(true);
|
||||
});
|
||||
|
||||
it("matches the CUSTOMER name — the case the old server filter missed", () => {
|
||||
// This is the regression that motivated moving search
|
||||
// client-side: the old filter only read delivery_note_number and
|
||||
// notes, so "what's outstanding for Acme" returned nothing on a
|
||||
// list that is grouped by company.
|
||||
const n = row({
|
||||
expand: {
|
||||
order: { order_number: "ORD-2607-118" },
|
||||
customer: { customer: "Andrew Bester" },
|
||||
company: { company: "Acme Bolts" },
|
||||
},
|
||||
});
|
||||
expect(matchesDeliveryNoteSearch(n, "acme")).toBe(true);
|
||||
expect(matchesDeliveryNoteSearch(n, "bester")).toBe(true);
|
||||
});
|
||||
|
||||
it("matches the company and the order number", () => {
|
||||
const n = row({
|
||||
expand: {
|
||||
order: { order_number: "ORD-2607-118" },
|
||||
customer: { customer: "Andrew Bester" },
|
||||
company: { company: "Acme Bolts" },
|
||||
},
|
||||
});
|
||||
expect(matchesDeliveryNoteSearch(n, "ORD-2607-118")).toBe(true);
|
||||
expect(matchesDeliveryNoteSearch(n, "acme bolts")).toBe(true);
|
||||
});
|
||||
|
||||
it("matches the customer's own PO reference", () => {
|
||||
expect(matchesDeliveryNoteSearch(row(), "PO-9931")).toBe(true);
|
||||
});
|
||||
|
||||
it("is case-insensitive", () => {
|
||||
expect(matchesDeliveryNoteSearch(row(), "COUNTER")).toBe(true);
|
||||
});
|
||||
|
||||
it("requires EVERY word, so 'acme bolt' narrows rather than 404s", () => {
|
||||
const n = row({
|
||||
expand: { company: { company: "Acme Bolts" } },
|
||||
});
|
||||
expect(matchesDeliveryNoteSearch(n, "acme bolt")).toBe(true);
|
||||
expect(matchesDeliveryNoteSearch(n, "acme missing")).toBe(false);
|
||||
});
|
||||
|
||||
it("returns everything for an empty or whitespace query", () => {
|
||||
expect(matchesDeliveryNoteSearch(row(), "")).toBe(true);
|
||||
expect(matchesDeliveryNoteSearch(row(), " ")).toBe(true);
|
||||
});
|
||||
|
||||
it("does not throw on a note with no expansions at all", () => {
|
||||
// Ad-hoc counter notes can carry a customer but no order, and a
|
||||
// note mid-creation can have neither. A search box that throws on
|
||||
// those rows takes the whole list down.
|
||||
const bare = row();
|
||||
expect(() => matchesDeliveryNoteSearch(bare, "anything")).not.toThrow();
|
||||
expect(matchesDeliveryNoteSearch(bare, "anything")).toBe(false);
|
||||
});
|
||||
|
||||
it("treats a quote in the query as ordinary text, not a filter break", () => {
|
||||
// The old server-side filter interpolated the term into a PB
|
||||
// filter string with no escaping, so a double quote produced a
|
||||
// malformed filter and a list that failed to load.
|
||||
const n = row({
|
||||
expand: { company: { company: 'Acme "Ltd" Bolts' } },
|
||||
});
|
||||
expect(matchesDeliveryNoteSearch(n, 'acme "ltd"')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -59,14 +59,103 @@ export function hasSignedCopy(note: Pick<DeliveryNote, "signed_copy">): boolean
|
|||
}
|
||||
|
||||
/**
|
||||
* Which tab a status belongs to. Kept next to `deriveDnStage` so the
|
||||
* tab buckets and the colour ladder can never disagree about what
|
||||
* "active" means.
|
||||
* Sub-filter for the Completed tab.
|
||||
*
|
||||
* The operator asked for cancelled and completed to live together
|
||||
* with a way to narrow between them, rather than as two separate
|
||||
* tabs. `both` is the default so the tab's count and its contents
|
||||
* always agree — a tab that says "Completed 40" and then shows 29
|
||||
* rows with the other 11 hidden is the sort of thing that makes a
|
||||
* list untrustworthy.
|
||||
*/
|
||||
export function dnTabForStatus(status: DeliveryNoteStatus): DnStage | "active" {
|
||||
if (status === "cancelled") return "cancelled";
|
||||
if (status === "invoiced" || status === "completed") return "invoiced";
|
||||
return "active";
|
||||
export type DoneFilter = "completed" | "cancelled" | "both";
|
||||
|
||||
/**
|
||||
* Does a status fall inside the "done" bucket (the Completed tab)?
|
||||
* Cancelled counts as done: the work is finished, one way or another.
|
||||
*/
|
||||
export function isDoneStatus(status: DeliveryNoteStatus): boolean {
|
||||
return status === "invoiced" || status === "completed" || status === "cancelled";
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply the sub-filter inside the done bucket.
|
||||
*
|
||||
* `"both"` means "both FINISHED kinds", not "any status at all" — it
|
||||
* still refuses an in-flight note. The filter currently only runs
|
||||
* inside the Completed tab so nothing can leak in practice, but a
|
||||
* function that would happily return a `draft` for `both` relies on
|
||||
* its call site staying correct. Guarding here means reusing it
|
||||
* elsewhere can't quietly put work-in-progress in a done list.
|
||||
*/
|
||||
export function matchesDoneFilter(
|
||||
status: DeliveryNoteStatus,
|
||||
filter: DoneFilter,
|
||||
): boolean {
|
||||
if (!isDoneStatus(status)) return false;
|
||||
if (filter === "both") return true;
|
||||
if (filter === "cancelled") return status === "cancelled";
|
||||
return status === "invoiced" || status === "completed";
|
||||
}
|
||||
|
||||
/**
|
||||
* Client-side search over a fetched delivery note.
|
||||
*
|
||||
* Deliberately matches EVERY field the row displays. The previous
|
||||
* server-side filter only covered `delivery_note_number` and `notes`,
|
||||
* so typing a customer or company name — the first thing anyone
|
||||
* types on a list grouped by company — returned nothing at all.
|
||||
*
|
||||
* Matching is case-insensitive substring across each field
|
||||
* independently, and multi-word queries match if EVERY word appears
|
||||
* somewhere on the row. That second rule matters more than it looks:
|
||||
* operators type "acme bolt" meaning *Acme* AND *bolt*, and a plain
|
||||
* whole-string substring test would find neither, because no single
|
||||
* field contains "acme bolt".
|
||||
*/
|
||||
export function matchesDeliveryNoteSearch(
|
||||
note: Pick<
|
||||
DeliveryNote,
|
||||
"delivery_note_number" | "notes" | "customer_po"
|
||||
> & {
|
||||
expand?: {
|
||||
order?: { order_number?: string | null } | null;
|
||||
customer?: { customer?: string | null } | null;
|
||||
company?: { company?: string | null } | null;
|
||||
};
|
||||
},
|
||||
query: string,
|
||||
): boolean {
|
||||
const q = query.trim().toLowerCase();
|
||||
if (!q) return true;
|
||||
|
||||
const haystack = [
|
||||
note.delivery_note_number ?? "",
|
||||
note.notes ?? "",
|
||||
note.customer_po ?? "",
|
||||
note.expand?.order?.order_number ?? "",
|
||||
note.expand?.customer?.customer ?? "",
|
||||
note.expand?.company?.company ?? "",
|
||||
]
|
||||
.join(" ")
|
||||
.toLowerCase();
|
||||
|
||||
// Every word must appear somewhere on the row.
|
||||
return q.split(/\s+/).every((word) => haystack.includes(word));
|
||||
}
|
||||
|
||||
/**
|
||||
* Which top-level tab a status belongs to. Kept next to
|
||||
* `deriveDnStage` so the tab buckets and the colour ladder can never
|
||||
* disagree about what "active" means.
|
||||
*
|
||||
* `cancelled` is deliberately NOT its own tab any more — the operator
|
||||
* asked for it to sit alongside completed under one "done" heading
|
||||
* with a filter between them, rather than as a third bucket that
|
||||
* needs a tab of its own for a small, cold set.
|
||||
*/
|
||||
export function dnTabForStatus(status: DeliveryNoteStatus): "active" | "done" {
|
||||
return isDoneStatus(status) ? "done" : "active";
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1065,17 +1065,39 @@ export async function updateCollectionPoolStatus(
|
|||
return updated;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch delivery notes for the list page.
|
||||
*
|
||||
* ## Why there is no `search` parameter any more
|
||||
*
|
||||
* This used to take a search term and push it into a PocketBase
|
||||
* filter over `delivery_note_number || notes`. Two problems, both
|
||||
* visible to the operator:
|
||||
*
|
||||
* 1. **It missed most of what a row shows.** Customer name, company
|
||||
* and order number were not searched — and "what's outstanding
|
||||
* for Acme" is the first thing anyone types on a list that is
|
||||
* grouped BY company. Those fields only arrive via `expand`, so
|
||||
* they can't be filtered server-side by this shape of query
|
||||
* without a join on every row.
|
||||
* 2. **The term was never escaped.** The `repId` clause on the very
|
||||
* next line escapes its quotes; the search clause didn't. A
|
||||
* customer called `Acme "Ltd"` — or anyone typing a stray
|
||||
* double quote — produced a broken filter literal and a list
|
||||
* that failed to load.
|
||||
*
|
||||
* So the search moved client-side (`matchesDeliveryNoteSearch` in
|
||||
* `src/lib/delivery-note-stage.ts`), over rows that are already
|
||||
* fetched WITH their customer / company / order expanded. One code
|
||||
* path, every visible field, and no way to build a malformed filter.
|
||||
*/
|
||||
export async function getDeliveryNotes(
|
||||
page = 1,
|
||||
perPage = 50,
|
||||
search = "",
|
||||
repId: string | null = null,
|
||||
) {
|
||||
const pb = getPocketBase();
|
||||
const clauses: string[] = [];
|
||||
if (search) {
|
||||
clauses.push(`(delivery_note_number ~ "${search}" || notes ~ "${search}")`);
|
||||
}
|
||||
if (repId) {
|
||||
clauses.push(`customer.rep = "${repId.replace(/"/g, '\\"')}"`);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue