From da73685f7172290151a279f8cf796628dbf6617e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Feb 2022 13:07:51 +0530 Subject: [PATCH 01/29] fix: Multiple fixes in Gross Profit report --- .../report/gross_profit/gross_profit.js | 10 +++-- .../report/gross_profit/gross_profit.py | 43 +++++++++++++------ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.js b/erpnext/accounts/report/gross_profit/gross_profit.js index 685f2d6176..c8a9a228c6 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.js +++ b/erpnext/accounts/report/gross_profit/gross_profit.js @@ -8,20 +8,22 @@ frappe.query_reports["Gross Profit"] = { "label": __("Company"), "fieldtype": "Link", "options": "Company", - "reqd": 1, - "default": frappe.defaults.get_user_default("Company") + "default": frappe.defaults.get_user_default("Company"), + "reqd": 1 }, { "fieldname":"from_date", "label": __("From Date"), "fieldtype": "Date", - "default": frappe.defaults.get_user_default("year_start_date") + "default": frappe.defaults.get_user_default("year_start_date"), + "reqd": 1 }, { "fieldname":"to_date", "label": __("To Date"), "fieldtype": "Date", - "default": frappe.defaults.get_user_default("year_end_date") + "default": frappe.defaults.get_user_default("year_end_date"), + "reqd": 1 }, { "fieldname":"sales_invoice", diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 84effc0f46..225b7c6426 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -369,20 +369,37 @@ class GrossProfitGenerator(object): return self.average_buying_rate[item_code] def get_last_purchase_rate(self, item_code, row): - condition = '' - if row.project: - condition += " AND a.project=%s" % (frappe.db.escape(row.project)) - elif row.cost_center: - condition += " AND a.cost_center=%s" % (frappe.db.escape(row.cost_center)) - if self.filters.to_date: - condition += " AND modified='%s'" % (self.filters.to_date) + purchase_invoice = frappe.qb.DocType("Purchase Invoice") + purchase_invoice_item = frappe.qb.DocType("Purchase Invoice Item") - last_purchase_rate = frappe.db.sql(""" - select (a.base_rate / a.conversion_factor) - from `tabPurchase Invoice Item` a - where a.item_code = %s and a.docstatus=1 - {0} - order by a.modified desc limit 1""".format(condition), item_code) + query = (frappe.qb.from_(purchase_invoice_item) + .inner_join( + purchase_invoice + ).on( + purchase_invoice.name == purchase_invoice_item.parent + ).select( + purchase_invoice_item.base_rate / purchase_invoice_item.conversion_factor + ).where( + purchase_invoice.docstatus == 1 + ).where( + purchase_invoice.posting_date <= self.filters.to_date + ).where( + purchase_invoice_item.item_code == item_code + )) + + if row.project: + query.where( + purchase_invoice_item.item_code == row.project + ) + + if row.cost_center: + query.where( + purchase_invoice_item.cost_center == row.cost_center + ) + + query.orderby(purchase_invoice.posting_date, order=frappe.qb.desc) + query.limit(1) + last_purchase_rate = query.run() return flt(last_purchase_rate[0][0]) if last_purchase_rate else 0 From 2172ab2d37d8be0c43d1f885a40657d352d255b4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 11 Feb 2022 14:48:39 +0530 Subject: [PATCH 02/29] fix: Update columns in new format --- .../report/gross_profit/gross_profit.json | 4 +- .../report/gross_profit/gross_profit.py | 80 ++++++------------- 2 files changed, 28 insertions(+), 56 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.json b/erpnext/accounts/report/gross_profit/gross_profit.json index 76c560ad24..0730ffd77e 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.json +++ b/erpnext/accounts/report/gross_profit/gross_profit.json @@ -1,5 +1,5 @@ { - "add_total_row": 0, + "add_total_row": 1, "columns": [], "creation": "2013-02-25 17:03:34", "disable_prepared_report": 0, @@ -9,7 +9,7 @@ "filters": [], "idx": 3, "is_standard": "Yes", - "modified": "2021-11-13 19:14:23.730198", + "modified": "2022-02-11 10:18:36.956558", "modified_by": "Administrator", "module": "Accounts", "name": "Gross Profit", diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 225b7c6426..c403b76f87 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -70,43 +70,42 @@ def get_data_when_grouped_by_invoice(columns, gross_profit_data, filters, group_ data.append(row) def get_data_when_not_grouped_by_invoice(gross_profit_data, filters, group_wise_columns, data): - for idx, src in enumerate(gross_profit_data.grouped_data): + for src in gross_profit_data.grouped_data: row = [] for col in group_wise_columns.get(scrub(filters.group_by)): row.append(src.get(col)) row.append(filters.currency) - if idx == len(gross_profit_data.grouped_data)-1: - row[0] = "Total" data.append(row) def get_columns(group_wise_columns, filters): columns = [] column_map = frappe._dict({ - "parent": _("Sales Invoice") + ":Link/Sales Invoice:120", - "invoice_or_item": _("Sales Invoice") + ":Link/Sales Invoice:120", - "posting_date": _("Posting Date") + ":Date:100", - "posting_time": _("Posting Time") + ":Data:100", - "item_code": _("Item Code") + ":Link/Item:100", - "item_name": _("Item Name") + ":Data:100", - "item_group": _("Item Group") + ":Link/Item Group:100", - "brand": _("Brand") + ":Link/Brand:100", - "description": _("Description") +":Data:100", - "warehouse": _("Warehouse") + ":Link/Warehouse:100", - "qty": _("Qty") + ":Float:80", - "base_rate": _("Avg. Selling Rate") + ":Currency/currency:100", - "buying_rate": _("Valuation Rate") + ":Currency/currency:100", - "base_amount": _("Selling Amount") + ":Currency/currency:100", - "buying_amount": _("Buying Amount") + ":Currency/currency:100", - "gross_profit": _("Gross Profit") + ":Currency/currency:100", - "gross_profit_percent": _("Gross Profit %") + ":Percent:100", - "project": _("Project") + ":Link/Project:100", - "sales_person": _("Sales person"), - "allocated_amount": _("Allocated Amount") + ":Currency/currency:100", - "customer": _("Customer") + ":Link/Customer:100", - "customer_group": _("Customer Group") + ":Link/Customer Group:100", - "territory": _("Territory") + ":Link/Territory:100" + "parent": {"label": _('Sales Invoice'), "fieldname": "parent_invoice", "fieldtype": "Link", "options": "Sales Invoice", "width": 120}, + "invoice_or_item": {"label": _('Sales Invoice'), "fieldtype": "Link", "options": "Sales Invoice", "width": 120}, + "posting_date": {"label": _('Posting Date'), "fieldname": "posting_date", "fieldtype": "Date", "width": 100}, + "posting_time": {"label": _('Posting Time'), "fieldname": "posting_time", "fieldtype": "Data", "width": 100}, + "item_code": {"label": _('Item Code'), "fieldname": "item_code", "fieldtype": "Link", "options": "Item", "width": 100}, + "item_name": {"label": _('Item Name'), "fieldname": "item_name", "fieldtype": "Data", "width": 100}, + "item_group": {"label": _('Item Group'), "fieldname": "item_group", "fieldtype": "Link", "options": "Item Group", "width": 100}, + "brand": {"label": _('Brand'), "fieldtype": "Link", "options": "Brand", "width": 100}, + "description": {"label": _('Description'), "fieldname": "description", "fieldtype": "Data", "width": 100}, + "warehouse": {"label": _('Warehouse'), "fieldname": "warehouse", "fieldtype": "Link", "options": "warehouse", "width": 100}, + "qty": {"label": _('Qty'), "fieldname": "qty", "fieldtype": "Float", "width": 80}, + "base_rate": {"label": _('Avg. Selling Rate'), "fieldname": "avg._selling_rate", "fieldtype": "Currency", "options": "currency", "width": 100}, + "buying_rate": {"label": _('Valuation Rate'), "fieldname": "valuation_rate", "fieldtype": "Currency", "options": "currency", "width": 100}, + "base_amount": {"label": _('Selling Amount'), "fieldname": "selling_amount", "fieldtype": "Currency", "options": "currency", "width": 100}, + "buying_amount": {"label": _('Buying Amount'), "fieldname": "buying_amount", "fieldtype": "Currency", "options": "currency", "width": 100}, + "gross_profit": {"label": _('Gross Profit'), "fieldname": "gross_profit", "fieldtype": "Currency", "options": "currency", "width": 100}, + "gross_profit_percent": {"label": _('Gross Profit Percent'), "fieldname": "gross_profit_%", + "fieldtype": "Percent", "width": 100}, + "project": {"label": _('Project'), "fieldname": "project", "fieldtype": "Link", "options": "Project", "width": 100}, + "sales_person": {"label": _('Sales Person'), "fieldname": "sales_person", "fieldtype": "Data","width": 100}, + "allocated_amount": {"label": _('Allocated Amount'), "fieldname": "allocated_amount", "fieldtype": "Currency", "options": "currency", "width": 100}, + "customer": {"label": _('Customer'), "fieldname": "customer", "fieldtype": "Link", "options": "Customer", "width": 100}, + "customer_group": {"label": _('Customer Group'), "fieldname": "customer_group", "fieldtype": "Link", "options": "customer", "width": 100}, + "territory": {"label": _('Territory'), "fieldname": "territory", "fieldtype": "Link", "options": "territory", "width": 100}, }) for col in group_wise_columns.get(scrub(filters.group_by)): @@ -223,16 +222,6 @@ class GrossProfitGenerator(object): self.get_average_rate_based_on_group_by() def get_average_rate_based_on_group_by(self): - # sum buying / selling totals for group - self.totals = frappe._dict( - qty=0, - base_amount=0, - buying_amount=0, - gross_profit=0, - gross_profit_percent=0, - base_rate=0, - buying_rate=0 - ) for key in list(self.grouped): if self.filters.get("group_by") != "Invoice": for i, row in enumerate(self.grouped[key]): @@ -244,7 +233,6 @@ class GrossProfitGenerator(object): new_row.base_amount += flt(row.base_amount, self.currency_precision) new_row = self.set_average_rate(new_row) self.grouped_data.append(new_row) - self.add_to_totals(new_row) else: for i, row in enumerate(self.grouped[key]): if row.indent == 1.0: @@ -258,17 +246,6 @@ class GrossProfitGenerator(object): if (flt(row.qty) or row.base_amount): row = self.set_average_rate(row) self.grouped_data.append(row) - self.add_to_totals(row) - - self.set_average_gross_profit(self.totals) - - if self.filters.get("group_by") == "Invoice": - self.totals.indent = 0.0 - self.totals.parent_invoice = "" - self.totals.invoice_or_item = "Total" - self.si_list.append(self.totals) - else: - self.grouped_data.append(self.totals) def is_not_invoice_row(self, row): return (self.filters.get("group_by") == "Invoice" and row.indent != 0.0) or self.filters.get("group_by") != "Invoice" @@ -284,11 +261,6 @@ class GrossProfitGenerator(object): new_row.gross_profit_percent = flt(((new_row.gross_profit / new_row.base_amount) * 100.0), self.currency_precision) \ if new_row.base_amount else 0 - def add_to_totals(self, new_row): - for key in self.totals: - if new_row.get(key): - self.totals[key] += new_row[key] - def get_returned_invoice_items(self): returned_invoices = frappe.db.sql(""" select @@ -389,7 +361,7 @@ class GrossProfitGenerator(object): if row.project: query.where( - purchase_invoice_item.item_code == row.project + purchase_invoice_item.project == row.project ) if row.cost_center: From 07bcbc6c7e10f977bc5a6ff8f5b48d91ec9b2b70 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sat, 12 Feb 2022 19:05:03 +0530 Subject: [PATCH 03/29] fix: Remove unused param --- erpnext/accounts/report/gross_profit/gross_profit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index c403b76f87..ebb929aaac 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -172,7 +172,7 @@ class GrossProfitGenerator(object): buying_amount = 0 for row in reversed(self.si_list): - if self.skip_row(row, self.product_bundles): + if self.skip_row(row): continue row.base_amount = flt(row.base_net_amount, self.currency_precision) @@ -278,7 +278,7 @@ class GrossProfitGenerator(object): self.returned_invoices.setdefault(inv.return_against, frappe._dict())\ .setdefault(inv.item_code, []).append(inv) - def skip_row(self, row, product_bundles): + def skip_row(self, row): if self.filters.get("group_by") != "Invoice": if not row.get(scrub(self.filters.get("group_by", ""))): return True From 973f6b1bbd53594e5b2a51a1dcdf7d9e38dd46a8 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 14 Feb 2022 22:14:17 +0530 Subject: [PATCH 04/29] fix: Gross profit for credit notes --- erpnext/accounts/report/gross_profit/gross_profit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index ebb929aaac..b03bb9bb13 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -282,8 +282,8 @@ class GrossProfitGenerator(object): if self.filters.get("group_by") != "Invoice": if not row.get(scrub(self.filters.get("group_by", ""))): return True - elif row.get("is_return") == 1: - return True + + return False def get_buying_amount_from_product_bundle(self, row, product_bundle): buying_amount = 0.0 From 00e8565868e3bb8a1547abeedd2d158a9b7e5bf4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 17:41:23 +0530 Subject: [PATCH 05/29] fix: round off increments in numeric item variant --- erpnext/stock/doctype/item/item.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/item/item.js b/erpnext/stock/doctype/item/item.js index dfc09181ca..ffea9c2d6e 100644 --- a/erpnext/stock/doctype/item/item.js +++ b/erpnext/stock/doctype/item/item.js @@ -594,7 +594,7 @@ $.extend(erpnext.item, { const increment = r.message.increment; let values = []; - for(var i = from; i <= to; i += increment) { + for(var i = from; i <= to; i = flt(i + increment, 6)) { values.push(i); } attr_val_fields[d.attribute] = values; From f4af75f60b7bb594df4f9a6e6d0cb1ad949dfa33 Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Tue, 15 Feb 2022 11:51:52 +0530 Subject: [PATCH 06/29] feat: batchwise valuation flag This is required to avoid breaking behaviour in valuation of old batches --- erpnext/patches.txt | 1 + .../patches/v14_0/update_batch_valuation_flag.py | 12 ++++++++++++ erpnext/stock/doctype/batch/batch.json | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 erpnext/patches/v14_0/update_batch_valuation_flag.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index a93ceca437..52c29b22b9 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -353,3 +353,4 @@ erpnext.patches.v13_0.update_reserved_qty_closed_wo erpnext.patches.v13_0.update_exchange_rate_settings erpnext.patches.v14_0.delete_amazon_mws_doctype erpnext.patches.v13_0.set_work_order_qty_in_so_from_mr +erpnext.patches.v14_0.update_batch_valuation_flag diff --git a/erpnext/patches/v14_0/update_batch_valuation_flag.py b/erpnext/patches/v14_0/update_batch_valuation_flag.py new file mode 100644 index 0000000000..d9f08d8d97 --- /dev/null +++ b/erpnext/patches/v14_0/update_batch_valuation_flag.py @@ -0,0 +1,12 @@ +import frappe + + +def execute(): + """ + - Don't use batchwise valuation for existing batches. + - Only batches created after this patch shoule use it. + """ + frappe.db.sql(""" + UPDATE `tabBatch` + SET use_batchwise_valuation=0 + """) diff --git a/erpnext/stock/doctype/batch/batch.json b/erpnext/stock/doctype/batch/batch.json index fc4cf1dbdb..0d28ea0919 100644 --- a/erpnext/stock/doctype/batch/batch.json +++ b/erpnext/stock/doctype/batch/batch.json @@ -9,6 +9,8 @@ "field_order": [ "sb_disabled", "disabled", + "column_break_24", + "use_batchwise_valuation", "sb_batch", "batch_id", "item", @@ -186,6 +188,18 @@ "fieldtype": "Float", "label": "Produced Qty", "read_only": 1 + }, + { + "fieldname": "column_break_24", + "fieldtype": "Column Break" + }, + { + "default": "1", + "fieldname": "use_batchwise_valuation", + "fieldtype": "Check", + "label": "Use Batch-wise Valuation", + "read_only": 1, + "set_only_once": 1 } ], "icon": "fa fa-archive", @@ -193,7 +207,7 @@ "image_field": "image", "links": [], "max_attachments": 5, - "modified": "2021-07-08 16:22:01.343105", + "modified": "2021-10-11 13:38:12.806976", "modified_by": "Administrator", "module": "Stock", "name": "Batch", From ce0514c8db17d59f2f84b3f6c263cd7e5877a049 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 15 Feb 2022 11:41:41 +0530 Subject: [PATCH 07/29] feat: batch wise valuation rates start with most used case: negative inventory isn't enabled - simple addition of qty and value when new batch qty is added - fetch outgoing rate from stock movement of specific batch --- erpnext/stock/doctype/batch/test_batch.py | 46 ++++++++++++++++++++ erpnext/stock/stock_ledger.py | 52 +++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index 0a663c2a18..e7d04db454 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -7,6 +7,7 @@ from frappe.utils import cint, flt from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.stock.doctype.batch.batch import UnableToSelectBatchError, get_batch_no, get_batch_qty +from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details from erpnext.tests.utils import ERPNextTestCase @@ -300,6 +301,51 @@ class TestBatch(ERPNextTestCase): details = get_item_details(args) self.assertEqual(details.get('price_list_rate'), 400) + + def test_basic_batch_wise_valuation(self, batch_qty = 100): + item_code = "_TestBatchWiseVal" + warehouse = "_Test Warehouse - _TC" + self.make_batch_item(item_code) + + rates = [42, 420] + + batches = {} + for rate in rates: + se = make_stock_entry(item_code=item_code, qty=10, rate=rate, target=warehouse) + batches[se.items[0].batch_no] = rate + + LOW, HIGH = list(batches.keys()) + + # consume things out of order + consumption_plan = [ + (HIGH, 1), + (LOW, 2), + (HIGH, 2), + (HIGH, 4), + (LOW, 6), + ] + + stock_value = sum(rates) * 10 + qty_after_transaction = 20 + for batch, qty in consumption_plan: + # consume out of order + se = make_stock_entry(item_code=item_code, source=warehouse, qty=qty, batch_no=batch) + + sle = frappe.get_last_doc("Stock Ledger Entry", {"is_cancelled": 0, "voucher_no": se.name}) + + stock_value_difference = sle.actual_qty * batches[sle.batch_no] + self.assertAlmostEqual(sle.stock_value_difference, stock_value_difference) + + stock_value += stock_value_difference + self.assertAlmostEqual(sle.stock_value, stock_value) + + qty_after_transaction += sle.actual_qty + self.assertAlmostEqual(sle.qty_after_transaction, qty_after_transaction) + self.assertAlmostEqual(sle.valuation_rate, stock_value / qty_after_transaction) + + self.assertEqual(sle.stock_queue, []) # queues don't apply on batched items + + def create_batch(item_code, rate, create_item_price_for_batch): pi = make_purchase_invoice(company="_Test Company", warehouse= "Stores - _TC", cost_center = "Main - _TC", update_stock=1, diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 00ca81f2b4..c33cc12c2f 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -447,6 +447,8 @@ class update_entries_after(object): self.wh_data.qty_after_transaction = sle.qty_after_transaction self.wh_data.stock_value = flt(self.wh_data.qty_after_transaction) * flt(self.wh_data.valuation_rate) + elif sle.batch_no and frappe.db.get_value("Batch", sle.batch_no, "use_batchwise_valuation", cache=True): + self.update_batched_values(sle) else: if sle.voucher_type=="Stock Reconciliation" and not sle.batch_no: # assert @@ -481,6 +483,7 @@ class update_entries_after(object): if not self.args.get("sle_id"): self.update_outgoing_rate_on_transaction(sle) + def validate_negative_stock(self, sle): """ validate negative stock for entries current datetime onwards @@ -736,7 +739,22 @@ class update_entries_after(object): if not self.wh_data.stock_queue: self.wh_data.stock_queue.append([0, sle.incoming_rate or sle.outgoing_rate or self.wh_data.valuation_rate]) + def update_batched_values(self, sle): + incoming_rate = flt(sle.incoming_rate) + actual_qty = flt(sle.actual_qty) + self.wh_data.qty_after_transaction += actual_qty + + if actual_qty > 0: + stock_value_difference = incoming_rate * actual_qty + self.wh_data.stock_value += stock_value_difference + else: + outgoing_rate = _get_batch_outgoing_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + stock_value_difference = outgoing_rate * actual_qty + self.wh_data.stock_value += stock_value_difference + + if self.wh_data.qty_after_transaction: + self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction def check_if_allow_zero_valuation_rate(self, voucher_type, voucher_detail_no): ref_item_dt = "" @@ -897,6 +915,40 @@ def get_sle_by_voucher_detail_no(voucher_detail_no, excluded_sle=None): ['item_code', 'warehouse', 'posting_date', 'posting_time', 'timestamp(posting_date, posting_time) as timestamp'], as_dict=1) +def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation): + + batch_details = frappe.db.sql(""" + select sum(stock_value_difference) as batch_value, sum(actual_qty) as batch_qty + from `tabStock Ledger Entry` + where + item_code = %(item_code)s + and warehouse = %(warehouse)s + and batch_no = %(batch_no)s + and is_cancelled = 0 + and ( + timestamp(posting_date, posting_time) < timestamp(%(posting_date)s, %(posting_time)s) + or ( + timestamp(posting_date, posting_time) = timestamp(%(posting_date)s, %(posting_time)s) + and creation < %(creation)s + ) + ) + """, + { + "item_code": item_code, + "warehouse": warehouse, + "batch_no": batch_no, + "posting_date": posting_date, + "posting_time": posting_time, + "creation": creation, + }, + as_dict=True + ) + + if batch_details and batch_details[0].batch_qty: + return batch_details[0].batch_value / batch_details[0].batch_qty + + + def get_valuation_rate(item_code, warehouse, voucher_type, voucher_no, allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True): From 342d09a671c522031f73ba777950c70983cea31a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 14:28:51 +0530 Subject: [PATCH 08/29] feat: get_valuation_rate batch wise This function is used to show valuation rate on frontend and also as fallback in case values aren't available. Add "batch_no" param to get batch specific valuation rates. Co-Authored-By: Alan Tom <2.alan.tom@gmail.com> --- erpnext/controllers/buying_controller.py | 1 + .../controllers/sales_and_purchase_return.py | 1 + erpnext/controllers/selling_controller.py | 1 + erpnext/public/js/controllers/transaction.js | 1 + erpnext/stock/doctype/batch/test_batch.py | 39 +++++++++++++++++ .../stock/doctype/stock_entry/stock_entry.js | 3 ++ .../stock/doctype/stock_entry/stock_entry.py | 3 +- erpnext/stock/stock_ledger.py | 43 +++++++++++++------ erpnext/stock/utils.py | 2 +- 9 files changed, 79 insertions(+), 15 deletions(-) diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index a181af7313..b831557200 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -249,6 +249,7 @@ class BuyingController(StockController, Subcontracting): "posting_time": self.get('posting_time'), "qty": -1 * flt(d.get('stock_qty')), "serial_no": d.get('serial_no'), + "batch_no": d.get("batch_no"), "company": self.company, "voucher_type": self.doctype, "voucher_no": self.name, diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index df3c5f10c1..8c3aab442b 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -420,6 +420,7 @@ def get_rate_for_return(voucher_type, voucher_no, item_code, return_against=None "posting_time": sle.get('posting_time'), "qty": sle.actual_qty, "serial_no": sle.get('serial_no'), + "batch_no": sle.get("batch_no"), "company": sle.company, "voucher_type": sle.voucher_type, "voucher_no": sle.voucher_no diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 31b2209399..e918cde7c4 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -394,6 +394,7 @@ class SellingController(StockController): "posting_time": self.get('posting_time') or nowtime(), "qty": qty if cint(self.get("is_return")) else (-1 * qty), "serial_no": d.get('serial_no'), + "batch_no": d.get("batch_no"), "company": self.company, "voucher_type": self.doctype, "voucher_no": self.name, diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 136e1edb6b..933ced0bd7 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -719,6 +719,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe 'posting_time': posting_time, 'qty': item.qty * item.conversion_factor, 'serial_no': item.serial_no, + 'batch_no': item.batch_no, 'voucher_type': voucher_type, 'company': company, 'allow_zero_valuation_rate': item.allow_zero_valuation_rate diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index e7d04db454..73a48b3f13 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -8,7 +8,11 @@ from frappe.utils import cint, flt from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.stock.doctype.batch.batch import UnableToSelectBatchError, get_batch_no, get_batch_qty from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry +from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, +) from erpnext.stock.get_item_details import get_item_details +from erpnext.stock.stock_ledger import get_valuation_rate from erpnext.tests.utils import ERPNextTestCase @@ -345,6 +349,41 @@ class TestBatch(ERPNextTestCase): self.assertEqual(sle.stock_queue, []) # queues don't apply on batched items + def test_moving_batch_valuation_rates(self): + item_code = "_TestBatchWiseVal" + warehouse = "_Test Warehouse - _TC" + self.make_batch_item(item_code) + + def assertValuation(expected): + actual = get_valuation_rate(item_code, warehouse, "voucher_type", "voucher_no", batch_no=batch_no) + self.assertAlmostEqual(actual, expected) + + se = make_stock_entry(item_code=item_code, qty=100, rate=10, target=warehouse) + batch_no = se.items[0].batch_no + assertValuation(10) + + # consumption should never affect current valuation rate + make_stock_entry(item_code=item_code, qty=20, source=warehouse) + assertValuation(10) + + make_stock_entry(item_code=item_code, qty=30, source=warehouse) + assertValuation(10) + + # 50 * 10 = 500 current value, add more item with higher valuation + make_stock_entry(item_code=item_code, qty=50, rate=20, target=warehouse, batch_no=batch_no) + assertValuation(15) + + # consuming again shouldn't do anything + make_stock_entry(item_code=item_code, qty=20, source=warehouse) + assertValuation(15) + + # reset rate with stock reconiliation + create_stock_reconciliation(item_code=item_code, warehouse=warehouse, qty=10, rate=25, batch_no=batch_no) + assertValuation(25) + + make_stock_entry(item_code=item_code, qty=20, rate=20, target=warehouse, batch_no=batch_no) + assertValuation((20 * 20 + 10 * 25) / (10 + 20)) + def create_batch(item_code, rate, create_item_price_for_batch): pi = make_purchase_invoice(company="_Test Company", diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.js b/erpnext/stock/doctype/stock_entry/stock_entry.js index c4b8131305..5c9da3a205 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.js +++ b/erpnext/stock/doctype/stock_entry/stock_entry.js @@ -425,6 +425,7 @@ frappe.ui.form.on('Stock Entry', { 'posting_time' : frm.doc.posting_time, 'warehouse' : cstr(item.s_warehouse) || cstr(item.t_warehouse), 'serial_no' : item.serial_no, + 'batch_no' : item.batch_no, 'company' : frm.doc.company, 'qty' : item.s_warehouse ? -1*flt(item.transfer_qty) : flt(item.transfer_qty), 'voucher_type' : frm.doc.doctype, @@ -457,6 +458,7 @@ frappe.ui.form.on('Stock Entry', { 'warehouse': cstr(child.s_warehouse) || cstr(child.t_warehouse), 'transfer_qty': child.transfer_qty, 'serial_no': child.serial_no, + 'batch_no': child.batch_no, 'qty': child.s_warehouse ? -1* child.transfer_qty : child.transfer_qty, 'posting_date': frm.doc.posting_date, 'posting_time': frm.doc.posting_time, @@ -680,6 +682,7 @@ frappe.ui.form.on('Stock Entry Detail', { 'warehouse' : cstr(d.s_warehouse) || cstr(d.t_warehouse), 'transfer_qty' : d.transfer_qty, 'serial_no' : d.serial_no, + 'batch_no' : d.batch_no, 'bom_no' : d.bom_no, 'expense_account' : d.expense_account, 'cost_center' : d.cost_center, diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 9ba007a186..99cf4de5de 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -510,7 +510,7 @@ class StockEntry(StockController): d.basic_rate = get_valuation_rate(d.item_code, d.t_warehouse, self.doctype, self.name, d.allow_zero_valuation_rate, currency=erpnext.get_company_currency(self.company), company=self.company, - raise_error_if_no_rate=raise_error_if_no_rate) + raise_error_if_no_rate=raise_error_if_no_rate, batch_no=d.batch_no) d.basic_rate = flt(d.basic_rate, d.precision("basic_rate")) if d.is_process_loss: @@ -541,6 +541,7 @@ class StockEntry(StockController): "posting_time": self.posting_time, "qty": item.s_warehouse and -1*flt(item.transfer_qty) or flt(item.transfer_qty), "serial_no": item.serial_no, + "batch_no": item.batch_no, "voucher_type": self.doctype, "voucher_no": self.name, "company": self.company, diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index c33cc12c2f..53bfed8722 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -634,7 +634,7 @@ class update_entries_after(object): if not allow_zero_rate: self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company) + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) def get_incoming_value_for_serial_nos(self, sle, serial_nos): # get rate from serial nos within same company @@ -702,7 +702,7 @@ class update_entries_after(object): if not allow_zero_valuation_rate: self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company) + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) def update_queue_values(self, sle): incoming_rate = flt(sle.incoming_rate) @@ -722,7 +722,7 @@ class update_entries_after(object): if not allow_zero_valuation_rate: return get_valuation_rate(sle.item_code, sle.warehouse, sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company) + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) else: return 0.0 @@ -950,21 +950,38 @@ def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posti def get_valuation_rate(item_code, warehouse, voucher_type, voucher_no, - allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True): + allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True, batch_no=None): if not company: company = frappe.get_cached_value("Warehouse", warehouse, "company") + last_valuation_rate = None + + # Get moving average rate of a specific batch number + if warehouse and batch_no and frappe.db.get_value("Batch", batch_no, "use_batchwise_valuation"): + last_valuation_rate = frappe.db.sql(""" + select sum(stock_value_difference) / sum(actual_qty) + from `tabStock Ledger Entry` + where + item_code = %s + AND warehouse = %s + AND batch_no = %s + AND is_cancelled = 0 + AND NOT (voucher_no = %s AND voucher_type = %s) + """, + (item_code, warehouse, batch_no, voucher_no, voucher_type)) + # Get valuation rate from last sle for the same item and warehouse - last_valuation_rate = frappe.db.sql("""select valuation_rate - from `tabStock Ledger Entry` force index (item_warehouse) - where - item_code = %s - AND warehouse = %s - AND valuation_rate >= 0 - AND is_cancelled = 0 - AND NOT (voucher_no = %s AND voucher_type = %s) - order by posting_date desc, posting_time desc, name desc limit 1""", (item_code, warehouse, voucher_no, voucher_type)) + if not last_valuation_rate or last_valuation_rate[0][0] is None: + last_valuation_rate = frappe.db.sql("""select valuation_rate + from `tabStock Ledger Entry` force index (item_warehouse) + where + item_code = %s + AND warehouse = %s + AND valuation_rate >= 0 + AND is_cancelled = 0 + AND NOT (voucher_no = %s AND voucher_type = %s) + order by posting_date desc, posting_time desc, name desc limit 1""", (item_code, warehouse, voucher_no, voucher_type)) if not last_valuation_rate: # Get valuation rate from last sle for the item against any warehouse diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 7263e39cc9..3be252e593 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -231,7 +231,7 @@ def get_incoming_rate(args, raise_error_if_no_rate=True): in_rate = get_valuation_rate(args.get('item_code'), args.get('warehouse'), args.get('voucher_type'), voucher_no, args.get('allow_zero_valuation'), currency=erpnext.get_company_currency(args.get('company')), company=args.get('company'), - raise_error_if_no_rate=raise_error_if_no_rate) + raise_error_if_no_rate=raise_error_if_no_rate, batch_no=args.get("batch_no")) return flt(in_rate) From ab926521bd0c9802666032cb3c32aa803655bde0 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 15:37:03 +0530 Subject: [PATCH 09/29] fix: correct incoming rate for batched items --- erpnext/stock/stock_ledger.py | 5 ++--- erpnext/stock/utils.py | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 53bfed8722..4748ad4e46 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -749,7 +749,7 @@ class update_entries_after(object): stock_value_difference = incoming_rate * actual_qty self.wh_data.stock_value += stock_value_difference else: - outgoing_rate = _get_batch_outgoing_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) stock_value_difference = outgoing_rate * actual_qty self.wh_data.stock_value += stock_value_difference @@ -915,7 +915,7 @@ def get_sle_by_voucher_detail_no(voucher_detail_no, excluded_sle=None): ['item_code', 'warehouse', 'posting_date', 'posting_time', 'timestamp(posting_date, posting_time) as timestamp'], as_dict=1) -def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation): +def get_batch_incoming_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation=None): batch_details = frappe.db.sql(""" select sum(stock_value_difference) as batch_value, sum(actual_qty) as batch_qty @@ -948,7 +948,6 @@ def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posti return batch_details[0].batch_value / batch_details[0].batch_qty - def get_valuation_rate(item_code, warehouse, voucher_type, voucher_no, allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True, batch_no=None): diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 3be252e593..e2bd2f197d 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -209,13 +209,28 @@ def _create_bin(item_code, warehouse): @frappe.whitelist() def get_incoming_rate(args, raise_error_if_no_rate=True): """Get Incoming Rate based on valuation method""" - from erpnext.stock.stock_ledger import get_previous_sle, get_valuation_rate + from erpnext.stock.stock_ledger import ( + get_batch_incoming_rate, + get_previous_sle, + get_valuation_rate, + ) if isinstance(args, str): args = json.loads(args) - in_rate = 0 + voucher_no = args.get('voucher_no') or args.get('name') + + in_rate = None if (args.get("serial_no") or "").strip(): in_rate = get_avg_purchase_rate(args.get("serial_no")) + elif args.get("batch_no") and \ + frappe.db.get_value("Batch", args.get("batch_no"), "use_batchwise_valuation", cache=True): + in_rate = get_batch_incoming_rate( + item_code=args.get('item_code'), + warehouse=args.get('warehouse'), + batch_no=args.get("batch_no"), + posting_date=args.get("posting_date"), + posting_time=args.get("posting_time"), + ) else: valuation_method = get_valuation_method(args.get("item_code")) previous_sle = get_previous_sle(args) @@ -226,8 +241,7 @@ def get_incoming_rate(args, raise_error_if_no_rate=True): elif valuation_method == 'Moving Average': in_rate = previous_sle.get('valuation_rate') or 0 - if not in_rate: - voucher_no = args.get('voucher_no') or args.get('name') + if in_rate is None: in_rate = get_valuation_rate(args.get('item_code'), args.get('warehouse'), args.get('voucher_type'), voucher_no, args.get('allow_zero_valuation'), currency=erpnext.get_company_currency(args.get('company')), company=args.get('company'), From 102fff24c886b49d08776307d513d68ffd56e918 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 15:51:04 +0530 Subject: [PATCH 10/29] refactor: convert query to QB and make creation optional --- erpnext/stock/doctype/batch/test_batch.py | 4 +- erpnext/stock/stock_ledger.py | 53 ++++++++++++----------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index 73a48b3f13..6495b56e92 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -1,6 +1,8 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +import json + import frappe from frappe.exceptions import ValidationError from frappe.utils import cint, flt @@ -347,7 +349,7 @@ class TestBatch(ERPNextTestCase): self.assertAlmostEqual(sle.qty_after_transaction, qty_after_transaction) self.assertAlmostEqual(sle.valuation_rate, stock_value / qty_after_transaction) - self.assertEqual(sle.stock_queue, []) # queues don't apply on batched items + self.assertEqual(json.loads(sle.stock_queue), []) # queues don't apply on batched items def test_moving_batch_valuation_rates(self): item_code = "_TestBatchWiseVal" diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 4748ad4e46..cacec408ce 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -8,7 +8,9 @@ from typing import Optional import frappe from frappe import _ from frappe.model.meta import get_field_precision +from frappe.query_builder.functions import Sum from frappe.utils import cint, cstr, flt, get_link_to_form, getdate, now, nowdate +from pypika import CustomFunction import erpnext from erpnext.stock.doctype.bin.bin import update_qty as update_bin_qty @@ -24,7 +26,6 @@ class NegativeStockError(frappe.ValidationError): pass class SerialNoExistsInFutureTransaction(frappe.ValidationError): pass -_exceptions = frappe.local('stockledger_exceptions') def make_sl_entries(sl_entries, allow_negative_stock=False, via_landed_cost_voucher=False): from erpnext.controllers.stock_controller import future_sle_exists @@ -917,32 +918,32 @@ def get_sle_by_voucher_detail_no(voucher_detail_no, excluded_sle=None): def get_batch_incoming_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation=None): - batch_details = frappe.db.sql(""" - select sum(stock_value_difference) as batch_value, sum(actual_qty) as batch_qty - from `tabStock Ledger Entry` - where - item_code = %(item_code)s - and warehouse = %(warehouse)s - and batch_no = %(batch_no)s - and is_cancelled = 0 - and ( - timestamp(posting_date, posting_time) < timestamp(%(posting_date)s, %(posting_time)s) - or ( - timestamp(posting_date, posting_time) = timestamp(%(posting_date)s, %(posting_time)s) - and creation < %(creation)s - ) + Timestamp = CustomFunction('timestamp', ['date', 'time']) + + sle = frappe.qb.DocType("Stock Ledger Entry") + + timestamp_condition = (Timestamp(sle.posting_date, sle.posting_time) < Timestamp(posting_date, posting_time)) + if creation: + timestamp_condition |= ( + (Timestamp(sle.posting_date, sle.posting_time) == Timestamp(posting_date, posting_time)) + & (sle.creation < creation) ) - """, - { - "item_code": item_code, - "warehouse": warehouse, - "batch_no": batch_no, - "posting_date": posting_date, - "posting_time": posting_time, - "creation": creation, - }, - as_dict=True - ) + + batch_details = ( + frappe.qb + .from_(sle) + .select( + Sum(sle.stock_value_difference).as_("batch_value"), + Sum(sle.actual_qty).as_("batch_qty") + ) + .where( + (sle.item_code == item_code) + & (sle.warehouse == warehouse) + & (sle.batch_no == batch_no) + & (sle.is_cancelled == 0) + ) + .where(timestamp_condition) + ).run(as_dict=True) if batch_details and batch_details[0].batch_qty: return batch_details[0].batch_value / batch_details[0].batch_qty From d130233ffc79d085b61bc1b63956d18c03de7a88 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 16:14:15 +0530 Subject: [PATCH 11/29] test: fix expected test failures --- .../stock_reconciliation/test_stock_reconciliation.py | 11 ++++++----- erpnext/stock/stock_ledger.py | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 86af0a0cf3..2ffe127d9a 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -200,7 +200,6 @@ class TestStockReconciliation(ERPNextTestCase): def test_stock_reco_for_batch_item(self): to_delete_records = [] - to_delete_serial_nos = [] # Add new serial nos item_code = "Stock-Reco-batch-Item-1" @@ -208,20 +207,22 @@ class TestStockReconciliation(ERPNextTestCase): sr = create_stock_reconciliation(item_code=item_code, warehouse = warehouse, qty=5, rate=200, do_not_submit=1) - sr.save(ignore_permissions=True) + sr.save() sr.submit() - self.assertTrue(sr.items[0].batch_no) + batch_no = sr.items[0].batch_no + self.assertTrue(batch_no) to_delete_records.append(sr.name) sr1 = create_stock_reconciliation(item_code=item_code, - warehouse = warehouse, qty=6, rate=300, batch_no=sr.items[0].batch_no) + warehouse = warehouse, qty=6, rate=300, batch_no=batch_no) args = { "item_code": item_code, "warehouse": warehouse, "posting_date": nowdate(), "posting_time": nowtime(), + "batch_no": batch_no, } valuation_rate = get_incoming_rate(args) @@ -230,7 +231,7 @@ class TestStockReconciliation(ERPNextTestCase): sr2 = create_stock_reconciliation(item_code=item_code, - warehouse = warehouse, qty=0, rate=0, batch_no=sr.items[0].batch_no) + warehouse = warehouse, qty=0, rate=0, batch_no=batch_no) stock_value = get_stock_value_on(warehouse, nowdate(), item_code) self.assertEqual(stock_value, 0) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index cacec408ce..2dd26643f7 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -751,6 +751,7 @@ class update_entries_after(object): self.wh_data.stock_value += stock_value_difference else: outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + # TODO: negative stock handling stock_value_difference = outgoing_rate * actual_qty self.wh_data.stock_value += stock_value_difference From 312db429e4605d6d0ce47d1034662fdf0ec053b7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 16:26:17 +0530 Subject: [PATCH 12/29] refactor: use qb for patching flag --- erpnext/patches/v14_0/update_batch_valuation_flag.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/erpnext/patches/v14_0/update_batch_valuation_flag.py b/erpnext/patches/v14_0/update_batch_valuation_flag.py index d9f08d8d97..55c8c48aa2 100644 --- a/erpnext/patches/v14_0/update_batch_valuation_flag.py +++ b/erpnext/patches/v14_0/update_batch_valuation_flag.py @@ -6,7 +6,6 @@ def execute(): - Don't use batchwise valuation for existing batches. - Only batches created after this patch shoule use it. """ - frappe.db.sql(""" - UPDATE `tabBatch` - SET use_batchwise_valuation=0 - """) + + batch = frappe.qb.DocType("Batch") + frappe.qb.update(batch).set(batch.use_batchwise_valuation, 0).run() From 683ef8a60397b728bd18e1a3c3c317e2f155793c Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Sat, 19 Feb 2022 16:19:30 +0530 Subject: [PATCH 13/29] test: more tests for batchwise valuation Co-Authored-By: Ankush Menat --- .../purchase_receipt/test_purchase_receipt.py | 1 + .../test_stock_ledger_entry.py | 278 ++++++++++++++++++ 2 files changed, 279 insertions(+) diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index 5ab7929a2a..d481689c13 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -1540,6 +1540,7 @@ def make_purchase_receipt(**args): "conversion_factor": args.conversion_factor or 1.0, "stock_qty": flt(qty) * (flt(args.conversion_factor) or 1.0), "serial_no": args.serial_no, + "batch_no": args.batch_no, "stock_uom": args.stock_uom or "_Test UOM", "uom": uom, "cost_center": args.cost_center or frappe.get_cached_value('Company', pr.company, 'cost_center'), diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index a1030d5496..60fea9613a 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -1,6 +1,10 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +import json +from operator import itemgetter +from uuid import uuid4 + import frappe from frappe.core.page.permission_manager.permission_manager import reset from frappe.utils import add_days, today @@ -349,6 +353,170 @@ class TestStockLedgerEntry(ERPNextTestCase): frappe.set_user("Administrator") user.remove_roles("Stock Manager") + def test_batchwise_item_valuation_moving_average(self): + suffix = get_unique_suffix() + item, warehouses, batches = setup_item_valuation_test( + valuation_method="Moving Average", suffix=suffix + ) + + # Incoming Entries for Stock Value check + pr_entry_list = [ + (item, warehouses[0], batches[0], 1, 100), + (item, warehouses[0], batches[1], 1, 50), + (item, warehouses[0], batches[0], 1, 150), + (item, warehouses[0], batches[1], 1, 100), + ] + prs = create_purchase_receipt_entries_for_batchwise_item_valuation_test(pr_entry_list) + sle_details = fetch_sle_details_for_doc_list(prs, ['stock_value']) + sv_list = [d['stock_value'] for d in sle_details] + expected_sv = [100, 150, 300, 400] + self.assertEqual(expected_sv, sv_list, "Incorrect 'Stock Value' values") + + # Outgoing Entries for Stock Value Difference check + dn_entry_list = [ + (item, warehouses[0], batches[1], 1, 200), + (item, warehouses[0], batches[0], 1, 200), + (item, warehouses[0], batches[1], 1, 200), + (item, warehouses[0], batches[0], 1, 200) + ] + dns = create_delivery_note_entries_for_batchwise_item_valuation_test(dn_entry_list) + sle_details = fetch_sle_details_for_doc_list(dns, ['stock_value_difference']) + svd_list = [-1 * d['stock_value_difference'] for d in sle_details] + expected_incoming_rates = expected_abs_svd = [75, 125, 75, 125] + + self.assertEqual(expected_abs_svd, svd_list, "Incorrect 'Stock Value Difference' values") + for dn, incoming_rate in zip(dns, expected_incoming_rates): + self.assertEqual( + dn.items[0].incoming_rate, incoming_rate, + "Incorrect 'Incoming Rate' values fetched for DN items" + ) + + + def assertSLEs(self, doc, expected_sles): + """ Compare sorted SLEs, useful for vouchers that create multiple SLEs for same line""" + sles = frappe.get_all("Stock Ledger Entry", fields=["*"], + filters={"voucher_no": doc.name, "voucher_type": doc.doctype, "is_cancelled":0}, + order_by="timestamp(posting_date, posting_time), creation") + + for exp_sle, act_sle in zip(expected_sles, sles): + for k, v in exp_sle.items(): + self.assertEqual(v, act_sle[k], msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") + + def test_batchwise_item_valuation_stock_reco(self): + suffix = get_unique_suffix() + item, warehouses, batches = setup_item_valuation_test( + valuation_method="FIFO", suffix=suffix + ) + state = { + "stock_value" : 0.0, + "qty": 0.0 + } + def update_invariants(exp_sles): + for sle in exp_sles: + state["stock_value"] += sle["stock_value_difference"] + state["qty"] += sle["actual_qty"] + sle["stock_value"] = state["stock_value"] + sle["qty_after_transaction"] = state["qty"] + + osr1 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=10, rate=100, batch_no=batches[1]) + expected_sles = [ + {"actual_qty": 10, "stock_value_difference": 1000}, + ] + update_invariants(expected_sles) + self.assertSLEs(osr1, expected_sles) + + osr2 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=13, rate=200, batch_no=batches[0]) + expected_sles = [ + {"actual_qty": 13, "stock_value_difference": 200*13}, + ] + update_invariants(expected_sles) + self.assertSLEs(osr2, expected_sles) + + sr1 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=5, rate=50, batch_no=batches[1]) + + expected_sles = [ + {"actual_qty": -10, "stock_value_difference": -10 * 100}, + {"actual_qty": 5, "stock_value_difference": 250} + ] + update_invariants(expected_sles) + self.assertSLEs(sr1, expected_sles) + + sr2 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=20, rate=75, batch_no=batches[0]) + expected_sles = [ + {"actual_qty": -13, "stock_value_difference": -13 * 200}, + {"actual_qty": 20, "stock_value_difference": 20 * 75} + ] + update_invariants(expected_sles) + self.assertSLEs(sr2, expected_sles) + + def test_legacy_item_valuation_stock_entry(self): + suffix = get_unique_suffix() + columns = [ + 'stock_value_difference', + 'stock_value', + 'actual_qty', + 'qty_after_transaction', + 'stock_queue', + ] + item, warehouses, batches = setup_item_valuation_test( + valuation_method="FIFO", suffix=suffix, use_batchwise_valuation=0 + ) + + def check_sle_details_against_expected(sle_details, expected_sle_details, detail, columns): + for i, (sle_vals, ex_sle_vals) in enumerate(zip(sle_details, expected_sle_details)): + for col, sle_val, ex_sle_val in zip(columns, sle_vals, ex_sle_vals): + if col == 'stock_queue': + sle_val = get_stock_value_from_q(sle_val) + ex_sle_val = get_stock_value_from_q(ex_sle_val) + self.assertEqual( + sle_val, ex_sle_val, + f"Incorrect {col} value on transaction #: {i} in {detail}" + ) + + # List used to defer assertions to prevent commits cause of error skipped rollback + details_list = [] + + + # Test Material Receipt Entries + se_entry_list_mr = [ + (item, None, warehouses[0], batches[0], 1, 50, "2021-01-21"), + (item, None, warehouses[0], batches[1], 1, 100, "2021-01-23"), + ] + ses = create_stock_entry_entries_for_batchwise_item_valuation_test( + se_entry_list_mr, "Material Receipt" + ) + sle_details = fetch_sle_details_for_doc_list(ses, columns=columns, as_dict=0) + expected_sle_details = [ + (50.0, 50.0, 1.0, 1.0, '[[1.0, 50.0]]'), + (100.0, 150.0, 1.0, 2.0, '[[1.0, 50.0], [1.0, 100.0]]'), + ] + details_list.append(( + sle_details, expected_sle_details, + "Material Receipt Entries", columns + )) + + + # Test Material Issue Entries + se_entry_list_mi = [ + (item, warehouses[0], None, batches[1], 1, None, "2021-01-29"), + ] + ses = create_stock_entry_entries_for_batchwise_item_valuation_test( + se_entry_list_mi, "Material Issue" + ) + sle_details = fetch_sle_details_for_doc_list(ses, columns=columns, as_dict=0) + expected_sle_details = [ + (-50.0, 100.0, -1.0, 1.0, '[[1, 100.0]]') + ] + details_list.append(( + sle_details, expected_sle_details, + "Material Issue Entries", columns + )) + + + # Run assertions + for details in details_list: + check_sle_details_against_expected(*details) + def create_repack_entry(**args): args = frappe._dict(args) @@ -412,3 +580,113 @@ def create_items(): make_item(d, properties=properties) return items + +def setup_item_valuation_test(valuation_method, suffix, use_batchwise_valuation=1, batches_list=['X', 'Y']): + from erpnext.stock.doctype.batch.batch import make_batch + from erpnext.stock.doctype.item.test_item import make_item + from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse + + item = make_item( + f"IV - Test Item {valuation_method} {suffix}", + dict(valuation_method=valuation_method, has_batch_no=1) + ) + warehouses = [create_warehouse(f"IV - Test Warehouse {i}") for i in ['J', 'K']] + batches = [f"IV - Test Batch {i} {valuation_method} {suffix}" for i in batches_list] + + for i, batch_id in enumerate(batches): + if not frappe.db.exists("Batch", batch_id): + ubw = use_batchwise_valuation + if isinstance(use_batchwise_valuation, (list, tuple)): + ubw = use_batchwise_valuation[i] + make_batch( + frappe._dict( + batch_id=batch_id, + item=item.item_code, + use_batchwise_valuation=ubw + ) + ) + + return item.item_code, warehouses, batches + +def create_purchase_receipt_entries_for_batchwise_item_valuation_test(pr_entry_list): + from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt + prs = [] + + for item, warehouse, batch_no, qty, rate in pr_entry_list: + pr = make_purchase_receipt(item=item, warehouse=warehouse, qty=qty, rate=rate, batch_no=batch_no) + prs.append(pr) + + return prs + +def create_delivery_note_entries_for_batchwise_item_valuation_test(dn_entry_list): + from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + dns = [] + for item, warehouse, batch_no, qty, rate in dn_entry_list: + so = make_sales_order( + rate=rate, + qty=qty, + item=item, + warehouse=warehouse, + against_blanket_order=0 + ) + + dn = make_delivery_note(so.name) + dn.items[0].batch_no = batch_no + dn.insert() + dn.submit() + dns.append(dn) + return dns + +def fetch_sle_details_for_doc_list(doc_list, columns, as_dict=1): + return frappe.db.sql(f""" + SELECT { ', '.join(columns)} + FROM `tabStock Ledger Entry` + WHERE + voucher_no IN %(voucher_nos)s + and docstatus = 1 + ORDER BY timestamp(posting_date, posting_time) ASC, CREATION ASC + """, dict( + voucher_nos=[doc.name for doc in doc_list] + ), as_dict=as_dict) + +def get_stock_value_from_q(q): + return sum(r*q for r,q in json.loads(q)) + +def create_stock_entry_entries_for_batchwise_item_valuation_test(se_entry_list, purpose): + ses = [] + for item, source, target, batch, qty, rate, posting_date in se_entry_list: + args = dict( + item_code=item, + qty=qty, + company="_Test Company", + batch_no=batch, + posting_date=posting_date, + purpose=purpose + ) + + if purpose == "Material Receipt": + args.update( + dict(to_warehouse=target, rate=rate) + ) + + elif purpose == "Material Issue": + args.update( + dict(from_warehouse=source) + ) + + elif purpose == "Material Transfer": + args.update( + dict(from_warehouse=source, to_warehouse=target) + ) + + else: + raise ValueError(f"Invalid purpose: {purpose}") + ses.append(make_stock_entry(**args)) + + return ses + +def get_unique_suffix(): + # Used to isolate valuation sensitive + # tests to prevent future tests from failing. + return str(uuid4())[:8].upper() From 5718777a2b3018e07ea310e87e5a2ea26ff3eb1b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 18:36:16 +0530 Subject: [PATCH 14/29] fix: consider batch_no when getting incoming rate --- erpnext/controllers/buying_controller.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index b831557200..b740476481 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -279,7 +279,8 @@ class BuyingController(StockController, Subcontracting): "posting_date": self.posting_date, "posting_time": self.posting_time, "qty": -1 * d.consumed_qty, - "serial_no": d.serial_no + "serial_no": d.serial_no, + "batch_no": d.batch_no, }) if rate > 0: From 60b8bae85f00b6a6bf4a26c7604e28e0b075bb52 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:18:35 +0530 Subject: [PATCH 15/29] test: batch wise valuation for transfer and intermediate --- .../test_stock_ledger_entry.py | 99 ++++++++++++++++--- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 60fea9613a..c298b5a096 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -354,10 +354,7 @@ class TestStockLedgerEntry(ERPNextTestCase): user.remove_roles("Stock Manager") def test_batchwise_item_valuation_moving_average(self): - suffix = get_unique_suffix() - item, warehouses, batches = setup_item_valuation_test( - valuation_method="Moving Average", suffix=suffix - ) + item, warehouses, batches = setup_item_valuation_test(valuation_method="Moving Average") # Incoming Entries for Stock Value check pr_entry_list = [ @@ -403,10 +400,7 @@ class TestStockLedgerEntry(ERPNextTestCase): self.assertEqual(v, act_sle[k], msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") def test_batchwise_item_valuation_stock_reco(self): - suffix = get_unique_suffix() - item, warehouses, batches = setup_item_valuation_test( - valuation_method="FIFO", suffix=suffix - ) + item, warehouses, batches = setup_item_valuation_test() state = { "stock_value" : 0.0, "qty": 0.0 @@ -449,8 +443,86 @@ class TestStockLedgerEntry(ERPNextTestCase): update_invariants(expected_sles) self.assertSLEs(sr2, expected_sles) + def test_batch_wise_valuation_across_warehouse(self): + item_code, warehouses, batches = setup_item_valuation_test() + source = warehouses[0] + target = warehouses[1] + + unrelated_batch = make_stock_entry(item_code=item_code, target=source, batch_no=batches[1], + qty=5, rate=10) + self.assertSLEs(unrelated_batch, [ + {"actual_qty": 5, "stock_value_difference": 10 * 5}, + ]) + + reciept = make_stock_entry(item_code=item_code, target=source, batch_no=batches[0], qty=5, rate=10) + self.assertSLEs(reciept, [ + {"actual_qty": 5, "stock_value_difference": 10 * 5}, + ]) + + transfer = make_stock_entry(item_code=item_code, source=source, target=target, batch_no=batches[0], qty=5) + self.assertSLEs(transfer, [ + {"actual_qty": -5, "stock_value_difference": -10 * 5, "warehouse": source}, + {"actual_qty": 5, "stock_value_difference": 10 * 5, "warehouse": target} + ]) + + backdated_receipt = make_stock_entry(item_code=item_code, target=source, batch_no=batches[0], + qty=5, rate=20, posting_date=add_days(today(), -1)) + self.assertSLEs(backdated_receipt, [ + {"actual_qty": 5, "stock_value_difference": 20 * 5}, + ]) + + # check reposted average rate in *future* transfer + self.assertSLEs(transfer, [ + {"actual_qty": -5, "stock_value_difference": -15 * 5, "warehouse": source, "stock_value": 15 * 5 + 10 * 5}, + {"actual_qty": 5, "stock_value_difference": 15 * 5, "warehouse": target, "stock_value": 15 * 5} + ]) + + transfer_unrelated = make_stock_entry(item_code=item_code, source=source, + target=target, batch_no=batches[1], qty=5) + self.assertSLEs(transfer_unrelated, [ + {"actual_qty": -5, "stock_value_difference": -10 * 5, "warehouse": source, "stock_value": 15 * 5}, + {"actual_qty": 5, "stock_value_difference": 10 * 5, "warehouse": target, "stock_value": 15 * 5 + 10 * 5} + ]) + + def test_intermediate_average_batch_wise_valuation(self): + """ A batch has moving average up until posting time, + check if same is respected when backdated entry is inserted in middle""" + item_code, warehouses, batches = setup_item_valuation_test() + warehouse = warehouses[0] + + batch = batches[0] + + yesterday = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batch, + qty=1, rate=10, posting_date=add_days(today(), -1)) + self.assertSLEs(yesterday, [ + {"actual_qty": 1, "stock_value_difference": 10}, + ]) + + tomorrow = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=1, rate=30, posting_date=add_days(today(), 1)) + self.assertSLEs(tomorrow, [ + {"actual_qty": 1, "stock_value_difference": 30}, + ]) + + create_today = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=1, rate=20) + self.assertSLEs(create_today, [ + {"actual_qty": 1, "stock_value_difference": 20}, + ]) + + consume_today = make_stock_entry(item_code=item_code, source=warehouse, batch_no=batches[0], + qty=1) + self.assertSLEs(consume_today, [ + {"actual_qty": -1, "stock_value_difference": -15}, + ]) + + consume_tomorrow = make_stock_entry(item_code=item_code, source=warehouse, batch_no=batches[0], + qty=2, posting_date=add_days(today(), 2)) + self.assertSLEs(consume_tomorrow, [ + {"stock_value_difference": -(30 + 15), "stock_value": 0, "qty_after_transaction": 0}, + ]) + def test_legacy_item_valuation_stock_entry(self): - suffix = get_unique_suffix() columns = [ 'stock_value_difference', 'stock_value', @@ -458,9 +530,7 @@ class TestStockLedgerEntry(ERPNextTestCase): 'qty_after_transaction', 'stock_queue', ] - item, warehouses, batches = setup_item_valuation_test( - valuation_method="FIFO", suffix=suffix, use_batchwise_valuation=0 - ) + item, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0) def check_sle_details_against_expected(sle_details, expected_sle_details, detail, columns): for i, (sle_vals, ex_sle_vals) in enumerate(zip(sle_details, expected_sle_details)): @@ -581,11 +651,14 @@ def create_items(): return items -def setup_item_valuation_test(valuation_method, suffix, use_batchwise_valuation=1, batches_list=['X', 'Y']): +def setup_item_valuation_test(valuation_method="FIFO", suffix=None, use_batchwise_valuation=1, batches_list=['X', 'Y']): from erpnext.stock.doctype.batch.batch import make_batch from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse + if not suffix: + suffix = get_unique_suffix() + item = make_item( f"IV - Test Item {valuation_method} {suffix}", dict(valuation_method=valuation_method, has_batch_no=1) From c5bd34d2383982e99db825cef1b5ec8215ccabee Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:21:12 +0530 Subject: [PATCH 16/29] test: multi-batch stock entry --- .../doctype/stock_entry/test_stock_entry.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index 306f2c3e69..6c6513beff 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -1107,6 +1107,52 @@ class TestStockEntry(ERPNextTestCase): posting_date='2021-09-02', # backdated consumption of 2nd batch purpose='Material Issue') + def test_multi_batch_value_diff(self): + """ Test value difference on stock entry in case of multi-batch. + | Stock entry | batch | qty | rate | value diff on SE | + | --- | --- | --- | --- | --- | + | receipt | A | 1 | 10 | 30 | + | receipt | B | 1 | 20 | | + | issue | A | -1 | 10 | -30 (to assert after submit) | + | issue | B | -1 | 20 | | + """ + from erpnext.stock.doctype.batch.test_batch import TestBatch + + batch_nos = [] + + item_code = '_TestMultibatchFifo' + TestBatch.make_batch_item(item_code) + warehouse = '_Test Warehouse - _TC' + receipt = make_stock_entry( + item_code=item_code, + qty=1, + rate=10, + to_warehouse=warehouse, + purpose='Material Receipt', + do_not_save=True + ) + receipt.append("items", frappe.copy_doc(receipt.items[0], ignore_no_copy=False).update({"basic_rate": 20}) ) + receipt.save() + receipt.submit() + batch_nos.extend(row.batch_no for row in receipt.items) + self.assertEqual(receipt.value_difference, 30) + + issue = make_stock_entry( + item_code=item_code, + qty=1, + from_warehouse=warehouse, + purpose='Material Issue', + do_not_save=True + ) + issue.append("items", frappe.copy_doc(issue.items[0], ignore_no_copy=False)) + for row, batch_no in zip(issue.items, batch_nos): + row.batch_no = batch_no + issue.save() + issue.submit() + + issue.reload() # reload because reposting current voucher updates rate + self.assertEqual(issue.value_difference, -30) + def make_serialized_item(**args): args = frappe._dict(args) se = frappe.copy_doc(test_records[0]) From d7ca83ef0b42af42bca94e43c18c26cbf8e19ed3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:35:33 +0530 Subject: [PATCH 17/29] refactor: code duplication for fallback rates --- erpnext/stock/stock_ledger.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 2dd26643f7..9339b3ea23 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -633,9 +633,7 @@ class update_entries_after(object): if not self.wh_data.valuation_rate and sle.voucher_detail_no: allow_zero_rate = self.check_if_allow_zero_valuation_rate(sle.voucher_type, sle.voucher_detail_no) if not allow_zero_rate: - self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, - sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + self.wh_data.valuation_rate = self.get_fallback_rate(sle) def get_incoming_value_for_serial_nos(self, sle, serial_nos): # get rate from serial nos within same company @@ -701,9 +699,7 @@ class update_entries_after(object): if not self.wh_data.valuation_rate and sle.voucher_detail_no: allow_zero_valuation_rate = self.check_if_allow_zero_valuation_rate(sle.voucher_type, sle.voucher_detail_no) if not allow_zero_valuation_rate: - self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, - sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + self.wh_data.valuation_rate = self.get_fallback_rate(sle) def update_queue_values(self, sle): incoming_rate = flt(sle.incoming_rate) @@ -721,9 +717,7 @@ class update_entries_after(object): def rate_generator() -> float: allow_zero_valuation_rate = self.check_if_allow_zero_valuation_rate(sle.voucher_type, sle.voucher_detail_no) if not allow_zero_valuation_rate: - return get_valuation_rate(sle.item_code, sle.warehouse, - sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + return self.get_fallback_rate(sle) else: return 0.0 @@ -771,6 +765,13 @@ class update_entries_after(object): else: return 0 + def get_fallback_rate(self, sle) -> float: + """When exact incoming rate isn't available use any of other "average" rates as fallback. + This should only get used for negative stock.""" + return get_valuation_rate(sle.item_code, sle.warehouse, + sle.voucher_type, sle.voucher_no, self.allow_zero_rate, + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + def get_sle_before_datetime(self, args): """get previous stock ledger entry before current time-bucket""" sle = get_stock_ledger_entries(args, "<", "desc", "limit 1", for_update=False) From aba7a7ce4e4dc1fb264023db0034df5e906b5571 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:36:28 +0530 Subject: [PATCH 18/29] fix: handle negative inventory inside a batch --- erpnext/stock/stock_ledger.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 9339b3ea23..edbe755329 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -742,13 +742,17 @@ class update_entries_after(object): if actual_qty > 0: stock_value_difference = incoming_rate * actual_qty - self.wh_data.stock_value += stock_value_difference else: outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) - # TODO: negative stock handling + if outgoing_rate is None: + # This can *only* happen if qty available for the batch is zero. + # in such case fall back various other rates. + # future entries will correct the overall accounting as each + # batch individually uses moving average rates. + outgoing_rate = self.get_fallback_rate(sle) stock_value_difference = outgoing_rate * actual_qty - self.wh_data.stock_value += stock_value_difference + self.wh_data.stock_value += stock_value_difference if self.wh_data.qty_after_transaction: self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction From b534fee2c7220390ed749d9ee87759663558a019 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 20:58:36 +0530 Subject: [PATCH 19/29] refactor: use queue difference instead of actual values --- erpnext/stock/stock_ledger.py | 19 ++++++++++++------- erpnext/stock/tests/test_valuation.py | 12 ++++++------ erpnext/stock/valuation.py | 12 ++++++------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index edbe755329..677266ee0c 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -19,7 +19,7 @@ from erpnext.stock.utils import ( get_or_make_bin, get_valuation_method, ) -from erpnext.stock.valuation import FIFOValuation, LIFOValuation +from erpnext.stock.valuation import FIFOValuation, LIFOValuation, round_off_if_near_zero class NegativeStockError(frappe.ValidationError): pass @@ -465,7 +465,6 @@ class update_entries_after(object): self.wh_data.stock_value = flt(self.wh_data.qty_after_transaction) * flt(self.wh_data.valuation_rate) else: self.update_queue_values(sle) - self.wh_data.qty_after_transaction += flt(sle.actual_qty) # rounding as per precision self.wh_data.stock_value = flt(self.wh_data.stock_value, self.precision) @@ -706,11 +705,15 @@ class update_entries_after(object): actual_qty = flt(sle.actual_qty) outgoing_rate = flt(sle.outgoing_rate) + self.wh_data.qty_after_transaction = round_off_if_near_zero(self.wh_data.qty_after_transaction + actual_qty) + if self.valuation_method == "LIFO": stock_queue = LIFOValuation(self.wh_data.stock_queue) else: stock_queue = FIFOValuation(self.wh_data.stock_queue) + _prev_qty, prev_stock_value = stock_queue.get_total_stock_and_value() + if actual_qty > 0: stock_queue.add_stock(qty=actual_qty, rate=incoming_rate) else: @@ -723,17 +726,19 @@ class update_entries_after(object): stock_queue.remove_stock(qty=abs(actual_qty), outgoing_rate=outgoing_rate, rate_generator=rate_generator) - stock_qty, stock_value = stock_queue.get_total_stock_and_value() + _qty, stock_value = stock_queue.get_total_stock_and_value() + + stock_value_difference = stock_value - prev_stock_value self.wh_data.stock_queue = stock_queue.state - self.wh_data.stock_value = stock_value - if stock_qty: - self.wh_data.valuation_rate = stock_value / stock_qty - + self.wh_data.stock_value = round_off_if_near_zero(self.wh_data.stock_value + stock_value_difference) if not self.wh_data.stock_queue: self.wh_data.stock_queue.append([0, sle.incoming_rate or sle.outgoing_rate or self.wh_data.valuation_rate]) + if self.wh_data.qty_after_transaction: + self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction + def update_batched_values(self, sle): incoming_rate = flt(sle.incoming_rate) actual_qty = flt(sle.actual_qty) diff --git a/erpnext/stock/tests/test_valuation.py b/erpnext/stock/tests/test_valuation.py index 648d4406ca..bdb768f1ad 100644 --- a/erpnext/stock/tests/test_valuation.py +++ b/erpnext/stock/tests/test_valuation.py @@ -7,7 +7,7 @@ from hypothesis import strategies as st from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry -from erpnext.stock.valuation import FIFOValuation, LIFOValuation, _round_off_if_near_zero +from erpnext.stock.valuation import FIFOValuation, LIFOValuation, round_off_if_near_zero from erpnext.tests.utils import ERPNextTestCase qty_gen = st.floats(min_value=-1e6, max_value=1e6) @@ -113,11 +113,11 @@ class TestFIFOValuation(unittest.TestCase): self.assertTotalQty(0) def test_rounding_off_near_zero(self): - self.assertEqual(_round_off_if_near_zero(0), 0) - self.assertEqual(_round_off_if_near_zero(1), 1) - self.assertEqual(_round_off_if_near_zero(-1), -1) - self.assertEqual(_round_off_if_near_zero(-1e-8), 0) - self.assertEqual(_round_off_if_near_zero(1e-8), 0) + self.assertEqual(round_off_if_near_zero(0), 0) + self.assertEqual(round_off_if_near_zero(1), 1) + self.assertEqual(round_off_if_near_zero(-1), -1) + self.assertEqual(round_off_if_near_zero(-1e-8), 0) + self.assertEqual(round_off_if_near_zero(1e-8), 0) def test_totals(self): self.queue.add_stock(1, 10) diff --git a/erpnext/stock/valuation.py b/erpnext/stock/valuation.py index ee9477ed74..e2bd1ad4df 100644 --- a/erpnext/stock/valuation.py +++ b/erpnext/stock/valuation.py @@ -34,7 +34,7 @@ class BinWiseValuation(ABC): total_qty += flt(qty) total_value += flt(qty) * flt(rate) - return _round_off_if_near_zero(total_qty), _round_off_if_near_zero(total_value) + return round_off_if_near_zero(total_qty), round_off_if_near_zero(total_value) def __repr__(self): return str(self.state) @@ -136,7 +136,7 @@ class FIFOValuation(BinWiseValuation): fifo_bin = self.queue[index] if qty >= fifo_bin[QTY]: # consume current bin - qty = _round_off_if_near_zero(qty - fifo_bin[QTY]) + qty = round_off_if_near_zero(qty - fifo_bin[QTY]) to_consume = self.queue.pop(index) consumed_bins.append(list(to_consume)) @@ -148,7 +148,7 @@ class FIFOValuation(BinWiseValuation): break else: # qty found in current bin consume it and exit - fifo_bin[QTY] = _round_off_if_near_zero(fifo_bin[QTY] - qty) + fifo_bin[QTY] = round_off_if_near_zero(fifo_bin[QTY] - qty) consumed_bins.append([qty, fifo_bin[RATE]]) qty = 0 @@ -231,7 +231,7 @@ class LIFOValuation(BinWiseValuation): stock_bin = self.stack[index] if qty >= stock_bin[QTY]: # consume current bin - qty = _round_off_if_near_zero(qty - stock_bin[QTY]) + qty = round_off_if_near_zero(qty - stock_bin[QTY]) to_consume = self.stack.pop(index) consumed_bins.append(list(to_consume)) @@ -243,14 +243,14 @@ class LIFOValuation(BinWiseValuation): break else: # qty found in current bin consume it and exit - stock_bin[QTY] = _round_off_if_near_zero(stock_bin[QTY] - qty) + stock_bin[QTY] = round_off_if_near_zero(stock_bin[QTY] - qty) consumed_bins.append([qty, stock_bin[RATE]]) qty = 0 return consumed_bins -def _round_off_if_near_zero(number: float, precision: int = 7) -> float: +def round_off_if_near_zero(number: float, precision: int = 7) -> float: """Rounds off the number to zero only if number is close to zero for decimal specified in precision. Precision defaults to 7. """ From b1555fd477923a968a203c2fde68e754777a1e08 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 21:21:39 +0530 Subject: [PATCH 20/29] chore: batch flag and consumption rate in invariant report --- .../stock_ledger_invariant_check.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) 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 cb35bf75d1..7826d34422 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 @@ -60,6 +60,9 @@ def add_invariant_check_fields(sles): fifo_qty += qty fifo_value += qty * rate + if sle.actual_qty < 0: + sle.consumption_rate = sle.stock_value_difference / sle.actual_qty + balance_qty += sle.actual_qty balance_stock_value += sle.stock_value_difference if sle.voucher_type == "Stock Reconciliation" and not sle.batch_no: @@ -90,6 +93,9 @@ def add_invariant_check_fields(sles): sle.fifo_stock_diff = sle.fifo_stock_value - sles[idx - 1].fifo_stock_value sle.fifo_difference_diff = sle.fifo_stock_diff - sle.stock_value_difference + if sle.batch_no: + sle.use_batchwise_valuation = frappe.db.get_value("Batch", sle.batch_no, "use_batchwise_valuation", cache=True) + return sles @@ -134,6 +140,11 @@ def get_columns(): "label": "Batch", "options": "Batch", }, + { + "fieldname": "use_batchwise_valuation", + "fieldtype": "Check", + "label": "Batchwise Valuation", + }, { "fieldname": "actual_qty", "fieldtype": "Float", @@ -145,9 +156,9 @@ def get_columns(): "label": "Incoming Rate", }, { - "fieldname": "outgoing_rate", + "fieldname": "consumption_rate", "fieldtype": "Float", - "label": "Outgoing Rate", + "label": "Consumption Rate", }, { "fieldname": "qty_after_transaction", From 76b395d62ee5f9ffb96e3c3e4920fa6eebaec175 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 22:01:34 +0530 Subject: [PATCH 21/29] test: old/new mix batches valuation consumption --- .../test_stock_ledger_entry.py | 83 ++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index c298b5a096..b0df45ffd4 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -397,7 +397,15 @@ class TestStockLedgerEntry(ERPNextTestCase): for exp_sle, act_sle in zip(expected_sles, sles): for k, v in exp_sle.items(): - self.assertEqual(v, act_sle[k], msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") + act_value = act_sle[k] + if k == "stock_queue": + act_value = json.loads(act_value) + if act_value and act_value[0][0] == 0: + # ignore empty fifo bins + continue + + self.assertEqual(v, act_value, msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") + def test_batchwise_item_valuation_stock_reco(self): item, warehouses, batches = setup_item_valuation_test() @@ -587,6 +595,77 @@ class TestStockLedgerEntry(ERPNextTestCase): for details in details_list: check_sle_details_against_expected(*details) + def test_mixed_valuation_batches(self): + item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0) + warehouse = warehouses[0] + + state = { + "qty": 0.0, + "stock_value": 0.0 + } + def update_invariants(exp_sles): + for sle in exp_sles: + state["stock_value"] += sle["stock_value_difference"] + state["qty"] += sle["actual_qty"] + sle["stock_value"] = state["stock_value"] + sle["qty_after_transaction"] = state["qty"] + return exp_sles + + old1 = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=10, rate=10) + self.assertSLEs(old1, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*10, "stock_queue": [[10, 10]]}, + ])) + old2 = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[1], + qty=10, rate=20) + self.assertSLEs(old2, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*20, "stock_queue": [[10, 10], [10, 20]]}, + ])) + old3 = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=5, rate=15) + + self.assertSLEs(old3, update_invariants([ + {"actual_qty": 5, "stock_value_difference": 5*15, "stock_queue": [[10, 10], [10, 20], [5, 15]]}, + ])) + + new1 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=40) + batches.append(new1.items[0].batch_no) + # assert old queue remains + self.assertSLEs(new1, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*40, "stock_queue": [[10, 10], [10, 20], [5, 15]]}, + ])) + + new2 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=42) + batches.append(new2.items[0].batch_no) + self.assertSLEs(new2, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*42, "stock_queue": [[10, 10], [10, 20], [5, 15]]}, + ])) + + # consume old batch as per FIFO + consume_old1 = make_stock_entry(item_code=item_code, source=warehouse, qty=15, batch_no=batches[0]) + self.assertSLEs(consume_old1, update_invariants([ + {"actual_qty": -15, "stock_value_difference": -10*10 - 5*20, "stock_queue": [[5, 20], [5, 15]]}, + ])) + + # consume new batch as per batch + consume_new2 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-1]) + self.assertSLEs(consume_new2, update_invariants([ + {"actual_qty": -10, "stock_value_difference": -10*42, "stock_queue": [[5, 20], [5, 15]]}, + ])) + + # finish all old batches + consume_old2 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[1]) + self.assertSLEs(consume_old2, update_invariants([ + {"actual_qty": -10, "stock_value_difference": -5*20 - 5*15, "stock_queue": []}, + ])) + + # finish all new batches + consume_new1 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-2]) + self.assertSLEs(consume_new1, update_invariants([ + {"actual_qty": -10, "stock_value_difference": -10*40, "stock_queue": []}, + ])) + + def create_repack_entry(**args): args = frappe._dict(args) @@ -661,7 +740,7 @@ def setup_item_valuation_test(valuation_method="FIFO", suffix=None, use_batchwis item = make_item( f"IV - Test Item {valuation_method} {suffix}", - dict(valuation_method=valuation_method, has_batch_no=1) + dict(valuation_method=valuation_method, has_batch_no=1, create_new_batch=1) ) warehouses = [create_warehouse(f"IV - Test Warehouse {i}") for i in ['J', 'K']] batches = [f"IV - Test Batch {i} {valuation_method} {suffix}" for i in batches_list] From 35483242b3864e09c635979afe7793aac7f12596 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 22:22:27 +0530 Subject: [PATCH 22/29] fix: extend round_off_if_near_zero fix to other methods --- erpnext/stock/stock_ledger.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 677266ee0c..de6c409d7c 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -743,12 +743,14 @@ class update_entries_after(object): incoming_rate = flt(sle.incoming_rate) actual_qty = flt(sle.actual_qty) - self.wh_data.qty_after_transaction += actual_qty + self.wh_data.qty_after_transaction = round_off_if_near_zero(self.wh_data.qty_after_transaction + actual_qty) if actual_qty > 0: stock_value_difference = incoming_rate * actual_qty else: - outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, + warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, + posting_time=sle.posting_time, creation=sle.creation) if outgoing_rate is None: # This can *only* happen if qty available for the batch is zero. # in such case fall back various other rates. @@ -757,7 +759,7 @@ class update_entries_after(object): outgoing_rate = self.get_fallback_rate(sle) stock_value_difference = outgoing_rate * actual_qty - self.wh_data.stock_value += stock_value_difference + self.wh_data.stock_value = round_off_if_near_zero(self.wh_data.stock_value + stock_value_difference) if self.wh_data.qty_after_transaction: self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction From 609d2fccad2a1b60a1e7ffd93f504f0e1329136d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 20 Feb 2022 11:35:53 +0530 Subject: [PATCH 23/29] fix: reset stock value if no qty --- erpnext/stock/stock_ledger.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index de6c409d7c..1b90086440 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -468,6 +468,8 @@ class update_entries_after(object): # rounding as per precision self.wh_data.stock_value = flt(self.wh_data.stock_value, self.precision) + if not self.wh_data.qty_after_transaction: + self.wh_data.stock_value = 0.0 stock_value_difference = self.wh_data.stock_value - self.wh_data.prev_stock_value self.wh_data.prev_stock_value = self.wh_data.stock_value From 6b0bc350636776fbec3edc254086462a7670649c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 20 Feb 2022 12:05:58 +0530 Subject: [PATCH 24/29] test: mixed moving average items --- .../test_stock_ledger_entry.py | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index b0df45ffd4..9e819dd658 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -595,7 +595,7 @@ class TestStockLedgerEntry(ERPNextTestCase): for details in details_list: check_sle_details_against_expected(*details) - def test_mixed_valuation_batches(self): + def test_mixed_valuation_batches_fifo(self): item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0) warehouse = warehouses[0] @@ -665,6 +665,34 @@ class TestStockLedgerEntry(ERPNextTestCase): {"actual_qty": -10, "stock_value_difference": -10*40, "stock_queue": []}, ])) + def test_mixed_valuation_batches_moving_average(self): + item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0, valuation_method="Moving Average") + warehouse = warehouses[0] + + make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=10, rate=10) + make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[1], + qty=10, rate=20) + make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=5, rate=15) + + new1 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=40) + batches.append(new1.items[0].batch_no) + new2 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=42) + batches.append(new2.items[0].batch_no) + + # consume old batch as per FIFO + make_stock_entry(item_code=item_code, source=warehouse, qty=15, batch_no=batches[0]) + # consume new batch as per batch + make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-1]) + # finish all old batches + make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[1]) + + # finish all new batches + consume_new1 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-2]) + self.assertSLEs(consume_new1, ([ + {"stock_value": 0}, + ])) def create_repack_entry(**args): From f38690f7037c75bb1c5a5d946d686b40392a111a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 20 Feb 2022 12:58:53 +0530 Subject: [PATCH 25/29] fix: check if Moving average item can use batchwise valuation --- erpnext/stock/doctype/batch/batch.py | 32 ++++++++++++++++++++++++++++ erpnext/stock/utils.py | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index 96751d6eae..b5e56ad301 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -6,6 +6,7 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.model.naming import make_autoname, revert_series_if_last +from frappe.query_builder.functions import Sum from frappe.utils import cint, flt, get_link_to_form from frappe.utils.data import add_days from frappe.utils.jinja import render_template @@ -110,11 +111,15 @@ class Batch(Document): def validate(self): self.item_has_batch_enabled() + self.set_batchwise_valuation() def item_has_batch_enabled(self): if frappe.db.get_value("Item", self.item, "has_batch_no") == 0: frappe.throw(_("The selected item cannot have Batch")) + def set_batchwise_valuation(self): + self.use_batchwise_valuation = int(can_use_batchwise_valuation(self.item)) + def before_save(self): has_expiry_date, shelf_life_in_days = frappe.db.get_value('Item', self.item, ['has_expiry_date', 'shelf_life_in_days']) if not self.expiry_date and has_expiry_date and shelf_life_in_days: @@ -338,3 +343,30 @@ def get_pos_reserved_batch_qty(filters): flt_reserved_batch_qty = flt(reserved_batch_qty[0][0]) return flt_reserved_batch_qty + +def can_use_batchwise_valuation(item_code: str) -> bool: + """ Check if item can use batchwise valuation. + + Note: Item with existing moving average batches can't use batchwise valuation + until they are exhausted. + """ + from erpnext.stock.stock_ledger import get_valuation_method + batch = frappe.qb.DocType("Batch") + + if get_valuation_method(item_code) != "Moving Average": + return True + + batch_qty = ( + frappe.qb + .from_(batch) + .select(Sum(batch.batch_qty)) + .where( + (batch.use_batchwise_valuation == 0) + & (batch.item == item_code) + ) + ).run() + + if batch_qty and batch_qty[0][0]: + return False + + return True diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index e2bd2f197d..f85a04f944 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -261,7 +261,7 @@ def get_valuation_method(item_code): """get valuation method from item or default""" val_method = frappe.db.get_value('Item', item_code, 'valuation_method', cache=True) if not val_method: - val_method = frappe.db.get_value("Stock Settings", None, "valuation_method") or "FIFO" + val_method = frappe.db.get_value("Stock Settings", None, "valuation_method", cache=True) or "FIFO" return val_method def get_fifo_rate(previous_stock_queue, qty): From 75fb5616987066b83b69455b4eb59d1a715b280e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 11:08:57 +0530 Subject: [PATCH 26/29] test: force correct flag in test data --- .../doctype/stock_ledger_entry/test_stock_ledger_entry.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 9e819dd658..c65ed2888e 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -778,13 +778,15 @@ def setup_item_valuation_test(valuation_method="FIFO", suffix=None, use_batchwis ubw = use_batchwise_valuation if isinstance(use_batchwise_valuation, (list, tuple)): ubw = use_batchwise_valuation[i] - make_batch( - frappe._dict( + batch = frappe.get_doc(frappe._dict( + doctype="Batch", batch_id=batch_id, item=item.item_code, use_batchwise_valuation=ubw ) - ) + ).insert() + batch.use_batchwise_valuation = ubw + batch.db_update() return item.item_code, warehouses, batches From af9fa049c749c9f72f0b21a5960111cb6ec57c12 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 12:28:19 +0530 Subject: [PATCH 27/29] fix: batchwise valuation can only be used by FIFO/LIFO --- erpnext/stock/doctype/batch/batch.py | 24 ++------------- .../test_stock_ledger_entry.py | 30 ------------------- 2 files changed, 2 insertions(+), 52 deletions(-) diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index b5e56ad301..93e8d41367 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -6,7 +6,6 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.model.naming import make_autoname, revert_series_if_last -from frappe.query_builder.functions import Sum from frappe.utils import cint, flt, get_link_to_form from frappe.utils.data import add_days from frappe.utils.jinja import render_template @@ -347,26 +346,7 @@ def get_pos_reserved_batch_qty(filters): def can_use_batchwise_valuation(item_code: str) -> bool: """ Check if item can use batchwise valuation. - Note: Item with existing moving average batches can't use batchwise valuation - until they are exhausted. - """ + Note: Moving average valuation method can not use batch_wise_valuation.""" from erpnext.stock.stock_ledger import get_valuation_method - batch = frappe.qb.DocType("Batch") - if get_valuation_method(item_code) != "Moving Average": - return True - - batch_qty = ( - frappe.qb - .from_(batch) - .select(Sum(batch.batch_qty)) - .where( - (batch.use_batchwise_valuation == 0) - & (batch.item == item_code) - ) - ).run() - - if batch_qty and batch_qty[0][0]: - return False - - return True + return get_valuation_method(item_code) != "Moving Average" diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index c65ed2888e..0864ece995 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -665,36 +665,6 @@ class TestStockLedgerEntry(ERPNextTestCase): {"actual_qty": -10, "stock_value_difference": -10*40, "stock_queue": []}, ])) - def test_mixed_valuation_batches_moving_average(self): - item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0, valuation_method="Moving Average") - warehouse = warehouses[0] - - make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], - qty=10, rate=10) - make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[1], - qty=10, rate=20) - make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], - qty=5, rate=15) - - new1 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=40) - batches.append(new1.items[0].batch_no) - new2 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=42) - batches.append(new2.items[0].batch_no) - - # consume old batch as per FIFO - make_stock_entry(item_code=item_code, source=warehouse, qty=15, batch_no=batches[0]) - # consume new batch as per batch - make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-1]) - # finish all old batches - make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[1]) - - # finish all new batches - consume_new1 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-2]) - self.assertSLEs(consume_new1, ([ - {"stock_value": 0}, - ])) - - def create_repack_entry(**args): args = frappe._dict(args) repack = frappe.new_doc("Stock Entry") From 9661058cc7daf9802e054f3fcd99c7852ff935a4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 18:16:10 +0530 Subject: [PATCH 28/29] fix: only set batchwise valuation flag if new batch --- erpnext/stock/doctype/batch/batch.json | 6 ++++-- erpnext/stock/doctype/batch/batch.py | 13 ++++--------- erpnext/stock/doctype/batch/test_batch.py | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/erpnext/stock/doctype/batch/batch.json b/erpnext/stock/doctype/batch/batch.json index 0d28ea0919..967c5729bf 100644 --- a/erpnext/stock/doctype/batch/batch.json +++ b/erpnext/stock/doctype/batch/batch.json @@ -194,7 +194,7 @@ "fieldtype": "Column Break" }, { - "default": "1", + "default": "0", "fieldname": "use_batchwise_valuation", "fieldtype": "Check", "label": "Use Batch-wise Valuation", @@ -207,10 +207,11 @@ "image_field": "image", "links": [], "max_attachments": 5, - "modified": "2021-10-11 13:38:12.806976", + "modified": "2022-02-21 08:08:23.999236", "modified_by": "Administrator", "module": "Stock", "name": "Batch", + "naming_rule": "By fieldname", "owner": "Administrator", "permissions": [ { @@ -231,6 +232,7 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "batch_id", "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index 93e8d41367..c9b4c147f1 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -117,7 +117,10 @@ class Batch(Document): frappe.throw(_("The selected item cannot have Batch")) def set_batchwise_valuation(self): - self.use_batchwise_valuation = int(can_use_batchwise_valuation(self.item)) + from erpnext.stock.stock_ledger import get_valuation_method + + if self.is_new() and get_valuation_method(self.item) != "Moving Average": + self.use_batchwise_valuation = 1 def before_save(self): has_expiry_date, shelf_life_in_days = frappe.db.get_value('Item', self.item, ['has_expiry_date', 'shelf_life_in_days']) @@ -342,11 +345,3 @@ def get_pos_reserved_batch_qty(filters): flt_reserved_batch_qty = flt(reserved_batch_qty[0][0]) return flt_reserved_batch_qty - -def can_use_batchwise_valuation(item_code: str) -> bool: - """ Check if item can use batchwise valuation. - - Note: Moving average valuation method can not use batch_wise_valuation.""" - from erpnext.stock.stock_ledger import get_valuation_method - - return get_valuation_method(item_code) != "Moving Average" diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index 6495b56e92..baa03024af 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -6,6 +6,7 @@ import json import frappe from frappe.exceptions import ValidationError from frappe.utils import cint, flt +from frappe.utils.data import add_to_date, getdate from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.stock.doctype.batch.batch import UnableToSelectBatchError, get_batch_no, get_batch_qty @@ -387,6 +388,25 @@ class TestBatch(ERPNextTestCase): assertValuation((20 * 20 + 10 * 25) / (10 + 20)) + def test_update_batch_properties(self): + item_code = "_TestBatchWiseVal" + self.make_batch_item(item_code) + + se = make_stock_entry(item_code=item_code, qty=100, rate=10, target="_Test Warehouse - _TC") + batch_no = se.items[0].batch_no + batch = frappe.get_doc("Batch", batch_no) + + expiry_date = add_to_date(batch.manufacturing_date, days=30) + + batch.expiry_date = expiry_date + batch.save() + + batch.reload() + + self.assertEqual(getdate(batch.expiry_date), getdate(expiry_date)) + + + def create_batch(item_code, rate, create_item_price_for_batch): pi = make_purchase_invoice(company="_Test Company", warehouse= "Stores - _TC", cost_center = "Main - _TC", update_stock=1, From e4c4dc402e75d3ec501095fa3e914553fcd07a4d Mon Sep 17 00:00:00 2001 From: Sagar Sharma Date: Mon, 21 Feb 2022 19:49:19 +0530 Subject: [PATCH 29/29] fix: JobCard TimeLog to_date (#29872) --- erpnext/manufacturing/doctype/job_card/job_card.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index 8d00019b7d..9f4ace296e 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -62,7 +62,7 @@ class JobCard(Document): if self.get('time_logs'): for d in self.get('time_logs'): - if get_datetime(d.from_time) > get_datetime(d.to_time): + if d.to_time and get_datetime(d.from_time) > get_datetime(d.to_time): frappe.throw(_("Row {0}: From time must be less than to time").format(d.idx)) data = self.get_overlap_for(d)