From 18bd330a590055287f6dc8c509f65452664e45d3 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sun, 17 Dec 2023 17:41:51 +0530 Subject: [PATCH] fix: incorrect available qty for backdated stock reco with batch (backport #37858) (#38811) fix: incorrect available qty for backdated stock reco with batch (#37858) * fix: incorrect available qty for backdated stock reco with batch * test: added test case (cherry picked from commit d4c0dbfacc7e99da6cba2c5d389f0a662490b0eb) Co-authored-by: rohitwaghchaure <rohitw1991@gmail.com> --- .../serial_and_batch_bundle.py | 13 +- .../stock_reconciliation.py | 135 +++++++++++++----- .../test_stock_reconciliation.py | 69 ++++++++- .../stock_reconciliation_item.json | 3 +- .../stock/report/stock_ledger/stock_ledger.py | 8 ++ .../stock_ledger_invariant_check.py | 13 +- erpnext/stock/stock_ledger.py | 68 +++++---- 7 files changed, 229 insertions(+), 80 deletions(-) diff --git a/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py b/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py index 28b7dc337b..48002323c2 100644 --- a/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py +++ b/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py @@ -157,7 +157,7 @@ class SerialandBatchBundle(Document): def throw_error_message(self, message, exception=frappe.ValidationError): frappe.throw(_(message), exception, title=_("Error")) - def set_incoming_rate(self, row=None, save=False): + def set_incoming_rate(self, row=None, save=False, allow_negative_stock=False): if self.type_of_transaction not in ["Inward", "Outward"] or self.voucher_type in [ "Installation Note", "Job Card", @@ -167,7 +167,9 @@ class SerialandBatchBundle(Document): return if self.type_of_transaction == "Outward": - self.set_incoming_rate_for_outward_transaction(row, save) + self.set_incoming_rate_for_outward_transaction( + row, save, allow_negative_stock=allow_negative_stock + ) else: self.set_incoming_rate_for_inward_transaction(row, save) @@ -188,7 +190,9 @@ class SerialandBatchBundle(Document): def get_serial_nos(self): return [d.serial_no for d in self.entries if d.serial_no] - def set_incoming_rate_for_outward_transaction(self, row=None, save=False): + def set_incoming_rate_for_outward_transaction( + self, row=None, save=False, allow_negative_stock=False + ): sle = self.get_sle_for_outward_transaction() if self.has_serial_no: @@ -217,7 +221,8 @@ class SerialandBatchBundle(Document): if self.docstatus == 1: available_qty += flt(d.qty) - self.validate_negative_batch(d.batch_no, available_qty) + if not allow_negative_stock: + self.validate_negative_batch(d.batch_no, available_qty) d.stock_value_difference = flt(d.qty) * flt(d.incoming_rate) diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 0482467dca..e8d652e2b2 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -6,7 +6,7 @@ from typing import Optional import frappe from frappe import _, bold, msgprint from frappe.query_builder.functions import CombineDatetime, Sum -from frappe.utils import cint, cstr, flt +from frappe.utils import add_to_date, cint, cstr, flt import erpnext from erpnext.accounts.utils import get_company_default @@ -116,9 +116,12 @@ class StockReconciliation(StockController): self.repost_future_sle_and_gle() self.delete_auto_created_batches() - def set_current_serial_and_batch_bundle(self): + def set_current_serial_and_batch_bundle(self, voucher_detail_no=None, save=False) -> None: """Set Serial and Batch Bundle for each item""" for item in self.items: + if voucher_detail_no and voucher_detail_no != item.name: + continue + item_details = frappe.get_cached_value( "Item", item.item_code, ["has_serial_no", "has_batch_no"], as_dict=1 ) @@ -176,6 +179,7 @@ class StockReconciliation(StockController): "warehouse": item.warehouse, "posting_date": self.posting_date, "posting_time": self.posting_time, + "ignore_voucher_nos": [self.name], } ) ) @@ -191,11 +195,36 @@ class StockReconciliation(StockController): ) if not serial_and_batch_bundle.entries: + if voucher_detail_no: + return + continue - item.current_serial_and_batch_bundle = serial_and_batch_bundle.save().name + serial_and_batch_bundle.save() + item.current_serial_and_batch_bundle = serial_and_batch_bundle.name item.current_qty = abs(serial_and_batch_bundle.total_qty) item.current_valuation_rate = abs(serial_and_batch_bundle.avg_rate) + if save: + sle_creation = frappe.db.get_value( + "Serial and Batch Bundle", item.serial_and_batch_bundle, "creation" + ) + creation = add_to_date(sle_creation, seconds=-1) + item.db_set( + { + "current_serial_and_batch_bundle": item.current_serial_and_batch_bundle, + "current_qty": item.current_qty, + "current_valuation_rate": item.current_valuation_rate, + "creation": creation, + } + ) + + serial_and_batch_bundle.db_set( + { + "creation": creation, + "voucher_no": self.name, + "voucher_detail_no": voucher_detail_no, + } + ) def set_new_serial_and_batch_bundle(self): for item in self.items: @@ -737,56 +766,84 @@ class StockReconciliation(StockController): else: self._cancel() - def recalculate_current_qty(self, item_code, batch_no): + def recalculate_current_qty(self, voucher_detail_no, sle_creation, add_new_sle=False): from erpnext.stock.stock_ledger import get_valuation_rate sl_entries = [] + for row in self.items: - if ( - not (row.item_code == item_code and row.batch_no == batch_no) - and not row.serial_and_batch_bundle - ): + if voucher_detail_no != row.name: continue + current_qty = 0.0 if row.current_serial_and_batch_bundle: - self.recalculate_qty_for_serial_and_batch_bundle(row) - continue - - current_qty = get_batch_qty_for_stock_reco( - item_code, row.warehouse, batch_no, self.posting_date, self.posting_time, self.name - ) + current_qty = self.get_qty_for_serial_and_batch_bundle(row) + elif row.batch_no: + current_qty = get_batch_qty_for_stock_reco( + row.item_code, row.warehouse, row.batch_no, self.posting_date, self.posting_time, self.name + ) precesion = row.precision("current_qty") - if flt(current_qty, precesion) == flt(row.current_qty, precesion): - continue + if flt(current_qty, precesion) != flt(row.current_qty, precesion): + val_rate = get_valuation_rate( + row.item_code, + row.warehouse, + self.doctype, + self.name, + company=self.company, + batch_no=row.batch_no, + serial_and_batch_bundle=row.current_serial_and_batch_bundle, + ) - val_rate = get_valuation_rate( - item_code, row.warehouse, self.doctype, self.name, company=self.company, batch_no=batch_no - ) + row.current_valuation_rate = val_rate + row.current_qty = current_qty + row.db_set( + { + "current_qty": row.current_qty, + "current_valuation_rate": row.current_valuation_rate, + "current_amount": flt(row.current_qty * row.current_valuation_rate), + } + ) - row.current_valuation_rate = val_rate - if not row.current_qty and current_qty: - sle = self.get_sle_for_items(row) - sle.actual_qty = current_qty * -1 - sle.valuation_rate = val_rate - sl_entries.append(sle) + if ( + add_new_sle + and not frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_detail_no": row.name, "actual_qty": ("<", 0), "is_cancelled": 0}, + "name", + ) + and (not row.current_serial_and_batch_bundle and not row.batch_no) + ): + self.set_current_serial_and_batch_bundle(voucher_detail_no, save=True) + row.reload() - row.current_qty = current_qty - row.db_set( - { - "current_qty": row.current_qty, - "current_valuation_rate": row.current_valuation_rate, - "current_amount": flt(row.current_qty * row.current_valuation_rate), - } - ) + if row.current_qty > 0 and row.current_serial_and_batch_bundle: + new_sle = self.get_sle_for_items(row) + new_sle.actual_qty = row.current_qty * -1 + new_sle.valuation_rate = row.current_valuation_rate + new_sle.creation_time = add_to_date(sle_creation, seconds=-1) + new_sle.serial_and_batch_bundle = row.current_serial_and_batch_bundle + new_sle.qty_after_transaction = 0.0 + sl_entries.append(new_sle) if sl_entries: - self.make_sl_entries(sl_entries, allow_negative_stock=True) + self.make_sl_entries(sl_entries, allow_negative_stock=self.has_negative_stock_allowed()) + if not frappe.db.exists("Repost Item Valuation", {"voucher_no": self.name, "status": "Queued"}): + self.repost_future_sle_and_gle(force=True) - def recalculate_qty_for_serial_and_batch_bundle(self, row): + def has_negative_stock_allowed(self): + allow_negative_stock = cint(frappe.db.get_single_value("Stock Settings", "allow_negative_stock")) + + if all(d.serial_and_batch_bundle and flt(d.qty) == flt(d.current_qty) for d in self.items): + allow_negative_stock = True + + return allow_negative_stock + + def get_qty_for_serial_and_batch_bundle(self, row): doc = frappe.get_doc("Serial and Batch Bundle", row.current_serial_and_batch_bundle) precision = doc.entries[0].precision("qty") + current_qty = 0 for d in doc.entries: qty = ( get_batch_qty( @@ -799,10 +856,12 @@ class StockReconciliation(StockController): or 0 ) * -1 - if flt(d.qty, precision) == flt(qty, precision): - continue + if flt(d.qty, precision) != flt(qty, precision): + d.db_set("qty", qty) - d.db_set("qty", qty) + current_qty += qty + + return abs(current_qty) def get_batch_qty_for_stock_reco( diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 34a7cbaa72..1ec99bf9a5 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -742,13 +742,6 @@ class TestStockReconciliation(FrappeTestCase, StockTestMixin): se2.cancel() - self.assertTrue(frappe.db.exists("Repost Item Valuation", {"voucher_no": stock_reco.name})) - - self.assertEqual( - frappe.db.get_value("Repost Item Valuation", {"voucher_no": stock_reco.name}, "status"), - "Completed", - ) - sle = frappe.get_all( "Stock Ledger Entry", filters={"item_code": item_code, "warehouse": warehouse, "is_cancelled": 0}, @@ -766,6 +759,68 @@ class TestStockReconciliation(FrappeTestCase, StockTestMixin): self.assertEqual(flt(sle[0].actual_qty), flt(-100.0)) + def test_backdated_stock_reco_entry_with_batch(self): + from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry + + item_code = self.make_item( + "Test New Batch Item ABCVSD", + { + "is_stock_item": 1, + "has_batch_no": 1, + "batch_number_series": "BNS9.####", + "create_new_batch": 1, + }, + ).name + + warehouse = "_Test Warehouse - _TC" + + # Stock Reco for 100, Balace Qty 100 + stock_reco = create_stock_reconciliation( + item_code=item_code, + posting_date=nowdate(), + posting_time="11:00:00", + warehouse=warehouse, + qty=100, + rate=100, + ) + + sles = frappe.get_all( + "Stock Ledger Entry", + fields=["actual_qty"], + filters={"voucher_no": stock_reco.name, "is_cancelled": 0}, + ) + + self.assertEqual(len(sles), 1) + + stock_reco.reload() + batch_no = get_batch_from_bundle(stock_reco.items[0].serial_and_batch_bundle) + + # Stock Reco for 100, Balace Qty 100 + stock_reco1 = create_stock_reconciliation( + item_code=item_code, + posting_date=add_days(nowdate(), -1), + posting_time="11:00:00", + batch_no=batch_no, + warehouse=warehouse, + qty=60, + rate=100, + ) + + sles = frappe.get_all( + "Stock Ledger Entry", + fields=["actual_qty"], + filters={"voucher_no": stock_reco.name, "is_cancelled": 0}, + ) + + stock_reco1.reload() + new_batch_no = get_batch_from_bundle(stock_reco1.items[0].serial_and_batch_bundle) + + self.assertEqual(len(sles), 2) + + for row in sles: + if row.actual_qty < 0: + self.assertEqual(row.actual_qty, -60) + def test_update_stock_reconciliation_while_reposting(self): from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry diff --git a/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json b/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json index ca19bbb96a..d9cbf95710 100644 --- a/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json +++ b/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json @@ -205,6 +205,7 @@ "fieldname": "current_serial_and_batch_bundle", "fieldtype": "Link", "label": "Current Serial / Batch Bundle", + "no_copy": 1, "options": "Serial and Batch Bundle", "read_only": 1 }, @@ -216,7 +217,7 @@ ], "istable": 1, "links": [], - "modified": "2023-07-26 12:54:34.011915", + "modified": "2023-11-02 15:47:07.929550", "modified_by": "Administrator", "module": "Stock", "name": "Stock Reconciliation Item", diff --git a/erpnext/stock/report/stock_ledger/stock_ledger.py b/erpnext/stock/report/stock_ledger/stock_ledger.py index eeef39641b..e59f2fe644 100644 --- a/erpnext/stock/report/stock_ledger/stock_ledger.py +++ b/erpnext/stock/report/stock_ledger/stock_ledger.py @@ -249,6 +249,13 @@ def get_columns(filters): "options": "Serial No", "width": 100, }, + { + "label": _("Serial and Batch Bundle"), + "fieldname": "serial_and_batch_bundle", + "fieldtype": "Link", + "options": "Serial and Batch Bundle", + "width": 100, + }, {"label": _("Balance Serial No"), "fieldname": "balance_serial_no", "width": 100}, { "label": _("Project"), @@ -287,6 +294,7 @@ def get_stock_ledger_entries(filters, items): sle.voucher_type, sle.qty_after_transaction, sle.stock_value_difference, + sle.serial_and_batch_bundle, sle.voucher_no, sle.stock_value, sle.batch_no, diff --git a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py index ca15afe444..fb392f7e36 100644 --- a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py +++ b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py @@ -24,6 +24,7 @@ SLE_FIELDS = ( "stock_value_difference", "valuation_rate", "voucher_detail_no", + "serial_and_batch_bundle", ) @@ -64,7 +65,11 @@ def add_invariant_check_fields(sles): balance_qty += sle.actual_qty balance_stock_value += sle.stock_value_difference - if sle.voucher_type == "Stock Reconciliation" and not sle.batch_no: + if ( + sle.voucher_type == "Stock Reconciliation" + and not sle.batch_no + and not sle.serial_and_batch_bundle + ): balance_qty = frappe.db.get_value("Stock Reconciliation Item", sle.voucher_detail_no, "qty") if balance_qty is None: balance_qty = sle.qty_after_transaction @@ -143,6 +148,12 @@ def get_columns(): "label": _("Batch"), "options": "Batch", }, + { + "fieldname": "serial_and_batch_bundle", + "fieldtype": "Link", + "label": _("Serial and Batch Bundle"), + "options": "Serial and Batch Bundle", + }, { "fieldname": "use_batchwise_valuation", "fieldtype": "Check", diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index a53180442f..9142a27f4c 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -210,6 +210,11 @@ def make_entry(args, allow_negative_stock=False, via_landed_cost_voucher=False): sle.allow_negative_stock = allow_negative_stock sle.via_landed_cost_voucher = via_landed_cost_voucher sle.submit() + + # Added to handle the case when the stock ledger entry is created from the repostig + if args.get("creation_time") and args.get("voucher_type") == "Stock Reconciliation": + sle.db_set("creation", args.get("creation_time")) + return sle @@ -696,9 +701,11 @@ class update_entries_after(object): if ( sle.voucher_type == "Stock Reconciliation" - and (sle.batch_no or (sle.has_batch_no and sle.serial_and_batch_bundle)) + and ( + sle.batch_no or (sle.has_batch_no and sle.serial_and_batch_bundle and not sle.has_serial_no) + ) and sle.voucher_detail_no - and sle.actual_qty < 0 + and not self.args.get("sle_id") ): self.reset_actual_qty_for_stock_reco(sle) @@ -765,27 +772,22 @@ class update_entries_after(object): self.update_outgoing_rate_on_transaction(sle) def reset_actual_qty_for_stock_reco(self, sle): - if sle.serial_and_batch_bundle: - current_qty = frappe.get_cached_value( - "Serial and Batch Bundle", sle.serial_and_batch_bundle, "total_qty" + doc = frappe.get_cached_doc("Stock Reconciliation", sle.voucher_no) + doc.recalculate_current_qty(sle.voucher_detail_no, sle.creation, sle.actual_qty > 0) + + if sle.actual_qty < 0: + sle.actual_qty = ( + flt(frappe.db.get_value("Stock Reconciliation Item", sle.voucher_detail_no, "current_qty")) + * -1 ) - if current_qty is not None: - current_qty = abs(current_qty) - else: - current_qty = frappe.get_cached_value( - "Stock Reconciliation Item", sle.voucher_detail_no, "current_qty" - ) - - if current_qty: - sle.actual_qty = current_qty * -1 - elif current_qty == 0: - sle.is_cancelled = 1 + if abs(sle.actual_qty) == 0.0: + sle.is_cancelled = 1 def calculate_valuation_for_serial_batch_bundle(self, sle): doc = frappe.get_cached_doc("Serial and Batch Bundle", sle.serial_and_batch_bundle) - doc.set_incoming_rate(save=True) + doc.set_incoming_rate(save=True, allow_negative_stock=self.allow_negative_stock) doc.calculate_qty_and_amount(save=True) self.wh_data.stock_value = round_off_if_near_zero(self.wh_data.stock_value + doc.total_amount) @@ -1472,6 +1474,7 @@ def get_valuation_rate( currency=None, company=None, raise_error_if_no_rate=True, + batch_no=None, serial_and_batch_bundle=None, ): @@ -1480,6 +1483,25 @@ def get_valuation_rate( if not company: company = frappe.get_cached_value("Warehouse", warehouse, "company") + if warehouse and batch_no and frappe.db.get_value("Batch", batch_no, "use_batchwise_valuation"): + table = frappe.qb.DocType("Stock Ledger Entry") + query = ( + frappe.qb.from_(table) + .select(Sum(table.stock_value_difference) / Sum(table.actual_qty)) + .where( + (table.item_code == item_code) + & (table.warehouse == warehouse) + & (table.batch_no == batch_no) + & (table.is_cancelled == 0) + & (table.voucher_no != voucher_no) + & (table.voucher_type != voucher_type) + ) + ) + + last_valuation_rate = query.run() + if last_valuation_rate: + return flt(last_valuation_rate[0][0]) + # Get moving average rate of a specific batch number if warehouse and serial_and_batch_bundle: batch_obj = BatchNoValuation( @@ -1574,8 +1596,6 @@ def update_qty_in_future_sle(args, allow_negative_stock=False): next_stock_reco_detail = get_next_stock_reco(args) if next_stock_reco_detail: detail = next_stock_reco_detail[0] - if detail.batch_no or (detail.serial_and_batch_bundle and detail.has_batch_no): - regenerate_sle_for_batch_stock_reco(detail) # add condition to update SLEs before this date & time datetime_limit_condition = get_datetime_limit_condition(detail) @@ -1604,16 +1624,6 @@ def update_qty_in_future_sle(args, allow_negative_stock=False): validate_negative_qty_in_future_sle(args, allow_negative_stock) -def regenerate_sle_for_batch_stock_reco(detail): - doc = frappe.get_cached_doc("Stock Reconciliation", detail.voucher_no) - doc.recalculate_current_qty(detail.item_code, detail.batch_no) - - if not frappe.db.exists( - "Repost Item Valuation", {"voucher_no": doc.name, "status": "Queued", "docstatus": "1"} - ): - doc.repost_future_sle_and_gle(force=True) - - def get_stock_reco_qty_shift(args): stock_reco_qty_shift = 0 if args.get("is_cancelled"):