diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index b2d6e7838a1..8eaf33c084d 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -6,6 +6,7 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.utils import getdate, nowdate +from pypika.terms import ExistsCriterion from erpnext.controllers.selling_controller import SellingController @@ -358,22 +359,31 @@ def get_list_context(context=None): def set_expired_status(): - # filter out submitted non expired quotations whose validity has been ended - cond = "`tabQuotation`.docstatus = 1 and `tabQuotation`.status NOT IN ('Expired', 'Lost') and `tabQuotation`.valid_till < %s" - # check if those QUO have SO against it - so_against_quo = """ - SELECT - so.name FROM `tabSales Order` so, `tabSales Order Item` so_item - WHERE - so_item.docstatus = 1 and so.docstatus = 1 - and so_item.parent = so.name - and so_item.prevdoc_docname = `tabQuotation`.name""" + quotation = frappe.qb.DocType("Quotation") + so = frappe.qb.DocType("Sales Order") + so_item = frappe.qb.DocType("Sales Order Item") - # if not exists any SO, set status as Expired - frappe.db.multisql( - { - "mariadb": f"""UPDATE `tabQuotation` SET `tabQuotation`.status = 'Expired' WHERE {cond} and not exists({so_against_quo})""", - "postgres": f"""UPDATE `tabQuotation` SET status = 'Expired' FROM `tabSales Order`, `tabSales Order Item` WHERE {cond} and not exists({so_against_quo})""", - }, - (nowdate()), + # submitted Sales Orders raised against the quotation (correlated to the quotation being updated) + so_against_quo = ( + frappe.qb.from_(so) + .from_(so_item) + .select(so.name) + .where( + (so_item.docstatus == 1) + & (so.docstatus == 1) + & (so_item.parent == so.name) + & (so_item.prevdoc_docname == quotation.name) + ) ) + + # expire submitted, non-expired/lost quotations whose validity has ended and that have no SO + ( + frappe.qb.update(quotation) + .set(quotation.status, "Expired") + .where( + (quotation.docstatus == 1) + & (quotation.status.notin(["Expired", "Lost"])) + & (quotation.valid_till < nowdate()) + & ExistsCriterion(so_against_quo).negate() + ) + ).run() diff --git a/erpnext/selling/page/point_of_sale/point_of_sale.py b/erpnext/selling/page/point_of_sale/point_of_sale.py index df675272c68..83e7bac3fef 100644 --- a/erpnext/selling/page/point_of_sale/point_of_sale.py +++ b/erpnext/selling/page/point_of_sale/point_of_sale.py @@ -5,7 +5,7 @@ import json import frappe -from frappe.query_builder import DocType, Order +from frappe.query_builder import Criterion, DocType, Order from frappe.utils import cint, get_datetime from frappe.utils.nestedset import get_root_of @@ -155,50 +155,55 @@ def get_items( if not frappe.db.exists("Item Group", item_group): item_group = get_root_of("Item Group") - condition = get_conditions(search_term) - condition += get_item_group_condition(pos_profile) - lft, rgt = frappe.db.get_value("Item Group", item_group, ["lft", "rgt"]) - bin_join_selection, bin_join_condition = "", "" - if hide_unavailable_items: - bin_join_selection = "LEFT JOIN `tabBin` bin ON bin.item_code = item.name" - bin_join_condition = "AND (item.is_stock_item = 0 OR (item.is_stock_item = 1 AND bin.warehouse = %(warehouse)s AND bin.actual_qty > 0))" + item = frappe.qb.DocType("Item") + item_group_dt = frappe.qb.DocType("Item Group") - items_data = frappe.db.sql( - """ - SELECT - item.name AS item_code, + item_group_subquery = ( + frappe.qb.from_(item_group_dt) + .select(item_group_dt.name) + .where((item_group_dt.lft >= lft) & (item_group_dt.rgt <= rgt)) + ) + + query = ( + frappe.qb.from_(item) + .select( + item.name.as_("item_code"), item.item_name, item.description, item.stock_uom, - item.image AS item_image, + item.image.as_("item_image"), item.is_stock_item, - item.sales_uom - FROM - `tabItem` item {bin_join_selection} - WHERE - item.disabled = 0 - AND item.has_variants = 0 - AND item.is_sales_item = 1 - AND item.is_fixed_asset = 0 - AND item.item_group in (SELECT name FROM `tabItem Group` WHERE lft >= {lft} AND rgt <= {rgt}) - AND {condition} - {bin_join_condition} - ORDER BY - item.name asc - LIMIT - {page_length} offset {start}""".format( - start=cint(start), - page_length=cint(page_length), - lft=cint(lft), - rgt=cint(rgt), - condition=condition, - bin_join_selection=bin_join_selection, - bin_join_condition=bin_join_condition, - ), - {"warehouse": warehouse}, - as_dict=1, + item.sales_uom, + ) + .where( + (item.disabled == 0) + & (item.has_variants == 0) + & (item.is_sales_item == 1) + & (item.is_fixed_asset == 0) + & (item.item_group.isin(item_group_subquery)) + & get_conditions(search_term, item) + ) + ) + + item_group_condition = get_item_group_condition(pos_profile, item) + if item_group_condition is not None: + query = query.where(item_group_condition) + + if hide_unavailable_items: + bin_dt = frappe.qb.DocType("Bin") + query = ( + query.left_join(bin_dt) + .on(bin_dt.item_code == item.name) + .where( + (item.is_stock_item == 0) + | ((item.is_stock_item == 1) & (bin_dt.warehouse == warehouse) & (bin_dt.actual_qty > 0)) + ) + ) + + items_data = ( + query.orderby(item.name, order=Order.asc).limit(cint(page_length)).offset(cint(start)).run(as_dict=1) ) # return (empty) list if there are no results @@ -269,56 +274,63 @@ def search_for_serial_or_batch_or_barcode_number(search_value: str) -> dict[str, return scan_barcode(search_value) -def get_conditions(search_term): - condition = "(" - condition += """item.name like {search_term} - or item.item_name like {search_term}""".format(search_term=frappe.db.escape("%" + search_term + "%")) - condition += add_search_fields_condition(search_term) - condition += ")" +def get_conditions(search_term, item=None): + if item is None: + item = frappe.qb.DocType("Item") - return condition + pattern = f"%{search_term}%" + conditions = [item.name.like(pattern), item.item_name.like(pattern)] + conditions += add_search_fields_condition(search_term, item) + + return Criterion.any(conditions) -def add_search_fields_condition(search_term): - condition = "" +def add_search_fields_condition(search_term, item=None): + if item is None: + item = frappe.qb.DocType("Item") + + pattern = f"%{search_term}%" + conditions = [] search_fields = frappe.get_all("POS Search Fields", fields=["fieldname"]) - if search_fields: - for field in search_fields: - if not field.get("fieldname"): - continue - condition += " or item.`{}` like {}".format( - field["fieldname"], frappe.db.escape("%" + search_term + "%") - ) - return condition + for field in search_fields: + if not field.get("fieldname"): + continue + conditions.append(item[field["fieldname"]].like(pattern)) + + return conditions -def get_item_group_condition(pos_profile): - cond = "and 1=1" +def get_item_group_condition(pos_profile, item=None): + if item is None: + item = frappe.qb.DocType("Item") + item_groups = get_item_groups(pos_profile) if item_groups: - cond = "and item.item_group in (%s)" % (", ".join(["%s"] * len(item_groups))) + return item.item_group.isin(item_groups) - return cond % tuple(item_groups) + return None @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def item_group_query(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - item_groups = [] - cond = "1=1" pos_profile = filters.get("pos_profile") + item_filters = [["name", "like", f"%{txt}%"]] if pos_profile: item_groups = get_item_groups(pos_profile) - if item_groups: - cond = "name in (%s)" % (", ".join(["%s"] * len(item_groups))) - cond = cond % tuple(item_groups) + item_filters.append(["name", "in", item_groups]) - return frappe.db.sql( - f""" select distinct name from `tabItem Group` - where {cond} and (name like %(txt)s) limit {page_len} offset {start}""", - {"txt": "%%%s%%" % txt}, + return frappe.get_all( + "Item Group", + filters=item_filters, + fields=["name"], + distinct=True, + order_by="", # original raw SQL had no ORDER BY; suppress the injected default (creation desc on MariaDB) + limit_start=start, + limit_page_length=page_len, + as_list=True, ) diff --git a/erpnext/selling/page/point_of_sale/test_point_of_sale.py b/erpnext/selling/page/point_of_sale/test_point_of_sale.py new file mode 100644 index 00000000000..dcfe6e7edb3 --- /dev/null +++ b/erpnext/selling/page/point_of_sale/test_point_of_sale.py @@ -0,0 +1,137 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.utils import random_string + +from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile +from erpnext.selling.page.point_of_sale.point_of_sale import get_items +from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry +from erpnext.tests.utils import ERPNextTestSuite + + +class TestPointOfSaleGetItems(ERPNextTestSuite): + """Covers the raw-SQL -> frappe.qb conversion of point_of_sale.get_items.""" + + def setUp(self): + super().setUp() + # Reuse the bootstrap leaf item group; an item assigned directly to it + # falls inside its own (lft, rgt) subtree, which is what get_items filters on. + self.item_group = "_Test Item Group" + + # A non-stock sales item keeps get_stock_availability cheap (no Bin needed) + # and keeps the item out of the hide_unavailable_items branch. + self.item_code = "_Test POS Item " + random_string(10) + item = frappe.get_doc( + { + "doctype": "Item", + "item_code": self.item_code, + "item_name": self.item_code, + "item_group": self.item_group, + "stock_uom": "_Test UOM", + "is_stock_item": 0, + "is_sales_item": 1, + "is_fixed_asset": 0, + "has_variants": 0, + "disabled": 0, + } + ) + item.insert() + self.item = item + + # make_pos_profile builds "_Test POS Profile" (hide_unavailable_items unset, + # no item_groups restriction). Rolled back by tearDown. + self.pos_profile = make_pos_profile().name + + def _get_item_codes(self, search_term): + result = get_items( + start=0, + page_length=100, + price_list="Standard Selling", + item_group=self.item_group, + pos_profile=self.pos_profile, + search_term=search_term, + ) + # get_items returns {"items": [...]} when the qb query yields rows, + # and a bare (empty) list when nothing matches. + items = result["items"] if isinstance(result, dict) else result + return [row.get("item_code") for row in items] + + def _make_stock_item(self): + # Fresh stock item in the filtered item group so it passes the + # item_group.isin(subquery) clause and reaches the Bin left-join. + item_code = "_Test POS Stock Item " + random_string(10) + frappe.get_doc( + { + "doctype": "Item", + "item_code": item_code, + "item_name": item_code, + "item_group": self.item_group, + "stock_uom": "_Test UOM", + "is_stock_item": 1, + "is_sales_item": 1, + "is_fixed_asset": 0, + "has_variants": 0, + "disabled": 0, + } + ).insert() + return item_code + + def test_matching_search_term_returns_item(self): + # search_term matches Item.name / Item.item_name via the LIKE OR-condition; + # scan_barcode finds nothing for this value, so the converted qb query runs. + item_codes = self._get_item_codes(self.item_code) + self.assertIn(self.item_code, item_codes) + + def test_non_matching_search_term_excludes_item(self): + non_matching = "zzz_no_such_item_" + random_string(10) + item_codes = self._get_item_codes(non_matching) + self.assertNotIn(self.item_code, item_codes) + + def test_partial_search_term_matches_on_item_name(self): + # A substring of the item code must still match (LIKE %term%), + # proving the OR/LIKE clause survived the SQL->qb conversion. + partial = self.item_code.split(" ")[-1] + item_codes = self._get_item_codes(partial) + self.assertIn(self.item_code, item_codes) + + def test_disabled_item_is_excluded(self): + # disabled == 0 is part of the converted WHERE clause; flipping it + # must drop the item even when the search term matches. + frappe.db.set_value("Item", self.item_code, "disabled", 1) + item_codes = self._get_item_codes(self.item_code) + self.assertNotIn(self.item_code, item_codes) + + def test_non_sales_item_is_excluded(self): + # is_sales_item == 1 is part of the converted WHERE clause. + frappe.db.set_value("Item", self.item_code, "is_sales_item", 0) + item_codes = self._get_item_codes(self.item_code) + self.assertNotIn(self.item_code, item_codes) + + def test_hide_unavailable_items_filters_on_bin_actual_qty(self): + # Covers the hide_unavailable_items branch: the Bin left-join only keeps a + # stock item when bin.warehouse == profile warehouse AND bin.actual_qty > 0. + # A second stock item with no Bin row (no stock) must be hidden. + warehouse = frappe.db.get_value("POS Profile", self.pos_profile, "warehouse") + frappe.db.set_value("POS Profile", self.pos_profile, "hide_unavailable_items", 1) + + in_stock_item = self._make_stock_item() + out_of_stock_item = self._make_stock_item() + + # Material Receipt gives in_stock_item actual_qty > 0 in the profile warehouse; + # out_of_stock_item gets no Bin row at all. + make_stock_entry(item_code=in_stock_item, target=warehouse, qty=5, basic_rate=100) + + # Sanity-check the precondition the branch keys off of. + self.assertGreater( + frappe.db.get_value("Bin", {"item_code": in_stock_item, "warehouse": warehouse}, "actual_qty") + or 0, + 0, + ) + self.assertFalse(frappe.db.exists("Bin", {"item_code": out_of_stock_item})) + + in_stock_codes = self._get_item_codes(in_stock_item) + self.assertIn(in_stock_item, in_stock_codes) + + out_of_stock_codes = self._get_item_codes(out_of_stock_item) + self.assertNotIn(out_of_stock_item, out_of_stock_codes) diff --git a/erpnext/selling/page/sales_funnel/sales_funnel.py b/erpnext/selling/page/sales_funnel/sales_funnel.py index 11fd698c27b..6ce192e95e2 100644 --- a/erpnext/selling/page/sales_funnel/sales_funnel.py +++ b/erpnext/selling/page/sales_funnel/sales_funnel.py @@ -5,6 +5,7 @@ from itertools import groupby import frappe from frappe import _ +from frappe.query_builder.functions import Count, Date from frappe.utils import flt from erpnext.accounts.report.utils import convert @@ -22,33 +23,47 @@ def validate_filters(from_date, to_date, company): def get_funnel_data(from_date: str, to_date: str, company: str): validate_filters(from_date, to_date, company) - active_leads = frappe.db.sql( - """select count(*) from `tabLead` - where (date(`creation`) between %s and %s) - and company=%s""", - (from_date, to_date, company), + lead = frappe.qb.DocType("Lead") + active_leads = ( + frappe.qb.from_(lead) + .select(Count("*")) + .where(Date(lead.creation).between(from_date, to_date) & (lead.company == company)) + .run() )[0][0] - opportunities = frappe.db.sql( - """select count(*) from `tabOpportunity` - where (date(`creation`) between %s and %s) - and opportunity_from='Lead' and company=%s""", - (from_date, to_date, company), + opportunity = frappe.qb.DocType("Opportunity") + opportunities = ( + frappe.qb.from_(opportunity) + .select(Count("*")) + .where( + Date(opportunity.creation).between(from_date, to_date) + & (opportunity.opportunity_from == "Lead") + & (opportunity.company == company) + ) + .run() )[0][0] - quotations = frappe.db.sql( - """select count(*) from `tabQuotation` - where docstatus = 1 and (date(`creation`) between %s and %s) - and (opportunity!="" or quotation_to="Lead") and company=%s""", - (from_date, to_date, company), + quotation = frappe.qb.DocType("Quotation") + quotations = ( + frappe.qb.from_(quotation) + .select(Count("*")) + .where( + (quotation.docstatus == 1) + & Date(quotation.creation).between(from_date, to_date) + & ((quotation.opportunity != "") | (quotation.quotation_to == "Lead")) + & (quotation.company == company) + ) + .run() )[0][0] - converted = frappe.db.sql( - """select count(*) from `tabCustomer` - JOIN `tabLead` ON `tabLead`.name = `tabCustomer`.lead_name - WHERE (date(`tabCustomer`.creation) between %s and %s) - and `tabLead`.company=%s""", - (from_date, to_date, company), + customer = frappe.qb.DocType("Customer") + converted = ( + frappe.qb.from_(customer) + .inner_join(lead) + .on(lead.name == customer.lead_name) + .select(Count("*")) + .where(Date(customer.creation).between(from_date, to_date) & (lead.company == company)) + .run() )[0][0] return [ diff --git a/erpnext/selling/page/sales_funnel/test_sales_funnel.py b/erpnext/selling/page/sales_funnel/test_sales_funnel.py new file mode 100644 index 00000000000..32835eb77b6 --- /dev/null +++ b/erpnext/selling/page/sales_funnel/test_sales_funnel.py @@ -0,0 +1,125 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.utils import add_days, random_string, today + +from erpnext.crm.doctype.opportunity.test_opportunity import make_opportunity +from erpnext.selling.doctype.quotation.test_quotation import make_quotation +from erpnext.selling.page.sales_funnel.sales_funnel import get_funnel_data +from erpnext.tests.utils import ERPNextTestSuite + + +class TestSalesFunnel(ERPNextTestSuite): + def get_stage_value(self, data, title): + for stage in data: + if stage["title"] == title: + return stage["value"] + self.fail(f"Stage {title!r} not found in funnel data: {data}") + + def make_lead(self, company): + # The funnel filters Lead on `company`, which the shared crm make_lead() + # helper does not set, so build the Lead directly here. + return frappe.get_doc( + { + "doctype": "Lead", + "first_name": "_Test Funnel", + "last_name": random_string(6), + "email_id": f"funnel_{random_string(8)}@example.com", + "company": company, + "status": "Lead", + } + ).insert(ignore_permissions=True) + + def test_funnel_lead_and_opportunity_counts(self): + company = "_Test Company" + # validate_filters() rejects from_date >= to_date, and the query matches on + # Date(creation), so use [today, tomorrow] to capture docs created today. + from_date, to_date = today(), add_days(today(), 1) + + # Baseline before creating anything (robust against pre-existing rows). + baseline = get_funnel_data(from_date, to_date, company) + baseline_leads = self.get_stage_value(baseline, "Active Leads") + baseline_opportunities = self.get_stage_value(baseline, "Opportunities") + + # Create two leads for this company today. + lead_1 = self.make_lead(company) + self.make_lead(company) + + # Create one opportunity (opportunity_from='Lead') against one of the leads. + opportunity = make_opportunity( + company=company, + opportunity_from="Lead", + lead=lead_1.name, + ) + self.assertEqual(opportunity.opportunity_from, "Lead") + self.assertEqual(opportunity.party_name, lead_1.name) + + after = get_funnel_data(from_date, to_date, company) + after_leads = self.get_stage_value(after, "Active Leads") + after_opportunities = self.get_stage_value(after, "Opportunities") + + # The two new leads and one new opportunity must be reflected exactly. + self.assertEqual(after_leads - baseline_leads, 2) + self.assertEqual(after_opportunities - baseline_opportunities, 1) + + # Sanity: counts are at least what we created. + self.assertGreaterEqual(after_leads, 2) + self.assertGreaterEqual(after_opportunities, 1) + + def test_funnel_filters_by_company(self): + # A lead for a different company must not inflate the target company's count. + company = "_Test Company" + other_company = "_Test Company 1" + from_date, to_date = today(), add_days(today(), 1) + + baseline_leads = self.get_stage_value(get_funnel_data(from_date, to_date, company), "Active Leads") + + # Lead created for a different company. + self.make_lead(other_company) + + after_leads = self.get_stage_value(get_funnel_data(from_date, to_date, company), "Active Leads") + self.assertEqual(after_leads, baseline_leads) + + def test_funnel_quotations_count(self): + # A submitted Quotation linked to an Opportunity (the `opportunity != ""` + # branch of the funnel filter) must be reflected in the Quotations stage. + company = "_Test Company" + from_date, to_date = today(), add_days(today(), 1) + + baseline_quotations = self.get_stage_value(get_funnel_data(from_date, to_date, company), "Quotations") + + opportunity = make_opportunity(company=company, opportunity_from="Customer") + + quotation = make_quotation(party_name="_Test Customer", company=company, do_not_submit=True) + quotation.opportunity = opportunity.name + quotation.submit() + self.assertEqual(quotation.docstatus, 1) + + after_quotations = self.get_stage_value(get_funnel_data(from_date, to_date, company), "Quotations") + self.assertEqual(after_quotations - baseline_quotations, 1) + self.assertGreaterEqual(after_quotations, 1) + + def test_funnel_converted_count(self): + # A Customer joined to a Lead of this company (Customer INNER JOIN Lead on + # lead_name) must be reflected in the Converted stage. + company = "_Test Company" + from_date, to_date = today(), add_days(today(), 1) + + baseline_converted = self.get_stage_value(get_funnel_data(from_date, to_date, company), "Converted") + + lead = self.make_lead(company) + frappe.get_doc( + { + "doctype": "Customer", + "customer_name": f"_Test Funnel Customer {random_string(6)}", + "customer_type": "Company", + "customer_group": "_Test Customer Group", + "territory": "_Test Territory", + "lead_name": lead.name, + } + ).insert(ignore_permissions=True) + + after_converted = self.get_stage_value(get_funnel_data(from_date, to_date, company), "Converted") + self.assertEqual(after_converted - baseline_converted, 1) + self.assertGreaterEqual(after_converted, 1) diff --git a/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py b/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py index b312abd4607..0c93800bd8b 100644 --- a/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py +++ b/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py @@ -12,7 +12,7 @@ def execute(filters=None): columns = get_columns() iwq_map = get_item_warehouse_quantity_map() - item_map = get_item_details() + item_map = get_item_details(list(iwq_map.keys())) data = [] for sbom, warehouse in iwq_map.items(): total = 0 @@ -53,48 +53,67 @@ def get_columns(): return columns -def get_item_details(): +def get_item_details(item_codes): + # only the bundle items actually shown in the report need detail lookup, not the whole catalogue + if not item_codes: + return {} item_map = {} - for item in frappe.db.sql( - """SELECT name, item_name, description, stock_uom - from `tabItem`""", - as_dict=1, + for item in frappe.get_all( + "Item", + filters={"name": ["in", item_codes]}, + fields=["name", "item_name", "description", "stock_uom"], ): item_map.setdefault(item.name, item) return item_map def get_item_warehouse_quantity_map(): - query = """SELECT parent, warehouse, MIN(qty) AS qty - FROM (SELECT b.parent, bi.item_code, bi.warehouse, - sum(bi.projected_qty) / b.qty AS qty - FROM tabBin AS bi, (SELECT pb.new_item_code as parent, b.item_code, b.qty, w.name - FROM `tabProduct Bundle Item` b, `tabWarehouse` w, - `tabProduct Bundle` pb - where b.parent = pb.name - and pb.is_active = 1 and pb.docstatus = 1) AS b - WHERE bi.item_code = b.item_code - AND bi.warehouse = b.name - GROUP BY b.parent, b.item_code, bi.warehouse - UNION ALL - SELECT b.parent, b.item_code, b.name, 0 AS qty - FROM (SELECT pb.new_item_code as parent, b.item_code, b.qty, w.name - FROM `tabProduct Bundle Item` b, `tabWarehouse` w, - `tabProduct Bundle` pb - where b.parent = pb.name - and pb.is_active = 1 and pb.docstatus = 1) AS b - WHERE NOT EXISTS(SELECT * - FROM `tabBin` AS bi - WHERE bi.item_code = b.item_code - AND bi.warehouse = b.name)) AS r - GROUP BY parent, warehouse - HAVING MIN(qty) != 0""" - result = frappe.db.sql(query, as_dict=1) - last_sbom = "" + # Components of every active product bundle: (bundle item code, component item, qty per bundle) + pb = frappe.qb.DocType("Product Bundle") + pbi = frappe.qb.DocType("Product Bundle Item") + bundle_components = ( + frappe.qb.from_(pbi) + .inner_join(pb) + .on(pbi.parent == pb.name) + .select(pb.new_item_code.as_("parent"), pbi.item_code, pbi.qty) + .where((pb.is_active == 1) & (pb.docstatus == 1)) + .run(as_dict=True) + ) + + if not bundle_components: + return {} + + component_items = list({c.item_code for c in bundle_components}) + + bin_projected = { + (b.item_code, b.warehouse): flt(b.projected_qty) + for b in frappe.get_all( + "Bin", + filters={"item_code": ["in", component_items]}, + fields=["item_code", "warehouse", "projected_qty"], + ) + } + + # Only warehouses that hold at least one component can yield a non-zero packable qty; a warehouse + # missing any component yields MIN()=0 and is dropped below, so scanning every warehouse in the + # system is wasted work. Scope the loop to warehouses present in the Bin result. + bin_warehouses = {wh for (_, wh) in bin_projected} + + # For each (bundle, warehouse) the number of complete bundles that can be packed is the + # MIN over components of (component projected_qty in that warehouse / component qty per bundle). + # A component with no Bin in a warehouse contributes 0 (the original UNION ALL/NOT EXISTS branch). + packable_qty = {} + for component in bundle_components: + if not component.qty: + continue + for warehouse in bin_warehouses: + qty = bin_projected.get((component.item_code, warehouse), 0) / flt(component.qty) + key = (component.parent, warehouse) + packable_qty[key] = min(packable_qty[key], qty) if key in packable_qty else qty + sbom_map = {} - for line in result: - if line.get("parent") != last_sbom: - last_sbom = line.get("parent") - actual_dict = sbom_map.setdefault(last_sbom, {}) - actual_dict.setdefault(line.get("warehouse"), line.get("qty")) + for (parent, warehouse), qty in packable_qty.items(): + if qty != 0: # HAVING MIN(qty) != 0 + sbom_map.setdefault(parent, {})[warehouse] = qty + return sbom_map diff --git a/erpnext/selling/report/available_stock_for_packing_items/test_available_stock_for_packing_items.py b/erpnext/selling/report/available_stock_for_packing_items/test_available_stock_for_packing_items.py new file mode 100644 index 00000000000..75c116e00a0 --- /dev/null +++ b/erpnext/selling/report/available_stock_for_packing_items/test_available_stock_for_packing_items.py @@ -0,0 +1,160 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.utils import flt, random_string + +from erpnext.selling.report.available_stock_for_packing_items.available_stock_for_packing_items import ( + execute, +) +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.tests.utils import ERPNextTestSuite + +WAREHOUSE = "_Test Warehouse - _TC" + + +class TestAvailableStockForPackingItems(ERPNextTestSuite): + """Cover the MIN-over-components / per-warehouse logic of the rewritten report. + + The report computes, for each (active Product Bundle, warehouse): + packable bundles = MIN over components of (Bin.projected_qty / qty per bundle) + and drops rows where that MIN is 0. We use freshly created component items so the + only Bin rows and the only bundle referencing them are the ones built here -- this + keeps the asserted number exact and makes the test fail if the conversion breaks. + """ + + def make_component(self): + return make_item( + f"_Test Packing Component {random_string(10)}", + {"is_stock_item": 1}, + ).name + + def make_bundle_parent(self): + return make_item( + f"_Test Packing Bundle {random_string(10)}", + {"is_stock_item": 0, "is_sales_item": 1}, + ).name + + def set_bin_projected_qty(self, item_code, warehouse, projected_qty): + """Create (if needed) the Bin for (item, warehouse) and pin projected_qty. + + Bin recomputes projected_qty from actual/ordered/... on save, so after the + Bin exists we force the exact value with db.set_value (no controller recompute). + This is precisely the column the report reads back. + """ + name = frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": warehouse}) + if not name: + bin_doc = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse) + bin_doc.flags.ignore_permissions = True + bin_doc.insert() + name = bin_doc.name + frappe.db.set_value("Bin", name, "projected_qty", projected_qty) + return name + + def make_active_bundle(self, parent, components): + """components: list of (item_code, qty_per_bundle). Submitted => is_active, docstatus 1.""" + bundle = frappe.get_doc({"doctype": "Product Bundle", "new_item_code": parent}) + for item_code, qty in components: + bundle.append("items", {"item_code": item_code, "qty": qty}) + bundle.insert() + bundle.submit() + return bundle + + def report_rows_for(self, parent): + """Run the report and return the data rows whose Item Code == parent (drops Total rows).""" + _columns, data = execute(filters=None) + return [row for row in data if row and row[0] == parent] + + def test_min_over_components_binds(self): + comp_a = self.make_component() + comp_b = self.make_component() + parent = self.make_bundle_parent() + + # comp_a: 2 per bundle, projected 10 -> 5 bundles; comp_b: 1 per bundle, projected 3 -> 3 bundles + self.set_bin_projected_qty(comp_a, WAREHOUSE, 10) + self.set_bin_projected_qty(comp_b, WAREHOUSE, 3) + self.make_active_bundle(parent, [(comp_a, 2), (comp_b, 1)]) + + rows = self.report_rows_for(parent) + + # Exactly one (bundle, warehouse) row, and packable == MIN(5, 3) == 3. + self.assertEqual(len(rows), 1) + row = rows[0] + # row shape: [item_code, item_name, description, uom, warehouse, quantity] + self.assertEqual(row[4], WAREHOUSE) + self.assertEqual(flt(row[5]), 3.0) + + def test_per_warehouse_grouping(self): + comp_a = self.make_component() + comp_b = self.make_component() + parent = self.make_bundle_parent() + + other_wh = self.make_secondary_warehouse() + + # _Test Warehouse: comp_a 8/2=4, comp_b 6/1=6 -> MIN 4 + self.set_bin_projected_qty(comp_a, WAREHOUSE, 8) + self.set_bin_projected_qty(comp_b, WAREHOUSE, 6) + # other warehouse: comp_a 4/2=2, comp_b 9/1=9 -> MIN 2 + self.set_bin_projected_qty(comp_a, other_wh, 4) + self.set_bin_projected_qty(comp_b, other_wh, 9) + + self.make_active_bundle(parent, [(comp_a, 2), (comp_b, 1)]) + + rows = self.report_rows_for(parent) + by_warehouse = {row[4]: flt(row[5]) for row in rows} + + self.assertEqual(by_warehouse.get(WAREHOUSE), 4.0) + self.assertEqual(by_warehouse.get(other_wh), 2.0) + + def test_starved_component_drops_row(self): + comp_a = self.make_component() + comp_b = self.make_component() + parent = self.make_bundle_parent() + + # comp_a is plentiful, comp_b is absent in the warehouse (no Bin) -> MIN == 0 -> dropped. + self.set_bin_projected_qty(comp_a, WAREHOUSE, 50) + self.make_active_bundle(parent, [(comp_a, 2), (comp_b, 1)]) + + self.assertEqual(self.report_rows_for(parent), []) + + def test_zero_projected_component_drops_row(self): + comp_a = self.make_component() + comp_b = self.make_component() + parent = self.make_bundle_parent() + + # comp_b present but with projected 0 -> 0/1 == 0 -> MIN == 0 -> row dropped. + self.set_bin_projected_qty(comp_a, WAREHOUSE, 20) + self.set_bin_projected_qty(comp_b, WAREHOUSE, 0) + self.make_active_bundle(parent, [(comp_a, 2), (comp_b, 1)]) + + self.assertEqual(self.report_rows_for(parent), []) + + def test_inactive_bundle_excluded(self): + comp_a = self.make_component() + parent = self.make_bundle_parent() + + self.set_bin_projected_qty(comp_a, WAREHOUSE, 10) + bundle = self.make_active_bundle(parent, [(comp_a, 1)]) + + # Sanity: while active it shows up... + self.assertTrue(self.report_rows_for(parent)) + + # ...and disappears once cancelled (is_active cleared, docstatus 2). + bundle.cancel() + self.assertEqual(self.report_rows_for(parent), []) + + def make_secondary_warehouse(self): + """A second leaf warehouse under _Test Company so two warehouses can be asserted.""" + name = f"_Test Pack WH {random_string(6)}" + full_name = f"{name} - _TC" + if frappe.db.exists("Warehouse", full_name): + return full_name + wh = frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": name, + "company": "_Test Company", + } + ) + wh.insert() + return wh.name diff --git a/erpnext/selling/report/customer_acquisition_and_loyalty/customer_acquisition_and_loyalty.py b/erpnext/selling/report/customer_acquisition_and_loyalty/customer_acquisition_and_loyalty.py index dcdd2525d8e..098551cb297 100644 --- a/erpnext/selling/report/customer_acquisition_and_loyalty/customer_acquisition_and_loyalty.py +++ b/erpnext/selling/report/customer_acquisition_and_loyalty/customer_acquisition_and_loyalty.py @@ -112,8 +112,8 @@ def get_data_by_territory(filters, common_columns): customers_in = get_customer_stats(filters, tree_view=True) territory_dict = {} - for t in frappe.db.sql( - """SELECT name, lft, parent_territory, is_group FROM `tabTerritory` ORDER BY lft""", as_dict=1 + for t in frappe.get_all( + "Territory", fields=["name", "lft", "parent_territory", "is_group"], order_by="lft" ): territory_dict.update({t.name: {"parent": t.parent_territory, "is_group": t.is_group}}) @@ -155,19 +155,19 @@ def get_data_by_territory(filters, common_columns): def get_customer_stats(filters, tree_view=False): """Calculates number of new and repeated customers and revenue.""" - company_condition = "" - if filters.get("company"): - company_condition = " and company=%(company)s" - customers = [] customers_in = {} - for si in frappe.db.sql( - f"""select territory, posting_date, customer, base_grand_total from `tabSales Invoice` - where docstatus=1 and posting_date <= %(to_date)s - {company_condition} order by posting_date""", - filters, - as_dict=1, + si_filters = {"docstatus": 1, "posting_date": ["<=", filters.get("to_date")]} + if filters.get("company"): + si_filters["company"] = filters.get("company") + + for si in frappe.get_all( + "Sales Invoice", + filters=si_filters, + fields=["territory", "posting_date", "customer", "base_grand_total"], + # name tie-break makes the first-seen-per-customer classification deterministic across engines + order_by="posting_date, name", ): key = si.territory if tree_view else si.posting_date.strftime("%Y-%m") new_or_repeat = "new" if si.customer not in customers else "repeat" diff --git a/erpnext/selling/report/customer_acquisition_and_loyalty/test_customer_acquisition_and_loyalty.py b/erpnext/selling/report/customer_acquisition_and_loyalty/test_customer_acquisition_and_loyalty.py new file mode 100644 index 00000000000..b0e724cb7cf --- /dev/null +++ b/erpnext/selling/report/customer_acquisition_and_loyalty/test_customer_acquisition_and_loyalty.py @@ -0,0 +1,121 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.utils import getdate, random_string + +from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice +from erpnext.selling.report.customer_acquisition_and_loyalty.customer_acquisition_and_loyalty import ( + get_customer_stats, +) +from erpnext.tests.utils import ERPNextTestSuite + + +class TestCustomerAcquisitionAndLoyalty(ERPNextTestSuite): + def test_new_vs_repeat_classification(self): + # Use a posting month in the past so the YYYY-MM bucket is unlikely to collide + # with other fixtures; deltas vs a baseline still neutralise any overlap. + first_date = "2017-04-05" + second_date = "2017-04-20" + month_key = getdate(first_date).strftime("%Y-%m") + # source uses both filters.get(...) and attribute access (filters.from_date), + # so pass a frappe._dict the way the report's execute() does. + filters = frappe._dict( + {"from_date": "2017-01-01", "to_date": "2017-04-30", "company": "_Test Company"} + ) + + customer = frappe.get_doc( + { + "doctype": "Customer", + "customer_name": "_Test CAL Customer " + random_string(8), + "customer_group": "_Test Customer Group", + "customer_type": "Individual", + "territory": "_Test Territory", + } + ).insert() + + # Baseline before adding any activity for this customer. + base = get_customer_stats(filters) + base_bucket = base.get(month_key, {"new": [0, 0.0], "repeat": [0, 0.0]}) + base_new = base_bucket["new"][0] + base_new_rev = base_bucket["new"][1] + base_repeat = base_bucket["repeat"][0] + base_repeat_rev = base_bucket["repeat"][1] + + # Two submitted invoices for the SAME customer in the SAME month: + # the earlier one is the customer's FIRST invoice -> "new", the later -> "repeat". + si1 = create_sales_invoice( + customer=customer.name, company="_Test Company", posting_date=first_date, rate=100 + ) + si2 = create_sales_invoice( + customer=customer.name, company="_Test Company", posting_date=second_date, rate=250 + ) + + stats = get_customer_stats(filters) + bucket = stats.get(month_key) + self.assertIsNotNone(bucket, "expected a bucket for posting month " + month_key) + + # Exactly one NEW and one REPEAT were added for this customer's activity. + self.assertEqual(bucket["new"][0] - base_new, 1) + self.assertEqual(bucket["repeat"][0] - base_repeat, 1) + + # Revenue is attributed by base_grand_total of the corresponding invoice: + # the first (new) invoice carries si1's total, the second (repeat) carries si2's. + self.assertAlmostEqual(bucket["new"][1] - base_new_rev, si1.base_grand_total) + self.assertAlmostEqual(bucket["repeat"][1] - base_repeat_rev, si2.base_grand_total) + + def test_territory_tree_view_classification(self): + # Covers the tree_view=True path of get_customer_stats, where buckets are keyed + # by Sales Invoice territory instead of YYYY-MM. This is the keying that + # get_data_by_territory() (which also drives frappe.get_all("Territory", ...)) + # consumes. A fresh customer on "_Test Territory" makes the bucket deterministic. + territory = "_Test Territory" + first_date = "2017-05-05" + second_date = "2017-05-20" + # get_customer_stats reads filters.from_date (attribute) and filters.get("to_date"), + # so build the _dict the same way execute() does. + filters = frappe._dict( + {"from_date": "2017-01-01", "to_date": "2017-05-31", "company": "_Test Company"} + ) + + customer = frappe.get_doc( + { + "doctype": "Customer", + "customer_name": "_Test CAL Territory Customer " + random_string(8), + "customer_group": "_Test Customer Group", + "customer_type": "Individual", + "territory": territory, + } + ).insert() + + # Baseline for the territory bucket before this customer has any invoices. + base = get_customer_stats(filters, tree_view=True) + base_bucket = base.get(territory, {"new": [0, 0.0], "repeat": [0, 0.0]}) + base_new = base_bucket["new"][0] + base_new_rev = base_bucket["new"][1] + base_repeat = base_bucket["repeat"][0] + base_repeat_rev = base_bucket["repeat"][1] + + # get_party_details copies the customer's territory onto the invoice, so both + # invoices land in the "_Test Territory" bucket: first -> "new", second -> "repeat". + si1 = create_sales_invoice( + customer=customer.name, company="_Test Company", posting_date=first_date, rate=100 + ) + si2 = create_sales_invoice( + customer=customer.name, company="_Test Company", posting_date=second_date, rate=250 + ) + # Guard the test's premise: territory must really be on the invoices. + self.assertEqual(si1.territory, territory) + self.assertEqual(si2.territory, territory) + + stats = get_customer_stats(filters, tree_view=True) + bucket = stats.get(territory) + self.assertIsNotNone(bucket, "expected a bucket keyed by territory " + territory) + + # Exactly one NEW and one REPEAT attributable to this customer in the bucket. + self.assertEqual(bucket["new"][0] - base_new, 1) + self.assertEqual(bucket["repeat"][0] - base_repeat, 1) + + # Revenue follows base_grand_total of the corresponding invoice. + self.assertAlmostEqual(bucket["new"][1] - base_new_rev, si1.base_grand_total) + self.assertAlmostEqual(bucket["repeat"][1] - base_repeat_rev, si2.base_grand_total) diff --git a/erpnext/selling/report/customer_credit_balance/customer_credit_balance.py b/erpnext/selling/report/customer_credit_balance/customer_credit_balance.py index 6813060d414..4d3172c177f 100644 --- a/erpnext/selling/report/customer_credit_balance/customer_credit_balance.py +++ b/erpnext/selling/report/customer_credit_balance/customer_credit_balance.py @@ -77,17 +77,18 @@ def get_columns(customer_naming_type): def get_details(filters): - sql_query = """SELECT - c.name, c.customer_name, - ccl.bypass_credit_limit_check, - c.is_frozen, c.disabled - FROM `tabCustomer` c, `tabCustomer Credit Limit` ccl - WHERE - c.name = ccl.parent - AND ccl.company = %(company)s""" + c = frappe.qb.DocType("Customer") + ccl = frappe.qb.DocType("Customer Credit Limit") + query = ( + frappe.qb.from_(c) + .inner_join(ccl) + .on(c.name == ccl.parent) + .select(c.name, c.customer_name, ccl.bypass_credit_limit_check, c.is_frozen, c.disabled) + .where(ccl.company == filters.get("company")) + ) # customer filter is optional. if filters.get("customer"): - sql_query += " AND c.name = %(customer)s" + query = query.where(c.name == filters.get("customer")) - return frappe.db.sql(sql_query, filters, as_dict=1) + return query.run(as_dict=1) diff --git a/erpnext/selling/report/customer_credit_balance/test_customer_credit_balance.py b/erpnext/selling/report/customer_credit_balance/test_customer_credit_balance.py new file mode 100644 index 00000000000..06bb32a4e0c --- /dev/null +++ b/erpnext/selling/report/customer_credit_balance/test_customer_credit_balance.py @@ -0,0 +1,64 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.utils import random_string + +from erpnext.selling.report.customer_credit_balance.customer_credit_balance import get_details +from erpnext.tests.utils import ERPNextTestSuite + + +class TestCustomerCreditBalance(ERPNextTestSuite): + def test_get_details_returns_customer_with_credit_limit(self): + company = "_Test Company" + customer_name = "_Test Credit Balance " + random_string(8) + + customer = frappe.get_doc( + { + "doctype": "Customer", + "customer_name": customer_name, + "customer_group": "_Test Customer Group", + "territory": "_Test Territory", + "credit_limits": [ + { + "company": company, + "credit_limit": 50000, + "bypass_credit_limit_check": 1, + } + ], + } + ).insert() + + rows = get_details(frappe._dict(company=company, customer=customer.name)) + + # Inner join + company + customer filters must isolate exactly this customer's row. + self.assertEqual(len(rows), 1) + row = rows[0] + self.assertEqual(row.name, customer.name) + self.assertEqual(row.customer_name, customer_name) + self.assertEqual(row.bypass_credit_limit_check, 1) + + def test_get_details_excludes_other_company_credit_limit(self): + # Credit limit child row exists, but for a different company than the filter, + # so the company-filtered inner join must return nothing for this customer. + company = "_Test Company" + customer_name = "_Test Credit Balance " + random_string(8) + + customer = frappe.get_doc( + { + "doctype": "Customer", + "customer_name": customer_name, + "customer_group": "_Test Customer Group", + "territory": "_Test Territory", + "credit_limits": [ + { + "company": "_Test Company 1", + "credit_limit": 50000, + "bypass_credit_limit_check": 0, + } + ], + } + ).insert() + + rows = get_details(frappe._dict(company=company, customer=customer.name)) + self.assertEqual(len(rows), 0) diff --git a/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py b/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py index 82143a11eea..7d7ea42209f 100644 --- a/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py +++ b/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py @@ -4,6 +4,7 @@ import frappe from frappe import _ +from frappe.query_builder.functions import Max, Sum from frappe.utils import flt @@ -49,27 +50,28 @@ def get_columns(): def get_data(): - sales_order_entry = frappe.db.sql( - """ - SELECT + so = frappe.qb.DocType("Sales Order") + so_item = frappe.qb.DocType("Sales Order Item") + sales_order_entry = ( + frappe.qb.from_(so) + .inner_join(so_item) + .on(so.name == so_item.parent) + .select( so_item.item_code, - so_item.item_name, - so_item.description, + # non-grouped columns are constant per grouped so.name / item_code -> Max() keeps the + # GROUP BY valid on postgres while returning the same value MySQL picked. + Max(so_item.item_name).as_("item_name"), + Max(so_item.description).as_("description"), so.name, - so.transaction_date, - so.customer, - so.territory, - sum(so_item.qty) as total_qty, - so.company - FROM `tabSales Order` so, `tabSales Order Item` so_item - WHERE - so.docstatus = 1 - and so.name = so_item.parent - and so.status not in ('Closed','Completed','Cancelled') - GROUP BY - so.name,so_item.item_code - """, - as_dict=1, + Max(so.transaction_date).as_("transaction_date"), + Max(so.customer).as_("customer"), + Max(so.territory).as_("territory"), + Sum(so_item.qty).as_("total_qty"), + Max(so.company).as_("company"), + ) + .where((so.docstatus == 1) & so.status.notin(["Closed", "Completed", "Cancelled"])) + .groupby(so.name, so_item.item_code) + .run(as_dict=1) ) sales_orders = [row.name for row in sales_order_entry] diff --git a/erpnext/selling/report/sales_analytics/sales_analytics.py b/erpnext/selling/report/sales_analytics/sales_analytics.py index e36690b4384..93f1abe8222 100644 --- a/erpnext/selling/report/sales_analytics/sales_analytics.py +++ b/erpnext/selling/report/sales_analytics/sales_analytics.py @@ -510,10 +510,10 @@ class Analytics: self.depth_map = frappe._dict() - self.group_entries = frappe.db.sql( - f"""select name, lft, rgt , {parent} as parent - from `tab{self.filters.tree_type}` order by lft""", - as_dict=1, + self.group_entries = frappe.get_all( + self.filters.tree_type, + fields=["name", "lft", "rgt", f"{parent} as parent"], + order_by="lft", ) for d in self.group_entries: @@ -528,14 +528,19 @@ class Analytics: if not frappe.db.exists("DocType", self.filters.doc_type): frappe.throw(_("Invalid Document Type {0}").format(self.filters.doc_type)) - self.group_entries = frappe.db.sql( - f""" select * from (select "Order Types" as name, 0 as lft, - 2 as rgt, '' as parent union select distinct order_type as name, 1 as lft, 1 as rgt, "Order Types" as parent - from `tab{self.filters.doc_type}` where ifnull(order_type, '') != '') as b order by lft, name - """, - as_dict=1, + order_types = frappe.get_all( + self.filters.doc_type, + filters={"order_type": ["is", "set"]}, + pluck="order_type", + distinct=True, + order_by="order_type", ) + self.group_entries = [frappe._dict(name="Order Types", lft=0, rgt=2, parent="")] + self.group_entries += [ + frappe._dict(name=order_type, lft=1, rgt=1, parent="Order Types") for order_type in order_types + ] + for d in self.group_entries: if d.parent: self.depth_map.setdefault(d.name, self.depth_map.get(d.parent) + 1) @@ -544,7 +549,7 @@ class Analytics: def get_supplier_parent_child_map(self): self.parent_child_map = frappe._dict( - frappe.db.sql(""" select name, supplier_group from `tabSupplier`""") + frappe.get_all("Supplier", fields=["name", "supplier_group"], as_list=True) ) def get_chart_data(self): diff --git a/erpnext/selling/report/sales_analytics/test_sales_analytics.py b/erpnext/selling/report/sales_analytics/test_sales_analytics.py new file mode 100644 index 00000000000..b9827327b63 --- /dev/null +++ b/erpnext/selling/report/sales_analytics/test_sales_analytics.py @@ -0,0 +1,174 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + + +import frappe +from frappe.utils import flt + +from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order +from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order +from erpnext.selling.report.sales_analytics.sales_analytics import execute +from erpnext.tests.utils import ERPNextTestSuite + +# Bootstrap masters reused as-is (see erpnext/tests/utils.py): +# "_Test Customer" -> customer_group "_Test Customer Group", territory "_Test Territory" +# "_Test Supplier" -> supplier_group "_Test Supplier Group" (child of "All Supplier Groups") +# Sales Order.order_type defaults to "Sales" (reqd Select field) +COMPANY = "_Test Company" +CUSTOMER = "_Test Customer" +CUSTOMER_GROUP = "_Test Customer Group" +TERRITORY = "_Test Territory" +SUPPLIER = "_Test Supplier" +SUPPLIER_GROUP = "_Test Supplier Group" +FROM_DATE = "2019-04-01" +TO_DATE = "2019-06-30" + + +class TestSalesAnalytics(ERPNextTestSuite): + def setUp(self): + frappe.set_user("Administrator") + # Two submitted Sales Orders for the bootstrap customer inside the report window. + # These roll up into the tree roots the converted tree/order-type queries build. + self.orders = [ + make_sales_order( + company=COMPANY, + customer=CUSTOMER, + qty=5, + rate=100, + transaction_date="2019-04-10", + ), + make_sales_order( + company=COMPANY, + customer=CUSTOMER, + qty=3, + rate=100, + transaction_date="2019-05-15", + ), + ] + + def _base_filters(self, **overrides): + filters = { + "doc_type": "Sales Order", + "value_quantity": "Value", + "range": "Monthly", + "company": COMPANY, + "from_date": FROM_DATE, + "to_date": TO_DATE, + } + filters.update(overrides) + return filters + + def _expected_value_total(self): + return sum(flt(so.base_net_total) for so in self.orders) + + def _expected_qty_total(self): + return sum(flt(so.total_qty) for so in self.orders) + + def _row_by_entity(self, data): + return {row["entity"]: row for row in data} + + def test_customer_group_tree_rolls_up_to_root(self): + """tree_type='Customer Group' drives get_groups (tree get_all ordered by lft) + and get_rows_by_group, rolling child values up to the 'All Customer Groups' root.""" + columns, data, *_ = execute(self._base_filters(tree_type="Customer Group")) + + self.assertTrue(columns) + self.assertTrue(data) + + rows = self._row_by_entity(data) + # The whole tree is returned, so both the root and the customer's own group appear. + self.assertIn("All Customer Groups", rows) + self.assertIn(CUSTOMER_GROUP, rows) + + expected = self._expected_value_total() + self.assertGreater(expected, 0) + # Leaf group holds the orders; root receives the same total via roll-up. + self.assertAlmostEqual(rows[CUSTOMER_GROUP]["total"], expected, places=2) + self.assertAlmostEqual(rows["All Customer Groups"]["total"], expected, places=2) + # Roots of a tree report sit at indent 0. + self.assertEqual(rows["All Customer Groups"]["indent"], 0) + + def test_territory_tree_rolls_up_to_root(self): + """tree_type='Territory' exercises the same tree path against the Territory tree.""" + columns, data, *_ = execute(self._base_filters(tree_type="Territory")) + + self.assertTrue(columns) + rows = self._row_by_entity(data) + self.assertIn("All Territories", rows) + self.assertIn(TERRITORY, rows) + + expected = self._expected_value_total() + self.assertAlmostEqual(rows[TERRITORY]["total"], expected, places=2) + self.assertAlmostEqual(rows["All Territories"]["total"], expected, places=2) + + def test_order_type_synthetic_tree(self): + """tree_type='Order Type' drives get_teams: distinct order_type rebuilt in Python + under a synthetic 'Order Types' root, then rolled up via get_rows_by_group.""" + columns, data, *_ = execute(self._base_filters(tree_type="Order Type")) + + self.assertTrue(columns) + rows = self._row_by_entity(data) + # Synthetic root plus the default order_type the bootstrap Sales Orders carry. + self.assertIn("Order Types", rows) + self.assertIn("Sales", rows) + self.assertEqual(rows["Order Types"]["indent"], 0) + + expected = self._expected_value_total() + self.assertAlmostEqual(rows["Sales"]["total"], expected, places=2) + self.assertAlmostEqual(rows["Order Types"]["total"], expected, places=2) + + def test_customer_group_by_quantity(self): + """value_quantity='Quantity' switches the selected value column (total_qty).""" + _columns, data, *_ = execute( + self._base_filters(tree_type="Customer Group", value_quantity="Quantity") + ) + + rows = self._row_by_entity(data) + self.assertIn(CUSTOMER_GROUP, rows) + + expected_qty = self._expected_qty_total() + self.assertGreater(expected_qty, 0) + self.assertAlmostEqual(rows[CUSTOMER_GROUP]["total"], expected_qty, places=2) + self.assertAlmostEqual(rows["All Customer Groups"]["total"], expected_qty, places=2) + + def test_supplier_group_tree_maps_supplier_to_group(self): + """tree_type='Supplier Group' (doc_type='Purchase Order') exercises + get_supplier_parent_child_map: the query selects 'supplier' as entity, then + get_periodic_data remaps each supplier to its group via the parent->child map + built by frappe.get_all('Supplier', ['name', 'supplier_group'], as_list=True). + The group total then rolls up into the 'All Supplier Groups' root.""" + # Baseline the report before adding our Purchase Order so the assertion is + # robust to any pre-existing rows in the historical window. + base_filters = self._base_filters(tree_type="Supplier Group", doc_type="Purchase Order") + _columns, base_data, *_ = execute(base_filters) + base_rows = self._row_by_entity(base_data) + base_group_total = flt(base_rows.get(SUPPLIER_GROUP, {}).get("total", 0.0)) + + po = create_purchase_order( + company=COMPANY, + supplier=SUPPLIER, + qty=4, + rate=250, + transaction_date="2019-04-10", + ) + po_value = flt(po.base_net_total) + self.assertGreater(po_value, 0) + + columns, data, *_ = execute(base_filters) + + self.assertTrue(columns) + self.assertTrue(data) + + rows = self._row_by_entity(data) + # The supplier was remapped to its group; both the leaf group and the tree + # root appear as entities (no raw supplier name leaks into the output). + self.assertIn(SUPPLIER_GROUP, rows) + self.assertIn("All Supplier Groups", rows) + self.assertNotIn(SUPPLIER, rows) + # Roots of a tree report sit at indent 0. + self.assertEqual(rows["All Supplier Groups"]["indent"], 0) + + # The new PO lands in the supplier's group via the parent->child map. + self.assertAlmostEqual(rows[SUPPLIER_GROUP]["total"] - base_group_total, po_value, places=2) + # Roll-up: the root aggregates every group, so it covers at least this PO. + self.assertGreaterEqual(flt(rows["All Supplier Groups"]["total"]), po_value) diff --git a/erpnext/selling/report/sales_order_analysis/sales_order_analysis.py b/erpnext/selling/report/sales_order_analysis/sales_order_analysis.py index 90c33c323ce..93c7dcc2554 100644 --- a/erpnext/selling/report/sales_order_analysis/sales_order_analysis.py +++ b/erpnext/selling/report/sales_order_analysis/sales_order_analysis.py @@ -6,9 +6,9 @@ from collections import OrderedDict import frappe from frappe import _, qb -from frappe.query_builder import CustomFunction -from frappe.query_builder.functions import Max -from frappe.utils import date_diff, flt, getdate +from frappe.query_builder import Case, CustomFunction +from frappe.query_builder.functions import Coalesce, DateDiff, Max, Sum +from frappe.utils import date_diff, flt, getdate, nowdate def execute(filters=None): @@ -18,8 +18,7 @@ def execute(filters=None): validate_filters(filters) columns = get_columns(filters) - conditions = get_conditions(filters) - data = get_data(conditions, filters) + data = get_data(filters) so_elapsed_time = get_so_elapsed_time(data) if not data: @@ -39,64 +38,66 @@ def validate_filters(filters): frappe.throw(_("To Date cannot be before From Date.")) -def get_conditions(filters): - conditions = "" - if filters.get("from_date") and filters.get("to_date"): - conditions += " and so.transaction_date between %(from_date)s and %(to_date)s" +def get_data(filters): + so = qb.DocType("Sales Order") + soi = qb.DocType("Sales Order Item") + sii = qb.DocType("Sales Invoice Item") - if filters.get("company"): - conditions += " and so.company = %(company)s" + # Use the application's today (nowdate, System Settings timezone) rather than the database + # server's CURRENT_DATE: the two differ by a day when the DB server runs in a different timezone + # (e.g. UTC DB + IST app near midnight), which made delay_days non-deterministic on postgres CI. + # DateDiff is cross-database: DATEDIFF() on MariaDB, date subtraction on postgres; it casts the + # string date to a date on postgres. delivery_date is functionally dependent on the grouped + # soi.name primary key, so this is valid under both. + delay = DateDiff(nowdate(), soi.delivery_date) + conversion_rate = Coalesce(so.conversion_rate, 1) - if filters.get("sales_order"): - conditions += " and so.name in %(sales_order)s" - - if filters.get("status"): - conditions += " and so.status in %(status)s" - - if filters.get("warehouse"): - conditions += " and soi.warehouse = %(warehouse)s" - - return conditions - - -def get_data(conditions, filters): - data = frappe.db.sql( - f""" - SELECT - so.transaction_date as date, - soi.delivery_date as delivery_date, - so.name as sales_order, - so.status, so.customer, soi.item_code, - DATEDIFF(CURRENT_DATE, soi.delivery_date) as delay_days, - IF(so.status in ('Completed','To Bill'), 0, (SELECT delay_days)) as delay, - soi.qty, soi.delivered_qty, - (soi.qty - soi.delivered_qty) AS pending_qty, - IFNULL(SUM(sii.qty), 0) as billed_qty, - soi.base_amount as amount, - (soi.delivered_qty * soi.base_rate) as delivered_qty_amount, - (soi.billed_amt * IFNULL(so.conversion_rate, 1)) as billed_amount, - (soi.base_amount - (soi.billed_amt * IFNULL(so.conversion_rate, 1))) as pending_amount, - soi.warehouse as warehouse, - so.company, soi.name, - soi.description as description - FROM - `tabSales Order` so, - `tabSales Order Item` soi - LEFT JOIN `tabSales Invoice Item` sii - ON sii.so_detail = soi.name and sii.docstatus = 1 - WHERE - soi.parent = so.name - and so.status not in ('Stopped', 'On Hold') - and so.docstatus = 1 - {conditions} - GROUP BY soi.name - ORDER BY so.transaction_date ASC, soi.item_code ASC - """, - filters, - as_dict=1, + query = ( + qb.from_(so) + .join(soi) + .on(soi.parent == so.name) + .left_join(sii) + .on((sii.so_detail == soi.name) & (sii.docstatus == 1)) + .select( + so.transaction_date.as_("date"), + soi.delivery_date.as_("delivery_date"), + so.name.as_("sales_order"), + so.status, + so.customer, + soi.item_code, + delay.as_("delay_days"), + Case().when(so.status.isin(["Completed", "To Bill"]), 0).else_(delay).as_("delay"), + soi.qty, + soi.delivered_qty, + (soi.qty - soi.delivered_qty).as_("pending_qty"), + Coalesce(Sum(sii.qty), 0).as_("billed_qty"), + soi.base_amount.as_("amount"), + (soi.delivered_qty * soi.base_rate).as_("delivered_qty_amount"), + (soi.billed_amt * conversion_rate).as_("billed_amount"), + (soi.base_amount - (soi.billed_amt * conversion_rate)).as_("pending_amount"), + soi.warehouse.as_("warehouse"), + so.company, + soi.name, + soi.description.as_("description"), + ) + .where((so.status.notin(["Stopped", "On Hold"])) & (so.docstatus == 1)) + .groupby(soi.name, so.name) + .orderby(so.transaction_date) + .orderby(soi.item_code) ) - return data + if filters.get("from_date") and filters.get("to_date"): + query = query.where(so.transaction_date[filters.get("from_date") : filters.get("to_date")]) + if filters.get("company"): + query = query.where(so.company == filters.get("company")) + if filters.get("sales_order"): + query = query.where(so.name.isin(filters.get("sales_order"))) + if filters.get("status"): + query = query.where(so.status.isin(filters.get("status"))) + if filters.get("warehouse"): + query = query.where(soi.warehouse == filters.get("warehouse")) + + return query.run(as_dict=True) def get_so_elapsed_time(data): @@ -112,7 +113,17 @@ def get_so_elapsed_time(data): dn = qb.DocType("Delivery Note") dni = qb.DocType("Delivery Note Item") - to_seconds = CustomFunction("TO_SECONDS", ["date"]) + # TO_SECONDS is MariaDB-only. On postgres, subtracting dates yields days, so multiply + # by 86400 for the equivalent second delta. so.transaction_date is neither aggregated nor + # in the GROUP BY, but it is selectable under postgres' strict GROUP BY because it is + # functionally dependent on the grouped so.name (a doctype's `name` is always the PK). + if frappe.db.db_type == "postgres": + elapsed_seconds = ((Max(dn.posting_date) - so.transaction_date) * 86400).as_("elapsed_seconds") + else: + to_seconds = CustomFunction("TO_SECONDS", ["date"]) + elapsed_seconds = (to_seconds(Max(dn.posting_date)) - to_seconds(so.transaction_date)).as_( + "elapsed_seconds" + ) query = ( qb.from_(so) @@ -125,11 +136,11 @@ def get_so_elapsed_time(data): .select( so.name.as_("sales_order"), soi.item_code.as_("so_item_code"), - (to_seconds(Max(dn.posting_date)) - to_seconds(so.transaction_date)).as_("elapsed_seconds"), + elapsed_seconds, ) .where((so.name.isin(sales_orders)) & (dn.docstatus == 1)) .orderby(so.name, soi.name) - .groupby(soi.name) + .groupby(soi.name, so.name) ) dn_elapsed_time = query.run(as_dict=True)