From 289e6cd4ce05853d374c841e64deaef5859e4a4b Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Fri, 15 Jul 2022 15:43:38 +0530 Subject: [PATCH] fix: change frappe.db.sql to frappe.qb --- erpnext/controllers/stock_controller.py | 7 +- .../inventory_dimension.json | 4 +- .../inventory_dimension.py | 23 +- .../test_inventory_dimension.py | 43 ++- .../stock/doctype/stock_entry/stock_entry.py | 8 +- .../report/stock_balance/stock_balance.py | 2 +- .../stock/report/stock_ledger/stock_ledger.py | 351 +++++++++--------- 7 files changed, 253 insertions(+), 185 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 40bc1aa207..e27718a9b4 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -372,9 +372,10 @@ class StockController(AccountsController): return sl_dict def update_inventory_dimensions(self, row, sl_dict) -> None: - dimension = get_evaluated_inventory_dimension(row, sl_dict, parent_doc=self) - if dimension and row.get(dimension.source_fieldname): - sl_dict[dimension.target_fieldname] = row.get(dimension.source_fieldname) + dimensions = get_evaluated_inventory_dimension(row, sl_dict, parent_doc=self) + for dimension in dimensions: + if dimension and row.get(dimension.source_fieldname): + sl_dict[dimension.target_fieldname] = row.get(dimension.source_fieldname) def make_sl_entries(self, sl_entries, allow_negative_stock=False, via_landed_cost_voucher=False): from erpnext.stock.stock_ledger import make_sl_entries diff --git a/erpnext/stock/doctype/inventory_dimension/inventory_dimension.json b/erpnext/stock/doctype/inventory_dimension/inventory_dimension.json index cfac5cdfae..8b334d13d7 100644 --- a/erpnext/stock/doctype/inventory_dimension/inventory_dimension.json +++ b/erpnext/stock/doctype/inventory_dimension/inventory_dimension.json @@ -85,7 +85,7 @@ "default": "0", "fieldname": "apply_to_all_doctypes", "fieldtype": "Check", - "label": "Apply to All Document Types" + "label": "Apply to All Inventory Document Types" }, { "default": "0", @@ -144,7 +144,7 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2022-07-05 15:33:37.270373", + "modified": "2022-07-19 21:06:11.824976", "modified_by": "Administrator", "module": "Stock", "name": "Inventory Dimension", diff --git a/erpnext/stock/doctype/inventory_dimension/inventory_dimension.py b/erpnext/stock/doctype/inventory_dimension/inventory_dimension.py index 3b9a84a3f2..5a9541f060 100644 --- a/erpnext/stock/doctype/inventory_dimension/inventory_dimension.py +++ b/erpnext/stock/doctype/inventory_dimension/inventory_dimension.py @@ -11,6 +11,14 @@ class DoNotChangeError(frappe.ValidationError): pass +class CanNotBeChildDoc(frappe.ValidationError): + pass + + +class CanNotBeDefaultDimension(frappe.ValidationError): + pass + + class InventoryDimension(Document): def onload(self): if not self.is_new() and frappe.db.has_column("Stock Ledger Entry", self.target_fieldname): @@ -51,11 +59,11 @@ class InventoryDimension(Document): def validate_reference_document(self): if frappe.get_cached_value("DocType", self.reference_document, "istable") == 1: msg = f"The reference document {self.reference_document} can not be child table." - frappe.throw(_(msg)) + frappe.throw(_(msg), CanNotBeChildDoc) if self.reference_document in ["Batch", "Serial No", "Warehouse", "Item"]: msg = f"The reference document {self.reference_document} can not be an Inventory Dimension." - frappe.throw(_(msg)) + frappe.throw(_(msg), CanNotBeDefaultDimension) def set_source_and_target_fieldname(self) -> None: if not self.source_fieldname: @@ -122,8 +130,9 @@ def get_inventory_documents( ) -def get_evaluated_inventory_dimension(doc, sl_dict, parent_doc=None) -> dict: +def get_evaluated_inventory_dimension(doc, sl_dict, parent_doc=None): dimensions = get_document_wise_inventory_dimensions(doc.doctype) + filter_dimensions = [] for row in dimensions: if ( row.type_of_transaction == "Inward" @@ -142,8 +151,12 @@ def get_evaluated_inventory_dimension(doc, sl_dict, parent_doc=None) -> dict: if parent_doc: evals["parent"] = parent_doc - if frappe.safe_eval(row.condition, evals): - return row + if row.condition and frappe.safe_eval(row.condition, evals): + filter_dimensions.append(row) + else: + filter_dimensions.append(row) + + return filter_dimensions def get_document_wise_inventory_dimensions(doctype) -> dict: diff --git a/erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py b/erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py index a79de1a792..998a0e9d6a 100644 --- a/erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py +++ b/erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py @@ -4,6 +4,11 @@ import frappe from frappe.tests.utils import FrappeTestCase +from erpnext.stock.doctype.inventory_dimension.inventory_dimension import ( + CanNotBeChildDoc, + CanNotBeDefaultDimension, + DoNotChangeError, +) from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse @@ -12,11 +17,36 @@ class TestInventoryDimension(FrappeTestCase): def setUp(self): prepare_test_data() + def test_validate_inventory_dimension(self): + # Can not be child doc + inv_dim1 = create_inventory_dimension( + reference_document="Stock Entry Detail", + type_of_transaction="Outward", + dimension_name="Stock Entry", + apply_to_all_doctypes=0, + istable=0, + document_type="Stock Entry", + do_not_save=True, + ) + + self.assertRaises(CanNotBeChildDoc, inv_dim1.insert) + + inv_dim1 = create_inventory_dimension( + reference_document="Batch", + type_of_transaction="Outward", + dimension_name="Batch", + apply_to_all_doctypes=0, + document_type="Stock Entry Detail", + do_not_save=True, + ) + + self.assertRaises(CanNotBeDefaultDimension, inv_dim1.insert) + def test_inventory_dimension(self): warehouse = "Shelf Warehouse - _TC" item_code = "_Test Item" - create_inventory_dimension( + inv_dim1 = create_inventory_dimension( reference_document="Shelf", type_of_transaction="Outward", dimension_name="Shelf", @@ -47,7 +77,6 @@ class TestInventoryDimension(FrappeTestCase): inward.save() inward.submit() inward.load_from_db() - print(inward.name) sle_data = frappe.db.get_value( "Stock Ledger Entry", {"voucher_no": inward.name}, ["shelf", "warehouse"], as_dict=1 @@ -74,6 +103,12 @@ class TestInventoryDimension(FrappeTestCase): sle_shelf = frappe.db.get_value("Stock Ledger Entry", {"voucher_no": outward.name}, "shelf") self.assertEqual(sle_shelf, "Shelf 1") + inv_dim1.load_from_db() + inv_dim1.apply_to_all_doctypes = 1 + + self.assertTrue(inv_dim1.has_stock_ledger()) + self.assertRaises(DoNotChangeError, inv_dim1.save) + def prepare_test_data(): if not frappe.db.exists("DocType", "Shelf"): @@ -107,6 +142,8 @@ def create_inventory_dimension(**args): doc = frappe.new_doc("Inventory Dimension") doc.update(args) - doc.insert(ignore_permissions=True) + + if not args.do_not_save: + doc.insert(ignore_permissions=True) return doc diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 9c49408289..f719c1efda 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -478,10 +478,10 @@ class StockEntry(StockController): if not d.s_warehouse: frappe.throw(_("Source warehouse is mandatory for row {0}").format(d.idx)) - if ( - cstr(d.s_warehouse) == cstr(d.t_warehouse) - and not self.purpose == "Material Transfer for Manufacture" - ): + if cstr(d.s_warehouse) == cstr(d.t_warehouse) and self.purpose not in [ + "Material Transfer for Manufacture", + "Material Transfer", + ]: frappe.throw(_("Source and target warehouse cannot be same for row {0}").format(d.idx)) if not (d.s_warehouse or d.t_warehouse): diff --git a/erpnext/stock/report/stock_balance/stock_balance.py b/erpnext/stock/report/stock_balance/stock_balance.py index a1e1030d9e..679d234c9f 100644 --- a/erpnext/stock/report/stock_balance/stock_balance.py +++ b/erpnext/stock/report/stock_balance/stock_balance.py @@ -329,7 +329,7 @@ def get_stock_ledger_entries(filters: StockBalanceFilter, items: List[str]) -> L query = query.where(sle.item_code.isin(items)) query = apply_conditions(query, filters) - return query.run(as_dict=True, debug=1) + return query.run(as_dict=True) def get_inventory_dimension_fields(): diff --git a/erpnext/stock/report/stock_ledger/stock_ledger.py b/erpnext/stock/report/stock_ledger/stock_ledger.py index 807b800c7c..e18d4c7522 100644 --- a/erpnext/stock/report/stock_ledger/stock_ledger.py +++ b/erpnext/stock/report/stock_ledger/stock_ledger.py @@ -4,7 +4,9 @@ import frappe from frappe import _ +from frappe.query_builder.functions import CombineDatetime from frappe.utils import cint, flt +from pypika.terms import ExistsCriterion from erpnext.stock.doctype.inventory_dimension.inventory_dimension import get_inventory_dimensions from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos @@ -70,7 +72,7 @@ def update_available_serial_nos(available_serial_nos, sle): key = (sle.item_code, sle.warehouse) if key not in available_serial_nos: stock_balance = get_stock_balance_for( - sle.item_code, sle.warehouse, sle.date.split(" ")[0], sle.date.split(" ")[1] + sle.item_code, sle.warehouse, sle.posting_date, sle.posting_time ) serials = get_serial_nos(stock_balance["serial_nos"]) if stock_balance["serial_nos"] else [] available_serial_nos.setdefault(key, serials) @@ -109,116 +111,6 @@ def get_columns(filters): "options": "UOM", "width": 90, }, - { - "label": _("In Qty"), - "fieldname": "in_qty", - "fieldtype": "Float", - "width": 80, - "convertible": "qty", - }, - { - "label": _("Out Qty"), - "fieldname": "out_qty", - "fieldtype": "Float", - "width": 80, - "convertible": "qty", - }, - { - "label": _("Balance Qty"), - "fieldname": "qty_after_transaction", - "fieldtype": "Float", - "width": 100, - "convertible": "qty", - }, - { - "label": _("Voucher #"), - "fieldname": "voucher_no", - "fieldtype": "Dynamic Link", - "options": "voucher_type", - "width": 150, - }, - { - "label": _("Warehouse"), - "fieldname": "warehouse", - "fieldtype": "Link", - "options": "Warehouse", - "width": 150, - }, - { - "label": _("Item Group"), - "fieldname": "item_group", - "fieldtype": "Link", - "options": "Item Group", - "width": 100, - }, - { - "label": _("Brand"), - "fieldname": "brand", - "fieldtype": "Link", - "options": "Brand", - "width": 100, - }, - {"label": _("Description"), "fieldname": "description", "width": 200}, - { - "label": _("Incoming Rate"), - "fieldname": "incoming_rate", - "fieldtype": "Currency", - "width": 110, - "options": "Company:company:default_currency", - "convertible": "rate", - }, - { - "label": _("Valuation Rate"), - "fieldname": "valuation_rate", - "fieldtype": "Currency", - "width": 110, - "options": "Company:company:default_currency", - "convertible": "rate", - }, - { - "label": _("Balance Value"), - "fieldname": "stock_value", - "fieldtype": "Currency", - "width": 110, - "options": "Company:company:default_currency", - }, - { - "label": _("Value Change"), - "fieldname": "stock_value_difference", - "fieldtype": "Currency", - "width": 110, - "options": "Company:company:default_currency", - }, - {"label": _("Voucher Type"), "fieldname": "voucher_type", "width": 110}, - { - "label": _("Voucher #"), - "fieldname": "voucher_no", - "fieldtype": "Dynamic Link", - "options": "voucher_type", - "width": 100, - }, - { - "label": _("Batch"), - "fieldname": "batch_no", - "fieldtype": "Link", - "options": "Batch", - "width": 100, - }, - { - "label": _("Serial No"), - "fieldname": "serial_no", - "fieldtype": "Link", - "options": "Serial No", - "width": 100, - }, - {"label": _("Balance Serial No"), "fieldname": "balance_serial_no", "width": 100}, - { - "label": _("Project"), - "fieldname": "project", - "fieldtype": "Link", - "options": "Project", - "width": 100, - }, ] for dimension in get_inventory_dimensions(): @@ -232,72 +124,197 @@ def get_columns(filters): } ) - columns.append( - { - "label": _("Company"), - "fieldname": "company", - "fieldtype": "Link", - "options": "Company", - "width": 110, - } + columns.extend( + [ + { + "label": _("In Qty"), + "fieldname": "in_qty", + "fieldtype": "Float", + "width": 80, + "convertible": "qty", + }, + { + "label": _("Out Qty"), + "fieldname": "out_qty", + "fieldtype": "Float", + "width": 80, + "convertible": "qty", + }, + { + "label": _("Balance Qty"), + "fieldname": "qty_after_transaction", + "fieldtype": "Float", + "width": 100, + "convertible": "qty", + }, + { + "label": _("Voucher #"), + "fieldname": "voucher_no", + "fieldtype": "Dynamic Link", + "options": "voucher_type", + "width": 150, + }, + { + "label": _("Warehouse"), + "fieldname": "warehouse", + "fieldtype": "Link", + "options": "Warehouse", + "width": 150, + }, + { + "label": _("Item Group"), + "fieldname": "item_group", + "fieldtype": "Link", + "options": "Item Group", + "width": 100, + }, + { + "label": _("Brand"), + "fieldname": "brand", + "fieldtype": "Link", + "options": "Brand", + "width": 100, + }, + {"label": _("Description"), "fieldname": "description", "width": 200}, + { + "label": _("Incoming Rate"), + "fieldname": "incoming_rate", + "fieldtype": "Currency", + "width": 110, + "options": "Company:company:default_currency", + "convertible": "rate", + }, + { + "label": _("Valuation Rate"), + "fieldname": "valuation_rate", + "fieldtype": "Currency", + "width": 110, + "options": "Company:company:default_currency", + "convertible": "rate", + }, + { + "label": _("Balance Value"), + "fieldname": "stock_value", + "fieldtype": "Currency", + "width": 110, + "options": "Company:company:default_currency", + }, + { + "label": _("Value Change"), + "fieldname": "stock_value_difference", + "fieldtype": "Currency", + "width": 110, + "options": "Company:company:default_currency", + }, + {"label": _("Voucher Type"), "fieldname": "voucher_type", "width": 110}, + { + "label": _("Voucher #"), + "fieldname": "voucher_no", + "fieldtype": "Dynamic Link", + "options": "voucher_type", + "width": 100, + }, + { + "label": _("Batch"), + "fieldname": "batch_no", + "fieldtype": "Link", + "options": "Batch", + "width": 100, + }, + { + "label": _("Serial No"), + "fieldname": "serial_no", + "fieldtype": "Link", + "options": "Serial No", + "width": 100, + }, + {"label": _("Balance Serial No"), "fieldname": "balance_serial_no", "width": 100}, + { + "label": _("Project"), + "fieldname": "project", + "fieldtype": "Link", + "options": "Project", + "width": 100, + }, + { + "label": _("Company"), + "fieldname": "company", + "fieldtype": "Link", + "options": "Company", + "width": 110, + }, + ] ) return columns def get_stock_ledger_entries(filters, items): - item_conditions_sql = "" - if items: - item_conditions_sql = "and sle.item_code in ({})".format( - ", ".join(frappe.db.escape(i) for i in items) + sle = frappe.qb.DocType("Stock Ledger Entry") + query = ( + frappe.qb.from_(sle) + .select( + sle.item_code, + CombineDatetime(sle.posting_date, sle.posting_time).as_("date"), + sle.warehouse, + sle.posting_date, + sle.posting_time, + sle.actual_qty, + sle.incoming_rate, + sle.valuation_rate, + sle.company, + sle.voucher_type, + sle.qty_after_transaction, + sle.stock_value_difference, + sle.voucher_no, + sle.stock_value, + sle.batch_no, + sle.serial_no, + sle.project, ) - - sl_entries = frappe.db.sql( - """ - SELECT - concat_ws(' ', posting_date, posting_time) AS date, - item_code, - warehouse, - actual_qty, - qty_after_transaction, - incoming_rate, - valuation_rate, - stock_value, - voucher_type, - voucher_no, - batch_no, - serial_no, - company, - project, - stock_value_difference {get_dimension_fields} - FROM - `tabStock Ledger Entry` sle - WHERE - company = %(company)s - AND is_cancelled = 0 AND posting_date BETWEEN %(from_date)s AND %(to_date)s - {sle_conditions} - {item_conditions_sql} - ORDER BY - posting_date asc, posting_time asc, creation asc - """.format( - sle_conditions=get_sle_conditions(filters), - item_conditions_sql=item_conditions_sql, - get_dimension_fields=get_dimension_fields(), - ), - filters, - as_dict=1, + .where( + (sle.docstatus < 2) + & (sle.is_cancelled == 0) + & (sle.posting_date[filters.from_date : filters.to_date]) + ) + .orderby(CombineDatetime(sle.posting_date, sle.posting_time)) + .orderby(sle.creation) ) - return sl_entries + inventory_dimension_fields = get_inventory_dimension_fields() + if inventory_dimension_fields: + for fieldname in inventory_dimension_fields: + query = query.select(fieldname) + if fieldname in filters and filters.get(fieldname): + query = query.where(sle[fieldname].isin(filters.get(fieldname))) + + if items: + query = query.where(sle.item_code.isin(items)) + + for field in ["voucher_no", "batch_no", "project", "company"]: + if filters.get(field): + query = query.where(sle[field] == filters.get(field)) + + if warehouse := filters.get("warehouse"): + lft, rgt = frappe.db.get_value("Warehouse", warehouse, ["lft", "rgt"]) + + warehouse_table = frappe.qb.DocType("Warehouse") + chilren_subquery = ( + frappe.qb.from_(warehouse_table) + .select(warehouse_table.name) + .where( + (warehouse_table.lft >= lft) + & (warehouse_table.rgt <= rgt) + & (warehouse_table.name == sle.warehouse) + ) + ) + query = query.where(ExistsCriterion(chilren_subquery)) + + return query.run(as_dict=True) -def get_dimension_fields() -> str: - fields = "" - - for dimension in get_inventory_dimensions(): - fields += f", {dimension.fieldname}" - - return fields +def get_inventory_dimension_fields(): + return [dimension.fieldname for dimension in get_inventory_dimensions()] def get_items(filters): @@ -395,7 +412,7 @@ def get_opening_balance(filters, columns, sl_entries): for sle in sl_entries: if ( sle.get("voucher_type") == "Stock Reconciliation" - and sle.get("date").split()[0] == filters.from_date + and sle.posting_date == filters.from_date and frappe.db.get_value("Stock Reconciliation", sle.voucher_no, "purpose") == "Opening Stock" ): last_entry = sle