fix(buyer-tasks): use getFullList and tighten item_status filter for po_draft/po_sent
- Switch from getList(1, 500) to getFullList so candidate sets
aren't silently truncated at 500 rows (matches orders-service
and quotes-service conventions).
- Tighten the po_draft filter to item_status = po_generated so
items in downstream statuses (collection_pool, delivered, etc.)
don't get pulled over the wire.
- Tighten the po_sent filter to exclude item_status in
{ready_delivery, invoiced, delivered, cancelled} for the same
reason.
- Drop the redundant ready_delivery post-filter on po_sent now
that the PB layer handles it.
- Tests capture the filter string passed to getFullList and assert
its clauses so regressions are caught.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
49acc04fd1
commit
b4c9f2d2ce
2 changed files with 138 additions and 114 deletions
|
|
@ -57,20 +57,26 @@ const baseItem = (overrides: Record<string, unknown> = {}) => ({
|
|||
...overrides,
|
||||
});
|
||||
|
||||
// Helper to build a mock admin client. Returns the mock so callers
|
||||
// can spy on the filter passed to getFullList.
|
||||
const makeAdmin = (items: Record<string, unknown>[]) => {
|
||||
const getFullList = vi.fn().mockResolvedValue(items);
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn(), // unused — we now call getFullList
|
||||
getFullList,
|
||||
}),
|
||||
};
|
||||
return { admin, getFullList };
|
||||
};
|
||||
|
||||
describe("listBuyerTasks", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("returns groups for items in needs_po state", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [baseItem()],
|
||||
totalItems: 1,
|
||||
}),
|
||||
}),
|
||||
};
|
||||
const { admin, getFullList } = makeAdmin([baseItem()]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("needs_po");
|
||||
|
|
@ -86,20 +92,29 @@ describe("listBuyerTasks", () => {
|
|||
totalCost: 50,
|
||||
step: "needs_po",
|
||||
});
|
||||
expect(getFullList).toHaveBeenCalledTimes(1);
|
||||
expect(getFullList).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ filter: expect.stringContaining("getList") }),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses getFullList (not the paginated getList) to avoid silent truncation", async () => {
|
||||
const { admin, getFullList } = makeAdmin([baseItem()]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
const collectionMock = admin.collection as unknown as ReturnType<typeof vi.fn>;
|
||||
|
||||
await listBuyerTasks("needs_po");
|
||||
|
||||
expect(collectionMock).toHaveBeenCalledWith("order_items");
|
||||
expect(getFullList).toHaveBeenCalledTimes(1);
|
||||
expect(collectionMock.mock.results[0].value.getList).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("groups multiple items from the same (order, supplier) pair", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [
|
||||
baseItem(),
|
||||
baseItem({ id: "item-2", quantity: 3, cost_price: 10 }),
|
||||
],
|
||||
totalItems: 2,
|
||||
}),
|
||||
}),
|
||||
};
|
||||
const { admin } = makeAdmin([
|
||||
baseItem(),
|
||||
baseItem({ id: "item-2", quantity: 3, cost_price: 10 }),
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("needs_po");
|
||||
|
|
@ -110,17 +125,10 @@ describe("listBuyerTasks", () => {
|
|||
});
|
||||
|
||||
it("skips items with no supplier", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [
|
||||
baseItem(),
|
||||
baseItem({ id: "item-2", supplier: "", expand: { order: baseOrder, supplier: undefined, purchase_order: undefined } }),
|
||||
],
|
||||
totalItems: 2,
|
||||
}),
|
||||
}),
|
||||
};
|
||||
const { admin } = makeAdmin([
|
||||
baseItem(),
|
||||
baseItem({ id: "item-2", supplier: "", expand: { order: baseOrder, supplier: undefined, purchase_order: undefined } }),
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("needs_po");
|
||||
|
|
@ -129,25 +137,28 @@ describe("listBuyerTasks", () => {
|
|||
expect(groups[0].itemCount).toBe(1);
|
||||
});
|
||||
|
||||
it("builds the needs_po filter from processed + empty PO", async () => {
|
||||
const { admin, getFullList } = makeAdmin([baseItem()]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
await listBuyerTasks("needs_po");
|
||||
|
||||
const filter = (getFullList.mock.calls[0]?.[0] as { filter: string } | undefined)?.filter;
|
||||
expect(filter).toBe(`item_status = "processed" && purchase_order = ""`);
|
||||
});
|
||||
|
||||
it("returns groups filtered by po_draft tab", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: basePo,
|
||||
},
|
||||
}),
|
||||
],
|
||||
totalItems: 1,
|
||||
}),
|
||||
const { admin, getFullList } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: basePo,
|
||||
},
|
||||
}),
|
||||
};
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_draft");
|
||||
|
|
@ -156,27 +167,28 @@ describe("listBuyerTasks", () => {
|
|||
expect(groups[0].step).toBe("po_draft");
|
||||
expect(groups[0].purchaseOrderId).toBe("po-1");
|
||||
expect(groups[0].poStatus).toBe("draft");
|
||||
|
||||
// The po_draft filter must restrict to po_generated items so the
|
||||
// PB query doesn't pull items in downstream statuses (collection
|
||||
// pool, delivered, etc.) over the wire.
|
||||
const filter = (getFullList.mock.calls[0]?.[0] as { filter: string } | undefined)?.filter;
|
||||
expect(filter).toContain(`item_status = "po_generated"`);
|
||||
expect(filter).toContain(`purchase_order != ""`);
|
||||
expect(filter).not.toContain("ready_delivery");
|
||||
});
|
||||
|
||||
it("excludes sent/cancelled POs from po_draft tab", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: { id: "po-1", status: "sent" },
|
||||
},
|
||||
}),
|
||||
],
|
||||
totalItems: 1,
|
||||
}),
|
||||
const { admin } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: { id: "po-1", status: "sent" },
|
||||
},
|
||||
}),
|
||||
};
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_draft");
|
||||
|
|
@ -185,24 +197,53 @@ describe("listBuyerTasks", () => {
|
|||
});
|
||||
|
||||
it("returns groups for po_sent tab", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: { id: "po-1", status: "sent" },
|
||||
},
|
||||
}),
|
||||
],
|
||||
totalItems: 1,
|
||||
}),
|
||||
const { admin, getFullList } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "po_generated",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: { id: "po-1", status: "sent" },
|
||||
},
|
||||
}),
|
||||
};
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_sent");
|
||||
|
||||
expect(groups).toHaveLength(1);
|
||||
expect(groups[0].step).toBe("po_sent");
|
||||
|
||||
// The po_sent filter must keep the in-flight item statuses and
|
||||
// explicitly drop downstream ones (delivered, invoiced, cancelled)
|
||||
// so they don't pollute the wire payload.
|
||||
const filter = (getFullList.mock.calls[0]?.[0] as { filter: string } | undefined)?.filter;
|
||||
expect(filter).toContain(`purchase_order != ""`);
|
||||
expect(filter).toContain(`item_status != "ready_delivery"`);
|
||||
expect(filter).toContain(`item_status != "invoiced"`);
|
||||
expect(filter).toContain(`item_status != "delivered"`);
|
||||
expect(filter).toContain(`item_status != "cancelled"`);
|
||||
expect(filter).not.toContain(`item_status = "po_generated"`);
|
||||
});
|
||||
|
||||
it("does not include a separate ready_delivery exclusion in po_sent post-filter", async () => {
|
||||
// The ready_delivery exclusion moved to the PB filter (covered by
|
||||
// the filter-content assertions above). Verify the service still
|
||||
// accepts a PO-sent item in a non-ready_delivery status, which
|
||||
// means we don't accidentally re-introduce a redundant
|
||||
// post-filter that would silently drop legitimate groups.
|
||||
const { admin } = makeAdmin([
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "collection_pool",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: { id: "po-1", status: "sent" },
|
||||
},
|
||||
}),
|
||||
]);
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_sent");
|
||||
|
|
@ -210,30 +251,4 @@ describe("listBuyerTasks", () => {
|
|||
expect(groups).toHaveLength(1);
|
||||
expect(groups[0].step).toBe("po_sent");
|
||||
});
|
||||
|
||||
it("excludes ready_delivery items from po_sent tab", async () => {
|
||||
const admin = {
|
||||
collection: vi.fn().mockReturnValue({
|
||||
getList: vi.fn().mockResolvedValue({
|
||||
items: [
|
||||
baseItem({
|
||||
purchase_order: "po-1",
|
||||
item_status: "ready_delivery",
|
||||
expand: {
|
||||
order: baseOrder,
|
||||
supplier: baseSupplier,
|
||||
purchase_order: { id: "po-1", status: "sent" },
|
||||
},
|
||||
}),
|
||||
],
|
||||
totalItems: 1,
|
||||
}),
|
||||
}),
|
||||
};
|
||||
vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
|
||||
|
||||
const groups = await listBuyerTasks("po_sent");
|
||||
|
||||
expect(groups).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -37,18 +37,26 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise<BuyerTaskGroup[
|
|||
|
||||
// Item-level filter narrows the candidates. PO-status filtering
|
||||
// happens post-fetch because the field lives on a related record.
|
||||
// The item_status filter is tightened per tab to cut down the wire
|
||||
// payload: PB filters can't traverse relations, so we can't ask for
|
||||
// "items whose PO is draft" directly. Instead we ask for "items in
|
||||
// the item_status states that imply a draft PO" (po_generated) for
|
||||
// the po_draft tab, and the broader set of in-flight statuses for
|
||||
// the po_sent tab.
|
||||
const itemFilter = tab === "needs_po"
|
||||
? `item_status = "processed" && purchase_order = ""`
|
||||
: `purchase_order != "" && item_status != "ready_delivery"`;
|
||||
: tab === "po_draft"
|
||||
? `item_status = "po_generated" && purchase_order != ""`
|
||||
: `purchase_order != "" && item_status != "ready_delivery" && item_status != "invoiced" && item_status != "delivered" && item_status != "cancelled"`;
|
||||
|
||||
const itemsRes = await admin.collection("order_items").getList(1, 500, {
|
||||
const items = await admin.collection("order_items").getFullList({
|
||||
filter: itemFilter,
|
||||
expand: "order,order.customer,order.customer.company,order.company,supplier,purchase_order",
|
||||
});
|
||||
|
||||
// Group by (orderId, supplierId); skip items with no supplier.
|
||||
const groups = new Map<string, BuyerTaskGroup>();
|
||||
for (const item of itemsRes.items) {
|
||||
for (const item of items) {
|
||||
const orderId = item.order;
|
||||
const supplierId = item.supplier;
|
||||
if (!orderId || !supplierId) continue;
|
||||
|
|
@ -57,11 +65,12 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise<BuyerTaskGroup[
|
|||
| { id: string; status: BuyerTaskGroup["poStatus"] }
|
||||
| undefined;
|
||||
|
||||
// Post-filter by PO status (tab-specific).
|
||||
// Post-filter by PO status (tab-specific). The PB filter can't
|
||||
// traverse to the purchase_orders relation, so we still need to
|
||||
// slice by PO status here even though the item_status filter
|
||||
// already narrowed the candidate set on the wire.
|
||||
if (tab === "po_draft" && po?.status !== "draft") continue;
|
||||
if (tab === "po_sent" && !["sent", "sending"].includes(po?.status ?? "")) continue;
|
||||
// Exclude ready_delivery items from po_sent (already delivered).
|
||||
if (tab === "po_sent" && item.item_status === "ready_delivery") continue;
|
||||
|
||||
const groupId = `${orderId}::${supplierId}`;
|
||||
let group = groups.get(groupId);
|
||||
|
|
|
|||
Loading…
Reference in a new issue