Merge pull request #37586 from ruthra-kumar/overallocation_on_po_to_multiple_invoices
fix: overallocation on purchase order to multiple invoices
diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py
index 1d843ab..71bc498 100644
--- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py
+++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py
@@ -14,6 +14,7 @@
from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice
from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
from erpnext.accounts.party import get_party_account
+from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
from erpnext.stock.doctype.item.test_item import create_item
test_dependencies = ["Item"]
@@ -85,26 +86,44 @@
self.customer5 = make_customer("_Test PR Customer 5", "EUR")
def create_account(self):
- account_name = "Debtors EUR"
- if not frappe.db.get_value(
- "Account", filters={"account_name": account_name, "company": self.company}
- ):
- acc = frappe.new_doc("Account")
- acc.account_name = account_name
- acc.parent_account = "Accounts Receivable - _PR"
- acc.company = self.company
- acc.account_currency = "EUR"
- acc.account_type = "Receivable"
- acc.insert()
- else:
- name = frappe.db.get_value(
- "Account",
- filters={"account_name": account_name, "company": self.company},
- fieldname="name",
- pluck=True,
- )
- acc = frappe.get_doc("Account", name)
- self.debtors_eur = acc.name
+ accounts = [
+ {
+ "attribute": "debtors_eur",
+ "account_name": "Debtors EUR",
+ "parent_account": "Accounts Receivable - _PR",
+ "account_currency": "EUR",
+ "account_type": "Receivable",
+ },
+ {
+ "attribute": "creditors_usd",
+ "account_name": "Payable USD",
+ "parent_account": "Accounts Payable - _PR",
+ "account_currency": "USD",
+ "account_type": "Payable",
+ },
+ ]
+
+ for x in accounts:
+ x = frappe._dict(x)
+ if not frappe.db.get_value(
+ "Account", filters={"account_name": x.account_name, "company": self.company}
+ ):
+ acc = frappe.new_doc("Account")
+ acc.account_name = x.account_name
+ acc.parent_account = x.parent_account
+ acc.company = self.company
+ acc.account_currency = x.account_currency
+ acc.account_type = x.account_type
+ acc.insert()
+ else:
+ name = frappe.db.get_value(
+ "Account",
+ filters={"account_name": x.account_name, "company": self.company},
+ fieldname="name",
+ pluck=True,
+ )
+ acc = frappe.get_doc("Account", name)
+ setattr(self, x.attribute, acc.name)
def create_sales_invoice(
self, qty=1, rate=100, posting_date=nowdate(), do_not_save=False, do_not_submit=False
@@ -151,6 +170,64 @@
payment.posting_date = posting_date
return payment
+ def create_purchase_invoice(
+ self, qty=1, rate=100, posting_date=nowdate(), do_not_save=False, do_not_submit=False
+ ):
+ """
+ Helper function to populate default values in sales invoice
+ """
+ pinv = make_purchase_invoice(
+ qty=qty,
+ rate=rate,
+ company=self.company,
+ customer=self.supplier,
+ item_code=self.item,
+ item_name=self.item,
+ cost_center=self.cost_center,
+ warehouse=self.warehouse,
+ debit_to=self.debit_to,
+ parent_cost_center=self.cost_center,
+ update_stock=0,
+ currency="INR",
+ is_pos=0,
+ is_return=0,
+ return_against=None,
+ income_account=self.income_account,
+ expense_account=self.expense_account,
+ do_not_save=do_not_save,
+ do_not_submit=do_not_submit,
+ )
+ return pinv
+
+ def create_purchase_order(
+ self, qty=1, rate=100, posting_date=nowdate(), do_not_save=False, do_not_submit=False
+ ):
+ """
+ Helper function to populate default values in sales invoice
+ """
+ pord = create_purchase_order(
+ qty=qty,
+ rate=rate,
+ company=self.company,
+ customer=self.supplier,
+ item_code=self.item,
+ item_name=self.item,
+ cost_center=self.cost_center,
+ warehouse=self.warehouse,
+ debit_to=self.debit_to,
+ parent_cost_center=self.cost_center,
+ update_stock=0,
+ currency="INR",
+ is_pos=0,
+ is_return=0,
+ return_against=None,
+ income_account=self.income_account,
+ expense_account=self.expense_account,
+ do_not_save=do_not_save,
+ do_not_submit=do_not_submit,
+ )
+ return pord
+
def clear_old_entries(self):
doctype_list = [
"GL Entry",
@@ -163,13 +240,11 @@
for doctype in doctype_list:
qb.from_(qb.DocType(doctype)).delete().where(qb.DocType(doctype).company == self.company).run()
- def create_payment_reconciliation(self):
+ def create_payment_reconciliation(self, party_is_customer=True):
pr = frappe.new_doc("Payment Reconciliation")
pr.company = self.company
- pr.party_type = (
- self.party_type if hasattr(self, "party_type") and self.party_type else "Customer"
- )
- pr.party = self.customer
+ pr.party_type = "Customer" if party_is_customer else "Supplier"
+ pr.party = self.customer if party_is_customer else self.supplier
pr.receivable_payable_account = get_party_account(pr.party_type, pr.party, pr.company)
pr.from_invoice_date = pr.to_invoice_date = pr.from_payment_date = pr.to_payment_date = nowdate()
return pr
@@ -906,9 +981,13 @@
self.assertEqual(pr.allocation[0].difference_amount, 0)
def test_reconciliation_purchase_invoice_against_return(self):
- pi = make_purchase_invoice(
- supplier="_Test Supplier USD", currency="USD", conversion_rate=50
- ).submit()
+ self.supplier = "_Test Supplier USD"
+ pi = self.create_purchase_invoice(qty=5, rate=50, do_not_submit=True)
+ pi.supplier = self.supplier
+ pi.currency = "USD"
+ pi.conversion_rate = 50
+ pi.credit_to = self.creditors_usd
+ pi.save().submit()
pi_return = frappe.get_doc(pi.as_dict())
pi_return.name = None
@@ -918,11 +997,12 @@
pi_return.items[0].qty = -pi_return.items[0].qty
pi_return.submit()
- self.company = "_Test Company"
- self.party_type = "Supplier"
- self.customer = "_Test Supplier USD"
-
- pr = self.create_payment_reconciliation()
+ pr = frappe.get_doc("Payment Reconciliation")
+ pr.company = self.company
+ pr.party_type = "Supplier"
+ pr.party = self.supplier
+ pr.receivable_payable_account = self.creditors_usd
+ pr.from_invoice_date = pr.to_invoice_date = pr.from_payment_date = pr.to_payment_date = nowdate()
pr.get_unreconciled_entries()
invoices = []
@@ -931,6 +1011,7 @@
if invoice.invoice_number == pi.name:
invoices.append(invoice.as_dict())
break
+
for payment in pr.payments:
if payment.reference_name == pi_return.name:
payments.append(payment.as_dict())
@@ -941,6 +1022,121 @@
# Should not raise frappe.exceptions.ValidationError: Total Debit must be equal to Total Credit.
pr.reconcile()
+ def test_reconciliation_from_purchase_order_to_multiple_invoices(self):
+ """
+ Reconciling advance payment from PO/SO to multiple invoices should not cause overallocation
+ """
+
+ self.supplier = "_Test Supplier"
+
+ pi1 = self.create_purchase_invoice(qty=10, rate=100)
+ pi2 = self.create_purchase_invoice(qty=10, rate=100)
+ po = self.create_purchase_order(qty=20, rate=100)
+ pay = get_payment_entry(po.doctype, po.name)
+ # Overpay Puchase Order
+ pay.paid_amount = 3000
+ pay.save().submit()
+ # assert total allocated and unallocated before reconciliation
+ self.assertEqual(
+ (
+ pay.references[0].reference_doctype,
+ pay.references[0].reference_name,
+ pay.references[0].allocated_amount,
+ ),
+ (po.doctype, po.name, 2000),
+ )
+ self.assertEqual(pay.total_allocated_amount, 2000)
+ self.assertEqual(pay.unallocated_amount, 1000)
+ self.assertEqual(pay.difference_amount, 0)
+
+ pr = self.create_payment_reconciliation(party_is_customer=False)
+ pr.get_unreconciled_entries()
+
+ self.assertEqual(len(pr.invoices), 2)
+ self.assertEqual(len(pr.payments), 2)
+
+ for x in pr.payments:
+ self.assertEqual((x.reference_type, x.reference_name), (pay.doctype, pay.name))
+
+ invoices = [x.as_dict() for x in pr.invoices]
+ payments = [x.as_dict() for x in pr.payments]
+ pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments}))
+ # partial allocation on pi1 and full allocate on pi2
+ pr.allocation[0].allocated_amount = 100
+ pr.reconcile()
+
+ # assert references and total allocated and unallocated amount
+ pay.reload()
+ self.assertEqual(len(pay.references), 3)
+ self.assertEqual(
+ (
+ pay.references[0].reference_doctype,
+ pay.references[0].reference_name,
+ pay.references[0].allocated_amount,
+ ),
+ (po.doctype, po.name, 900),
+ )
+ self.assertEqual(
+ (
+ pay.references[1].reference_doctype,
+ pay.references[1].reference_name,
+ pay.references[1].allocated_amount,
+ ),
+ (pi1.doctype, pi1.name, 100),
+ )
+ self.assertEqual(
+ (
+ pay.references[2].reference_doctype,
+ pay.references[2].reference_name,
+ pay.references[2].allocated_amount,
+ ),
+ (pi2.doctype, pi2.name, 1000),
+ )
+ self.assertEqual(pay.total_allocated_amount, 2000)
+ self.assertEqual(pay.unallocated_amount, 1000)
+ self.assertEqual(pay.difference_amount, 0)
+
+ pr.get_unreconciled_entries()
+ self.assertEqual(len(pr.invoices), 1)
+ self.assertEqual(len(pr.payments), 2)
+
+ invoices = [x.as_dict() for x in pr.invoices]
+ payments = [x.as_dict() for x in pr.payments]
+ pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments}))
+ pr.reconcile()
+
+ # assert references and total allocated and unallocated amount
+ pay.reload()
+ self.assertEqual(len(pay.references), 3)
+ # PO references should be removed now
+ self.assertEqual(
+ (
+ pay.references[0].reference_doctype,
+ pay.references[0].reference_name,
+ pay.references[0].allocated_amount,
+ ),
+ (pi1.doctype, pi1.name, 100),
+ )
+ self.assertEqual(
+ (
+ pay.references[1].reference_doctype,
+ pay.references[1].reference_name,
+ pay.references[1].allocated_amount,
+ ),
+ (pi2.doctype, pi2.name, 1000),
+ )
+ self.assertEqual(
+ (
+ pay.references[2].reference_doctype,
+ pay.references[2].reference_name,
+ pay.references[2].allocated_amount,
+ ),
+ (pi1.doctype, pi1.name, 900),
+ )
+ self.assertEqual(pay.total_allocated_amount, 2000)
+ self.assertEqual(pay.unallocated_amount, 1000)
+ self.assertEqual(pay.difference_amount, 0)
+
def make_customer(customer_name, currency=None):
if not frappe.db.exists("Customer", customer_name):
diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py
index f2691fb..1c7052f 100644
--- a/erpnext/accounts/utils.py
+++ b/erpnext/accounts/utils.py
@@ -645,7 +645,7 @@
"outstanding_amount": d.outstanding_amount,
"allocated_amount": d.allocated_amount,
"exchange_rate": d.exchange_rate if d.exchange_gain_loss else payment_entry.get_exchange_rate(),
- "exchange_gain_loss": d.exchange_gain_loss, # only populated from invoice in case of advance allocation
+ "exchange_gain_loss": d.exchange_gain_loss,
"account": d.account,
}
@@ -658,22 +658,21 @@
existing_row.reference_doctype, existing_row.reference_name
).set_total_advance_paid()
- original_row = existing_row.as_dict().copy()
- existing_row.update(reference_details)
+ if d.allocated_amount <= existing_row.allocated_amount:
+ existing_row.allocated_amount -= d.allocated_amount
- if d.allocated_amount < original_row.allocated_amount:
new_row = payment_entry.append("references")
new_row.docstatus = 1
for field in list(reference_details):
- new_row.set(field, original_row[field])
+ new_row.set(field, reference_details[field])
- new_row.allocated_amount = original_row.allocated_amount - d.allocated_amount
else:
new_row = payment_entry.append("references")
new_row.docstatus = 1
new_row.update(reference_details)
payment_entry.flags.ignore_validate_update_after_submit = True
+ payment_entry.clear_unallocated_reference_document_rows()
payment_entry.setup_party_account_field()
payment_entry.set_missing_values()
if not skip_ref_details_update_for_pe:
diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py
index 83689a2..d8b5878 100644
--- a/erpnext/selling/doctype/sales_order/test_sales_order.py
+++ b/erpnext/selling/doctype/sales_order/test_sales_order.py
@@ -1784,10 +1784,10 @@
si.submit()
pe.load_from_db()
- self.assertEqual(pe.references[0].reference_name, si.name)
- self.assertEqual(pe.references[0].allocated_amount, 200)
- self.assertEqual(pe.references[1].reference_name, so.name)
- self.assertEqual(pe.references[1].allocated_amount, 300)
+ self.assertEqual(pe.references[0].reference_name, so.name)
+ self.assertEqual(pe.references[0].allocated_amount, 300)
+ self.assertEqual(pe.references[1].reference_name, si.name)
+ self.assertEqual(pe.references[1].allocated_amount, 200)
def test_delivered_item_material_request(self):
"SO -> MR (Manufacture) -> WO. Test if WO Qty is updated in SO."