diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index 74164ea653f..b1e6cc88f10 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -1123,15 +1123,14 @@ class BuyingController(SubcontractingController): asset = frappe.get_doc("Asset", asset.name) if delete_asset and is_auto_create_enabled: # need to delete movements to delete assets otherwise throws link exists error - movements = frappe.db.sql( - """SELECT asm.name - FROM `tabAsset Movement` asm, `tabAsset Movement Item` asm_item - WHERE asm_item.parent=asm.name and asm_item.asset=%s""", - asset.name, - as_dict=1, + movements = frappe.get_all( + "Asset Movement Item", + filters={"asset": asset.name}, + pluck="parent", + limit_page_length=0, # delete every movement of the asset (no default 20 cap) ) for movement in movements: - frappe.delete_doc("Asset Movement", movement.name, force=1) + frappe.delete_doc("Asset Movement", movement, force=1) frappe.delete_doc("Asset", asset.name, force=1) continue @@ -1224,17 +1223,12 @@ def validate_item_type(doc, fieldname, message): if not items: return - item_list = ", ".join(["%s" % frappe.db.escape(d) for d in items]) - - invalid_items = [ - d[0] - for d in frappe.db.sql( - f""" - select item_code from tabItem where name in ({item_list}) and {fieldname}=0 - """, - as_list=True, - ) - ] + invalid_items = frappe.get_all( + "Item", + filters={"name": ["in", items], fieldname: 0}, + pluck="item_code", + limit_page_length=0, # validate every item in the document (no default 20 cap) + ) if invalid_items: items = ", ".join([d for d in invalid_items]) diff --git a/erpnext/controllers/item_variant.py b/erpnext/controllers/item_variant.py index b125b15fe55..7f52699a816 100644 --- a/erpnext/controllers/item_variant.py +++ b/erpnext/controllers/item_variant.py @@ -443,13 +443,21 @@ def make_variant_item_code(template_item_code, template_item_name, variant): abbreviations = [] for attr in variant.attributes: - item_attribute = frappe.db.sql( - """select i.numeric_values, v.abbr - from `tabItem Attribute` i left join `tabItem Attribute Value` v - on (i.name=v.parent) - where i.name=%(attribute)s and (v.attribute_value=%(attribute_value)s or i.numeric_values = 1)""", - {"attribute": attr.attribute, "attribute_value": attr.attribute_value}, - as_dict=True, + ia = frappe.qb.DocType("Item Attribute") + iav = frappe.qb.DocType("Item Attribute Value") + item_attribute = ( + frappe.qb.from_(ia) + .left_join(iav) + .on(ia.name == iav.parent) + .select(ia.numeric_values, iav.abbr) + .where( + (ia.name == attr.attribute) + # attribute_value is a varchar column; cast the param to str so postgres doesn't choke on + # `varchar = numeric` for numeric attributes (where this side is irrelevant anyway, since + # numeric_values == 1 already satisfies the OR). Non-numeric values are already strings. + & ((iav.attribute_value == cstr(attr.attribute_value)) | (ia.numeric_values == 1)) + ) + .run(as_dict=True) ) if not item_attribute: diff --git a/erpnext/controllers/trends.py b/erpnext/controllers/trends.py index 9998c0c537d..03e2b483d14 100644 --- a/erpnext/controllers/trends.py +++ b/erpnext/controllers/trends.py @@ -111,6 +111,9 @@ def get_data(filters, conditions): elif filters.get("group_by") == "Supplier": sel_col = "t1.supplier" + # first column of the multi-column group_by = the based-on key the detail queries equate against + based_on_key = conditions["group_by"].split(",")[0].strip() + if filters.get("based_on") in ["Customer", "Supplier"]: inc = 3 elif filters.get("based_on") in ["Item"]: @@ -160,7 +163,7 @@ def get_data(filters, conditions): posting_date, "%s", "%s", - conditions["group_by"], + based_on_key, "%s", conditions.get("addl_tables_relational_cond"), cond, @@ -177,6 +180,7 @@ def get_data(filters, conditions): """ select t4.default_currency AS currency , {} , {} from `tab{}` t1, `tab{} Item` t2 {} where t2.parent = t1.name and t1.company = {} and {} between {} and {} and t1.docstatus = 1 and {} = {} and {} = {} {} {} + group by t4.default_currency, {} """.format( sel_col, conditions["period_wise_select"], @@ -189,10 +193,11 @@ def get_data(filters, conditions): "%s", sel_col, "%s", - conditions["group_by"], + based_on_key, "%s", conditions.get("addl_tables_relational_cond"), cond, + sel_col, ), (filters.get("company"), year_start_date, year_end_date, row[i][0], data1[d][0]), as_list=1, @@ -307,8 +312,8 @@ def get_period_wise_columns(bet_dates, period, pwc): def get_period_wise_query(bet_dates, trans_date, query_details): - query_details += """SUM(IF(t1.{trans_date} BETWEEN '{sd}' AND '{ed}', t2.stock_qty, NULL)), - SUM(IF(t1.{trans_date} BETWEEN '{sd}' AND '{ed}', t2.base_net_amount, NULL)), + query_details += """SUM(CASE WHEN t1.{trans_date} BETWEEN '{sd}' AND '{ed}' THEN t2.stock_qty ELSE NULL END), + SUM(CASE WHEN t1.{trans_date} BETWEEN '{sd}' AND '{ed}' THEN t2.base_net_amount ELSE NULL END), """.format( trans_date=trans_date, sd=bet_dates[0], @@ -365,7 +370,7 @@ def based_wise_columns_query(based_on, trans): if based_on == "Item": based_on_details["based_on_cols"] = ["Item:Link/Item:120", "Item Name:Data:120"] based_on_details["based_on_select"] = "t2.item_code, t2.item_name," - based_on_details["based_on_group_by"] = "t2.item_code" + based_on_details["based_on_group_by"] = "t2.item_code, t2.item_name" based_on_details["addl_tables"] = "" elif based_on == "Item Group": @@ -389,7 +394,11 @@ def based_wise_columns_query(based_on, trans): "Territory:Link/Territory:120", ] based_on_details["based_on_select"] = "t1.customer, t1.customer_name, t1.territory," - based_on_details["based_on_group_by"] = "t1.party_name" if trans == "Quotation" else "t1.customer" + based_on_details["based_on_group_by"] = ( + "t1.party_name, t1.customer_name, t1.territory" + if trans == "Quotation" + else "t1.customer, t1.customer_name, t1.territory" + ) based_on_details["addl_tables"] = "" elif based_on == "Customer Group": @@ -405,7 +414,7 @@ def based_wise_columns_query(based_on, trans): "Supplier Group:Link/Supplier Group:140", ] based_on_details["based_on_select"] = "t1.supplier, t1.supplier_name, t3.supplier_group," - based_on_details["based_on_group_by"] = "t1.supplier" + based_on_details["based_on_group_by"] = "t1.supplier, t1.supplier_name, t3.supplier_group" based_on_details["addl_tables"] = ",`tabSupplier` t3" based_on_details["addl_tables_relational_cond"] = " and t1.supplier = t3.name" @@ -437,6 +446,7 @@ def based_wise_columns_query(based_on, trans): frappe.throw(_("Project-wise data is not available for Quotation")) based_on_details["based_on_select"] += "t4.default_currency as currency," + based_on_details["based_on_group_by"] += ", t4.default_currency" based_on_details["based_on_cols"].append("Currency:Link/Currency:120") based_on_details["addl_tables"] += ", `tabCompany` t4" based_on_details["addl_tables_relational_cond"] = ( diff --git a/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py b/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py new file mode 100644 index 00000000000..9fbae8f2e31 --- /dev/null +++ b/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py @@ -0,0 +1,26 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +from erpnext.tests.utils import ERPNextTestSuite + + +class TestSalesOrderTrends(ERPNextTestSuite): + def test_report_executes_with_group_by(self): + # trends.get_data builds per-period SUM(CASE ...) aggregates (converted from MySQL SUM(IF)), + # with a GROUP BY widened to every selected non-aggregated column and a based_on_key for the + # group-by detail subqueries. Setting group_by exercises that full path on both engines. + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + from erpnext.selling.report.sales_order_trends.sales_order_trends import execute + + make_sales_order(item_code="_Test Item", qty=3, rate=100) + + filters = { + "company": "_Test Company", + "period": "Monthly", + "based_on": "Item", + "group_by": "Customer", + } + columns, data, _chart_none, _chart = execute(filters) + + self.assertTrue(columns) + self.assertTrue(any("_Test Item" in [str(cell) for cell in row] for row in data))