mirror of
https://github.com/frappe/erpnext.git
synced 2026-04-13 03:45:08 +00:00
Merge pull request #50471 from frappe/mergify/bp/version-15-hotfix/pr-50155
fix: ensure that additional discount amount is not mapped repeatedly (backport #50155)
This commit is contained in:
@@ -39,6 +39,8 @@ from erpnext.accounts.doctype.pricing_rule.utils import (
|
||||
)
|
||||
from erpnext.accounts.general_ledger import get_round_off_account_and_cost_center
|
||||
from erpnext.accounts.party import (
|
||||
PURCHASE_TRANSACTION_TYPES,
|
||||
SALES_TRANSACTION_TYPES,
|
||||
get_party_account,
|
||||
get_party_account_currency,
|
||||
get_party_gle_currency,
|
||||
@@ -2918,6 +2920,104 @@ class AccountsController(TransactionBase):
|
||||
x["transaction_currency"] = self.currency
|
||||
x["transaction_exchange_rate"] = self.get("conversion_rate") or 1
|
||||
|
||||
def after_mapping(self, source_doc):
|
||||
self.set_discount_amount_after_mapping(source_doc)
|
||||
|
||||
def set_discount_amount_after_mapping(self, source_doc):
|
||||
"""
|
||||
Ensures that Additional Discount Amount is not copied repeatedly
|
||||
for multiple mappings of a single source transaction.
|
||||
"""
|
||||
|
||||
# source and target doctypes should both be buying / selling
|
||||
for transaction_types in (PURCHASE_TRANSACTION_TYPES, SALES_TRANSACTION_TYPES):
|
||||
if self.doctype in transaction_types and source_doc.doctype in transaction_types:
|
||||
break
|
||||
|
||||
else:
|
||||
return
|
||||
|
||||
# ensure both doctypes have discount_amount field
|
||||
if not self.meta.get_field("discount_amount") or not source_doc.meta.get_field("discount_amount"):
|
||||
return
|
||||
|
||||
# ensure discount_amount is set in source doc
|
||||
if not source_doc.discount_amount:
|
||||
return
|
||||
|
||||
# ensure additional_discount_percentage is not set in the source doc
|
||||
if source_doc.get("additional_discount_percentage"):
|
||||
return
|
||||
|
||||
item_doctype = self.meta.get_field("items").options
|
||||
doctype_table = frappe.qb.DocType(self.doctype)
|
||||
item_table = frappe.qb.DocType(item_doctype)
|
||||
|
||||
is_same_doctype = self.doctype == source_doc.doctype
|
||||
is_return = self.get("is_return") and is_same_doctype
|
||||
|
||||
if is_same_doctype and not is_return:
|
||||
# should never happen
|
||||
# you don't map to the same doctype without it being a return
|
||||
return
|
||||
|
||||
query = (
|
||||
frappe.qb.from_(doctype_table)
|
||||
.where(doctype_table.docstatus == 1)
|
||||
.where(doctype_table.discount_amount != 0)
|
||||
.select(Sum(doctype_table.discount_amount))
|
||||
)
|
||||
|
||||
if is_return:
|
||||
query = query.where(doctype_table.is_return == 1).where(
|
||||
doctype_table.return_against == source_doc.name
|
||||
)
|
||||
|
||||
else:
|
||||
item_meta = frappe.get_meta(item_doctype)
|
||||
reference_fieldname = next(
|
||||
(
|
||||
row.fieldname
|
||||
for row in item_meta.fields
|
||||
if row.fieldtype == "Link"
|
||||
and row.options == source_doc.doctype
|
||||
and not row.get("is_custom_field")
|
||||
),
|
||||
None,
|
||||
)
|
||||
|
||||
if not reference_fieldname:
|
||||
return
|
||||
|
||||
query = query.where(
|
||||
doctype_table.name.isin(
|
||||
frappe.qb.from_(item_table)
|
||||
.select(item_table.parent)
|
||||
.where(item_table[reference_fieldname] == source_doc.name)
|
||||
.distinct()
|
||||
)
|
||||
)
|
||||
|
||||
result = query.run()
|
||||
if not result:
|
||||
return
|
||||
|
||||
discount_already_applied = result[0][0]
|
||||
if not discount_already_applied:
|
||||
return
|
||||
|
||||
if is_return:
|
||||
# returns have negative discount
|
||||
discount_already_applied *= -1
|
||||
|
||||
discount_amount = max(source_doc.discount_amount - discount_already_applied, 0)
|
||||
if discount_amount and is_return:
|
||||
discount_amount *= -1
|
||||
|
||||
self.discount_amount = flt(discount_amount, self.precision("discount_amount"))
|
||||
|
||||
self.calculate_taxes_and_totals()
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
def get_tax_rate(account_head):
|
||||
|
||||
@@ -682,6 +682,25 @@ class calculate_taxes_and_totals:
|
||||
self.doc.precision("discount_amount"),
|
||||
)
|
||||
|
||||
discount_amount = self.doc.discount_amount or 0
|
||||
grand_total = self.doc.grand_total
|
||||
|
||||
# validate that discount amount cannot exceed the total before discount
|
||||
if (
|
||||
(grand_total >= 0 and discount_amount > grand_total)
|
||||
or (grand_total < 0 and discount_amount < grand_total) # returns
|
||||
):
|
||||
frappe.throw(
|
||||
_(
|
||||
"Additional Discount Amount ({discount_amount}) cannot exceed "
|
||||
"the total before such discount ({total_before_discount})"
|
||||
).format(
|
||||
discount_amount=self.doc.get_formatted("discount_amount"),
|
||||
total_before_discount=self.doc.get_formatted("grand_total"),
|
||||
),
|
||||
title=_("Invalid Discount Amount"),
|
||||
)
|
||||
|
||||
def apply_discount_amount(self):
|
||||
if self.doc.discount_amount:
|
||||
if not self.doc.apply_discount_on:
|
||||
|
||||
@@ -2258,3 +2258,173 @@ class TestAccountsController(FrappeTestCase):
|
||||
self.assertRaises(frappe.ValidationError, si.save)
|
||||
si.contact_person = customer_contact.name
|
||||
si.save()
|
||||
|
||||
def test_discount_amount_not_mapped_repeatedly_for_sales_transactions(self):
|
||||
"""
|
||||
Test that additional discount amount is not copied repeatedly
|
||||
when creating multiple delivery notes from a single sales order with discount_amount set
|
||||
"""
|
||||
from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note
|
||||
from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
|
||||
|
||||
# Create a sales order with discount amount
|
||||
so = make_sales_order(qty=10, rate=100, do_not_submit=True)
|
||||
so.apply_discount_on = "Net Total"
|
||||
so.discount_amount = 100
|
||||
so.save()
|
||||
so.submit()
|
||||
|
||||
# Create first delivery note from sales order (partial qty)
|
||||
dn1 = make_delivery_note(so.name)
|
||||
dn1.items[0].qty = 5
|
||||
dn1.save()
|
||||
dn1.submit()
|
||||
|
||||
# First delivery note should have full discount amount
|
||||
self.assertEqual(dn1.discount_amount, 100)
|
||||
self.assertEqual(dn1.grand_total, 400)
|
||||
|
||||
# Create second delivery note from the same sales order (remaining qty)
|
||||
dn2 = make_delivery_note(so.name)
|
||||
dn2.items[0].qty = 5
|
||||
dn2.save()
|
||||
dn2.submit()
|
||||
|
||||
# Second delivery note should have discount_amount set to 0
|
||||
# because discount was already fully applied in first delivery note
|
||||
self.assertEqual(dn2.discount_amount, 0)
|
||||
self.assertEqual(dn2.grand_total, 500)
|
||||
|
||||
def test_discount_amount_not_mapped_repeatedly_for_purchase_transactions(self):
|
||||
"""
|
||||
Test that additional discount amount is not copied repeatedly
|
||||
when creating multiple purchase receipts from a single purchase order with discount_amount set
|
||||
"""
|
||||
from erpnext.buying.doctype.purchase_order.purchase_order import make_purchase_receipt
|
||||
from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
|
||||
|
||||
# Create a purchase order with discount amount
|
||||
po = create_purchase_order(qty=10, rate=100, do_not_submit=True)
|
||||
po.apply_discount_on = "Net Total"
|
||||
po.discount_amount = 100
|
||||
po.save()
|
||||
po.submit()
|
||||
|
||||
# Create first purchase receipt from purchase order (partial qty)
|
||||
pr1 = make_purchase_receipt(po.name)
|
||||
pr1.items[0].qty = 5
|
||||
pr1.save()
|
||||
pr1.submit()
|
||||
|
||||
# First purchase receipt should have full discount amount
|
||||
self.assertEqual(pr1.discount_amount, 100)
|
||||
self.assertEqual(pr1.grand_total, 400)
|
||||
|
||||
# Create second purchase receipt from the same purchase order (remaining qty)
|
||||
pr2 = make_purchase_receipt(po.name)
|
||||
pr2.items[0].qty = 5
|
||||
pr2.save()
|
||||
pr2.submit()
|
||||
|
||||
# Second purchase receipt should have discount_amount set to 0
|
||||
# because discount was already fully applied in first purchase receipt
|
||||
self.assertEqual(pr2.discount_amount, 0)
|
||||
self.assertEqual(pr2.grand_total, 500)
|
||||
|
||||
def test_discount_amount_partial_application_in_mapped_transactions(self):
|
||||
"""
|
||||
Test that discount amount is partially applied when some discount
|
||||
has already been used in previous mapped transactions
|
||||
"""
|
||||
from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice
|
||||
from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
|
||||
|
||||
# Create a sales order with discount amount
|
||||
so = make_sales_order(qty=10, rate=100, do_not_submit=True)
|
||||
so.apply_discount_on = "Net Total"
|
||||
so.discount_amount = 200
|
||||
so.save()
|
||||
so.submit()
|
||||
|
||||
self.assertEqual(so.discount_amount, 200)
|
||||
self.assertEqual(so.grand_total, 800)
|
||||
|
||||
# Create first invoice with partial discount (manually set lower discount)
|
||||
si1 = make_sales_invoice(so.name)
|
||||
si1.items[0].qty = 5
|
||||
si1.discount_amount = 50 # Partial discount application
|
||||
si1.save()
|
||||
si1.submit()
|
||||
|
||||
self.assertEqual(si1.discount_amount, 50)
|
||||
self.assertEqual(si1.grand_total, 450)
|
||||
|
||||
# Create second invoice from the same sales order
|
||||
si2 = make_sales_invoice(so.name)
|
||||
si2.items[0].qty = 5
|
||||
si2.save()
|
||||
si2.submit()
|
||||
|
||||
# Second invoice should have remaining discount (200 - 50 = 150)
|
||||
self.assertEqual(si2.discount_amount, 150)
|
||||
self.assertEqual(si2.grand_total, 350)
|
||||
|
||||
def test_discount_amount_not_mapped_when_percentage_is_set(self):
|
||||
"""
|
||||
Test that discount amount is not adjusted when additional_discount_percentage
|
||||
is set in the source document (as it will be recalculated based on percentage)
|
||||
"""
|
||||
from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note
|
||||
from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
|
||||
|
||||
# Create a sales order with discount percentage instead of amount
|
||||
so = make_sales_order(qty=10, rate=100, do_not_submit=True)
|
||||
so.apply_discount_on = "Net Total"
|
||||
so.additional_discount_percentage = 10 # 10% discount
|
||||
so.save()
|
||||
so.submit()
|
||||
|
||||
self.assertEqual(so.discount_amount, 100) # 10% of 1000
|
||||
self.assertEqual(so.grand_total, 900)
|
||||
|
||||
# Create delivery note from sales order
|
||||
dn = make_delivery_note(so.name)
|
||||
dn.items[0].qty = 5
|
||||
dn.save()
|
||||
|
||||
# Delivery note should have discount amount recalculated based on percentage
|
||||
# and not affected by the repeated mapping logic
|
||||
self.assertEqual(dn.additional_discount_percentage, 10)
|
||||
self.assertEqual(dn.discount_amount, 50) # 10% of 500
|
||||
|
||||
def test_discount_amount_for_multiple_returns(self):
|
||||
"""
|
||||
Test that discount amount is correctly adjusted when multiple return invoices
|
||||
are created against the same original invoice to prevent over-returning discount
|
||||
"""
|
||||
from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_sales_return
|
||||
|
||||
# Create original sales invoice with discount
|
||||
si = create_sales_invoice(qty=10, rate=100, do_not_submit=True)
|
||||
si.apply_discount_on = "Net Total"
|
||||
si.discount_amount = 100
|
||||
si.save()
|
||||
si.submit()
|
||||
|
||||
# Create first return - Frappe will copy full discount by default, we need to adjust it
|
||||
return_si_1 = make_sales_return(si.name)
|
||||
return_si_1.items[0].qty = -6 # Return 6 out of 10 items
|
||||
# Manually set discount to match the proportion (60% of discount)
|
||||
return_si_1.discount_amount = -60
|
||||
return_si_1.save()
|
||||
return_si_1.submit()
|
||||
|
||||
self.assertEqual(return_si_1.discount_amount, -60)
|
||||
|
||||
# Create second return for remaining items
|
||||
return_si_2 = make_sales_return(si.name)
|
||||
return_si_2.items[0].qty = -4 # Return remaining 4 out of 10 items
|
||||
return_si_2.save()
|
||||
|
||||
# Second return should only get remaining discount (100 - 60 = 40)
|
||||
self.assertEqual(return_si_2.discount_amount, -40)
|
||||
|
||||
Reference in New Issue
Block a user