diff --git a/erpnext/stock/deprecated_serial_batch.py b/erpnext/stock/deprecated_serial_batch.py index a9fc98ca776..4b0f3998c02 100644 --- a/erpnext/stock/deprecated_serial_batch.py +++ b/erpnext/stock/deprecated_serial_batch.py @@ -7,6 +7,7 @@ from frappe.query_builder.functions import Locate, Sum from frappe.utils import flt, nowtime from pypika import Order from pypika.functions import Coalesce, Concat +from pypika.terms import ExistsCriterion from erpnext.deprecation_dumpster import deprecated @@ -182,37 +183,79 @@ class DeprecatedBatchNoValuation: self.set_balance_value_for_non_batchwise_valuation_batches() + fallback_rate = self.get_pooled_fallback_rate() + for batch_no, ledger in self.batch_nos.items(): if batch_no not in self.non_batchwise_valuation_batches: continue - if not self.non_batchwise_balance_qty: - continue - - if not self.non_batchwise_balance_qty.get(batch_no): - self.batch_avg_rate[batch_no] = 0.0 - self.stock_value_differece[batch_no] = 0.0 - else: - self.batch_avg_rate[batch_no] = ( - self.non_batchwise_balance_value[batch_no] / self.non_batchwise_balance_qty[batch_no] - ) - self.stock_value_differece[batch_no] = self.non_batchwise_balance_value + self.batch_avg_rate[batch_no] = self.get_non_batchwise_avg_rate(batch_no, fallback_rate) + self.stock_value_differece[batch_no] = flt(self.non_batchwise_balance_value.get(batch_no)) stock_value_change = self.batch_avg_rate[batch_no] * ledger.qty self.stock_value_change += stock_value_change - self.non_batchwise_balance_value[batch_no] -= stock_value_change - self.non_batchwise_balance_qty[batch_no] -= ledger.qty + # ledger.qty is negative for outward entries, so adding drains the pool + self.non_batchwise_balance_value[batch_no] += stock_value_change + self.non_batchwise_balance_qty[batch_no] += ledger.qty + + # on the legacy batch_no field path the ledger is the Stock Ledger Entry itself, + # so there is no Serial and Batch Entry row to write the rate back to + if not self.sle.get("serial_and_batch_bundle") or not ledger.get("name"): + continue frappe.db.set_value( "Serial and Batch Entry", ledger.name, { "stock_value_difference": stock_value_change, - "incoming_rate": self.batch_avg_rate[batch_no], + # every reader of incoming_rate takes abs(), keep the stored value in step + "incoming_rate": abs(self.batch_avg_rate[batch_no]), }, ) + def get_non_batchwise_avg_rate(self, batch_no, fallback_rate): + balance_qty = flt(self.non_batchwise_balance_qty.get(batch_no)) + balance_value = flt(self.non_batchwise_balance_value.get(batch_no)) + + # The value and the qty of a batch are summed independently over its whole history + # and the two can drift apart: outward entries posted before batch level valuation + # existed were priced at the pooled warehouse rate, and a Stock Reconciliation posts + # value with no matching qty. A drained or a negative pool would otherwise yield a + # negative or an exploding rate, which is read back as abs() by the callers and + # silently overdraws the warehouse stock value. + if balance_qty > 0 and balance_value > 0: + return balance_value / balance_qty + + return fallback_rate + + def get_pooled_fallback_rate(self): + """Moving average rate of the stock that is not valued batch wise. + + `last_sle` carries the balance of the whole warehouse, so the batches that are + valued batch wise have to be netted off before it can price the ones that are not. + """ + last_sle = self.last_sle or frappe._dict() + + total_qty = flt(last_sle.qty_after_transaction) + total_value = flt(last_sle.stock_value) + + qty, value = total_qty, total_value + for batch_no in self.batchwise_valuation_batches: + qty -= flt(self.available_qty.get(batch_no)) + value -= flt(self.stock_value_differece.get(batch_no)) + + if qty > 0 and value > 0: + return value / qty + + # The batchwise batches can account for more than the warehouse holds when the + # legacy ledger is itself inconsistent, which is what an overdrawn history leaves + # behind. The plain warehouse rate is then the best basis left. + if total_qty > 0 and total_value > 0: + return total_value / total_qty + + return 0.0 + @deprecated( "erpnext.stock.serial_batch_bundle.BatchNoValuation.set_balance_value_for_non_batchwise_valuation_batches", "unknown", @@ -285,13 +328,9 @@ class DeprecatedBatchNoValuation: query = query.where(sle.name != self.sle.name) # Moving Average items with no Use Batch wise Valuation but want to use batch wise valuation - moving_avg_item_non_batch_value = False - if valuation_method := self.get_valuation_method(self.sle.item_code): - if valuation_method == "Moving Average" and not frappe.db.get_single_value( - "Stock Settings", "do_not_use_batchwise_valuation" - ): - query = query.where(batch.use_batchwise_valuation == 0) - moving_avg_item_non_batch_value = True + moving_avg_item_non_batch_value = self.use_batch_pool_for_moving_average() + if moving_avg_item_non_batch_value: + query = query.where(batch.use_batchwise_valuation == 0) batch_data = query.run(as_dict=True) for d in batch_data: @@ -373,11 +412,37 @@ class DeprecatedBatchNoValuation: if not posting_datetime and self.sle.posting_date: posting_datetime = get_combine_datetime(self.sle.posting_date, self.sle.posting_time) + sle_creation = self.sle.creation + if not sle_creation and self.sle.get("serial_and_batch_bundle"): + sle_creation = frappe.db.get_value( + "Stock Ledger Entry", + {"serial_and_batch_bundle": self.sle.serial_and_batch_bundle, "is_cancelled": 0}, + "creation", + ) + + if not sle_creation: + # the current entry is not in the ledger yet, so it sorts after everything posted + # at the same instant; nudge the boundary to take them in, the same way + # set_balance_value_from_sl_entries does, otherwise the two halves of one bundle + # are summed as of two different points in time + posting_datetime = posting_datetime + datetime.timedelta(milliseconds=1) + timestamp_condition = bundle.posting_datetime < posting_datetime - if self.sle.creation: - timestamp_condition |= (bundle.posting_datetime == posting_datetime) & ( - bundle.creation < self.sle.creation + if sle_creation: + sle_table = frappe.qb.DocType("Stock Ledger Entry") + + # bundle creation and SLE creation are different timelines (a bundle can be + # created much before its SLE), so break the tie on the creation of the + # bundle's own SLE, exactly like BatchNoValuation.get_batch_stock_before_date + timestamp_condition |= (bundle.posting_datetime == posting_datetime) & ExistsCriterion( + frappe.qb.from_(sle_table) + .select(sle_table.name) + .where( + (sle_table.serial_and_batch_bundle == bundle.name) + & (sle_table.is_cancelled == 0) + & (sle_table.creation < sle_creation) + ) ) query = ( @@ -411,13 +476,9 @@ class DeprecatedBatchNoValuation: query = query.where(bundle.voucher_type != "Pick List") # Moving Average items with no Use Batch wise Valuation but want to use batch wise valuation - moving_avg_item_non_batch_value = False - if valuation_method := self.get_valuation_method(self.sle.item_code): - if valuation_method == "Moving Average" and not frappe.db.get_single_value( - "Stock Settings", "do_not_use_batchwise_valuation" - ): - query = query.where(batch.use_batchwise_valuation == 0) - moving_avg_item_non_batch_value = True + moving_avg_item_non_batch_value = self.use_batch_pool_for_moving_average() + if moving_avg_item_non_batch_value: + query = query.where(batch.use_batchwise_valuation == 0) batch_data = query.run(as_dict=True) for d in batch_data: @@ -440,3 +501,13 @@ class DeprecatedBatchNoValuation: from erpnext.stock.utils import get_valuation_method return get_valuation_method(item_code, self.sle.company) + + def use_batch_pool_for_moving_average(self): + if not hasattr(self, "_use_batch_pool_for_moving_average"): + self._use_batch_pool_for_moving_average = self.get_valuation_method( + self.sle.item_code + ) == "Moving Average" and not frappe.get_single_value( + "Stock Settings", "do_not_use_batchwise_valuation" + ) + + return self._use_batch_pool_for_moving_average diff --git a/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py b/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py index 2ee0d7e1ec5..dd193a9d0cf 100644 --- a/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py +++ b/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py @@ -494,6 +494,153 @@ class TestSerialandBatchBundle(ERPNextTestSuite): self.assertEqual(flt(sle.stock_value), 0.0) self.assertEqual(flt(sle.qty_after_transaction), 0.0) + def test_moving_avg_item_with_mixed_batchwise_valuation(self): + """A Moving Average item holding both batchwise and non batchwise batches. + + The non batchwise batches were consumed at the pooled warehouse rate before batch + level valuation existed, so the sum of their stock value differences no longer + tracks the sum of their quantities. Valuing a later outward off that drained pool + used to hand back a negative rate, which the callers read as abs() and used to + overdraw the warehouse, driving stock value negative. + """ + frappe.db.set_single_value("Stock Settings", "do_not_use_batchwise_valuation", 0) + + item_code = "Old Batch Item Mixed Valuation 1" + make_item( + item_code, + { + "has_batch_no": 1, + "batch_number_series": "TEST-MIX-BAT-VAL-.#####", + "create_new_batch": 1, + "is_stock_item": 1, + "valuation_method": "Moving Average", + }, + ) + + warehouse = "_Test Warehouse - _TC" + non_batchwise_batch = "TEST-MIX-BAT-VAL-00001" + batchwise_batch = "TEST-MIX-BAT-VAL-00002" + + for batch_id, use_batchwise_valuation in ( + (non_batchwise_batch, 0), + (batchwise_batch, 1), + ): + if not frappe.db.exists("Batch", batch_id): + batch_doc = frappe.get_doc( + { + "doctype": "Batch", + "batch_id": batch_id, + "item": item_code, + "use_batchwise_valuation": use_batchwise_valuation, + } + ).insert(ignore_permissions=True) + + batch_doc.db_set("use_batchwise_valuation", use_batchwise_valuation) + + # ERPNextTestSuite.tearDown only rolls the db back, it does not restore + # frappe.local.flags, so these have to be put back even if a submit raises + previous_flags = ( + frappe.flags.ignore_serial_batch_bundle_validation, + frappe.flags.use_serial_and_batch_fields, + ) + frappe.flags.ignore_serial_batch_bundle_validation = True + frappe.flags.use_serial_and_batch_fields = True + + # Legacy ledger, written the way the pre batch-level-valuation code posted it: + # in 20 @ 50 of the non batchwise batch -> warehouse 20 qty / 1000 + # in 20 @ 450 of the batchwise batch -> warehouse 40 qty / 10000, rate 250 + # out 20 of the non batchwise batch, priced at the pooled rate of 250 + # which leaves the non batchwise batch with a pool of -4000 value against 0 qty. + legacy_entries = [ + (non_batchwise_batch, 20, 1000, 20, 1000), + (batchwise_batch, 20, 9000, 40, 10000), + (non_batchwise_batch, -20, -5000, 20, 5000), + ] + + try: + for batch_id, qty, svd, qty_after_transaction, stock_value in legacy_entries: + doc = frappe.get_doc( + { + "doctype": "Stock Ledger Entry", + "posting_date": today(), + "posting_time": nowtime(), + "batch_no": batch_id, + "incoming_rate": (svd / qty) if qty > 0 else 0, + "qty_after_transaction": qty_after_transaction, + "stock_value_difference": svd, + "stock_value": stock_value, + "balance_value": stock_value, + "valuation_rate": stock_value / qty_after_transaction, + "actual_qty": qty, + "item_code": item_code, + "warehouse": warehouse, + } + ) + + doc.set_posting_datetime() + doc.flags.ignore_permissions = True + doc.flags.ignore_mandatory = True + doc.flags.ignore_links = True + doc.flags.ignore_validate = True + doc.submit() + finally: + ( + frappe.flags.ignore_serial_batch_bundle_validation, + frappe.flags.use_serial_and_batch_fields, + ) = previous_flags + + # Refill the drained non batchwise batch, then consume it back out. + make_stock_entry( + item_code=item_code, + target=warehouse, + qty=10, + rate=50, + batch_no=non_batchwise_batch, + use_serial_batch_fields=True, + ) + + se = make_stock_entry( + item_code=item_code, + source=warehouse, + qty=10, + batch_no=non_batchwise_batch, + use_serial_batch_fields=True, + ) + + sle = frappe.db.get_value( + "Stock Ledger Entry", + {"item_code": item_code, "is_cancelled": 0, "voucher_no": se.name}, + ["qty_after_transaction", "stock_value", "stock_value_difference"], + as_dict=True, + ) + + self.assertEqual(flt(sle.qty_after_transaction), 20.0) + + # The non batchwise batch is left holding a pool of -3500 against 10 qty, so its + # own rate works out to -350. That used to be taken as abs() = 350 and the 10 units + # drew 3500 out of the warehouse, against the 50 each they were actually bought at. + # The drained pool is rejected now and the warehouse rate of 5500 / 30 is used. + self.assertEqual(flt(sle.stock_value_difference, 2), -1833.33) + self.assertEqual(flt(sle.stock_value, 2), 3666.67) + self.assertGreaterEqual(flt(sle.stock_value), 0.0) + + # a batch is never valued at a negative rate, and the rate stored on the ledger + # entry stays in step with the sign every reader applies to it + bundle = frappe.db.get_value( + "Stock Ledger Entry", + {"item_code": item_code, "is_cancelled": 0, "voucher_no": se.name}, + "serial_and_batch_bundle", + ) + + incoming_rate = frappe.db.get_value( + "Serial and Batch Entry", + {"parent": bundle, "batch_no": non_batchwise_batch}, + "incoming_rate", + ) + + self.assertGreaterEqual(flt(incoming_rate), 0.0) + self.assertEqual(flt(incoming_rate, 2), 183.33) + def test_old_serial_no_valuation(self): from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt