diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 7ff4c2594a1..c2cf93ca1d2 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -978,9 +978,7 @@ class StockReconciliation(StockController): if not self.expense_account: frappe.throw(_("Please enter Expense Account")) - elif self.purpose == "Opening Stock" or not frappe.db.sql( - """select name from `tabStock Ledger Entry` limit 1""" - ): + elif self.purpose == "Opening Stock" or not frappe.db.get_all("Stock Ledger Entry", limit=1): if frappe.db.get_value("Account", self.expense_account, "report_type") == "Profit and Loss": frappe.throw( _( @@ -1108,43 +1106,59 @@ def get_item_and_warehouses(item_code, warehouse): def get_items_for_stock_reco(warehouse, company): lft, rgt = frappe.db.get_value("Warehouse", warehouse, ["lft", "rgt"]) - items = frappe.db.sql( - f""" - select - i.name as item_code, i.item_name, bin.warehouse as warehouse, i.has_serial_no, i.has_batch_no - from - `tabBin` bin, `tabItem` i - where - i.name = bin.item_code - and IFNULL(i.disabled, 0) = 0 - and i.is_stock_item = 1 - and i.has_variants = 0 - and exists( - select name from `tabWarehouse` where lft >= {lft} and rgt <= {rgt} and name = bin.warehouse and is_group = 0 - ) - """, - as_dict=1, + + item = frappe.qb.DocType("Item") + bin_dt = frappe.qb.DocType("Bin") + wh = frappe.qb.DocType("Warehouse") + item_default = frappe.qb.DocType("Item Default") + + warehouses_in_tree = ( + frappe.qb.from_(wh).select(wh.name).where((wh.lft >= lft) & (wh.rgt <= rgt) & (wh.is_group == 0)) ) - items += frappe.db.sql( - """ - select - i.name as item_code, i.item_name, id.default_warehouse as warehouse, i.has_serial_no, i.has_batch_no - from - `tabItem` i, `tabItem Default` id - where - i.name = id.parent - and exists( - select name from `tabWarehouse` where lft >= %s and rgt <= %s and name=id.default_warehouse and is_group = 0 - ) - and i.is_stock_item = 1 - and i.has_variants = 0 - and IFNULL(i.disabled, 0) = 0 - and id.company = %s - group by i.name - """, - (lft, rgt, company), - as_dict=1, + items = ( + frappe.qb.from_(bin_dt) + .inner_join(item) + .on(item.name == bin_dt.item_code) + .select( + item.name.as_("item_code"), + item.item_name, + bin_dt.warehouse.as_("warehouse"), + item.has_serial_no, + item.has_batch_no, + ) + .where( + ((item.disabled == 0) | item.disabled.isnull()) + & (item.is_stock_item == 1) + & (item.has_variants == 0) + & bin_dt.warehouse.isin(warehouses_in_tree) + ) + .run(as_dict=1) + ) + + # Item Default holds at most one row per (item, company) -- enforced at the app layer by + # Item.validate_item_defaults ("Cannot set multiple Item Defaults for a company"), though not by a + # DB-level unique constraint -- so the company filter already yields one row per item and the + # original `group by i.name` (an arbitrary-row collapse) is a no-op for valid data. + items += ( + frappe.qb.from_(item) + .inner_join(item_default) + .on(item.name == item_default.parent) + .select( + item.name.as_("item_code"), + item.item_name, + item_default.default_warehouse.as_("warehouse"), + item.has_serial_no, + item.has_batch_no, + ) + .where( + item_default.default_warehouse.isin(warehouses_in_tree) + & (item.is_stock_item == 1) + & (item.has_variants == 0) + & ((item.disabled == 0) | item.disabled.isnull()) + & (item_default.company == company) + ) + .run(as_dict=1) ) # remove duplicates diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 02cd0e63e4a..57278dacb23 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -1841,6 +1841,46 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin): self.assertEqual(frappe.get_value("Serial No", serial_no, "status"), "Delivered") + def test_get_items_for_stock_reco_from_bin(self): + """get_items_for_stock_reco Bin branch (comma-join -> inner_join, ifnull(disabled) -> + disabled==0|isnull, warehouse-subtree EXISTS -> isin) must surface a stocked item.""" + 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.doctype.stock_reconciliation.stock_reconciliation import ( + get_items_for_stock_reco, + ) + + warehouse = "_Test Warehouse - _TC" + item = make_item(properties={"is_stock_item": 1}).name + make_stock_entry(item_code=item, target=warehouse, qty=5, basic_rate=100) + + returned = { + (d["item_code"], d["warehouse"]) for d in get_items_for_stock_reco(warehouse, "_Test Company") + } + self.assertIn((item, warehouse), returned) + + def test_get_items_for_stock_reco_from_item_default(self): + """get_items_for_stock_reco Item Default branch (EXISTS -> isin, dropped `group by i.name` — + sound because Item Default is one-per-(item, company)) must surface an item via its + default_warehouse even without stock.""" + from erpnext.stock.doctype.item.test_item import make_item + from erpnext.stock.doctype.stock_reconciliation.stock_reconciliation import ( + get_items_for_stock_reco, + ) + + warehouse = "_Test Warehouse - _TC" + item = make_item( + properties={ + "is_stock_item": 1, + "item_defaults": [{"company": "_Test Company", "default_warehouse": warehouse}], + } + ).name + + returned = { + (d["item_code"], d["warehouse"]) for d in get_items_for_stock_reco(warehouse, "_Test Company") + } + self.assertIn((item, warehouse), returned) + def create_batch_item_with_batch(item_name, batch_id): batch_item_doc = create_item(item_name, is_stock_item=1)