mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-10 05:01:47 +00:00
fix: incorrect batch-wise valuation rate for entries with same posting datetime (#57794)
* fix: incorrect batch-wise valuation rate for entries with same posting datetime The tie-breaker in get_batch_no_ledgers compared the bundle's creation against the SLE's creation. These are different timelines - a bundle can be created (drafted) much before its SLE (created at submission). For entries sharing a posting datetime (backdated / amended vouchers), this mis-ordered the entries against the ledger's replay order (SLE creation), causing double counting or omission of batch qty / value and runaway outgoing rates that no repost could heal. Now the tie is broken using the creation of the bundle's own SLE (same timeline on both sides). When the valuation runs through the bundle before its SLE exists, the entry is by definition last in its timestamp group, so all same-timestamp entries already in the ledger precede it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: batch-wise valuation ordering for same posting datetime entries Covers both tie-breaking branches of get_batch_no_ledgers: - submission (pre-insertion) branch: same-timestamp inward at a different rate plus a multi-row outward voucher (same item and warehouse), at submission and after a backdated repost - existing-SLE branch: a bundle created after its sibling's SLE, the ordering must follow the SLE creation and not the bundle creation Both tests fail with the previous parent.creation < sle.creation tie-breaker and pass with the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
rohitwaghchaure
@@ -5,7 +5,7 @@ import json
|
||||
|
||||
import frappe
|
||||
from frappe.tests.utils import FrappeTestCase, change_settings
|
||||
from frappe.utils import flt, nowtime, today
|
||||
from frappe.utils import add_days, add_to_date, flt, nowtime, today
|
||||
|
||||
from erpnext.stock.doctype.item.test_item import make_item
|
||||
from erpnext.stock.doctype.serial_and_batch_bundle.serial_and_batch_bundle import (
|
||||
@@ -1512,3 +1512,186 @@ class TestSerialandBatchBundleLogic(FrappeTestCase):
|
||||
|
||||
self.assertNotIn(bundles[1], bundle_wise_serial_nos)
|
||||
self.assertEqual(bundle_wise_serial_nos[bundles[0]], [serial_no])
|
||||
|
||||
@change_settings("Stock Settings", {"auto_create_serial_and_batch_bundle_for_outward": 1})
|
||||
def test_batchwise_valuation_for_same_posting_datetime_entries(self):
|
||||
# an inward at a different rate and multiple outward rows with the same
|
||||
# item and warehouse share the same posting datetime, the tie-breaking
|
||||
# must include the same-timestamp entries which are already part of the
|
||||
# ledger and must not let the outward rows count each other
|
||||
item_code = make_item(
|
||||
"Test Batchwise Same Posting Datetime Item 1",
|
||||
properties={
|
||||
"is_stock_item": 1,
|
||||
"has_batch_no": 1,
|
||||
"create_new_batch": 1,
|
||||
"batch_number_series": "TBSPD-ITEM1-.#####",
|
||||
"valuation_method": "FIFO",
|
||||
},
|
||||
).name
|
||||
|
||||
warehouse = "_Test Warehouse - _TC"
|
||||
|
||||
receipt = make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=10,
|
||||
rate=100,
|
||||
target=warehouse,
|
||||
posting_date=add_days(today(), -5),
|
||||
posting_time="12:00:00",
|
||||
)
|
||||
|
||||
batch_no = get_batch_from_bundle(receipt.items[0].serial_and_batch_bundle)
|
||||
self.assertTrue(frappe.db.get_value("Batch", batch_no, "use_batchwise_valuation"))
|
||||
|
||||
# same posting datetime as the outward rows below, at a different rate
|
||||
make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=20,
|
||||
rate=250,
|
||||
target=warehouse,
|
||||
batch_no=batch_no,
|
||||
use_serial_batch_fields=1,
|
||||
posting_date=add_days(today(), -3),
|
||||
posting_time="12:00:00",
|
||||
)
|
||||
|
||||
issue = make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=2,
|
||||
source=warehouse,
|
||||
posting_date=add_days(today(), -3),
|
||||
posting_time="12:00:00",
|
||||
do_not_save=True,
|
||||
)
|
||||
|
||||
for qty in [3, 4]:
|
||||
issue.append(
|
||||
"items",
|
||||
{
|
||||
"item_code": item_code,
|
||||
"s_warehouse": warehouse,
|
||||
"qty": qty,
|
||||
"conversion_factor": 1,
|
||||
},
|
||||
)
|
||||
|
||||
issue.save()
|
||||
issue.submit()
|
||||
|
||||
# (10 * 100 + 20 * 250) / 30 = 200
|
||||
self.assert_batchwise_outgoing_rate(item_code, outgoing_rate=200.0, balance_value=4200.0)
|
||||
|
||||
# backdated receipt reposts the same posting datetime cluster
|
||||
make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=10,
|
||||
rate=100,
|
||||
target=warehouse,
|
||||
batch_no=batch_no,
|
||||
use_serial_batch_fields=1,
|
||||
posting_date=add_days(today(), -4),
|
||||
posting_time="12:00:00",
|
||||
)
|
||||
|
||||
# (20 * 100 + 20 * 250) / 40 = 175
|
||||
self.assert_batchwise_outgoing_rate(item_code, outgoing_rate=175.0, balance_value=5425.0)
|
||||
|
||||
@change_settings("Stock Settings", {"auto_create_serial_and_batch_bundle_for_outward": 1})
|
||||
def test_batchwise_valuation_when_bundle_created_before_the_sle(self):
|
||||
# a bundle can be created (drafted) much before / after its SLE, the
|
||||
# tie-breaking for the same posting datetime entries must follow the
|
||||
# SLE creation and not the bundle creation
|
||||
item_code = make_item(
|
||||
"Test Batchwise Same Posting Datetime Item 2",
|
||||
properties={
|
||||
"is_stock_item": 1,
|
||||
"has_batch_no": 1,
|
||||
"create_new_batch": 1,
|
||||
"batch_number_series": "TBSPD-ITEM2-.#####",
|
||||
"valuation_method": "FIFO",
|
||||
},
|
||||
).name
|
||||
|
||||
warehouse = "_Test Warehouse - _TC"
|
||||
|
||||
receipt = make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=10,
|
||||
rate=100,
|
||||
target=warehouse,
|
||||
posting_date=add_days(today(), -5),
|
||||
posting_time="12:00:00",
|
||||
)
|
||||
|
||||
batch_no = get_batch_from_bundle(receipt.items[0].serial_and_batch_bundle)
|
||||
|
||||
# inward at a different rate, same posting datetime as the outward below
|
||||
inward = make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=10,
|
||||
rate=200,
|
||||
target=warehouse,
|
||||
batch_no=batch_no,
|
||||
use_serial_batch_fields=1,
|
||||
posting_date=add_days(today(), -3),
|
||||
posting_time="12:00:00",
|
||||
)
|
||||
|
||||
outward = make_stock_entry(
|
||||
item_code=item_code,
|
||||
qty=10,
|
||||
source=warehouse,
|
||||
posting_date=add_days(today(), -3),
|
||||
posting_time="12:00:00",
|
||||
)
|
||||
|
||||
# simulate the inward's bundle drafted after the outward's SLE, the
|
||||
# bundle creation timeline no longer matches the SLE creation timeline
|
||||
outward_sle_creation = frappe.db.get_value(
|
||||
"Stock Ledger Entry",
|
||||
{"voucher_no": outward.name, "is_cancelled": 0},
|
||||
"creation",
|
||||
)
|
||||
|
||||
frappe.db.set_value(
|
||||
"Serial and Batch Bundle",
|
||||
inward.items[0].serial_and_batch_bundle,
|
||||
"creation",
|
||||
add_to_date(outward_sle_creation, minutes=30),
|
||||
update_modified=False,
|
||||
)
|
||||
|
||||
repost = frappe.get_doc(
|
||||
{
|
||||
"doctype": "Repost Item Valuation",
|
||||
"based_on": "Item and Warehouse",
|
||||
"item_code": item_code,
|
||||
"warehouse": warehouse,
|
||||
"posting_date": add_days(today(), -6),
|
||||
"posting_time": "00:00:00",
|
||||
"allow_negative_stock": 1,
|
||||
}
|
||||
)
|
||||
|
||||
repost.submit()
|
||||
|
||||
# (10 * 100 + 10 * 200) / 20 = 150, the inward precedes the outward as
|
||||
# per the SLE creation even though its bundle was created afterwards
|
||||
self.assert_batchwise_outgoing_rate(item_code, outgoing_rate=150.0, balance_value=1500.0)
|
||||
|
||||
def assert_batchwise_outgoing_rate(self, item_code, outgoing_rate, balance_value):
|
||||
sl_entries = frappe.get_all(
|
||||
"Stock Ledger Entry",
|
||||
filters={"item_code": item_code, "is_cancelled": 0},
|
||||
fields=["actual_qty", "stock_value_difference", "stock_value"],
|
||||
order_by="posting_datetime, creation",
|
||||
)
|
||||
|
||||
for sle in sl_entries:
|
||||
if sle.actual_qty > 0:
|
||||
continue
|
||||
|
||||
self.assertEqual(flt(sle.stock_value_difference, 2), flt(sle.actual_qty * outgoing_rate, 2))
|
||||
|
||||
self.assertEqual(flt(sl_entries[-1].stock_value, 2), flt(balance_value, 2))
|
||||
|
||||
@@ -829,14 +829,43 @@ class BatchNoValuation(DeprecatedBatchNoValuation):
|
||||
parent = frappe.qb.DocType("Serial and Batch Bundle")
|
||||
child = frappe.qb.DocType("Serial and Batch Entry")
|
||||
|
||||
sle_creation = self.sle.creation if self.sle.get("name") else None
|
||||
if not self.sle.get("name") 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",
|
||||
)
|
||||
|
||||
timestamp_condition = ""
|
||||
if self.sle.posting_datetime:
|
||||
timestamp_condition = parent.posting_datetime < self.sle.posting_datetime
|
||||
|
||||
if self.sle.creation:
|
||||
timestamp_condition |= (parent.posting_datetime == self.sle.posting_datetime) & (
|
||||
parent.creation < self.sle.creation
|
||||
sle_table = frappe.qb.DocType("Stock Ledger Entry")
|
||||
if sle_creation:
|
||||
# bundle creation and SLE creation are different timelines (a
|
||||
# bundle can be created much before its SLE), so break the tie
|
||||
# using the creation of the bundle's own SLE
|
||||
tie_condition = ExistsCriterion(
|
||||
frappe.qb.from_(sle_table)
|
||||
.select(sle_table.name)
|
||||
.where(
|
||||
(sle_table.serial_and_batch_bundle == parent.name)
|
||||
& (sle_table.is_cancelled == 0)
|
||||
& (sle_table.creation < sle_creation)
|
||||
)
|
||||
)
|
||||
else:
|
||||
# the current entry is not yet in the ledger and will get the
|
||||
# latest creation, so the same-timestamp entries which are
|
||||
# already in the ledger precede it
|
||||
tie_condition = ExistsCriterion(
|
||||
frappe.qb.from_(sle_table)
|
||||
.select(sle_table.name)
|
||||
.where((sle_table.serial_and_batch_bundle == parent.name) & (sle_table.is_cancelled == 0))
|
||||
)
|
||||
|
||||
timestamp_condition |= (parent.posting_datetime == self.sle.posting_datetime) & tie_condition
|
||||
|
||||
query = (
|
||||
frappe.qb.from_(parent)
|
||||
|
||||
Reference in New Issue
Block a user