mirror of
https://github.com/frappe/erpnext.git
synced 2026-05-26 00:14:50 +00:00
Merge pull request #47985 from frappe/mergify/bp/version-15-hotfix/pr-47946
fix: patch and report for incorrect discount values (backport #47946)
This commit is contained in:
@@ -2806,6 +2806,17 @@ class TestPurchaseInvoice(FrappeTestCase, StockTestMixin):
|
|||||||
# Test 4 - Since this PI is overbilled by 130% and only 120% is allowed, it will fail
|
# Test 4 - Since this PI is overbilled by 130% and only 120% is allowed, it will fail
|
||||||
self.assertRaises(frappe.ValidationError, pi.submit)
|
self.assertRaises(frappe.ValidationError, pi.submit)
|
||||||
|
|
||||||
|
def test_discount_percentage_not_set_when_amount_is_manually_set(self):
|
||||||
|
pi = make_purchase_invoice(do_not_save=True)
|
||||||
|
discount_amount = 7
|
||||||
|
pi.discount_amount = discount_amount
|
||||||
|
pi.save()
|
||||||
|
self.assertEqual(pi.additional_discount_percentage, None)
|
||||||
|
pi.set_posting_time = 1
|
||||||
|
pi.posting_date = add_days(today(), -1)
|
||||||
|
pi.save()
|
||||||
|
self.assertEqual(pi.discount_amount, discount_amount)
|
||||||
|
|
||||||
|
|
||||||
def set_advance_flag(company, flag, default_account):
|
def set_advance_flag(company, flag, default_account):
|
||||||
frappe.db.set_value(
|
frappe.db.set_value(
|
||||||
|
|||||||
@@ -0,0 +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,
|
||||||
|
// },
|
||||||
|
// ],
|
||||||
|
// };
|
||||||
@@ -0,0 +1,38 @@
|
|||||||
|
{
|
||||||
|
"add_total_row": 0,
|
||||||
|
"add_translate_data": 0,
|
||||||
|
"columns": [],
|
||||||
|
"creation": "2025-06-06 17:09:50.681090",
|
||||||
|
"disabled": 0,
|
||||||
|
"docstatus": 0,
|
||||||
|
"doctype": "Report",
|
||||||
|
"filters": [],
|
||||||
|
"idx": 0,
|
||||||
|
"is_standard": "Yes",
|
||||||
|
"letter_head": "",
|
||||||
|
"letterhead": null,
|
||||||
|
"modified": "2025-06-06 18:09:18.221911",
|
||||||
|
"modified_by": "Administrator",
|
||||||
|
"module": "Accounts",
|
||||||
|
"name": "Calculated Discount Mismatch",
|
||||||
|
"owner": "Administrator",
|
||||||
|
"prepared_report": 0,
|
||||||
|
"ref_doctype": "Version",
|
||||||
|
"report_name": "Calculated Discount Mismatch",
|
||||||
|
"report_type": "Script Report",
|
||||||
|
"roles": [
|
||||||
|
{
|
||||||
|
"role": "System Manager"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"role": "Administrator"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"role": "Accounts Manager"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"role": "Accounts User"
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"timeout": 0
|
||||||
|
}
|
||||||
@@ -0,0 +1,174 @@
|
|||||||
|
# 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
|
||||||
|
from frappe.utils.formatters import format_value
|
||||||
|
|
||||||
|
AFFECTED_DOCTYPES = frozenset(
|
||||||
|
(
|
||||||
|
"POS Invoice",
|
||||||
|
"Purchase Invoice",
|
||||||
|
"Sales Invoice",
|
||||||
|
"Purchase Order",
|
||||||
|
"Supplier Quotation",
|
||||||
|
"Quotation",
|
||||||
|
"Sales Order",
|
||||||
|
"Delivery Note",
|
||||||
|
"Purchase Receipt",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
LAST_MODIFIED_DATE_THRESHOLD = "2025-05-30"
|
||||||
|
|
||||||
|
|
||||||
|
def execute(filters=None):
|
||||||
|
columns = get_columns()
|
||||||
|
data = get_data()
|
||||||
|
|
||||||
|
return columns, data
|
||||||
|
|
||||||
|
|
||||||
|
def get_columns():
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
"fieldname": "doctype",
|
||||||
|
"label": _("Transaction Type"),
|
||||||
|
"fieldtype": "Link",
|
||||||
|
"options": "DocType",
|
||||||
|
"width": 120,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"fieldname": "docname",
|
||||||
|
"label": _("Transaction Name"),
|
||||||
|
"fieldtype": "Dynamic Link",
|
||||||
|
"options": "doctype",
|
||||||
|
"width": 150,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"fieldname": "actual_discount_percentage",
|
||||||
|
"label": _("Discount Percentage in Transaction"),
|
||||||
|
"fieldtype": "Percent",
|
||||||
|
"width": 180,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"fieldname": "actual_discount_amount",
|
||||||
|
"label": _("Discount Amount in Transaction"),
|
||||||
|
"fieldtype": "Currency",
|
||||||
|
"width": 180,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"fieldname": "suspected_discount_amount",
|
||||||
|
"label": _("Suspected Discount Amount"),
|
||||||
|
"fieldtype": "Currency",
|
||||||
|
"width": 180,
|
||||||
|
},
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
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 []
|
||||||
|
|
||||||
|
VERSION = frappe.qb.DocType("Version")
|
||||||
|
|
||||||
|
versions = (
|
||||||
|
frappe.qb.from_(VERSION)
|
||||||
|
.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)
|
||||||
|
)
|
||||||
|
|
||||||
|
if not versions:
|
||||||
|
return []
|
||||||
|
|
||||||
|
version_map = {}
|
||||||
|
for version in versions:
|
||||||
|
key = (version.ref_doctype, version.docname)
|
||||||
|
if key not in version_map:
|
||||||
|
version_map[key] = []
|
||||||
|
|
||||||
|
version_map[key].append(version.data)
|
||||||
|
|
||||||
|
data = []
|
||||||
|
discount_amount_field_map = {
|
||||||
|
doctype: frappe.get_meta(doctype).get_field("discount_amount") for doctype in AFFECTED_DOCTYPES
|
||||||
|
}
|
||||||
|
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]
|
||||||
|
doctype = doc[0]
|
||||||
|
doc_values = transactions_with_discount_percentage.get(doc)
|
||||||
|
formatted_discount_amount = format_value(
|
||||||
|
doc_values.discount_amount,
|
||||||
|
df=discount_amount_field_map[doctype],
|
||||||
|
currency=doc_values.currency,
|
||||||
|
)
|
||||||
|
|
||||||
|
if new != formatted_discount_amount:
|
||||||
|
# if the discount amount in the version is not equal to the current value, skip
|
||||||
|
break
|
||||||
|
|
||||||
|
data.append(
|
||||||
|
{
|
||||||
|
"doctype": doctype,
|
||||||
|
"docname": doc_values.name,
|
||||||
|
"actual_discount_percentage": doc_values.additional_discount_percentage,
|
||||||
|
"actual_discount_amount": new,
|
||||||
|
"suspected_discount_amount": old,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
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
|
||||||
11
erpnext/change_log/v15/v15_64_0.md
Normal file
11
erpnext/change_log/v15/v15_64_0.md
Normal file
@@ -0,0 +1,11 @@
|
|||||||
|
There was a bug in the **Additional Discount** functionality of ERPNext in **v15.64.0**. This has since been fixed.
|
||||||
|
|
||||||
|
**If you've updated from a version older than v15.64.0, no action is needed on your side.**
|
||||||
|
|
||||||
|
If you're updating from v15.64.0, the **Additional Discount Amount** in some transactions may differ from the value you entered. This only affects cases where **Additional Discount Amount** is manually entered. If it is computed from **Additional Discount Percentage** entered by you, there shouldn't be any issue.
|
||||||
|
|
||||||
|
This report can help identify such transactions: [Calculated Discount Mismatch](/app/query-report/Calculated%20Discount%20Mismatch)
|
||||||
|
|
||||||
|
Please review and amend these as necessary.
|
||||||
|
|
||||||
|
We apologize for the inconvenience caused.
|
||||||
@@ -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)
|
execute:frappe.rename_doc("Report", "TDS Payable Monthly", "Tax Withholding Details", force=True)
|
||||||
erpnext.patches.v14_0.update_proprietorship_to_individual
|
erpnext.patches.v14_0.update_proprietorship_to_individual
|
||||||
erpnext.patches.v15_0.rename_subcontracting_fields
|
erpnext.patches.v15_0.rename_subcontracting_fields
|
||||||
|
erpnext.patches.v15_0.unset_incorrect_additional_discount_percentage
|
||||||
|
|
||||||
[post_model_sync]
|
[post_model_sync]
|
||||||
erpnext.patches.v15_0.create_asset_depreciation_schedules_from_assets
|
erpnext.patches.v15_0.create_asset_depreciation_schedules_from_assets
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user