* fix(stock): reset bin when no stock ledger entries remain
update_bin() only writes bins reachable through prev_sle_dict, and that
dict is empty once the last live sle for an item and warehouse is
cancelled or deleted. actual_qty is still recomputed, but stock_value
and valuation_rate stay stale and a repost cannot heal them, so bin
totals drift permanently from the stock balance.
zero those bins after the normal update, guarded by a re-check that no
live sle exists. also drop the prev_sle_dict seeding added earlier in
initialize_previous_data, which never took effect because
initialize_reposting() discards the dict before update_bin() reads it.
* test(stock): cover bin reset when ledger is empty
three cases that all leave an item and warehouse with no live sle:
cancelling the only voucher, deleting it with delete_linked_ledger_entries
on, and reposting over an already emptied ledger. each asserts actual_qty,
valuation_rate and stock_value are all zero.
* fix: skip zero-qty rows in make_sl_entries instead of reusing the previous entry
A row with zero actual_qty that is not a Stock Reconciliation never gets an
SLE, but the loop body still ran with the previous iteration's sle_doc:
repost_current_voucher and the bin update executed twice for the previous
row, or the whole call crashed with UnboundLocalError when the zero-qty row
came first. Skip such rows entirely.
* refactor: remove dead update_entries_after.update_bin_data
No callers anywhere in the codebase; it duplicates update_bin() with subtly
different semantics (no update_modified) and would only invite accidental
resurrection as a second Bin write path.
* refactor: rename bin.update_qty to update_qty_from_sle
Two unrelated functions circulated under the name update_bin_qty:
bin.update_qty (recomputes quantities from the ledger, aliased on import in
stock_ledger.py) and stock_balance.update_bin_qty (writes caller-supplied
absolute values, imported by six modules). Give the SLE-driven one a name
that states its semantics and drop the alias.
* test: cover both repost branches and the no-repost case
* fix: queue repost for entries backdated by a concurrent submit
---------
Co-authored-by: nareshkannasln <nareshkannashanmugam@gmail.com>
When the in-memory running rate is zero, the fallback went through
get_incoming_rate, whose previous-SLE lookup matches the same
posting_datetime and can land on a sibling line of the voucher being
replayed. Replace it with get_previous_sle_of_current_voucher excluding
the current voucher, keeping the get_valuation_rate chain when no
previous entry exists. get_incoming_rate is no longer used in this
module.
During repost, a return line with recalculate_rate resolved its moving
average rate through get_incoming_rate -> get_previous_sle, which matches
posting_datetime <= and orders by creation desc. For a multi-line return
of the same item, every line shares one posting_datetime, so the query
landed on a sibling line of the same voucher whose stored valuation_rate
was still the previous repost run's output, not the rate before the
voucher.
Each repost run therefore re-seeded the voucher from its own prior
output. The error gain per run is (qty returned at the stale rate) /
(qty remaining after the return), so whenever a return removes most of
the stock the loop diverges instead of converging, alternating sign and
growing until stock_value overflows decimal(21,9) and the repost dies
with 'Out of range value for column stock_value'.
Use the in-memory running valuation rate that update_entries_after
already tracks for the warehouse at this point in the repost. It is the
authoritative pre-entry state, is immune to sibling rows, and makes the
repost idempotent. The database lookup is kept only as a fallback for a
zero in-memory rate, preserving the existing zero-rate fallback chain.
* feat: validate stock value and stock closing entry before period closing
* fix: do not accept scoped stock closing entries as period closing prerequisite
* feat: seed batch valuation from stock closing balance and freeze closed-period stock
* fix: shift same-timestamp sibling SLEs when cancelling an entry
update_qty_in_future_sle compared against the reversal SLE's own
creation and skipped same-posting_datetime siblings on cancel, leaving
their qty_after_transaction stale and causing false negative stock
errors.
* fix: revert update_qty_in_future_sle cancel tie-break, it double-counted
Same hasattr pattern as repost_gate: an ERPNext ahead of its frappe build keeps
the status-quo serialization-failure retries instead of failing every stock
submission on postgres.
Postgres locking reads never see rows a concurrent transaction is inserting
(MariaDB's gap locks block the insert, then its locking reads return the fresh
row), so two concurrent writers for the same (item, warehouse) compute from the
same stale previous SLE and the loser overwrites Bin with a wrong absolute qty.
Today only the REPEATABLE READ serialization-failure retry catches this; the
gate makes correctness lock-based, covers the empty-history first-transaction
case (nothing exists to row-lock), and keeps negative-stock validation accurate
against concurrently inserted SLEs. Taken at the top of make_sl_entries (sorted
pairs, before the future_sle_exists cache warms) and in
update_entries_after.__init__ for the repost paths; re-entrant, released at
commit. MariaDB paths unchanged.
* fix: skip redundant reposting of dependent items
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: use earliest cascade datetime and batch repost item lookup
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
MariaDB falls back to the existing deadlock-retry path; the advisory-lock
serialization from #56697 now applies on Postgres only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- REPOST_LOCK_TIMEOUT 600 -> 300s: a contended waiter re-queues and frees its
long-queue worker slot sooner instead of pinning it for up to 10 minutes
(still well under the 1800s repost job timeout).
- Collision-free lock key: pass a ("stock_repost", item, warehouse) tuple so a
colon in item_code/warehouse can't map two distinct pairs onto one lock.
- Document that the gate is repost-vs-repost only; the synchronous
repost_current_voucher submit path is deliberately left ungated (gating a live
submit behind a background repost would be a worse regression).
Wraps per-(item, warehouse) reposting in repost_future_sle with a session-level advisory lock in front of the existing for-update row locks -- an outer gate that turns lock-order deadlocks into an orderly wait. Postgres/MariaDB only; nullcontext elsewhere. Row locks still enforce correctness. Requires frappe advisory_lock.
Conservative cleanup of frappe.throw/msgprint messages per the message style
guide; meaning, severity, and .format() arguments are unchanged:
- index bare {} placeholders as {0}/{1}/... so translators can reorder
- move f-strings / .format() / concatenation out of _() (they break gettext
extraction and never translate)
- wrap translatable dynamic values (DocType/Select labels) in _()
- fix grammar and colloquialisms
- drop no-op _() wrapping runtime-built strings
Part of #53976.
fix(stock): guard batchwise valuation-rate division against a zero divisor
get_valuation_rate's batchwise fallback selects
Sum(stock_value_difference) / Sum(actual_qty). When a batch's non-current
Stock Ledger Entries net to zero quantity (equal received and issued) the
divisor Sum(actual_qty) is 0. On MariaDB x/0 yields NULL and the caller's
`if last_valuation_rate and last_valuation_rate[0][0] is not None` check
falls through to the next strategy; on PostgreSQL float division by zero
raises `division by zero`, aborting the query (and the transaction).
Wrap the divisor in NullIf(Sum(actual_qty), 0) so a zero divisor yields
NULL on both engines, matching MariaDB and preserving the caller's
is-not-None fall-through. (stock_value_difference is Currency and actual_qty
is Float, so the division was already float — no integer-truncation change.)
It never references `self`. The deterministic-serial-value test added in #56249
called it as `get_incoming_value_for_serial_nos(None, sle, serial_nos)` — passing
None for self, which is fragile: a future `self.*` access would fail with an opaque
AttributeError. Declaring it @staticmethod makes the call honest
(`get_incoming_value_for_serial_nos(sle, serial_nos)`) and is backward compatible —
the method has no in-repo callers besides that test, and any `self.`-style call still
binds correctly to a staticmethod.
Addresses Greptile review feedback on #56249.
Convert four raw frappe.db.sql statements to frappe.qb:
- set_as_cancel (UPDATE -> frappe.qb.update)
- the invalid-serial-no incoming_rate lookup
- get_valuation_rate's last-valuation lookup
- get_future_sle_with_negative_qty
The serial-no comparisons (invalid-serial lookup and the get_stock_ledger_entries
condition builder, which stays raw) are wrapped in lower()/Lower() so serial
matching is case-insensitive on Postgres too -- MariaDB's collation already is,
so this is a no-op there. Deterministic creation/name tiebreakers are added to
the "ORDER BY posting_date DESC LIMIT 1" lookups so Postgres picks the same row
MariaDB did.
Surgical re-apply (not a whole-file port): develop's reposting valuation-recalc
clause (`recalculate_valuation_rate`) in update_entries_after and the
already-shipped Min()-wrapped get_items_to_be_repost GROUP BY are preserved. The
dynamic-condition / row-locking raw queries (get_previous_sle,
get_stock_ledger_entries builder, get_future_sle_with_negative_batch_qty, the
qty_shift UPDATE) are intentionally left raw.
Verified: full test_stock_ledger_entry suite 22/22 on MariaDB; added focused
tests for set_as_cancel / get_valuation_rate / get_future_sle_with_negative_qty
that pass on MariaDB and Postgres.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_items_to_be_repost selected posting_date/posting_time/creation/posting_datetime
alongside `group_by item_code, warehouse` with no aggregation -> arbitrary pick on
MariaDB, GroupingError on Postgres. Wrap the four columns in `Min()` (earliest row
per item+warehouse, the correct repost-start point; a single voucher's SLEs share
posting_date/time per group -> MariaDB-identical). This is reached by every stock
transaction submit/cancel via repost_future_sle_and_gle, so it unblocks the whole
transaction-heavy stock suite on Postgres (e.g. test_purchase_receipt 105/105).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
