From 7a5ee1aa5b6cc55ef16924bc3f8f8d7ffe95934b Mon Sep 17 00:00:00 2001
From: Jeppe B <2jepp9350@gmail.com>
Date: Mon, 3 Aug 2026 13:04:19 +0200
Subject: [PATCH] Fix invoice period tree review findings (#253)
## Summary
- preserve complete snapshot item payloads during inline edits and
reject partial text-field payloads
- force snapshot refreshes after parent/mutation changes with one
bounded recovery retry
- make legacy tree-action fallback create, confirm, and apply a fresh
compatible preview
- keep collection labeling localized and report the correct changed
count
## Verification
- focused object-tree and snapshot suites: 30 tests passed
- focused ESLint and `git diff --check` clean
- production build and selected-customer mocked Playwright flow passed
before final review fixes
- App Store Readiness and Qodana green on exact head; Automated Tests in
progress
- independent QA and reviewer gates: GO
Resolves all inline review threads on the current head.
---
.../components/InvoicingPeriodObjectTree.vue | 167 ++++++++++++------
.../services/invoicingPeriodTreeSnapshot.ts | 1 +
.../unit/invoicing-period-object-tree.spec.js | 165 ++++++++++++++++-
.../invoicing-period-tree-snapshot.spec.js | 1 +
4 files changed, 276 insertions(+), 58 deletions(-)
diff --git a/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/components/InvoicingPeriodObjectTree.vue b/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/components/InvoicingPeriodObjectTree.vue
index 8f3c89c2..5c2187ad 100644
--- a/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/components/InvoicingPeriodObjectTree.vue
+++ b/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/components/InvoicingPeriodObjectTree.vue
@@ -298,6 +298,18 @@ const refreshSnapshot = async ({ force = false, consumedKeys = [] as any[] } = {
}
}
};
+const refreshSnapshotAfterMutation = async ({ consumedKeys = [] as any[] } = {}) => {
+ if (!activeSnapshot.value) {
+ return null;
+ }
+ const refreshedSnapshot = await refreshSnapshot({ force: true, consumedKeys });
+ if (refreshedSnapshot || !activeSnapshot.value || legacySnapshotFallbackAllowed.value) {
+ return refreshedSnapshot;
+ }
+ // A mutation has already committed at this point. Give a transient snapshot
+ // failure one bounded retry so actions do not remain latched off indefinitely.
+ return refreshSnapshot({ force: true, consumedKeys });
+};
const expandableNodeKeys = computed(() =>
Object.values(nodeById.value)
.filter((node) => node?.isLeaf !== true && node?.disabled !== true && node?.id !== undefined && node?.id !== null)
@@ -345,9 +357,11 @@ watch(
);
watch(legacyDataSignature, () => {
- if (!activeSnapshot.value) {
- replaceRootNodes(buildLegacyRootNodes());
+ if (activeSnapshot.value) {
+ void refreshSnapshot({ force: true });
+ return;
}
+ replaceRootNodes(buildLegacyRootNodes());
});
watch(
@@ -1371,6 +1385,9 @@ const getNodeFlags = (node: TreeNode) => {
return [...entityFlags, ...fallbackFieldFlags];
};
const handleFlagStatusChanged = () => {
+ if (activeSnapshot.value) {
+ void refreshSnapshotAfterMutation();
+ }
emit("refresh");
};
@@ -1596,6 +1613,9 @@ const canEditOrderField = (order: any, column: string) => {
};
const canEditOrderItemField = () => SessionUser.hasPermission?.("edit_order_items") === true;
const refreshAfterInlineEdit = async () => {
+ if (activeSnapshot.value) {
+ await refreshSnapshotAfterMutation();
+ }
emit("refresh");
};
const collectionObjectForNode = (node: TreeNode) => {
@@ -1634,9 +1654,23 @@ const findCachedOrderItem = (itemId: number) => {
return match;
}
}
- return null;
+ return nodeById.value[makeNodeId(TREE_NODE_TYPES.ORDER_ITEM, itemId)]?.meta?.item ?? null;
};
const showEditOrderItemFieldForm = async (id: number, column: string, value: any, onAfterSubmit: any = null) => {
+ const item = findCachedOrderItem(toPositiveInteger(id) || 0);
+ const hasCompleteUpdatePayload =
+ item &&
+ Number.isFinite(Number(item.price)) &&
+ Number.isFinite(Number(item.quantity ?? item.amount)) &&
+ Object.prototype.hasOwnProperty.call(item, "reference") &&
+ Object.prototype.hasOwnProperty.call(item, "notes");
+ if (!hasCompleteUpdatePayload) {
+ return Swal.fire({
+ icon: "error",
+ title: loadFailedMessage(),
+ });
+ }
+
const result = await Swal.fire({
title: fieldLabel(
{
@@ -1658,21 +1692,18 @@ const showEditOrderItemFieldForm = async (id: number, column: string, value: any
return result;
}
- const item = findCachedOrderItem(toPositiveInteger(id) || 0);
const nextItem = {
- ...(item || {}),
+ ...item,
[column]: result.value,
};
await SessionUser.request("/order/items", "PUT", {
id,
- price: Number(nextItem.price ?? 0),
+ price: Number(nextItem.price),
notes: String(nextItem.notes ?? ""),
reference: String(nextItem.reference ?? ""),
- quantity: Number(nextItem.quantity ?? nextItem.amount ?? 1),
+ quantity: Number(nextItem.quantity ?? nextItem.amount),
});
- if (item) {
- item[column] = result.value;
- }
+ item[column] = result.value;
if (typeof onAfterSubmit === "function") {
await onAfterSubmit();
}
@@ -2049,6 +2080,9 @@ const runActionWithPreview = async (
timer: 1400,
showConfirmButton: false,
});
+ if (activeSnapshot.value) {
+ await refreshSnapshotAfterMutation();
+ }
emit("refresh");
} catch (error: any) {
await Swal.fire({
@@ -2123,6 +2157,47 @@ const chooseMergeTargetInvoiceCollection = async (invoiceCollectionIds: number[]
return normalizeInvoiceCollectionId(result.value);
};
+const confirmCollectionBulkActionPreview = async (action: string, previewRaw: any, preview: any) => {
+ const blockers = preview.blockers;
+ if (blockers.length > 0 || preview.changedCount === 0) {
+ await Swal.fire({
+ icon: blockers.length > 0 ? "error" : "info",
+ title:
+ blockers.length > 0
+ ? t("invoicing_period.invoice_collection_actions.preview.blocked_title")
+ : t("invoicing_period.invoice_collection_actions.preview.no_changes_title"),
+ html: renderBulkActionPreviewHtml(previewRaw),
+ });
+ return null;
+ }
+
+ const confirmation = await Swal.fire({
+ icon: "warning",
+ title: t("invoicing_period.invoice_collection_actions.preview.title", {
+ action: getBulkActionLabel(action),
+ }),
+ html: renderBulkActionPreviewHtml(previewRaw),
+ input: "text",
+ inputLabel: t("invoicing_period.invoice_collection_actions.preview.confirmation_label", {
+ phrase: preview.confirmationPhrase,
+ }),
+ inputPlaceholder: preview.confirmationPhrase,
+ showCancelButton: true,
+ confirmButtonText: t("invoicing_period.invoice_collection_actions.preview.confirm_button"),
+ cancelButtonText: commonText("cancel", "Annuller"),
+ inputValidator: (value) => {
+ if (String(value ?? "").trim() !== String(preview.confirmationPhrase ?? "")) {
+ return t("invoicing_period.invoice_collection_actions.preview.confirmation_mismatch", {
+ phrase: preview.confirmationPhrase,
+ });
+ }
+ return undefined;
+ },
+ });
+
+ return confirmation.isConfirmed ? confirmation : null;
+};
+
const runCollectionBulkAction = async (
action: string,
ids: any[] = collectionIds(),
@@ -2184,50 +2259,13 @@ const runCollectionBulkAction = async (
}
);
}
- const previewRaw = getApiPayload(previewResponse);
- const preview = normalizeInvoiceCollectionActionPreview(previewRaw, ids.length);
- const blockers = preview.blockers;
- const changedCount = preview.changedCount;
-
- if (blockers.length > 0 || changedCount === 0) {
- await Swal.fire({
- icon: blockers.length > 0 ? "error" : "info",
- title:
- blockers.length > 0
- ? t("invoicing_period.invoice_collection_actions.preview.blocked_title")
- : t("invoicing_period.invoice_collection_actions.preview.no_changes_title"),
- html: renderBulkActionPreviewHtml(previewRaw),
- });
- return;
- }
-
- const confirmation = await Swal.fire({
- icon: "warning",
- title: t("invoicing_period.invoice_collection_actions.preview.title", {
- action: getBulkActionLabel(action),
- }),
- html: renderBulkActionPreviewHtml(previewRaw),
- input: "text",
- inputLabel: t("invoicing_period.invoice_collection_actions.preview.confirmation_label", {
- phrase: preview.confirmationPhrase,
- }),
- inputPlaceholder: preview.confirmationPhrase,
- showCancelButton: true,
- confirmButtonText: t("invoicing_period.invoice_collection_actions.preview.confirm_button"),
- cancelButtonText: commonText("cancel", "Annuller"),
- inputValidator: (value) => {
- if (String(value ?? "").trim() !== String(preview.confirmationPhrase ?? "")) {
- return t("invoicing_period.invoice_collection_actions.preview.confirmation_mismatch", {
- phrase: preview.confirmationPhrase,
- });
- }
- return undefined;
- },
- });
-
- if (!confirmation.isConfirmed) {
+ let previewRaw = getApiPayload(previewResponse);
+ let preview = normalizeInvoiceCollectionActionPreview(previewRaw, ids.length);
+ let confirmation = await confirmCollectionBulkActionPreview(action, previewRaw, preview);
+ if (!confirmation) {
return;
}
+ let confirmationText = confirmation.value;
let applyResponse: any;
if (useTreeAction) {
@@ -2240,6 +2278,23 @@ const runCollectionBulkAction = async (
if (!isInvoiceCollectionTreeActionUnavailable(error)) {
throw error;
}
+ const legacyPreviewResponse = await SessionUser.objects.collectedOrderInvoices.functions.bulk_action_preview(
+ action,
+ ids,
+ options,
+ localeValue(),
+ {
+ customer_number: activeSnapshot.value?.customer_number,
+ snapshot_revision: activeSnapshot.value?.snapshot_revision,
+ }
+ );
+ previewRaw = getApiPayload(legacyPreviewResponse);
+ preview = normalizeInvoiceCollectionActionPreview(previewRaw, ids.length);
+ confirmation = await confirmCollectionBulkActionPreview(action, previewRaw, preview);
+ if (!confirmation) {
+ return;
+ }
+ confirmationText = confirmation.value;
useTreeAction = false;
}
}
@@ -2249,7 +2304,7 @@ const runCollectionBulkAction = async (
action,
invoice_collection_ids: ids,
options,
- confirmation_text: confirmation.value,
+ confirmation_text: confirmationText,
locale: localeValue(),
customer_number: activeSnapshot.value?.customer_number,
snapshot_revision: activeSnapshot.value?.snapshot_revision,
@@ -2285,7 +2340,7 @@ const runCollectionBulkAction = async (
rootNodes.value.forEach((node) => collectConsumedNodeKeys(node));
const consumedKeys = checkedKeys.value.filter((key) => consumedNodeKeys.has(String(key)));
if (activeSnapshot.value) {
- const refreshedSnapshot = await refreshSnapshot({ force: true, consumedKeys });
+ const refreshedSnapshot = await refreshSnapshotAfterMutation({ consumedKeys });
if (!refreshedSnapshot) {
const consumed = new Set(consumedKeys.map(String));
checkedKeys.value = checkedKeys.value.filter((key) => !consumed.has(String(key)));
@@ -2297,7 +2352,7 @@ const runCollectionBulkAction = async (
icon: "success",
title: t("invoicing_period.invoice_collection_actions.success_title"),
text: t("invoicing_period.invoice_collection_actions.success_text", {
- count: applied?.result?.changed_count ?? applied?.summary?.changed_count ?? changedCount,
+ count: applied?.result?.changed_count ?? applied?.summary?.changed_count ?? preview.changedCount,
}),
timer: 2000,
showConfirmButton: false,
@@ -2305,7 +2360,7 @@ const runCollectionBulkAction = async (
emit("refresh");
} catch (error: any) {
if (activeSnapshot.value && Number(error?.response?.status) === 409) {
- await refreshSnapshot({ force: true });
+ await refreshSnapshotAfterMutation();
}
await Swal.fire({
icon: "error",
diff --git a/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/services/invoicingPeriodTreeSnapshot.ts b/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/services/invoicingPeriodTreeSnapshot.ts
index 990ef86b..e49155d4 100644
--- a/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/services/invoicingPeriodTreeSnapshot.ts
+++ b/src/views/dashboards/superUserDashboard/InvoicingBillingPeriod/services/invoicingPeriodTreeSnapshot.ts
@@ -326,6 +326,7 @@ export const buildCompleteSnapshotRootNodes = (snapshot: CompleteInvoicingPeriod
const customer = snapshot.customer;
const collectionNodes = [...snapshot.collections].sort((left, right) => left.id - right.id).map((collection) => {
const node = makeCollectionNode(collection.id, collection.orders, customer, {
+ label: labels.collection(collection.id),
collectionSummary: collection,
invoiceState: asString(collection.invoice_state ?? collection.state),
completeOrderCount: collection.complete_order_count,
diff --git a/tests/unit/invoicing-period-object-tree.spec.js b/tests/unit/invoicing-period-object-tree.spec.js
index b986f6ef..8e2e1122 100644
--- a/tests/unit/invoicing-period-object-tree.spec.js
+++ b/tests/unit/invoicing-period-object-tree.spec.js
@@ -114,8 +114,8 @@ vi.mock("vue-i18n", () => ({
vi.mock("@/components/displays/buttons/EditableTableColumn.vue", () => ({
default: {
name: "EditableTableColumn",
- props: ["object", "column", "parseFunction"],
- template: `{{ parseFunction ? parseFunction(object?.[column]) : object?.[column] }}`,
+ props: ["object", "column", "parseFunction", "editFunction", "loadList", "cellTestId"],
+ template: ``,
},
}));
@@ -344,6 +344,17 @@ const expandNode = async (wrapper, key) => {
describe("InvoicingPeriodObjectTree", () => {
beforeEach(() => {
vi.clearAllMocks();
+ Swal.fire.mockReset().mockResolvedValue({ isConfirmed: false });
+ mocks.request.mockImplementation(async (url, method) => {
+ if (url === "/order/items" && method === "GET") {
+ return {
+ data: {
+ data: mocks.orderItems,
+ },
+ };
+ }
+ return { data: { data: [] } };
+ });
mocks.orderItems.splice(0, mocks.orderItems.length, {
id: 501,
order_id: 9001,
@@ -402,6 +413,89 @@ describe("InvoicingPeriodObjectTree", () => {
expect(price.find(".editable-table-column-stub").exists()).toBe(true);
});
+ it("preserves untouched snapshot item values during an inline edit", async () => {
+ mocks.orderItems[0].reference = "KEEP-REFERENCE";
+ mocks.orderItems[0].notes = "KEEP-NOTES";
+ mocks.request.mockImplementation(async (url, method) => {
+ if (url === "/superuser/invoicing/period/tree" && method === "GET") {
+ return snapshotResponse("rev-edit");
+ }
+ return { data: { data: [] } };
+ });
+ Swal.fire.mockResolvedValueOnce({ isConfirmed: true, value: "135" });
+ const wrapper = mountTree({ capabilities: { object_tree_v2: true } });
+ await flushPromises();
+
+ await expandNode(wrapper, "collected_order_invoice:3001");
+ await expandNode(wrapper, "category:3001:collection_orders");
+ await expandNode(wrapper, "order:9001");
+ await expandNode(wrapper, "category:9001:order_items");
+ await wrapper.get('[data-testid="invoice-period-tree-field-editor-order_item:501-price"]').trigger("click");
+ await flushPromises();
+
+ expect(mocks.request).toHaveBeenCalledWith("/order/items", "PUT", {
+ id: 501,
+ price: 135,
+ notes: "KEEP-NOTES",
+ reference: "KEEP-REFERENCE",
+ quantity: 2,
+ });
+ expect(mocks.request.mock.calls.filter(([url]) => url === "/superuser/invoicing/period/tree")).toHaveLength(2);
+ });
+
+ it("rejects partial snapshot items before a full inline update can erase text fields", async () => {
+ delete mocks.orderItems[0].notes;
+ mocks.request.mockImplementation(async (url, method) => {
+ if (url === "/superuser/invoicing/period/tree" && method === "GET") {
+ return snapshotResponse("rev-partial-item");
+ }
+ return { data: { data: [] } };
+ });
+ const wrapper = mountTree({ capabilities: { object_tree_v2: true } });
+ await flushPromises();
+
+ await expandNode(wrapper, "collected_order_invoice:3001");
+ await expandNode(wrapper, "category:3001:collection_orders");
+ await expandNode(wrapper, "order:9001");
+ await expandNode(wrapper, "category:9001:order_items");
+ await wrapper.get('[data-testid="invoice-period-tree-field-editor-order_item:501-price"]').trigger("click");
+ await flushPromises();
+
+ expect(mocks.request.mock.calls.some(([url, method]) => url === "/order/items" && method === "PUT")).toBe(false);
+ expect(Swal.fire).toHaveBeenCalledWith(expect.objectContaining({ icon: "error" }));
+ });
+
+ it("retries a transient snapshot failure after an inline mutation", async () => {
+ let snapshotCalls = 0;
+ mocks.request.mockImplementation(async (url, method) => {
+ if (url === "/superuser/invoicing/period/tree" && method === "GET") {
+ snapshotCalls += 1;
+ if (snapshotCalls === 2) {
+ throw new Error("temporary snapshot failure");
+ }
+ return snapshotResponse(`rev-retry-${snapshotCalls}`, { queue_economic: true });
+ }
+ return { data: { data: [] } };
+ });
+ Swal.fire.mockResolvedValueOnce({ isConfirmed: true, value: "135" });
+ const wrapper = mountTree({ capabilities: { object_tree_v2: true } });
+ await flushPromises();
+
+ await expandNode(wrapper, "collected_order_invoice:3001");
+ await expandNode(wrapper, "category:3001:collection_orders");
+ await expandNode(wrapper, "order:9001");
+ await expandNode(wrapper, "category:9001:order_items");
+ await wrapper.get('[data-testid="invoice-period-tree-field-editor-order_item:501-price"]').trigger("click");
+ await flushPromises();
+
+ expect(snapshotCalls).toBe(3);
+ expect(
+ wrapper
+ .get('[data-testid="invoice-period-tree-action-wheel-collected_order_invoice:3001"]')
+ .attributes("data-action-count")
+ ).toBe("1");
+ });
+
it("keeps a visible expand-all toggle and reuses loaded branches after collapse", async () => {
const wrapper = mountTree();
const toggle = () => wrapper.get('[data-testid="invoice-period-tree-toggle-all"]');
@@ -796,6 +890,73 @@ describe("InvoicingPeriodObjectTree", () => {
expect(mocks.request.mock.calls.some(([url]) => url === "/order/items")).toBe(false);
});
+ it("refreshes an active snapshot when parent transaction data changes", async () => {
+ mocks.request
+ .mockResolvedValueOnce(snapshotResponse("rev-parent-1", { queue_economic: true }))
+ .mockResolvedValueOnce(snapshotResponse("rev-parent-2", { queue_economic: true }));
+ const wrapper = mountTree({ capabilities: { object_tree_v2: true } });
+ await flushPromises();
+
+ await wrapper.setProps({
+ transactions: [{ ...mocks.order, booked: true, total_net_amount: 245 }],
+ });
+ await flushPromises();
+
+ expect(mocks.request.mock.calls.filter(([url]) => url === "/superuser/invoicing/period/tree")).toHaveLength(2);
+ });
+
+ it("creates and confirms a legacy preview before falling back from tree apply", async () => {
+ const actions = { queue_economic: true };
+ mocks.request.mockResolvedValueOnce(snapshotResponse("rev-fallback", actions));
+ SessionUser.objects.collectedOrderInvoices.functions.period_tree_action_preview.mockResolvedValueOnce({
+ data: {
+ data: {
+ preview_id: "tree-preview",
+ confirmation_phrase: "CONFIRM TREE",
+ summary: { collection_count: 1, changed_count: 1 },
+ blockers: [],
+ },
+ },
+ });
+ SessionUser.objects.collectedOrderInvoices.functions.period_tree_action_apply.mockRejectedValueOnce({
+ response: { status: 404 },
+ });
+ SessionUser.objects.collectedOrderInvoices.functions.bulk_action_preview.mockResolvedValueOnce({
+ data: {
+ data: {
+ preview_id: "legacy-preview",
+ confirmation_phrase: "CONFIRM LEGACY",
+ summary: { collection_count: 1, changed_count: 1 },
+ blockers: [],
+ },
+ },
+ });
+ SessionUser.objects.collectedOrderInvoices.functions.bulk_action_apply.mockResolvedValueOnce({
+ data: { data: { jobs: [{ id: 1 }] } },
+ });
+ Swal.fire
+ .mockResolvedValueOnce({ isConfirmed: true, value: "CONFIRM TREE" })
+ .mockResolvedValueOnce({ isConfirmed: true, value: "CONFIRM LEGACY" });
+
+ const wrapper = mountTree({ capabilities: { object_tree_v2: true } });
+ await flushPromises();
+ await wrapper.get('[data-node-key="collected_order_invoice:3001"] .b-checkbox-stub').trigger("click");
+ await flushPromises();
+ await wrapper.get('[data-testid="invoice-period-tree-actions-trigger-collected_order_invoice"]').trigger("click");
+ await wrapper.get('[data-testid="invoice-period-tree-action-collection:queue-economic"]').trigger("click");
+ await flushPromises();
+
+ expect(SessionUser.objects.collectedOrderInvoices.functions.bulk_action_preview).toHaveBeenCalledTimes(1);
+ expect(SessionUser.objects.collectedOrderInvoices.functions.bulk_action_apply).toHaveBeenCalledWith(
+ expect.objectContaining({
+ preview_id: "legacy-preview",
+ confirmation_text: "CONFIRM LEGACY",
+ })
+ );
+ expect(Swal.fire.mock.calls.some(([options]) => options?.icon === "success")).toBe(true);
+ expect(Swal.fire.mock.calls.some(([options]) => options?.icon === "error")).toBe(false);
+ });
+
it("keeps legacy collection mutations disabled while a superseding snapshot request is pending", async () => {
const pending = [];
mocks.request.mockImplementation((url, _method, _parameters, _catcher, _then, options) =>
diff --git a/tests/unit/invoicing-period-tree-snapshot.spec.js b/tests/unit/invoicing-period-tree-snapshot.spec.js
index 32af911d..6fabd58c 100644
--- a/tests/unit/invoicing-period-tree-snapshot.spec.js
+++ b/tests/unit/invoicing-period-tree-snapshot.spec.js
@@ -89,6 +89,7 @@ describe("invoicing period selected-customer tree snapshot", () => {
});
const roots = buildCompleteSnapshotRootNodes(snapshot, labels);
+ expect(roots[0].label).toBe("Collection 3001");
expect(roots[0].meta).toMatchObject({
periodOrderCount: 1,
periodTotalNetAmount: 200,