fix: changes to report and patch

(cherry picked from commit daad6137f8)
2025-06-09 13:52:19 +05:30
parent b3eb49d39d
commit 5237ff8d94
5 changed files with 229 additions and 104 deletions
--- a/erpnext/accounts/report/calculated_discount_mismatch/calculated_discount_mismatch.js
+++ b/erpnext/accounts/report/calculated_discount_mismatch/calculated_discount_mismatch.js
@@ -1,13 +1,13 @@
 // Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and contributors
 // For license information, please see license.txt

-frappe.query_reports["Calculated Discount Mismatch"] = {
-	filters: [
-		// {
-		// 	"fieldname": "my_filter",
-		// 	"label": __("My Filter"),
-		// 	"fieldtype": "Data",
-		// 	"reqd": 1,
-		// },
-	],
-};
+// frappe.query_reports["Calculated Discount Mismatch"] = {
+// 	filters: [
+// 		{
+// 			"fieldname": "my_filter",
+// 			"label": __("My Filter"),
+// 			"fieldtype": "Data",
+// 			"reqd": 1,
+// 		},
+// 	],
+// };
--- a/erpnext/accounts/report/calculated_discount_mismatch/calculated_discount_mismatch.py
+++ b/erpnext/accounts/report/calculated_discount_mismatch/calculated_discount_mismatch.py
@@ -1,10 +1,14 @@
 # Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and contributors
 # For license information, please see license.txt

+import json
+
 import frappe
 from frappe import _
+from frappe.query_builder import Order, Tuple
+from frappe.utils import flt

-DISCOUNT_DOCTYPES = frozenset(
+AFFECTED_DOCTYPES = frozenset(
 	(
 		"POS Invoice",
 		"Purchase Invoice",
@@ -20,67 +24,157 @@ DISCOUNT_DOCTYPES = frozenset(
 LAST_MODIFIED_DATE_THRESHOLD = "2025-05-30"


-def execute(filters: dict | None = None):
-	"""Return columns and data for the report.
-
-	This is the main entry point for the report. It accepts the filters as a
-	dictionary and should return columns and data. It is called by the framework
-	every time the report is refreshed or a filter is updated.
-	"""
+def execute():
 	columns = get_columns()
 	data = get_data()

 	return columns, data


-def get_columns() -> list[dict]:
-	"""Return columns for the report.
-
-	One field definition per column, just like a DocType field definition.
-	"""
+def get_columns():
 	return [
 		{
-			"label": _("Doctype"),
 			"fieldname": "doctype",
-			"fieldtype": "Data",
+			"label": _("Transaction Type"),
+			"fieldtype": "Link",
+			"options": "DocType",
+			"width": 120,
+		},
+		{
+			"fieldname": "docname",
+			"label": _("Transaction Name"),
+			"fieldtype": "Dynamic Link",
+			"options": "doctype",
 			"width": 150,
 		},
 		{
-			"label": _("Document Name"),
-			"fieldname": "document_name",
-			"fieldtype": "Dynamic Link",
-			"options": "doctype",
-			"width": 200,
+			"fieldname": "currency",
+			"label": _("Currency"),
+			"fieldtype": "Link",
+			"options": "Currency",
+		},
+		{
+			"fieldname": "actual_discount_percentage",
+			"label": _("Discount Percentage in Transaction"),
+			"fieldtype": "Percent",
+			"width": 180,
+		},
+		{
+			"fieldname": "actual_discount_amount",
+			"label": _("Discount Amount in Transaction"),
+			"fieldtype": "Currency",
+			"options": "currency",
+			"width": 180,
+		},
+		{
+			"fieldname": "suspected_discount_amount",
+			"label": _("Suspected Discount Amount"),
+			"fieldtype": "Currency",
+			"options": "currency",
+			"width": 180,
+		},
+		{
+			"fieldname": "difference",
+			"label": _("Difference"),
+			"fieldtype": "Currency",
+			"options": "currency",
+			"width": 180,
 		},
 	]


-def get_data() -> list[list]:
-	"""Return data for the report.
+def get_data():
+	transactions_with_discount_percentage = {}
+
+	for doctype in AFFECTED_DOCTYPES:
+		transactions = get_transactions_with_discount_percentage(doctype)
+
+		for transaction in transactions:
+			transactions_with_discount_percentage[(doctype, transaction.name)] = transaction
+
+	if not transactions_with_discount_percentage:
+		return []

-	The report data is a list of rows, with each row being a list of cell values.
-	"""
-	data = []
 	VERSION = frappe.qb.DocType("Version")

-	result = (
+	versions = (
 		frappe.qb.from_(VERSION)
-		.select(VERSION.ref_doctype, VERSION.docname, VERSION.data, VERSION.name)
-		.where(VERSION.modified > LAST_MODIFIED_DATE_THRESHOLD)
-		.where(VERSION.ref_doctype.isin(list(DISCOUNT_DOCTYPES)))
+		.select(VERSION.ref_doctype, VERSION.docname, VERSION.data)
+		.where(VERSION.creation > LAST_MODIFIED_DATE_THRESHOLD)
+		.where(Tuple(VERSION.ref_doctype, VERSION.docname).isin(list(transactions_with_discount_percentage)))
+		.where(
+			VERSION.data.like('%"discount\\_amount"%')
+			| VERSION.data.like('%"additional\\_discount\\_percentage"%')
+		)
+		.orderby(VERSION.creation, order=Order.desc)
 		.run(as_dict=True)
 	)

-	for row in result:
-		changed_data = {entry[0]: entry for entry in frappe.parse_json(row.data).get("changed", [])}
+	if not versions:
+		return []

-		docstatus = changed_data.get("docstatus")
-		if not docstatus or docstatus[2] != 1:
-			continue
+	version_map = {}
+	for version in versions:
+		key = (version.ref_doctype, version.docname)
+		if key not in version_map:
+			version_map[key] = []

-		if "discount_amount" not in changed_data:
-			continue
+		version_map[key].append(version.data)

-		data.append({"doctype": row.ref_doctype, "document_name": row.docname})
+	data = []
+	for doc, versions in version_map.items():
+		for version_data in versions:
+			if '"additional_discount_percentage"' in version_data:
+				# don't consider doc if additional_discount_percentage is changed in newest version
+				break
+
+			version_data = json.loads(version_data)
+			changed_values = version_data.get("changed")
+			if not changed_values:
+				continue
+
+			discount_values = next((row for row in changed_values if row[0] == "discount_amount"), None)
+			if not discount_values:
+				continue
+
+			old = discount_values[1]
+			new = discount_values[2]
+			doc_values = transactions_with_discount_percentage.get(doc)
+			if new != doc_values.discount_amount:
+				# if the discount amount in the version is not equal to the current value, skip
+				break
+
+			data.append(
+				{
+					"doctype": doc[0],
+					"docname": doc[1],
+					"currency": doc_values.currency,
+					"actual_discount_percentage": doc_values.additional_discount_percentage,
+					"actual_discount_amount": new,
+					"suspected_discount_amount": old,
+					"difference": flt(old - new, 9),
+				}
+			)
+			break

 	return data
+
+
+def get_transactions_with_discount_percentage(doctype):
+	transactions = frappe.get_all(
+		doctype,
+		fields=[
+			"name",
+			"currency",
+			"additional_discount_percentage",
+			"discount_amount",
+		],
+		filters={
+			"docstatus": 1,
+			"additional_discount_percentage": [">", 0],
+			"discount_amount": ["!=", 0],
+			"modified": [">", LAST_MODIFIED_DATE_THRESHOLD],
+		},
+	)
+
+	return transactions
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -262,6 +262,7 @@ erpnext.patches.v14_0.clear_reconciliation_values_from_singles
 execute:frappe.rename_doc("Report", "TDS Payable Monthly", "Tax Withholding Details", force=True)
 erpnext.patches.v14_0.update_proprietorship_to_individual
 erpnext.patches.v15_0.rename_subcontracting_fields
+erpnext.patches.v15_0.unset_incorrect_additional_discount_percentage

 [post_model_sync]
 erpnext.patches.v15_0.create_asset_depreciation_schedules_from_assets
@@ -408,4 +409,3 @@ erpnext.patches.v15_0.set_cancelled_status_to_cancelled_pos_invoice
 erpnext.patches.v15_0.rename_group_by_to_categorize_by_in_custom_reports
 erpnext.patches.v14_0.update_full_name_in_contract
 erpnext.patches.v15_0.drop_sle_indexes
-erpnext.patches.v15_0.set_additional_discount_percentage
--- a/erpnext/patches/v15_0/set_additional_discount_percentage.py
+++ b/erpnext/patches/v15_0/set_additional_discount_percentage.py
@@ -1,56 +0,0 @@
-import frappe
-from frappe import scrub
-from frappe.model.meta import get_field_precision
-from frappe.utils import flt
-
-from erpnext.accounts.report.calculated_discount_mismatch.calculated_discount_mismatch import (
-	DISCOUNT_DOCTYPES,
-	LAST_MODIFIED_DATE_THRESHOLD,
-)
-
-
-def execute():
-	for doctype in DISCOUNT_DOCTYPES:
-		documents = frappe.get_all(
-			doctype,
-			{
-				"docstatus": 0,
-				"modified": [">", LAST_MODIFIED_DATE_THRESHOLD],
-				"discount_amount": ["is", "set"],
-			},
-			[
-				"name",
-				"additional_discount_percentage",
-				"discount_amount",
-				"apply_discount_on",
-				"grand_total",
-				"net_total",
-			],
-		)
-
-		if not documents:
-			continue
-
-		precision = get_field_precision(frappe.get_meta(doctype).get_field("additional_discount_percentage"))
-		mismatched_documents = []
-
-		for doc in documents:
-			discount_applied_on = scrub(doc.apply_discount_on)
-
-			calculated_discount_amount = flt(
-				doc.additional_discount_percentage * doc.get(discount_applied_on) / 100,
-				precision,
-			)
-
-			if calculated_discount_amount != doc.discount_amount:
-				mismatched_documents.append(doc.name)
-
-		if mismatched_documents:
-			frappe.db.set_value(
-				doctype,
-				{
-					"name": ["in", mismatched_documents],
-				},
-				"additional_discount_percentage",
-				0,
-			)
--- a/erpnext/patches/v15_0/unset_incorrect_additional_discount_percentage.py
+++ b/erpnext/patches/v15_0/unset_incorrect_additional_discount_percentage.py
@@ -0,0 +1,87 @@
+import frappe
+from frappe import scrub
+from frappe.model.meta import get_field_precision
+from frappe.utils import flt
+from semantic_version import Version
+
+from erpnext.accounts.report.calculated_discount_mismatch.calculated_discount_mismatch import (
+	AFFECTED_DOCTYPES,
+	LAST_MODIFIED_DATE_THRESHOLD,
+)
+
+
+def execute():
+	# run this patch only if erpnext version before update is v15.64.0 or higher
+	version, git_branch = frappe.db.get_value(
+		"Installed Application",
+		{"app_name": "erpnext"},
+		["app_version", "git_branch"],
+	)
+
+	semantic_version = get_semantic_version(version)
+	if semantic_version and (
+		semantic_version.major < 15 or (git_branch == "version-15" and semantic_version.minor < 64)
+	):
+		return
+
+	for doctype in AFFECTED_DOCTYPES:
+		meta = frappe.get_meta(doctype)
+		filters = {
+			"modified": [">", LAST_MODIFIED_DATE_THRESHOLD],
+			"additional_discount_percentage": [">", 0],
+			"discount_amount": ["!=", 0],
+		}
+
+		# can't reverse calculate grand_total if shipping rule is set
+		if meta.has_field("shipping_rule"):
+			filters["shipping_rule"] = ["is", "not set"]
+
+		documents = frappe.get_all(
+			doctype,
+			fields=[
+				"name",
+				"additional_discount_percentage",
+				"discount_amount",
+				"apply_discount_on",
+				"grand_total",
+				"net_total",
+			],
+			filters=filters,
+		)
+
+		if not documents:
+			continue
+
+		precision = get_field_precision(frappe.get_meta(doctype).get_field("additional_discount_percentage"))
+		mismatched_documents = []
+
+		for doc in documents:
+			# we need grand_total before applying discount
+			doc.grand_total += doc.discount_amount
+			discount_applied_on = scrub(doc.apply_discount_on)
+			calculated_discount_amount = flt(
+				doc.additional_discount_percentage * doc.get(discount_applied_on) / 100,
+				precision,
+			)
+
+			# if difference is more than 0.02 (based on precision), unset the additional discount percentage
+			if abs(calculated_discount_amount - doc.discount_amount) > 2 / (10**precision):
+				mismatched_documents.append(doc.name)
+
+		if mismatched_documents:
+			# changing the discount percentage has no accounting effect
+			# so we can safely set it to 0 in the database
+			frappe.db.set_value(
+				doctype,
+				{"name": ["in", mismatched_documents]},
+				"additional_discount_percentage",
+				0,
+				update_modified=False,
+			)
+
+
+def get_semantic_version(version):
+	try:
+		return Version(version)
+	except Exception:
+		pass