Merge pull request #47946 from priyanshshah2442/fix_discount_mismatch
This commit is contained in:
@@ -2813,6 +2813,17 @@ class TestPurchaseInvoice(IntegrationTestCase, StockTestMixin):
|
||||
# Test 4 - Since this PI is overbilled by 130% and only 120% is allowed, it will fail
|
||||
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):
|
||||
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.
|
||||
@@ -260,6 +260,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
|
||||
|
||||
@@ -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