From 337707b9cc7e9b13fc1557588beb6f387c6905f3 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 19 Oct 2023 11:32:21 +0530 Subject: [PATCH 1/4] fix: overallocation on Payment with PO/SO (cherry picked from commit 23df4205f8abfca6764d84665cb9877703c1a470) --- erpnext/accounts/utils.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 4abd04b6c218..3da22cea7fc6 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -648,7 +648,7 @@ def update_reference_in_payment_entry( "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, } @@ -661,22 +661,21 @@ def update_reference_in_payment_entry( 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: From 9600a2cdb7c79f90509cc640de999cd5c34d03a2 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 19 Oct 2023 14:03:21 +0530 Subject: [PATCH 2/4] test: overalloction on reconciliation when PO is involved (cherry picked from commit 946228d783cb58cd2809f4b0bc0a854c49cc4060) --- .../test_payment_reconciliation.py | 183 +++++++++++++++++- 1 file changed, 178 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py index 1d843abde1d9..48d1cf2cc266 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"] @@ -151,6 +152,64 @@ def create_payment_entry(self, amount=100, posting_date=nowdate(), customer=None 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 +222,11 @@ def clear_old_entries(self): 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 @@ -931,6 +988,7 @@ def test_reconciliation_purchase_invoice_against_return(self): 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 +999,121 @@ def test_reconciliation_purchase_invoice_against_return(self): # 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): From 034375b4f56b8af05ca9e153b8539695e2065439 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 19 Oct 2023 14:57:05 +0530 Subject: [PATCH 3/4] refactor(test): make use of utility methods (cherry picked from commit 547993f80103fa192563a82447c39fe122918767) --- .../test_payment_reconciliation.py | 79 ++++++++++++------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py index 48d1cf2cc266..71bc498b494f 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -86,26 +86,44 @@ def create_customer(self): 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 @@ -963,9 +981,13 @@ def test_no_difference_amount_for_base_currency_accounts(self): 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 @@ -975,11 +997,12 @@ def test_reconciliation_purchase_invoice_against_return(self): 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 = [] From 2bec89a5bf339e8cd1db189cc27b556842e2def0 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 24 Oct 2023 08:09:22 +0530 Subject: [PATCH 4/4] chore: fix flakiness `test_sales_order_partial_advance_payment` (cherry picked from commit 4dff2c7a0dad1de840a2b1f53d51e9fe1682fa7f) --- erpnext/selling/doctype/sales_order/test_sales_order.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 83689a2b0bab..d8b5878aa300 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 @@ def test_sales_order_partial_advance_payment(self): 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."