mirror of
https://github.com/frappe/erpnext.git
synced 2026-09-24 05:47:15 +00:00
fix: do not zero out backdated stock at a stock reco adjustment entry (#59271)
* fix: do not zero out backdated stock at a stock reco adjustment entry * fix: keep a stock reco adjustment entry value-only on cancel and refresh * fix: read stock reco adjustment rows once and value them from the ledger
This commit is contained in:
@@ -800,7 +800,19 @@ class StockReconciliation(StockController):
|
||||
)
|
||||
)
|
||||
|
||||
def get_stranded_stock_value(self, row) -> float:
|
||||
def get_balance_before_reconciliation(self, row) -> dict:
|
||||
from erpnext.stock.stock_ledger import get_previous_sle
|
||||
|
||||
return get_previous_sle(
|
||||
{
|
||||
"item_code": row.item_code,
|
||||
"warehouse": row.warehouse,
|
||||
"posting_date": self.posting_date,
|
||||
"posting_time": self.posting_time,
|
||||
}
|
||||
)
|
||||
|
||||
def get_stranded_stock_value(self, row, previous_sle=None) -> float:
|
||||
"""Stock value the ledger still carries for an item-warehouse that has no quantity on hand.
|
||||
|
||||
This is what an adjustment entry writes off. The write-off is measured at item-warehouse
|
||||
@@ -809,16 +821,10 @@ class StockReconciliation(StockController):
|
||||
at an already empty batch while other batches of the same item still hold stock would
|
||||
otherwise write off the valuation of the stock that remains.
|
||||
"""
|
||||
from erpnext.stock.stock_ledger import get_previous_sle, get_stock_value_difference
|
||||
from erpnext.stock.stock_ledger import get_stock_value_difference
|
||||
|
||||
previous_sle = get_previous_sle(
|
||||
{
|
||||
"item_code": row.item_code,
|
||||
"warehouse": row.warehouse,
|
||||
"posting_date": self.posting_date,
|
||||
"posting_time": self.posting_time,
|
||||
}
|
||||
)
|
||||
if previous_sle is None:
|
||||
previous_sle = self.get_balance_before_reconciliation(row)
|
||||
|
||||
if flt(previous_sle.get("qty_after_transaction")):
|
||||
return 0.0
|
||||
@@ -828,13 +834,23 @@ class StockReconciliation(StockController):
|
||||
)
|
||||
|
||||
def make_adjustment_entry(self, row, sl_entries):
|
||||
difference_amount = self.get_stranded_stock_value(row)
|
||||
previous_sle = self.get_balance_before_reconciliation(row)
|
||||
difference_amount = self.get_stranded_stock_value(row, previous_sle=previous_sle)
|
||||
|
||||
if not difference_amount:
|
||||
# rounded, so float dust does not post an entry whose GL counterpart rounds away to zero
|
||||
if not flt(difference_amount, self.precision("difference_amount")):
|
||||
return
|
||||
|
||||
args = self.get_sle_for_items(row)
|
||||
args.update({"stock_value_difference": -1 * difference_amount, "is_adjustment_entry": 1})
|
||||
args.update(
|
||||
{
|
||||
"stock_value_difference": -1 * difference_amount,
|
||||
# the row carries no rate, so carry the running one forward rather than stamp a zero
|
||||
# that later rate lookups would read back as the last known valuation
|
||||
"valuation_rate": flt(previous_sle.get("valuation_rate")),
|
||||
"is_adjustment_entry": 1,
|
||||
}
|
||||
)
|
||||
|
||||
sl_entries.append(args)
|
||||
|
||||
@@ -897,7 +913,16 @@ class StockReconciliation(StockController):
|
||||
has_dimensions = True
|
||||
|
||||
if self.docstatus == 2:
|
||||
if row.current_qty and current_bundle:
|
||||
if self.is_adjustment_row(row):
|
||||
# Reversing a value-only entry must not shift any quantity, so mirror the balance the
|
||||
# ledger carried across it and let get_stock_reco_qty_shift resolve to zero.
|
||||
data.actual_qty = 0.0
|
||||
data.qty_after_transaction = flt(row.current_qty)
|
||||
data.previous_qty_after_transaction = flt(row.current_qty)
|
||||
data.valuation_rate = flt(row.current_valuation_rate)
|
||||
data.stock_value = flt(row.current_amount)
|
||||
data.stock_value_difference = -1 * flt(row.amount_difference)
|
||||
elif row.current_qty and current_bundle:
|
||||
data.actual_qty = -1 * row.current_qty
|
||||
data.qty_after_transaction = flt(row.current_qty)
|
||||
data.previous_qty_after_transaction = flt(row.qty)
|
||||
@@ -1029,9 +1054,14 @@ class StockReconciliation(StockController):
|
||||
|
||||
for row in self.items:
|
||||
stock_value_difference = flt(get_row_stock_value_difference(self.doctype, self.name, row.name))
|
||||
amount_difference = flt(stock_value_difference, row.precision("amount_difference"))
|
||||
|
||||
if self.is_adjustment_row(row):
|
||||
self.set_adjustment_row_values(row, amount_difference)
|
||||
difference_amount += amount_difference
|
||||
continue
|
||||
|
||||
amount = flt(flt(row.qty) * flt(row.valuation_rate), row.precision("amount"))
|
||||
amount_difference = flt(stock_value_difference, row.precision("amount_difference"))
|
||||
current_amount = flt(amount - amount_difference, row.precision("current_amount"))
|
||||
|
||||
current_qty = self.get_current_qty_from_ledger(row)
|
||||
@@ -1061,6 +1091,50 @@ class StockReconciliation(StockController):
|
||||
update_modified=False,
|
||||
)
|
||||
|
||||
def is_adjustment_row(self, row) -> bool:
|
||||
# Read once for the whole voucher: both callers run per row, and a reconciliation
|
||||
# submits and cancels synchronously for up to 100 of them.
|
||||
if self.flags.adjustment_rows is None:
|
||||
self.flags.adjustment_rows = set(
|
||||
frappe.get_all(
|
||||
"Stock Ledger Entry",
|
||||
filters={
|
||||
"voucher_type": self.doctype,
|
||||
"voucher_no": self.name,
|
||||
"is_adjustment_entry": 1,
|
||||
"is_cancelled": 0,
|
||||
},
|
||||
pluck="voucher_detail_no",
|
||||
)
|
||||
)
|
||||
|
||||
return row.name in self.flags.adjustment_rows
|
||||
|
||||
def set_adjustment_row_values(self, row, amount_difference):
|
||||
"""Refresh a value-only row: it moves no stock, so both sides carry the ledger's own figures
|
||||
and ``amount_difference`` is the write-off booked to the GL, not a change in what is on hand.
|
||||
"""
|
||||
previous_sle = self.get_previous_ledger_entry(row) or frappe._dict()
|
||||
|
||||
current_qty = flt(previous_sle.get("qty_after_transaction"), row.precision("current_qty"))
|
||||
current_valuation_rate = flt(
|
||||
previous_sle.get("valuation_rate"), row.precision("current_valuation_rate")
|
||||
)
|
||||
# from the ledger's stock value, since rounding the rate first loses money on large qtys
|
||||
current_amount = flt(previous_sle.get("stock_value"), row.precision("current_amount"))
|
||||
|
||||
row.db_set(
|
||||
{
|
||||
"amount": current_amount,
|
||||
"current_qty": current_qty,
|
||||
"current_valuation_rate": current_valuation_rate,
|
||||
"current_amount": current_amount,
|
||||
"quantity_difference": 0.0,
|
||||
"amount_difference": amount_difference,
|
||||
},
|
||||
update_modified=False,
|
||||
)
|
||||
|
||||
def get_current_qty_from_ledger(self, row):
|
||||
"""Current (pre-reconciliation) qty for a row, recomputed from the ledger after reposting.
|
||||
|
||||
@@ -1075,6 +1149,14 @@ class StockReconciliation(StockController):
|
||||
)
|
||||
return abs(flt(total_qty, row.precision("current_qty")))
|
||||
|
||||
previous_sle = self.get_previous_ledger_entry(row)
|
||||
if previous_sle is None:
|
||||
return flt(row.current_qty, row.precision("current_qty"))
|
||||
|
||||
return flt(previous_sle.get("qty_after_transaction"), row.precision("current_qty"))
|
||||
|
||||
def get_previous_ledger_entry(self, row):
|
||||
"""Balance, rate and value carried just before this row's own entries, or None if it has none."""
|
||||
reco_sle = frappe.db.get_value(
|
||||
"Stock Ledger Entry",
|
||||
{
|
||||
@@ -1087,12 +1169,12 @@ class StockReconciliation(StockController):
|
||||
as_dict=True,
|
||||
)
|
||||
if not reco_sle:
|
||||
return flt(row.current_qty, row.precision("current_qty"))
|
||||
return None
|
||||
|
||||
sle = frappe.qb.DocType("Stock Ledger Entry")
|
||||
previous_sle = (
|
||||
frappe.qb.from_(sle)
|
||||
.select(sle.qty_after_transaction)
|
||||
.select(sle.qty_after_transaction, sle.valuation_rate, sle.stock_value)
|
||||
.where(
|
||||
(sle.item_code == row.item_code)
|
||||
& (sle.warehouse == row.warehouse)
|
||||
@@ -1108,9 +1190,9 @@ class StockReconciliation(StockController):
|
||||
.orderby(sle.posting_datetime, order=frappe.qb.desc)
|
||||
.orderby(sle.creation, order=frappe.qb.desc)
|
||||
.limit(1)
|
||||
).run()
|
||||
).run(as_dict=True)
|
||||
|
||||
return flt(previous_sle[0][0], row.precision("current_qty")) if previous_sle else 0.0
|
||||
return previous_sle[0] if previous_sle else frappe._dict()
|
||||
|
||||
def submit(self):
|
||||
if len(self.items) > 100:
|
||||
|
||||
@@ -2215,6 +2215,211 @@ class TestStockReconciliation(FrappeTestCase, StockTestMixin):
|
||||
self.assertEqual(sles[0].qty_after_transaction, 0)
|
||||
self.assertEqual(flt(sles[0].stock_value_difference), -100.0)
|
||||
|
||||
def test_adjustment_entry_clears_value_stranded_at_zero_qty(self):
|
||||
"""bal_qty 0 with bal_val 500: the write-off has to bring the reported value to zero."""
|
||||
from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry
|
||||
from erpnext.stock.report.stock_balance.stock_balance import execute
|
||||
|
||||
item_code = self.make_item("Test Stock Reco Stranded Value Non Batch").name
|
||||
warehouse = "_Test Warehouse - _TC"
|
||||
|
||||
receipt = make_stock_entry(
|
||||
item_code=item_code,
|
||||
target=warehouse,
|
||||
qty=10,
|
||||
basic_rate=100,
|
||||
posting_date=add_days(nowdate(), -3),
|
||||
)
|
||||
make_stock_entry(item_code=item_code, source=warehouse, qty=10, posting_date=add_days(nowdate(), -2))
|
||||
|
||||
# strand 500 of value: qty nets out, stock_value_difference does not
|
||||
receipt_sle = frappe.db.get_value(
|
||||
"Stock Ledger Entry", {"voucher_no": receipt.name, "is_cancelled": 0}, "name"
|
||||
)
|
||||
frappe.db.set_value(
|
||||
"Stock Ledger Entry",
|
||||
receipt_sle,
|
||||
"stock_value_difference",
|
||||
flt(frappe.db.get_value("Stock Ledger Entry", receipt_sle, "stock_value_difference")) + 500,
|
||||
update_modified=False,
|
||||
)
|
||||
|
||||
report_filters = frappe._dict(
|
||||
{"item_code": [item_code], "warehouse": [warehouse], "company": "_Test Company"}
|
||||
)
|
||||
|
||||
# this is what the user sees before the reconciliation
|
||||
_columns, data = execute(filters=report_filters)
|
||||
self.assertEqual(flt(data[0].get("bal_qty")), 0.0)
|
||||
self.assertEqual(flt(data[0].get("bal_val")), 500.0)
|
||||
|
||||
sr = create_stock_reconciliation(
|
||||
item_code=item_code, warehouse=warehouse, qty=0, rate=0, do_not_save=1
|
||||
)
|
||||
sr.items[0].allow_zero_valuation_rate = 1
|
||||
sr.save()
|
||||
sr.submit()
|
||||
|
||||
sles = frappe.get_all(
|
||||
"Stock Ledger Entry",
|
||||
filters={"voucher_no": sr.name, "is_cancelled": 0},
|
||||
fields=["actual_qty", "qty_after_transaction", "stock_value_difference", "is_adjustment_entry"],
|
||||
)
|
||||
|
||||
self.assertEqual(len(sles), 1)
|
||||
self.assertEqual(sles[0].is_adjustment_entry, 1)
|
||||
self.assertEqual(flt(sles[0].actual_qty), 0.0)
|
||||
self.assertEqual(flt(sles[0].qty_after_transaction), 0.0)
|
||||
self.assertEqual(flt(sles[0].stock_value_difference), -500.0)
|
||||
|
||||
# the report, and the GL basis behind it, both land on zero
|
||||
# (the row drops out entirely once every figure on it is zero)
|
||||
_columns, data = execute(filters=report_filters)
|
||||
self.assertEqual(flt(data[0].get("bal_val")) if data else 0.0, 0.0)
|
||||
self.assertEqual(
|
||||
flt(get_stock_value_on(warehouses=warehouse, posting_date=nowdate(), item_code=item_code)),
|
||||
0.0,
|
||||
)
|
||||
|
||||
def _make_backdated_adjustment_scenario(self, item_name, valuation_method, backdated_qty=4):
|
||||
"""Strand 100 of value at zero qty, write it off, then backdate a receipt before the write-off."""
|
||||
from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry
|
||||
|
||||
item_code = self.make_item(item_name, {"valuation_method": valuation_method}).name
|
||||
warehouse = "_Test Warehouse - _TC"
|
||||
|
||||
receipt = make_stock_entry(
|
||||
item_code=item_code,
|
||||
target=warehouse,
|
||||
qty=10,
|
||||
basic_rate=100,
|
||||
posting_date=add_days(nowdate(), -10),
|
||||
)
|
||||
make_stock_entry(item_code=item_code, source=warehouse, qty=10, posting_date=add_days(nowdate(), -9))
|
||||
|
||||
# strand 100 of value on the ledger: qty nets out, stock_value_difference does not
|
||||
receipt_sle = frappe.db.get_value(
|
||||
"Stock Ledger Entry", {"voucher_no": receipt.name, "is_cancelled": 0}, "name"
|
||||
)
|
||||
frappe.db.set_value(
|
||||
"Stock Ledger Entry",
|
||||
receipt_sle,
|
||||
"stock_value_difference",
|
||||
flt(frappe.db.get_value("Stock Ledger Entry", receipt_sle, "stock_value_difference")) + 100,
|
||||
update_modified=False,
|
||||
)
|
||||
|
||||
sr = create_stock_reconciliation(
|
||||
item_code=item_code,
|
||||
warehouse=warehouse,
|
||||
qty=0,
|
||||
rate=0,
|
||||
posting_date=add_days(nowdate(), -5),
|
||||
do_not_save=1,
|
||||
)
|
||||
sr.items[0].allow_zero_valuation_rate = 1
|
||||
sr.save()
|
||||
sr.submit()
|
||||
|
||||
self.assertTrue(
|
||||
frappe.db.exists("Stock Ledger Entry", {"voucher_no": sr.name, "is_adjustment_entry": 1})
|
||||
)
|
||||
|
||||
# a backdated receipt lands before the write-off
|
||||
if backdated_qty:
|
||||
make_stock_entry(
|
||||
item_code=item_code,
|
||||
target=warehouse,
|
||||
qty=backdated_qty,
|
||||
basic_rate=50,
|
||||
posting_date=add_days(nowdate(), -7),
|
||||
)
|
||||
|
||||
return item_code, warehouse, sr
|
||||
|
||||
def _assert_backdated_stock_survives(self, item_code, warehouse, sr):
|
||||
adjustment_sle = frappe.db.get_value(
|
||||
"Stock Ledger Entry",
|
||||
{"voucher_no": sr.name, "is_cancelled": 0},
|
||||
["qty_after_transaction", "stock_value", "stock_value_difference", "valuation_rate"],
|
||||
as_dict=True,
|
||||
)
|
||||
|
||||
# the backdated stock is carried through the adjustment entry, not wiped out by it
|
||||
self.assertEqual(flt(adjustment_sle.qty_after_transaction), 4.0)
|
||||
self.assertEqual(flt(adjustment_sle.stock_value), 200.0)
|
||||
self.assertEqual(flt(adjustment_sle.valuation_rate), 50.0)
|
||||
|
||||
# and the write-off still lands the running ledger value on the stock value it holds
|
||||
self.assertEqual(
|
||||
flt(get_stock_value_on(warehouses=warehouse, posting_date=nowdate(), item_code=item_code)),
|
||||
200.0,
|
||||
)
|
||||
self.assertEqual(get_stock_balance(item_code, warehouse), 4.0)
|
||||
|
||||
def test_adjustment_entry_does_not_zero_out_backdated_stock(self):
|
||||
"""An adjustment entry restates value, so a backdated receipt posted before it must survive."""
|
||||
item_code, warehouse, sr = self._make_backdated_adjustment_scenario(
|
||||
"Test Stock Reco Backdated Adjustment", "FIFO"
|
||||
)
|
||||
self._assert_backdated_stock_survives(item_code, warehouse, sr)
|
||||
|
||||
def test_adjustment_entry_does_not_zero_out_backdated_stock_moving_average(self):
|
||||
"""Same, through the moving average path rather than the queue."""
|
||||
item_code, warehouse, sr = self._make_backdated_adjustment_scenario(
|
||||
"Test Stock Reco Backdated Adjustment MA", "Moving Average"
|
||||
)
|
||||
self._assert_backdated_stock_survives(item_code, warehouse, sr)
|
||||
|
||||
def test_adjustment_row_amount_is_not_distorted_by_rate_rounding(self):
|
||||
"""The refreshed amount comes from the ledger's stock value, not from a rounded rate."""
|
||||
from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry
|
||||
|
||||
item_code, warehouse, sr = self._make_backdated_adjustment_scenario(
|
||||
"Test Stock Reco Adjustment Rounding", "FIFO", backdated_qty=0
|
||||
)
|
||||
|
||||
# a backdated receipt whose value does not divide evenly into a 2 decimal rate
|
||||
make_stock_entry(
|
||||
item_code=item_code,
|
||||
target=warehouse,
|
||||
qty=10000,
|
||||
basic_rate=1.2345,
|
||||
posting_date=add_days(nowdate(), -7),
|
||||
)
|
||||
|
||||
sr.reload()
|
||||
row = sr.items[0]
|
||||
|
||||
self.assertEqual(flt(row.current_qty), 10000.0)
|
||||
self.assertEqual(flt(row.current_amount), 12345.0)
|
||||
|
||||
def test_cancelling_adjustment_entry_shifts_no_qty(self):
|
||||
"""Reversing a value-only entry must not push the preserved quantity into later entries."""
|
||||
from erpnext.stock.stock_ledger import get_stock_reco_qty_shift
|
||||
|
||||
_item_code, _warehouse, sr = self._make_backdated_adjustment_scenario(
|
||||
"Test Stock Reco Adjustment Cancel", "FIFO"
|
||||
)
|
||||
|
||||
sr.reload()
|
||||
row = sr.items[0]
|
||||
|
||||
# the refreshed document reports the balance the ledger carries and no quantity movement
|
||||
self.assertEqual(flt(row.current_qty), 4.0)
|
||||
self.assertEqual(flt(row.quantity_difference), 0.0)
|
||||
self.assertEqual(flt(row.current_valuation_rate), 50.0)
|
||||
self.assertEqual(flt(row.current_amount), 200.0)
|
||||
self.assertEqual(flt(row.amount_difference), -100.0)
|
||||
|
||||
# the reversal built on cancellation moves nothing, so later entries are not shifted
|
||||
sr.docstatus = 2
|
||||
args = sr.get_sle_for_items(row)
|
||||
args.actual_qty = -flt(args.actual_qty) # as make_sl_entries flips it for a cancellation
|
||||
|
||||
self.assertEqual(flt(args.actual_qty), 0.0)
|
||||
self.assertEqual(flt(get_stock_reco_qty_shift(args)), 0.0)
|
||||
|
||||
|
||||
def create_batch_item_with_batch(item_name, batch_id):
|
||||
batch_item_doc = create_item(item_name, is_stock_item=1)
|
||||
|
||||
@@ -1057,6 +1057,8 @@ class update_entries_after:
|
||||
else:
|
||||
if (
|
||||
sle.voucher_type == "Stock Reconciliation"
|
||||
# an adjustment entry counted nothing, so it must not assert a balance
|
||||
and not sle.is_adjustment_entry
|
||||
and not sle.batch_no
|
||||
and not sle.has_batch_no
|
||||
and not has_dimensions
|
||||
@@ -1118,26 +1120,21 @@ class update_entries_after:
|
||||
|
||||
sle.stock_value_difference = stock_value_difference
|
||||
|
||||
if (
|
||||
sle.is_adjustment_entry
|
||||
and flt(sle.qty_after_transaction, self.flt_precision) == 0
|
||||
and (
|
||||
flt(sle.stock_value, self.currency_precision) != 0
|
||||
or flt(sle.stock_value_difference, self.currency_precision) == 0
|
||||
)
|
||||
):
|
||||
sle.stock_value_difference = (
|
||||
get_stock_value_difference(
|
||||
sle.item_code,
|
||||
sle.warehouse,
|
||||
sle.posting_date,
|
||||
sle.posting_time,
|
||||
voucher_detail_no=sle.voucher_detail_no,
|
||||
creation=sle.creation,
|
||||
)
|
||||
* -1
|
||||
# Re-derive the write-off on every repost: whatever brings the running sum of
|
||||
# stock_value_difference back in line with the stock value held at this point. A non-zero
|
||||
# difference above means the entry moved something, so it is not a write-off and is left alone.
|
||||
if sle.is_adjustment_entry and flt(sle.stock_value_difference, self.currency_precision) == 0:
|
||||
value_till_now = get_stock_value_difference(
|
||||
sle.item_code,
|
||||
sle.warehouse,
|
||||
sle.posting_date,
|
||||
sle.posting_time,
|
||||
voucher_detail_no=sle.voucher_detail_no,
|
||||
creation=sle.creation,
|
||||
)
|
||||
|
||||
sle.stock_value_difference = flt(flt(sle.stock_value) - value_till_now, self.currency_precision)
|
||||
|
||||
sle.doctype = "Stock Ledger Entry"
|
||||
sle.modified = now()
|
||||
frappe.get_doc(sle).db_update()
|
||||
|
||||
Reference in New Issue
Block a user