feat: enhance offline batch and serial number management

[MohamedAliSmk]

• Added functions to persist and consume batch and serial data for items in offline mode.
• Implemented caching for batch and serial numbers during item searches.
• Improved handling of cached data to ensure accurate updates and retrievals when offline.
• Enhanced error handling for background caching operations.

---

🛑 Review action list — do not merge/close until all checked

Full review · 11 inline comments · details in each

Blocking

• T1 Batch qty subtracted twice offline — delete consumeCachedBatchQty
• T2 Batch consume has no return path (4 drift paths) — fixed by T1
• T3 Sold serials returned to cache on offline submit — flag in clearCart

Should fix

• T4 Cache mutators non-atomic — wrap in db.transaction("rw", …)
• T5 Offline serial fallback skips isCacheValid (warehouse + TTL)
• T6 Up to 25 sequential POSTs per search — drop pagination sites, dedupe

Nice to have

• T7 Phantom db.items rows + undefined warehouse guard
• T8 Vitest config + one items.test.js (repo has zero tests)
• T9 Wrap 118-char import

[engahmed1190]

🔍 Full review — code logic, security, performance, coverage

Verdict: request changes. The offline read fix is correct and should ship. The consumption-accounting layer bolted on top has three high-severity defects and wasn't part of PN-83.

Structural read

What actually fixes "Batch/Serial unable to select at offline mode" is ~30 lines:

Change	Why it fixes PN-83
persistItemBatchSerialData in batchesResource.onSuccess	the dialog's own batch fetch was never written to IndexedDB
persistItemBatchSerialData in fetchSerials	serial store results were never written to IndexedDB
the two new return guards in loadBatchesOrSerials	offline previously fell through to batchesResource.reload(), which fails

Everything else — the three new cacheBatchSerialForItems call sites, consumeCachedBatchQty, consumeCachedSerials, returnCachedSerials, and the create-on-missing db.items.put — is offline inventory accounting. Every defect below lives in that second layer. Cutting it removes ~90 of the 161 added lines and every high-severity finding.

Security

No new attack surface. No new endpoints, no new server-side code, no HTML/URL/query sink. get_batch_serial_data_for_items is @frappe.whitelist() and unchanged by this diff. All new code writes client-side IndexedDB, already trusted-per-device here.

The security-relevant findings are the inventory-integrity ones: T1/T2 understate stock, T3 lets the same serial reach two invoices. For a POS that's the risk surface that matters.

Coverage

Zero. find . -name "*.test.*" returns nothing, and there's no test block in POS/vite.config.js despite test / test:run / test:coverage in package.json — yarn test cannot run today. This diff adds four pure-ish mutators with real arithmetic (set diffing, Math.max(0, qty - n), sort-merge); they're the most testable and highest-consequence code in the changeset.

Verified

• Patch applies cleanly to current develop (git apply --3way) — no conflicts, no duplicated call sites.
• npx biome lint on the changed files: no new violations (one pre-existing useOptionalChain at BatchSerialDialog.vue:365).
• T1 reproduced with a standalone script — see the inline comment.

---

✅ Task list — please do not merge or close this PR until every box is checked

🔴 Blocking

• T1 — Batch qty subtracted twice offline. Delete consumeCachedBatchQty; availableBatches already handles cart subtraction. BatchSerialDialog.vue:516
• T2 — Batch consumption has no return path (4 divergence paths: line remove, qty edit, addItem uom-merge dropping batch_no, addItem throw). Resolved by T1. items.js:254
• T3 — Sold serials returned to durable cache on offline submit. Fix in clearCart (useInvoice.js:1182) with a returnSerials flag, false from the two post-submit call sites. serialNumber.js:182

🟡 Should fix

• T4 — Wrap the surviving cache mutators in db.transaction("rw", db.items, ...); they're non-atomic read-modify-write with .catch(() => {}) at every call site. items.js:210
• T5 — Offline serial fallback bypasses isCacheValid (warehouse + 5-min TTL). BatchSerialDialog.vue:460
• T6 — Drop the two pagination cacheBatchSerialForItems sites; keep the search one with a recently-fetched Set + TTL. Up to 25 sequential POSTs per search today. itemSearch.js:1224, 1339, 1725

🟢 Nice to have

• T7 — Don't put phantom item rows (no item_name/barcodes); guard the undefined warehouse in returnCachedSerials. items.js:173, 236
• T8 — Add a test block to POS/vite.config.js + one items.test.js with db mocked, covering the consume/return round-trip, Math.max(0, …) clamping, and parseSerialNumbers on both string and array input. Not a suite — one file.
• T9 — Wrap the 118-char import at BatchSerialDialog.vue:304.

Latent / no action needed this PR

• Persisted batch/serial data has no warehouse key. Harmless today (dialog is bound to shiftStore.profileWarehouse, POSSale.vue:613) but a shift/profile change leaves stale data with nothing to invalidate it. Worth an issue.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_016WM1HqLHEhF8GhYjB4EJHp

[MohamedAliSmk]

Review follow-up (local, not pushed yet)

Addressed T1–T9 as 9 separate commits on PN-83-Batch-Serial-unable-to-select-at-offline-mode. Inline replies are on each thread.

Commit	Point
4b5eb310	T1 — remove consumeCachedBatchQty call
59e6f51a	T2 — delete consumeCachedBatchQty
65fba4f3	T3 — clearCart({ returnSerials: false }) post-submit
b2adb1cf	T4 — Dexie transactions on surviving mutators
ddf82e24	T5 — drop invalid in-memory serial fallback
bd6f34d4	T6 — drop pagination sites; TTL-dedupe search
3d879240	T7 — no phantom rows; warehouse guard on return
93d58875	T8 — vite test block + items.test.js (5 tests green)
438f9ddb	T9 — wrap offline items import

T8 note: Math.max(0, …) clamping lived only in consumeCachedBatchQty, which left with T1/T2 — tests cover consume/return round-trip, parseSerialNumbers, phantom skip, and warehouse guard instead.

Holding push until author review of the local commits.

[MohamedAliSmk]

T8 — tests

Agreed the mutators were the most testable / highest-consequence part of the changeset, and yarn test couldn’t run without a Vite test block.

Done in 93d58875:

• added a test block to POS/vite.config.js (jsdom, src/**/*.{test,spec}.{js,ts})
• added POS/src/utils/offline/items.test.js with db mocked

Coverage included:

• parseSerialNumbers on string + array (+ empty)
• consume → return round-trip
• phantom-row skip when item is missing
• warehouse guard when cache is empty

Note: Math.max(0, …) clamping lived only inside consumeCachedBatchQty, which was removed in T1/T2, so that case isn’t tested anymore.

yarn test:run src/utils/offline/items.test.js → 5 passed.