From 077f3928cfafd413cbb0e6968dade5604fe3a888 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 1 Nov 2023 20:11:00 +0530 Subject: [PATCH 1/6] refactor: `split_invoices_based_on_payment_terms` - Invoices were in the wrong order due to the logic. The invoices with payment terms were added first and the rest after. - Overly long function with unnecessary loops (reduced to one main loop) and complexity - The split row as per payment terms was not ordered. So the second installment was allocated first (cherry picked from commit 6bd56d2d5f62814eb2f777934ed894bc5709e947) # Conflicts: # erpnext/accounts/doctype/payment_entry/payment_entry.py --- .../doctype/payment_entry/payment_entry.py | 89 +++++++++++++++++-- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 35207eae0e50..70f01f81c086 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1549,13 +1549,42 @@ def get_outstanding_reference_documents(args): return data -def split_invoices_based_on_payment_terms(outstanding_invoices, company): - invoice_ref_based_on_payment_terms = {} +def split_invoices_based_on_payment_terms(outstanding_invoices, company) -> list: + """Split a list of invoices based on their payment terms.""" + exc_rates = get_currency_data(outstanding_invoices, company) + outstanding_invoices_after_split = [] + for entry in outstanding_invoices: + if entry.voucher_type in ["Sales Invoice", "Purchase Invoice"]: + if payment_term_template := frappe.db.get_value( + entry.voucher_type, entry.voucher_no, "payment_terms_template" + ): + split_rows = get_split_invoice_rows(entry, payment_term_template, exc_rates) + if not split_rows: + continue + + frappe.msgprint( + _("Spliting {} {} into {} row(s) as per Payment Terms").format( + split_rows[0]["voucher_type"], split_rows[0]["voucher_no"], len(split_rows) + ), + alert=True, + ) + outstanding_invoices_after_split += split_rows + continue + + # If not an invoice or no payment terms template, add as it is + outstanding_invoices_after_split.append(entry) + + return outstanding_invoices_after_split + + +def get_currency_data(outstanding_invoices: list, company: str = None) -> dict: + """Get currency and conversion data for a list of invoices.""" + exc_rates = frappe._dict() company_currency = ( frappe.db.get_value("Company", company, "default_currency") if company else None ) - exc_rates = frappe._dict() + for doctype in ["Sales Invoice", "Purchase Invoice"]: invoices = [x.voucher_no for x in outstanding_invoices if x.voucher_type == doctype] for x in frappe.db.get_all( @@ -1570,11 +1599,52 @@ def split_invoices_based_on_payment_terms(outstanding_invoices, company): company_currency=company_currency, ) - for idx, d in enumerate(outstanding_invoices): - if d.voucher_type in ["Sales Invoice", "Purchase Invoice"]: - payment_term_template = frappe.db.get_value( - d.voucher_type, d.voucher_no, "payment_terms_template" + return exc_rates + + +def get_split_invoice_rows(invoice: dict, payment_term_template: str, exc_rates: dict) -> list: + """Split invoice based on its payment schedule table.""" + split_rows = [] + allocate_payment_based_on_payment_terms = frappe.db.get_value( + "Payment Terms Template", payment_term_template, "allocate_payment_based_on_payment_terms" + ) + + if not allocate_payment_based_on_payment_terms: + return [invoice] + + payment_schedule = frappe.get_all( + "Payment Schedule", filters={"parent": invoice.voucher_no}, fields=["*"], order_by="due_date" + ) + for payment_term in payment_schedule: + if not payment_term.outstanding > 0.1: + continue + + doc_details = exc_rates.get(payment_term.parent, None) + is_multi_currency_acc = (doc_details.currency != doc_details.company_currency) and ( + doc_details.party_account_currency != doc_details.company_currency + ) + payment_term_outstanding = flt(payment_term.outstanding) + if not is_multi_currency_acc: + payment_term_outstanding = doc_details.conversion_rate * flt(payment_term.outstanding) + + split_rows.append( + frappe._dict( + { + "due_date": invoice.due_date, + "currency": invoice.currency, + "voucher_no": invoice.voucher_no, + "voucher_type": invoice.voucher_type, + "posting_date": invoice.posting_date, + "invoice_amount": flt(invoice.invoice_amount), + "outstanding_amount": payment_term_outstanding + if payment_term_outstanding + else invoice.outstanding_amount, + "payment_term_outstanding": payment_term_outstanding, + "payment_amount": payment_term.payment_amount, + "payment_term": payment_term.payment_term, + } ) +<<<<<<< HEAD if payment_term_template: allocate_payment_based_on_payment_terms = frappe.db.get_value( "Payment Terms Template", payment_term_template, "allocate_payment_based_on_payment_terms" @@ -1635,6 +1705,11 @@ def split_invoices_based_on_payment_terms(outstanding_invoices, company): outstanding_invoices_after_split += outstanding_invoices return outstanding_invoices_after_split +======= + ) + + return split_rows +>>>>>>> 6bd56d2d5f (refactor: `split_invoices_based_on_payment_terms`) def get_orders_to_be_billed( From a7e286752687e7bab98e28be1fca5326937983a6 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 7 Nov 2023 14:44:04 +0100 Subject: [PATCH 2/6] test: `get_outstanding_reference_documents` (triggered via UI) (cherry picked from commit 162c0497d1db69e3af5f142459e3a875f03962af) --- .../payment_entry/test_payment_entry.py | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 2de009f8c438..0115809af9f1 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -6,10 +6,11 @@ import frappe from frappe import qb from frappe.tests.utils import FrappeTestCase, change_settings -from frappe.utils import flt, nowdate +from frappe.utils import add_days, flt, nowdate from erpnext.accounts.doctype.payment_entry.payment_entry import ( InvalidPaymentEntry, + get_outstanding_reference_documents, get_payment_entry, get_reference_details, ) @@ -1219,6 +1220,42 @@ def test_allocation_validation_for_sales_order(self): so.reload() self.assertEqual(so.advance_paid, so.rounded_total) + def test_outstanding_invoices_api(self): + """ + Test if `get_outstanding_reference_documents` fetches invoices in the right order. + """ + customer = create_customer("Max Mustermann", "INR") + create_payment_terms_template() + + si = create_sales_invoice(qty=1, rate=100, customer=customer) + si2 = create_sales_invoice(do_not_save=1, qty=1, rate=100, customer=customer) + si2.payment_terms_template = "Test Receivable Template" + si2.submit() + + args = { + "posting_date": nowdate(), + "company": "_Test Company", + "party_type": "Customer", + "payment_type": "Pay", + "party": customer, + "party_account": "Debtors - _TC", + } + args.update( + { + "get_outstanding_invoices": True, + "from_posting_date": nowdate(), + "to_posting_date": add_days(nowdate(), 7), + } + ) + references = get_outstanding_reference_documents(args) + + self.assertEqual(len(references), 3) + self.assertEqual(references[0].voucher_no, si.name) + self.assertEqual(references[1].voucher_no, si2.name) + self.assertEqual(references[2].voucher_no, si2.name) + self.assertEqual(references[1].payment_term, "Basic Amount Receivable") + self.assertEqual(references[2].payment_term, "Tax Receivable") + def create_payment_entry(**args): payment_entry = frappe.new_doc("Payment Entry") @@ -1279,6 +1316,9 @@ def create_payment_terms_template(): def create_payment_terms_template_with_discount( name=None, discount_type=None, discount=None, template_name=None ): + """ + Create a Payment Terms Template with % or amount discount. + """ create_payment_term(name or "30 Credit Days with 10% Discount") template_name = template_name or "Test Discount Template" From 0813f02ca5a9da156d02f8ece054d93cf415e554 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 9 Nov 2023 12:50:44 +0100 Subject: [PATCH 3/6] fix: Alert message and make sure invoice due dates are different for effective test - Make invoice due dates are different so that the invoice with the earliest due date is allocated first in the test - Translate voucher type, simplify alert message. The invoice could be "split" into 1 row, no. of rows in the message seems unnecessary. (cherry picked from commit 56ac3424d22b68bbc551913133803686431d1694) --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 4 ++-- .../accounts/doctype/payment_entry/test_payment_entry.py | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 70f01f81c086..f7856f11c05c 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1564,8 +1564,8 @@ def split_invoices_based_on_payment_terms(outstanding_invoices, company) -> list continue frappe.msgprint( - _("Spliting {} {} into {} row(s) as per Payment Terms").format( - split_rows[0]["voucher_type"], split_rows[0]["voucher_no"], len(split_rows) + _("Splitting {0} {1} as per Payment Terms").format( + _(entry.voucher_type), frappe.bold(entry.voucher_no) ), alert=True, ) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 0115809af9f1..169a077f6a82 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1227,7 +1227,10 @@ def test_outstanding_invoices_api(self): customer = create_customer("Max Mustermann", "INR") create_payment_terms_template() - si = create_sales_invoice(qty=1, rate=100, customer=customer) + # SI has an earlier due date and SI2 has a later due date + si = create_sales_invoice( + qty=1, rate=100, customer=customer, posting_date=add_days(nowdate(), -4) + ) si2 = create_sales_invoice(do_not_save=1, qty=1, rate=100, customer=customer) si2.payment_terms_template = "Test Receivable Template" si2.submit() @@ -1243,8 +1246,8 @@ def test_outstanding_invoices_api(self): args.update( { "get_outstanding_invoices": True, - "from_posting_date": nowdate(), - "to_posting_date": add_days(nowdate(), 7), + "from_posting_date": add_days(nowdate(), -4), + "to_posting_date": add_days(nowdate(), 2), } ) references = get_outstanding_reference_documents(args) From c840f30a20d348d76b7e4bb218610953f7ed0f7d Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 9 Nov 2023 13:03:52 +0100 Subject: [PATCH 4/6] style: Remove spaces introduced via merge conflict (cherry picked from commit 4b4b176fcf99241bf17067989ebc850b2de24bcb) # Conflicts: # erpnext/accounts/doctype/payment_entry/test_payment_entry.py --- .../payment_entry/test_payment_entry.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 169a077f6a82..78d745cc76d5 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1258,6 +1258,42 @@ def test_outstanding_invoices_api(self): self.assertEqual(references[2].voucher_no, si2.name) self.assertEqual(references[1].payment_term, "Basic Amount Receivable") self.assertEqual(references[2].payment_term, "Tax Receivable") +<<<<<<< HEAD +======= + + def test_receive_payment_from_payable_party_type(self): + pe = create_payment_entry( + party_type="Supplier", + party="_Test Supplier", + payment_type="Receive", + paid_from="Creditors - _TC", + paid_to="_Test Cash - _TC", + save=True, + submit=True, + ) + self.voucher_no = pe.name + self.expected_gle = [ + {"account": "_Test Cash - _TC", "debit": 1000.0, "credit": 0.0}, + {"account": "Creditors - _TC", "debit": 0.0, "credit": 1000.0}, + ] + self.check_gl_entries() + + def check_gl_entries(self): + gle = frappe.qb.DocType("GL Entry") + gl_entries = ( + frappe.qb.from_(gle) + .select( + gle.account, + gle.debit, + gle.credit, + ) + .where((gle.voucher_no == self.voucher_no) & (gle.is_cancelled == 0)) + .orderby(gle.account) + ).run(as_dict=True) + for row in range(len(self.expected_gle)): + for field in ["account", "debit", "credit"]: + self.assertEqual(self.expected_gle[row][field], gl_entries[row][field]) +>>>>>>> 4b4b176fcf (style: Remove spaces introduced via merge conflict) def create_payment_entry(**args): From fb19e7f15eb22bab4a778fd0dc922733b0dd00ae Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 9 Nov 2023 15:01:58 +0100 Subject: [PATCH 5/6] fix: Re-add no.of rows split in alert message (cherry picked from commit 1fc5844025d97de150195b94070b0c4061378992) --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index f7856f11c05c..19b2ee89d108 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1564,8 +1564,8 @@ def split_invoices_based_on_payment_terms(outstanding_invoices, company) -> list continue frappe.msgprint( - _("Splitting {0} {1} as per Payment Terms").format( - _(entry.voucher_type), frappe.bold(entry.voucher_no) + _("Splitting {0} {1} into {2} rows as per Payment Terms").format( + _(entry.voucher_type), frappe.bold(entry.voucher_no), len(split_rows) ), alert=True, ) From e7183e3ea935638a7737dc20f48e1aa7624936cb Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 22 Nov 2023 16:16:37 +0100 Subject: [PATCH 6/6] fix: Merge conflicts --- .../doctype/payment_entry/payment_entry.py | 63 ------------------- .../payment_entry/test_payment_entry.py | 36 ----------- 2 files changed, 99 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 19b2ee89d108..84c723f1c313 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1644,72 +1644,9 @@ def get_split_invoice_rows(invoice: dict, payment_term_template: str, exc_rates: "payment_term": payment_term.payment_term, } ) -<<<<<<< HEAD - if payment_term_template: - allocate_payment_based_on_payment_terms = frappe.db.get_value( - "Payment Terms Template", payment_term_template, "allocate_payment_based_on_payment_terms" - ) - if allocate_payment_based_on_payment_terms: - payment_schedule = frappe.get_all( - "Payment Schedule", filters={"parent": d.voucher_no}, fields=["*"] - ) - - for payment_term in payment_schedule: - if payment_term.outstanding > 0.1: - doc_details = exc_rates.get(payment_term.parent, None) - is_multi_currency_acc = (doc_details.currency != doc_details.company_currency) and ( - doc_details.party_account_currency != doc_details.company_currency - ) - payment_term_outstanding = flt(payment_term.outstanding) - if not is_multi_currency_acc: - payment_term_outstanding = doc_details.conversion_rate * flt(payment_term.outstanding) - - invoice_ref_based_on_payment_terms.setdefault(idx, []) - invoice_ref_based_on_payment_terms[idx].append( - frappe._dict( - { - "due_date": d.due_date, - "currency": d.currency, - "voucher_no": d.voucher_no, - "voucher_type": d.voucher_type, - "posting_date": d.posting_date, - "invoice_amount": flt(d.invoice_amount), - "outstanding_amount": payment_term_outstanding - if payment_term_outstanding - else d.outstanding_amount, - "payment_term_outstanding": payment_term_outstanding, - "payment_amount": payment_term.payment_amount, - "payment_term": payment_term.payment_term, - } - ) - ) - - outstanding_invoices_after_split = [] - if invoice_ref_based_on_payment_terms: - for idx, ref in invoice_ref_based_on_payment_terms.items(): - voucher_no = ref[0]["voucher_no"] - voucher_type = ref[0]["voucher_type"] - - frappe.msgprint( - _("Spliting {} {} into {} row(s) as per Payment Terms").format( - voucher_type, voucher_no, len(ref) - ), - alert=True, - ) - - outstanding_invoices_after_split += invoice_ref_based_on_payment_terms[idx] - - existing_row = list(filter(lambda x: x.get("voucher_no") == voucher_no, outstanding_invoices)) - index = outstanding_invoices.index(existing_row[0]) - outstanding_invoices.pop(index) - - outstanding_invoices_after_split += outstanding_invoices - return outstanding_invoices_after_split -======= ) return split_rows ->>>>>>> 6bd56d2d5f (refactor: `split_invoices_based_on_payment_terms`) def get_orders_to_be_billed( diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 78d745cc76d5..169a077f6a82 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1258,42 +1258,6 @@ def test_outstanding_invoices_api(self): self.assertEqual(references[2].voucher_no, si2.name) self.assertEqual(references[1].payment_term, "Basic Amount Receivable") self.assertEqual(references[2].payment_term, "Tax Receivable") -<<<<<<< HEAD -======= - - def test_receive_payment_from_payable_party_type(self): - pe = create_payment_entry( - party_type="Supplier", - party="_Test Supplier", - payment_type="Receive", - paid_from="Creditors - _TC", - paid_to="_Test Cash - _TC", - save=True, - submit=True, - ) - self.voucher_no = pe.name - self.expected_gle = [ - {"account": "_Test Cash - _TC", "debit": 1000.0, "credit": 0.0}, - {"account": "Creditors - _TC", "debit": 0.0, "credit": 1000.0}, - ] - self.check_gl_entries() - - def check_gl_entries(self): - gle = frappe.qb.DocType("GL Entry") - gl_entries = ( - frappe.qb.from_(gle) - .select( - gle.account, - gle.debit, - gle.credit, - ) - .where((gle.voucher_no == self.voucher_no) & (gle.is_cancelled == 0)) - .orderby(gle.account) - ).run(as_dict=True) - for row in range(len(self.expected_gle)): - for field in ["account", "debit", "credit"]: - self.assertEqual(self.expected_gle[row][field], gl_entries[row][field]) ->>>>>>> 4b4b176fcf (style: Remove spaces introduced via merge conflict) def create_payment_entry(**args):