Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: overallocation on purchase order to multiple invoices (backport #37586) #37646

Merged
merged 5 commits into from
Oct 24, 2023
Merged

fix: overallocation on purchase order to multiple invoices (backport #37586) #37646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d71b885
fix: overallocation on Payment with PO/SO
ruthra-kumar Oct 19, 2023
c8922ad
test: overalloction on reconciliation when PO is involved
ruthra-kumar Oct 19, 2023
c921d7d
refactor(test): make use of utility methods
ruthra-kumar Oct 19, 2023
7b9daef
chore: fix flakiness `test_sales_order_partial_advance_payment`
ruthra-kumar Oct 24, 2023
d576fc7
chore: resolve conflict
ruthra-kumar Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
262 changes: 229 additions & 33 deletions erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -85,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
Expand Down Expand Up @@ -151,6 +170,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",
Expand All @@ -163,13 +240,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
Expand Down Expand Up @@ -906,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
Expand All @@ -918,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 = []
Expand All @@ -931,6 +1011,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())
Expand All @@ -941,6 +1022,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):
Expand Down
11 changes: 5 additions & 6 deletions erpnext/accounts/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,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,
}

if d.voucher_detail_no:
Expand All @@ -636,22 +636,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:
Expand Down
8 changes: 4 additions & 4 deletions erpnext/selling/doctype/sales_order/test_sales_order.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1893,10 +1893,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."
Expand Down