From ed6a682779d24be3e516c45dee04b6909fd95004 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sun, 21 Jun 2026 10:03:04 +0530 Subject: [PATCH] fix(selling): keep Lost Quotations % fractional on Postgres (integer division) The "Lost Quotations %" column computed Count(distinct) / total_quotations * 100, where both operands are integers. Postgres does integer division on int/int, so any group that is a strict minority of the total truncated to 0 (e.g. 1 of 4 -> 0%); MariaDB always divides as decimal. Multiply by 100.0 before dividing so the division is done in floating point on both engines. The "Lost Value %" column already divided Sum(Currency)/Sum(Currency) (numeric), so it was unaffected; left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../report/lost_quotations/lost_quotations.py | 3 +- .../lost_quotations/test_lost_quotations.py | 63 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 erpnext/selling/report/lost_quotations/test_lost_quotations.py diff --git a/erpnext/selling/report/lost_quotations/lost_quotations.py b/erpnext/selling/report/lost_quotations/lost_quotations.py index a48e945172c..c3bcd54cfd3 100644 --- a/erpnext/selling/report/lost_quotations/lost_quotations.py +++ b/erpnext/selling/report/lost_quotations/lost_quotations.py @@ -83,7 +83,8 @@ def get_data(company: str, from_date: str, to_date: str, group_by: Literal["Lost .select( Coalesce(dimension[fieldname], _("Not Specified")), Count(q.name).distinct(), - Round((Count(q.name).distinct() / total_quotations * 100), 2), + # `* 100.0` before dividing: count/count is integer division on Postgres (truncates to 0) + Round((Count(q.name).distinct() * 100.0 / total_quotations), 2), Sum(q.base_net_total), Round((Sum(q.base_net_total) / total_value * 100), 2), ) diff --git a/erpnext/selling/report/lost_quotations/test_lost_quotations.py b/erpnext/selling/report/lost_quotations/test_lost_quotations.py new file mode 100644 index 00000000000..857fbb3b897 --- /dev/null +++ b/erpnext/selling/report/lost_quotations/test_lost_quotations.py @@ -0,0 +1,63 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe + +from erpnext.selling.doctype.quotation.test_quotation import make_quotation +from erpnext.selling.report.lost_quotations.lost_quotations import execute +from erpnext.tests.utils import ERPNextTestSuite + + +class TestLostQuotations(ERPNextTestSuite): + def setUp(self): + self.company = "_Test Company" + self.reason_a = self._ensure_lost_reason("_Test Lost Reason A") + self.reason_b = self._ensure_lost_reason("_Test Lost Reason B") + + def test_lost_quotations_percentage_is_not_integer_divided(self): + # `lost_quotations_pct` is count(group) / count(total) * 100. count/count is integer division on + # Postgres, which truncates a proper fraction to 0; this asserts the percentage stays fractional. + quotations = [] + # reason A on one quotation, reason B on three -> A is a strict minority of the total + quotations.append(self._make_lost_quotation(self.reason_a)) + for _ in range(3): + quotations.append(self._make_lost_quotation(self.reason_b)) + for qo in quotations: + self.addCleanup(self._cancel_and_delete, qo.name) + + _columns, data = execute( + frappe._dict({"company": self.company, "timespan": "This Year", "group_by": "Lost Reason"}) + ) + + # row layout: (lost_reason, lost_quotations, lost_quotations_pct, lost_value, lost_value_pct) + row_a = next(row for row in data if row[0] == self.reason_a) + self.assertEqual(row_a[1], 1) + # with integer division this is 0; with correct division it is a positive fraction + self.assertGreater(row_a[2], 0) + self.assertLess(row_a[2], 100) + + def _ensure_lost_reason(self, name): + # only clean up reasons this test created, so a pre-existing master is left intact + if not frappe.db.exists("Quotation Lost Reason", name): + frappe.get_doc({"doctype": "Quotation Lost Reason", "order_lost_reason": name}).insert() + self.addCleanup(self._delete_lost_reason, name) + return name + + @staticmethod + def _delete_lost_reason(name): + if frappe.db.exists("Quotation Lost Reason", name): + frappe.delete_doc("Quotation Lost Reason", name, force=1) + + def _make_lost_quotation(self, reason): + qo = make_quotation(company=self.company, qty=1, rate=100) + qo.declare_enquiry_lost([{"lost_reason": reason}], []) + return qo + + @staticmethod + def _cancel_and_delete(name): + if not frappe.db.exists("Quotation", name): + return + doc = frappe.get_doc("Quotation", name) + if doc.docstatus == 1: + doc.cancel() + frappe.delete_doc("Quotation", name, force=1)