From c06a6bfc099680ea8f1f69b7432927c4597a241f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Apr 2023 09:46:54 +0530 Subject: [PATCH 01/54] refactor: book exchange gain/loss through journal (cherry picked from commit 81cd7873d343d893a6e2a3b41107f17e03eedcd8) --- .../doctype/payment_entry/payment_entry.js | 2 +- .../doctype/payment_entry/payment_entry.py | 19 +- .../payment_reconciliation.py | 10 +- .../doctype/sales_invoice/sales_invoice.py | 3 +- erpnext/accounts/utils.py | 34 +++- erpnext/controllers/accounts_controller.py | 170 ++++++++++++------ 6 files changed, 171 insertions(+), 67 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js index 74a90fe2e8f..3c2fb1dd0ed 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -7,7 +7,7 @@ cur_frm.cscript.tax_table = "Advance Taxes and Charges"; frappe.ui.form.on('Payment Entry', { onload: function(frm) { - frm.ignore_doctypes_on_cancel_all = ['Sales Invoice', 'Purchase Invoice', "Repost Payment Ledger"]; + frm.ignore_doctypes_on_cancel_all = ['Sales Invoice', 'Purchase Invoice', "Journal Entry", "Repost Payment Ledger"]; if(frm.doc.__islocal) { if (!frm.doc.paid_from) frm.set_value("paid_from_account_currency", null); diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index d3f0043fe6a..f0c36cef47f 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -101,6 +101,7 @@ class PaymentEntry(AccountsController): "Repost Payment Ledger", "Repost Payment Ledger Items", ) + super(PaymentEntry, self).on_cancel() self.make_gl_entries(cancel=1) self.update_outstanding_amounts() self.update_advance_paid() @@ -783,10 +784,25 @@ class PaymentEntry(AccountsController): flt(d.allocated_amount) * flt(exchange_rate), self.precision("base_paid_amount") ) else: + + # Use source/target exchange rate, so no difference amount is calculated. + # then update exchange gain/loss amount in refernece table + # if there is an amount, submit a JE for that + + exchange_rate = 1 + if self.payment_type == "Receive": + exchange_rate = self.source_exchange_rate + elif self.payment_type == "Pay": + exchange_rate = self.target_exchange_rate + base_allocated_amount += flt( - flt(d.allocated_amount) * flt(d.exchange_rate), self.precision("base_paid_amount") + flt(d.allocated_amount) * flt(exchange_rate), self.precision("base_paid_amount") ) + allocated_amount_in_pe_exchange_rate = flt( + flt(d.allocated_amount) * flt(d.exchange_rate), self.precision("base_paid_amount") + ) + d.exchange_gain_loss = base_allocated_amount - allocated_amount_in_pe_exchange_rate return base_allocated_amount def set_total_allocated_amount(self): @@ -977,6 +993,7 @@ class PaymentEntry(AccountsController): gl_entries = self.build_gl_map() gl_entries = process_gl_map(gl_entries) make_gl_entries(gl_entries, cancel=cancel, adv_adj=adv_adj) + self.make_exchange_gain_loss_journal() def add_party_gl_entries(self, gl_entries): if self.party_account: diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 05623c09932..ab03699785a 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -347,11 +347,11 @@ class PaymentReconciliation(Document): payment_details = self.get_payment_details(row, dr_or_cr) reconciled_entry.append(payment_details) - if payment_details.difference_amount and row.reference_type not in [ - "Sales Invoice", - "Purchase Invoice", - ]: - self.make_difference_entry(payment_details) + # if payment_details.difference_amount and row.reference_type not in [ + # "Sales Invoice", + # "Purchase Invoice", + # ]: + # self.make_difference_entry(payment_details) if entry_list: reconcile_against_document(entry_list, skip_ref_details_update_for_pe) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index c2804d19a34..9e9cd61e203 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -1046,6 +1046,8 @@ class SalesInvoice(SellingController): merge_entries=False, from_repost=from_repost, ) + + self.make_exchange_gain_loss_journal() elif self.docstatus == 2: make_reverse_gl_entries(voucher_type=self.doctype, voucher_no=self.name) @@ -1071,7 +1073,6 @@ class SalesInvoice(SellingController): self.make_customer_gl_entry(gl_entries) self.make_tax_gl_entries(gl_entries) - self.make_exchange_gain_loss_gl_entries(gl_entries) self.make_internal_transfer_gl_entries(gl_entries) self.make_item_gl_entries(gl_entries) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 5662e99c5e7..adb789e7493 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -612,9 +612,7 @@ def update_reference_in_payment_entry( "total_amount": d.grand_total, "outstanding_amount": d.outstanding_amount, "allocated_amount": d.allocated_amount, - "exchange_rate": d.exchange_rate - if not d.exchange_gain_loss - else payment_entry.get_exchange_rate(), + "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 } @@ -657,11 +655,41 @@ def update_reference_in_payment_entry( if not skip_ref_details_update_for_pe: payment_entry.set_missing_ref_details() payment_entry.set_amounts() + payment_entry.make_exchange_gain_loss_journal() if not do_not_save: payment_entry.save(ignore_permissions=True) +def cancel_exchange_gain_loss_journal(parent_doc: dict | object) -> None: + """ + Cancel Exchange Gain/Loss for Sales/Purchase Invoice, if they have any. + """ + if parent_doc.doctype in ["Sales Invoice", "Purchase Invoice", "Payment Entry"]: + journals = frappe.db.get_all( + "Journal Entry Account", + filters={ + "reference_type": parent_doc.doctype, + "reference_name": parent_doc.name, + "docstatus": 1, + }, + fields=["parent"], + as_list=1, + ) + if journals: + exchange_journals = frappe.db.get_all( + "Journal Entry", + filters={ + "name": ["in", [x[0] for x in journals]], + "voucher_type": "Exchange Gain Or Loss", + "docstatus": 1, + }, + as_list=1, + ) + for doc in exchange_journals: + frappe.get_doc("Journal Entry", doc[0]).cancel() + + def unlink_ref_doc_from_payment_entries(ref_doc): remove_ref_doc_link_from_jv(ref_doc.doctype, ref_doc.name) remove_ref_doc_link_from_pe(ref_doc.doctype, ref_doc.name) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 9912dd47f8b..dcaaff08ade 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -5,7 +5,7 @@ import json import frappe -from frappe import _, bold, throw +from frappe import _, bold, qb, throw from frappe.model.workflow import get_workflow_name, is_transition_condition_satisfied from frappe.query_builder.functions import Abs, Sum from frappe.utils import ( @@ -962,67 +962,119 @@ class AccountsController(TransactionBase): d.exchange_gain_loss = difference - def make_exchange_gain_loss_gl_entries(self, gl_entries): - if self.get("doctype") in ["Purchase Invoice", "Sales Invoice"]: - for d in self.get("advances"): - if d.exchange_gain_loss: - is_purchase_invoice = self.get("doctype") == "Purchase Invoice" - party = self.supplier if is_purchase_invoice else self.customer - party_account = self.credit_to if is_purchase_invoice else self.debit_to - party_type = "Supplier" if is_purchase_invoice else "Customer" - - gain_loss_account = frappe.get_cached_value( - "Company", self.company, "exchange_gain_loss_account" - ) - if not gain_loss_account: - frappe.throw( - _("Please set default Exchange Gain/Loss Account in Company {}").format(self.get("company")) - ) - account_currency = get_account_currency(gain_loss_account) - if account_currency != self.company_currency: - frappe.throw( - _("Currency for {0} must be {1}").format(gain_loss_account, self.company_currency) - ) - - # for purchase - dr_or_cr = "debit" if d.exchange_gain_loss > 0 else "credit" - if not is_purchase_invoice: - # just reverse for sales? - dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" - - gl_entries.append( - self.get_gl_dict( - { - "account": gain_loss_account, - "account_currency": account_currency, - "against": party, - dr_or_cr + "_in_account_currency": abs(d.exchange_gain_loss), - dr_or_cr: abs(d.exchange_gain_loss), - "cost_center": self.cost_center or erpnext.get_default_cost_center(self.company), - "project": self.project, - }, - item=d, + def make_exchange_gain_loss_journal(self) -> None: + """ + Make Exchange Gain/Loss journal for Invoices and Payments + """ + # Cancelling is existing exchange gain/loss journals is handled in on_cancel event + if self.docstatus == 1: + if self.get("doctype") == "Payment Entry": + gain_loss_to_book = [x for x in self.references if x.exchange_gain_loss != 0] + booked = [] + if gain_loss_to_book: + vtypes = [x.reference_doctype for x in gain_loss_to_book] + vnames = [x.reference_name for x in gain_loss_to_book] + je = qb.DocType("Journal Entry") + jea = qb.DocType("Journal Entry Account") + parents = ( + qb.from_(jea) + .select(jea.parent) + .where( + (jea.reference_type == "Payment Entry") + & (jea.reference_name == self.name) + & (jea.docstatus == 1) ) + .run() ) - dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + booked = [] + if parents: + booked = ( + qb.from_(je) + .inner_join(jea) + .on(je.name == jea.parent) + .select(jea.reference_type, jea.reference_name, jea.reference_detail_no) + .where( + (je.docstatus == 1) + & (je.name.isin(parents)) + & (je.voucher_type == "Exchange Gain or Loss") + ) + .run() + ) - gl_entries.append( - self.get_gl_dict( + for d in gain_loss_to_book: + if d.exchange_gain_loss and ( + (d.reference_doctype, d.reference_name, str(d.idx)) not in booked + ): + journal_entry = frappe.new_doc("Journal Entry") + journal_entry.voucher_type = "Exchange Gain Or Loss" + journal_entry.company = self.company + journal_entry.posting_date = nowdate() + journal_entry.multi_currency = 1 + + if self.payment_type == "Receive": + party_account = self.paid_from + elif self.payment_type == "Pay": + party_account = self.paid_to + + party_account_currency = frappe.get_cached_value( + "Account", party_account, "account_currency" + ) + dr_or_cr = "debit" if d.exchange_gain_loss > 0 else "credit" + reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + + gain_loss_account = frappe.get_cached_value( + "Company", self.company, "exchange_gain_loss_account" + ) + if not gain_loss_account: + frappe.throw( + _("Please set default Exchange Gain/Loss Account in Company {}").format( + self.get("company") + ) + ) + gain_loss_account_currency = get_account_currency(gain_loss_account) + if gain_loss_account_currency != self.company_currency: + frappe.throw( + _("Currency for {0} must be {1}").format(gain_loss_account, self.company_currency) + ) + + journal_account = frappe._dict( { "account": party_account, - "party_type": party_type, - "party": party, - "against": gain_loss_account, - dr_or_cr + "_in_account_currency": flt(abs(d.exchange_gain_loss) / self.conversion_rate), + "party_type": self.party_type, + "party": self.party, + "account_currency": party_account_currency, + "exchange_rate": 0, + "cost_center": erpnext.get_default_cost_center(self.company), + "reference_type": d.reference_doctype, + "reference_name": d.reference_name, + "reference_detail_no": d.idx, dr_or_cr: abs(d.exchange_gain_loss), - "cost_center": self.cost_center, - "project": self.project, - }, - self.party_account_currency, - item=self, + dr_or_cr + "_in_account_currency": 0, + } ) - ) + + journal_entry.append("accounts", journal_account) + + journal_account = frappe._dict( + { + "account": gain_loss_account, + "account_currency": gain_loss_account_currency, + "exchange_rate": 1, + "cost_center": erpnext.get_default_cost_center(self.company), + "reference_type": self.doctype, + "reference_name": self.name, + "reference_detail_no": d.idx, + reverse_dr_or_cr + "_in_account_currency": abs(d.exchange_gain_loss), + reverse_dr_or_cr: abs(d.exchange_gain_loss), + } + ) + + journal_entry.append("accounts", journal_account) + + journal_entry.save() + journal_entry.submit() + # frappe.throw("stopping...") def make_precision_loss_gl_entry(self, gl_entries): round_off_account, round_off_cost_center = get_round_off_account_and_cost_center( @@ -1111,9 +1163,15 @@ class AccountsController(TransactionBase): reconcile_against_document(lst) def on_cancel(self): - from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries + from erpnext.accounts.utils import ( + cancel_exchange_gain_loss_journal, + unlink_ref_doc_from_payment_entries, + ) + + if self.doctype in ["Sales Invoice", "Purchase Invoice", "Payment Entry"]: + # Cancel Exchange Gain/Loss Journal before unlinking + cancel_exchange_gain_loss_journal(self) - if self.doctype in ["Sales Invoice", "Purchase Invoice"]: if frappe.db.get_single_value("Accounts Settings", "unlink_payment_on_cancellation_of_invoice"): unlink_ref_doc_from_payment_entries(self) From 44110860b4c0ed5922283c28b32f8a56ff773e2e Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 15 Jun 2023 16:55:56 +0530 Subject: [PATCH 02/54] test: different scenarios for exchange booking (cherry picked from commit 5e1cd1f22701b7675422b05b3616253d9a3a28db) --- .../tests/test_accounts_controller.py | 501 ++++++++++++++++++ 1 file changed, 501 insertions(+) create mode 100644 erpnext/controllers/tests/test_accounts_controller.py diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py new file mode 100644 index 00000000000..31aa857c8f5 --- /dev/null +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -0,0 +1,501 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import unittest + +import frappe +from frappe import qb +from frappe.tests.utils import FrappeTestCase, change_settings +from frappe.utils import add_days, flt, nowdate + +from erpnext import get_default_cost_center +from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry +from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry +from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice +from erpnext.accounts.party import get_party_account +from erpnext.stock.doctype.item.test_item import create_item + + +def make_customer(customer_name, currency=None): + if not frappe.db.exists("Customer", customer_name): + customer = frappe.new_doc("Customer") + customer.customer_name = customer_name + customer.type = "Individual" + + if currency: + customer.default_currency = currency + customer.save() + return customer.name + else: + return customer_name + + +class TestAccountsController(FrappeTestCase): + """ + Test Exchange Gain/Loss booking on various scenarios + """ + + def setUp(self): + self.create_company() + self.create_account() + self.create_item() + self.create_customer() + self.clear_old_entries() + + def tearDown(self): + frappe.db.rollback() + + def create_company(self): + company_name = "_Test Company MC" + self.company_abbr = abbr = "_CM" + if frappe.db.exists("Company", company_name): + company = frappe.get_doc("Company", company_name) + else: + company = frappe.get_doc( + { + "doctype": "Company", + "company_name": company_name, + "country": "India", + "default_currency": "INR", + "create_chart_of_accounts_based_on": "Standard Template", + "chart_of_accounts": "Standard", + } + ) + company = company.save() + + self.company = company.name + self.cost_center = company.cost_center + self.warehouse = "Stores - " + abbr + self.finished_warehouse = "Finished Goods - " + abbr + self.income_account = "Sales - " + abbr + self.expense_account = "Cost of Goods Sold - " + abbr + self.debit_to = "Debtors - " + abbr + self.debit_usd = "Debtors USD - " + abbr + self.cash = "Cash - " + abbr + self.creditors = "Creditors - " + abbr + + def create_item(self): + item = create_item( + item_code="_Test Notebook", is_stock_item=0, company=self.company, warehouse=self.warehouse + ) + self.item = item if isinstance(item, str) else item.item_code + + def create_customer(self): + self.customer = make_customer("_Test MC Customer USD", "USD") + + def create_account(self): + account_name = "Debtors USD" + 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 - " + self.company_abbr + acc.company = self.company + acc.account_currency = "USD" + 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_usd = acc.name + + def create_sales_invoice( + self, qty=1, rate=1, posting_date=nowdate(), do_not_save=False, do_not_submit=False + ): + """ + Helper function to populate default values in sales invoice + """ + sinv = create_sales_invoice( + qty=qty, + rate=rate, + company=self.company, + customer=self.customer, + item_code=self.item, + item_name=self.item, + cost_center=self.cost_center, + warehouse=self.warehouse, + debit_to=self.debit_usd, + parent_cost_center=self.cost_center, + update_stock=0, + currency="USD", + conversion_rate=80, + 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 sinv + + def create_payment_entry( + self, amount=1, source_exc_rate=75, posting_date=nowdate(), customer=None + ): + """ + Helper function to populate default values in payment entry + """ + payment = create_payment_entry( + company=self.company, + payment_type="Receive", + party_type="Customer", + party=customer or self.customer, + paid_from=self.debit_usd, + paid_to=self.cash, + paid_amount=amount, + ) + payment.source_exchange_rate = source_exc_rate + payment.received_amount = source_exc_rate * amount + payment.posting_date = posting_date + return payment + + def clear_old_entries(self): + doctype_list = [ + "GL Entry", + "Payment Ledger Entry", + "Sales Invoice", + "Purchase Invoice", + "Payment Entry", + "Journal Entry", + ] + for doctype in doctype_list: + qb.from_(qb.DocType(doctype)).delete().where(qb.DocType(doctype).company == self.company).run() + + def create_payment_reconciliation(self): + pr = frappe.new_doc("Payment Reconciliation") + pr.company = self.company + pr.party_type = "Customer" + pr.party = self.customer + 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 + + def create_journal_entry( + self, acc1=None, acc2=None, amount=0, posting_date=None, cost_center=None + ): + je = frappe.new_doc("Journal Entry") + je.posting_date = posting_date or nowdate() + je.company = self.company + je.user_remark = "test" + if not cost_center: + cost_center = self.cost_center + je.set( + "accounts", + [ + { + "account": acc1, + "cost_center": cost_center, + "debit_in_account_currency": amount if amount > 0 else 0, + "credit_in_account_currency": abs(amount) if amount < 0 else 0, + }, + { + "account": acc2, + "cost_center": cost_center, + "credit_in_account_currency": amount if amount > 0 else 0, + "debit_in_account_currency": abs(amount) if amount < 0 else 0, + }, + ], + ) + return je + + def get_journals_for(self, voucher_type: str, voucher_no: str) -> list: + journals = [] + if voucher_type and voucher_no: + journals = frappe.db.get_all( + "Journal Entry Account", + filters={"reference_type": voucher_type, "reference_name": voucher_no, "docstatus": 1}, + fields=["parent"], + ) + return journals + + def test_01_payment_against_invoice(self): + # Invoice in Foreign Currency + si = self.create_sales_invoice(qty=1, rate=1) + # Payment + pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() + pe.append( + "references", + {"reference_doctype": si.doctype, "reference_name": si.name, "allocated_amount": 1}, + ) + pe = pe.save().submit() + + si.reload() + self.assertEqual(si.outstanding_amount, 0) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_si[0], exc_je_for_pe[0]) + + # Cancel Payment + pe.cancel() + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_pe, []) + + def test_02_advance_against_invoice(self): + # Advance Payment + adv = self.create_payment_entry(amount=1, source_exc_rate=85).save().submit() + adv.reload() + + # Invoice in Foreign Currency + si = self.create_sales_invoice(qty=1, rate=1, do_not_submit=True) + si.append( + "advances", + { + "doctype": "Sales Invoice Advance", + "reference_type": adv.doctype, + "reference_name": adv.name, + "advance_amount": 1, + "allocated_amount": 1, + "ref_exchange_rate": 85, + "remarks": "Test", + }, + ) + si = si.save() + si = si.submit() + + adv.reload() + self.assertEqual(si.outstanding_amount, 0) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) + + # Cancel Invoice + si.cancel() + + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_adv, []) + + def test_03_partial_advance_and_payment_for_invoice(self): + """ + Invoice with partial advance payment, and a normal payment + """ + # Partial Advance + adv = self.create_payment_entry(amount=1, source_exc_rate=85).save().submit() + adv.reload() + + # Invoice in Foreign Currency linked with advance + si = self.create_sales_invoice(qty=2, rate=1, do_not_submit=True) + si.append( + "advances", + { + "doctype": "Sales Invoice Advance", + "reference_type": adv.doctype, + "reference_name": adv.name, + "advance_amount": 1, + "allocated_amount": 1, + "ref_exchange_rate": 85, + "remarks": "Test", + }, + ) + si = si.save() + si = si.submit() + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + + # Exchange Gain/Loss Journal should've been created for the partial advance + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) + + # Payment + pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() + pe.append( + "references", + {"reference_doctype": si.doctype, "reference_name": si.name, "allocated_amount": 1}, + ) + pe = pe.save().submit() + + si.reload() + self.assertEqual(si.outstanding_amount, 0) + + # Exchange Gain/Loss Journal should've been created for the payment + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + + self.assertNotEqual(exc_je_for_si, []) + # There should be 2 JE's now. One for the advance and one for the payment + self.assertEqual(len(exc_je_for_si), 2) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_si, exc_je_for_pe + exc_je_for_adv) + + # Cancel Invoice + si.reload() + si.cancel() + + # Exchange Gain/Loss Journal should been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_pe, []) + self.assertEqual(exc_je_for_adv, []) + + def test_04_partial_advance_and_payment_for_invoice_with_cancellation(self): + """ + Invoice with partial advance payment, and a normal payment. Cancel advance and payment. + """ + # Partial Advance + adv = self.create_payment_entry(amount=1, source_exc_rate=85).save().submit() + adv.reload() + + # Invoice in Foreign Currency linked with advance + si = self.create_sales_invoice(qty=2, rate=1, do_not_submit=True) + si.append( + "advances", + { + "doctype": "Sales Invoice Advance", + "reference_type": adv.doctype, + "reference_name": adv.name, + "advance_amount": 1, + "allocated_amount": 1, + "ref_exchange_rate": 85, + "remarks": "Test", + }, + ) + si = si.save() + si = si.submit() + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + + # Exchange Gain/Loss Journal should've been created for the partial advance + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) + + # Payment + pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() + pe.append( + "references", + {"reference_doctype": si.doctype, "reference_name": si.name, "allocated_amount": 1}, + ) + pe = pe.save().submit() + + si.reload() + self.assertEqual(si.outstanding_amount, 0) + + # Exchange Gain/Loss Journal should've been created for the payment + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + + self.assertNotEqual(exc_je_for_si, []) + # There should be 2 JE's now. One for the advance and one for the payment + self.assertEqual(len(exc_je_for_si), 2) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_si, exc_je_for_pe + exc_je_for_adv) + + adv.reload() + adv.cancel() + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + + # Exchange Gain/Loss Journal for advance should been cancelled + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_adv, []) + + def test_05_same_payment_split_against_invoice(self): + # Invoice in Foreign Currency + si = self.create_sales_invoice(qty=2, rate=1) + # Payment + pe = self.create_payment_entry(amount=2, source_exc_rate=75).save() + pe.append( + "references", + {"reference_doctype": si.doctype, "reference_name": si.name, "allocated_amount": 1}, + ) + pe = pe.save().submit() + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_si[0], exc_je_for_pe[0]) + + # Reconcile the remaining amount + pr = frappe.get_doc("Payment Reconciliation") + pr.company = self.company + pr.party_type = "Customer" + pr.party = self.customer + pr.receivable_payable_account = self.debit_usd + + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + + # Test exact payment allocation + 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() + self.assertEqual(len(pr.invoices), 0) + self.assertEqual(len(pr.payments), 0) + + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + self.assertEqual(len(exc_je_for_si), 2) + self.assertEqual(len(exc_je_for_pe), 2) + self.assertEqual(exc_je_for_si, exc_je_for_pe) + + # Cancel Payment + pe.reload() + pe.cancel() + + si.reload() + self.assertEqual(si.outstanding_amount, 2) + + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_pe, []) From db46987d4b83fda5bf2ba7ea3f28588eb9956623 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 11:34:11 +0530 Subject: [PATCH 03/54] refactor: replace with new method in purchase invoice (cherry picked from commit 7e94a1c51b428202820858f72a7e4a864cde0e9c) --- erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 5a7ff1c0d1c..cefb502ede1 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -543,6 +543,7 @@ class PurchaseInvoice(BuyingController): merge_entries=False, from_repost=from_repost, ) + self.make_exchange_gain_loss_journal() elif self.docstatus == 2: provisional_entries = [a for a in gl_entries if a.voucher_type == "Purchase Receipt"] make_reverse_gl_entries(voucher_type=self.doctype, voucher_no=self.name) @@ -587,7 +588,6 @@ class PurchaseInvoice(BuyingController): self.get_asset_gl_entry(gl_entries) self.make_tax_gl_entries(gl_entries) - self.make_exchange_gain_loss_gl_entries(gl_entries) self.make_internal_transfer_gl_entries(gl_entries) gl_entries = make_regional_gl_entries(gl_entries, self) From 287af687cfab88f37eb7289895d794423b35cdd1 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 12:32:21 +0530 Subject: [PATCH 04/54] chore: patch to update property setter for Journal Entry Accounts (cherry picked from commit 0587338435a6ffeeb59669ff20dbd9779b9ac740) --- erpnext/patches.txt | 1 + ...eference_type_in_journal_entry_accounts.py | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 erpnext/patches/v14_0/update_reference_type_in_journal_entry_accounts.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 75f728afa88..4a0c9b3b410 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -328,6 +328,7 @@ erpnext.patches.v14_0.set_pick_list_status erpnext.patches.v13_0.update_docs_link erpnext.patches.v14_0.enable_all_leads execute:frappe.db.set_single_value("Accounts Settings", "merge_similar_account_heads", 0) +erpnext.patches.v14_0.update_reference_type_in_journal_entry_accounts # below migration patches should always run last erpnext.patches.v14_0.migrate_gl_to_payment_ledger erpnext.patches.v14_0.update_company_in_ldc diff --git a/erpnext/patches/v14_0/update_reference_type_in_journal_entry_accounts.py b/erpnext/patches/v14_0/update_reference_type_in_journal_entry_accounts.py new file mode 100644 index 00000000000..48b6bcf755f --- /dev/null +++ b/erpnext/patches/v14_0/update_reference_type_in_journal_entry_accounts.py @@ -0,0 +1,22 @@ +import frappe + + +def execute(): + """ + Update Propery Setters for Journal Entry with new 'Entry Type' + """ + new_reference_type = "Payment Entry" + prop_setter = frappe.db.get_list( + "Property Setter", + filters={ + "doc_type": "Journal Entry Account", + "field_name": "reference_type", + "property": "options", + }, + ) + if prop_setter: + property_setter_doc = frappe.get_doc("Property Setter", prop_setter[0].get("name")) + + if new_reference_type not in property_setter_doc.value.split("\n"): + property_setter_doc.value += "\n" + new_reference_type + property_setter_doc.save() From 72005bdb3955d7c11e67597e6cc0172d4445fd85 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 14:01:48 +0530 Subject: [PATCH 05/54] refactor: add new reference type in journal entry account (cherry picked from commit 13febcac811507c7c61bc116ca797857d0b5baf5) --- .../doctype/journal_entry_account/journal_entry_account.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json b/erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json index 47ad19e0f98..3ba8cea94bb 100644 --- a/erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json +++ b/erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json @@ -203,7 +203,7 @@ "fieldtype": "Select", "label": "Reference Type", "no_copy": 1, - "options": "\nSales Invoice\nPurchase Invoice\nJournal Entry\nSales Order\nPurchase Order\nExpense Claim\nAsset\nLoan\nPayroll Entry\nEmployee Advance\nExchange Rate Revaluation\nInvoice Discounting\nFees\nFull and Final Statement" + "options": "\nSales Invoice\nPurchase Invoice\nJournal Entry\nSales Order\nPurchase Order\nExpense Claim\nAsset\nLoan\nPayroll Entry\nEmployee Advance\nExchange Rate Revaluation\nInvoice Discounting\nFees\nFull and Final Statement\nPayment Entry" }, { "fieldname": "reference_name", @@ -284,7 +284,7 @@ "idx": 1, "istable": 1, "links": [], - "modified": "2022-10-26 20:03:10.906259", + "modified": "2023-06-16 14:11:13.507807", "modified_by": "Administrator", "module": "Accounts", "name": "Journal Entry Account", From a9da619903e6e4a13cb3ec5319b48a6e56c01137 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 14:07:44 +0530 Subject: [PATCH 06/54] chore: fix logic for purchase invoice and some typos (cherry picked from commit 34b5e849a290ee02d9b653286dfbe6590d35a800) --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 4 ++-- erpnext/controllers/accounts_controller.py | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index f0c36cef47f..65c55686bce 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -786,8 +786,8 @@ class PaymentEntry(AccountsController): else: # Use source/target exchange rate, so no difference amount is calculated. - # then update exchange gain/loss amount in refernece table - # if there is an amount, submit a JE for that + # then update exchange gain/loss amount in reference table + # if there is an exchange gain/loss amount in reference table, submit a JE for that exchange_rate = 1 if self.payment_type == "Receive": diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index dcaaff08ade..f9d7b0deb50 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -966,7 +966,7 @@ class AccountsController(TransactionBase): """ Make Exchange Gain/Loss journal for Invoices and Payments """ - # Cancelling is existing exchange gain/loss journals is handled in on_cancel event + # Cancelling existing exchange gain/loss journals is handled in on_cancel event in accounts/utils.py if self.docstatus == 1: if self.get("doctype") == "Payment Entry": gain_loss_to_book = [x for x in self.references if x.exchange_gain_loss != 0] @@ -1021,6 +1021,10 @@ class AccountsController(TransactionBase): "Account", party_account, "account_currency" ) dr_or_cr = "debit" if d.exchange_gain_loss > 0 else "credit" + + if d.reference_doctype == "Purchase Invoice": + dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" gain_loss_account = frappe.get_cached_value( @@ -1074,7 +1078,6 @@ class AccountsController(TransactionBase): journal_entry.save() journal_entry.submit() - # frappe.throw("stopping...") def make_precision_loss_gl_entry(self, gl_entries): round_off_account, round_off_cost_center = get_round_off_account_and_cost_center( From 6c6acef78eee6a7228f51a63342ad4edd5987a21 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 17:51:05 +0530 Subject: [PATCH 07/54] refactor: helper method (cherry picked from commit c1184585eda2e37b74718b95d541fa0419511bd9) --- .../doctype/payment_entry/test_payment_entry.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index ca1d317c38e..47bf6df37d1 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -31,6 +31,16 @@ class TestPaymentEntry(FrappeTestCase): def tearDown(self): frappe.db.rollback() + def get_journals_for(self, voucher_type: str, voucher_no: str) -> list: + journals = [] + if voucher_type and voucher_no: + journals = frappe.db.get_all( + "Journal Entry Account", + filters={"reference_type": voucher_type, "reference_name": voucher_no, "docstatus": 1}, + fields=["parent"], + ) + return journals + def test_payment_entry_against_order(self): so = make_sales_order() pe = get_payment_entry("Sales Order", so.name, bank_account="_Test Cash - _TC") From d030f4a1007fcdacede7657721015c83101e875f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 09:58:18 +0530 Subject: [PATCH 08/54] refactor: remove unused variable, pe should pull in parent exc rate 1. 'reference_doc' variable is never set. Hence, removing. 2. set_exchange_rate() relies on ref_doc, which was never set due to point [1]. Replacing it with 'doc'. 3. Sales/Purchase Invoice has 'conversion_rate' field for tracking exchange rate. Added a get statement for them as well. (cherry picked from commit 92ae9c220110ddcb32d90a1bb89f0b85e72ff7d0) --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 65c55686bce..b4c39f41063 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -362,7 +362,7 @@ class PaymentEntry(AccountsController): else: if ref_doc: if self.paid_from_account_currency == ref_doc.currency: - self.source_exchange_rate = ref_doc.get("exchange_rate") + self.source_exchange_rate = ref_doc.get("exchange_rate") or ref_doc.get("conversion_rate") if not self.source_exchange_rate: self.source_exchange_rate = get_exchange_rate( @@ -375,7 +375,7 @@ class PaymentEntry(AccountsController): elif self.paid_to and not self.target_exchange_rate: if ref_doc: if self.paid_to_account_currency == ref_doc.currency: - self.target_exchange_rate = ref_doc.get("exchange_rate") + self.target_exchange_rate = ref_doc.get("exchange_rate") or ref_doc.get("conversion_rate") if not self.target_exchange_rate: self.target_exchange_rate = get_exchange_rate( @@ -1895,7 +1895,6 @@ def get_payment_entry( payment_type=None, reference_date=None, ): - reference_doc = None doc = frappe.get_doc(dt, dn) over_billing_allowance = frappe.db.get_single_value("Accounts Settings", "over_billing_allowance") if dt in ("Sales Order", "Purchase Order") and flt(doc.per_billed, 2) >= ( @@ -2036,7 +2035,7 @@ def get_payment_entry( update_accounting_dimensions(pe, doc) if party_account and bank: - pe.set_exchange_rate(ref_doc=reference_doc) + pe.set_exchange_rate(ref_doc=doc) pe.set_amounts() if discount_amount: From 59b7e962555da99973c3798629a0eb4ccfcfa941 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 10:23:23 +0530 Subject: [PATCH 09/54] refactor: assert exchange gain/loss amount in reference table (cherry picked from commit 4ff53e106271e0562e2ba5604802a41c24c999c4) --- .../doctype/payment_entry/test_payment_entry.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 47bf6df37d1..5a1dda5c459 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -601,21 +601,15 @@ class TestPaymentEntry(FrappeTestCase): pe.target_exchange_rate = 45.263 pe.reference_no = "1" pe.reference_date = "2016-01-01" - - pe.append( - "deductions", - { - "account": "_Test Exchange Gain/Loss - _TC", - "cost_center": "_Test Cost Center - _TC", - "amount": 94.80, - }, - ) - pe.save() self.assertEqual(flt(pe.difference_amount, 2), 0.0) self.assertEqual(flt(pe.unallocated_amount, 2), 0.0) + # the exchange gain/loss amount is captured in reference table and a separate Journal will be submitted for them + # payment entry will not be generating difference amount + self.assertEqual(flt(pe.references[0].exchange_gain_loss, 2), -94.74) + def test_payment_entry_retrieves_last_exchange_rate(self): from erpnext.setup.doctype.currency_exchange.test_currency_exchange import ( save_new_records, From 1f76dde0256530172e56b970ed5bf88fe116babd Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 11:26:49 +0530 Subject: [PATCH 10/54] refactor(test): exc gain/loss booked through journal (cherry picked from commit 00a2e42a47fb064afbb31b27653a54d12b6c8097) --- .../payment_entry/test_payment_entry.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 5a1dda5c459..21379458874 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -796,33 +796,28 @@ class TestPaymentEntry(FrappeTestCase): pe.reference_no = "1" pe.reference_date = "2016-01-01" pe.source_exchange_rate = 55 - - pe.append( - "deductions", - { - "account": "_Test Exchange Gain/Loss - _TC", - "cost_center": "_Test Cost Center - _TC", - "amount": -500, - }, - ) pe.save() self.assertEqual(pe.unallocated_amount, 0) self.assertEqual(pe.difference_amount, 0) - + self.assertEqual(pe.references[0].exchange_gain_loss, 500) pe.submit() expected_gle = dict( (d[0], d) for d in [ - ["_Test Receivable USD - _TC", 0, 5000, si.name], + ["_Test Receivable USD - _TC", 0, 5500, si.name], ["_Test Bank USD - _TC", 5500, 0, None], - ["_Test Exchange Gain/Loss - _TC", 0, 500, None], ] ) self.validate_gl_entries(pe.name, expected_gle) + # Exchange gain/loss should have been posted through a journal + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + + self.assertEqual(exc_je_for_si, exc_je_for_pe) outstanding_amount = flt(frappe.db.get_value("Sales Invoice", si.name, "outstanding_amount")) self.assertEqual(outstanding_amount, 0) From 220bf245551f84b919040beafa3bcb3d45344ec6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 26 Jun 2023 17:34:28 +0530 Subject: [PATCH 11/54] refactor: exc booking logic for Journal Entry (cherry picked from commit 7b516f84636e7219ac17d972c80a8286c385e954) --- erpnext/accounts/utils.py | 3 + erpnext/controllers/accounts_controller.py | 78 +++++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index adb789e7493..034a404564f 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -459,6 +459,9 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n # update ref in advance entry if voucher_type == "Journal Entry": update_reference_in_journal_entry(entry, doc, do_not_save=True) + # advance section in sales/purchase invoice and reconciliation tool,both pass on exchange gain/loss + # amount and account in args + doc.make_exchange_gain_loss_journal(args) else: update_reference_in_payment_entry( entry, doc, do_not_save=True, skip_ref_details_update_for_pe=skip_ref_details_update_for_pe diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index f9d7b0deb50..a14c571241e 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -962,13 +962,89 @@ class AccountsController(TransactionBase): d.exchange_gain_loss = difference - def make_exchange_gain_loss_journal(self) -> None: + def make_exchange_gain_loss_journal(self, args=None) -> None: """ Make Exchange Gain/Loss journal for Invoices and Payments """ # Cancelling existing exchange gain/loss journals is handled in on_cancel event in accounts/utils.py if self.docstatus == 1: + if self.get("doctype") == "Journal Entry": + if args: + for arg in args: + print(arg) + if arg.get("difference_amount") != 0 and arg.get("difference_account"): + journal_entry = frappe.new_doc("Journal Entry") + journal_entry.voucher_type = "Exchange Gain Or Loss" + journal_entry.company = self.company + journal_entry.posting_date = nowdate() + journal_entry.multi_currency = 1 + + party_account = arg.account + party_account_currency = frappe.get_cached_value( + "Account", party_account, "account_currency" + ) + dr_or_cr = "debit" if arg.difference_amount > 0 else "credit" + + if arg.reference_doctype == "Purchase Invoice": + dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + + reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + + gain_loss_account = arg.difference_account + + if not gain_loss_account: + frappe.throw( + _("Please set default Exchange Gain/Loss Account in Company {}").format( + self.get("company") + ) + ) + + gain_loss_account_currency = get_account_currency(gain_loss_account) + if gain_loss_account_currency != self.company_currency: + frappe.throw( + _("Currency for {0} must be {1}").format(gain_loss_account, self.company_currency) + ) + + journal_account = frappe._dict( + { + "account": party_account, + "party_type": arg.party_type, + "party": arg.party, + "account_currency": party_account_currency, + "exchange_rate": 0, + "cost_center": erpnext.get_default_cost_center(self.company), + "reference_type": arg.against_voucher_type, + "reference_name": arg.against_voucher, + "reference_detail_no": arg.idx, + dr_or_cr: abs(arg.difference_amount), + dr_or_cr + "_in_account_currency": 0, + } + ) + + journal_entry.append("accounts", journal_account) + + journal_account = frappe._dict( + { + "account": gain_loss_account, + "account_currency": gain_loss_account_currency, + "exchange_rate": 1, + "cost_center": erpnext.get_default_cost_center(self.company), + # TODO: figure out a way to pass reference + # "reference_type": self.doctype, + # "reference_name": self.name, + # "reference_detail_no": arg.idx, + reverse_dr_or_cr + "_in_account_currency": abs(arg.difference_amount), + reverse_dr_or_cr: abs(arg.difference_amount), + } + ) + + journal_entry.append("accounts", journal_account) + + journal_entry.save() + journal_entry.submit() + if self.get("doctype") == "Payment Entry": + # For Payment Entry, exchange_gain_loss field in the `reference` table is the trigger for journal creation gain_loss_to_book = [x for x in self.references if x.exchange_gain_loss != 0] booked = [] if gain_loss_to_book: From 72e88d22ede487a93b4b5e525fbe76c6b2ee1a86 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 26 Jun 2023 21:43:20 +0530 Subject: [PATCH 12/54] chore: remove debugging statements and fixing failing unit tests (cherry picked from commit ee3ce82ea82df9dd2910e4d29a5c2c4f885be393) --- erpnext/controllers/accounts_controller.py | 23 +++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index a14c571241e..51c5c83f2b3 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -971,26 +971,25 @@ class AccountsController(TransactionBase): if self.get("doctype") == "Journal Entry": if args: for arg in args: - print(arg) - if arg.get("difference_amount") != 0 and arg.get("difference_account"): + if arg.get("difference_amount", 0) != 0 and arg.get("difference_account"): journal_entry = frappe.new_doc("Journal Entry") journal_entry.voucher_type = "Exchange Gain Or Loss" journal_entry.company = self.company journal_entry.posting_date = nowdate() journal_entry.multi_currency = 1 - party_account = arg.account + party_account = arg.get("account") party_account_currency = frappe.get_cached_value( "Account", party_account, "account_currency" ) - dr_or_cr = "debit" if arg.difference_amount > 0 else "credit" + dr_or_cr = "debit" if arg.get("difference_amount") > 0 else "credit" if arg.reference_doctype == "Purchase Invoice": dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" - gain_loss_account = arg.difference_account + gain_loss_account = arg.get("difference_account") if not gain_loss_account: frappe.throw( @@ -1008,14 +1007,14 @@ class AccountsController(TransactionBase): journal_account = frappe._dict( { "account": party_account, - "party_type": arg.party_type, - "party": arg.party, + "party_type": arg.get("party_type"), + "party": arg.get("party"), "account_currency": party_account_currency, "exchange_rate": 0, "cost_center": erpnext.get_default_cost_center(self.company), - "reference_type": arg.against_voucher_type, - "reference_name": arg.against_voucher, - "reference_detail_no": arg.idx, + "reference_type": arg.get("against_voucher_type"), + "reference_name": arg.get("against_voucher"), + "reference_detail_no": arg.get("idx"), dr_or_cr: abs(arg.difference_amount), dr_or_cr + "_in_account_currency": 0, } @@ -1033,8 +1032,8 @@ class AccountsController(TransactionBase): # "reference_type": self.doctype, # "reference_name": self.name, # "reference_detail_no": arg.idx, - reverse_dr_or_cr + "_in_account_currency": abs(arg.difference_amount), - reverse_dr_or_cr: abs(arg.difference_amount), + reverse_dr_or_cr + "_in_account_currency": abs(arg.get("difference_amount")), + reverse_dr_or_cr: abs(arg.get("difference_amount")), } ) From e2c35f8c856d9ffab14d616778c49ee739c773c3 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 11:16:52 +0530 Subject: [PATCH 13/54] refactor(test): assert Exc journal when reconciling Journa to invoic (cherry picked from commit 389cadf15715b1483986297b38ec2dbb268d2b26) --- .../test_payment_reconciliation.py | 16 +++++++++++++--- erpnext/controllers/accounts_controller.py | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py index 2ac7df0e39b..1d843abde1d 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -686,14 +686,24 @@ class TestPaymentReconciliation(FrappeTestCase): # Check if difference journal entry gets generated for difference amount after reconciliation pr.reconcile() - total_debit_amount = frappe.db.get_all( + total_credit_amount = frappe.db.get_all( "Journal Entry Account", {"account": self.debtors_eur, "docstatus": 1, "reference_name": si.name}, - "sum(debit) as amount", + "sum(credit) as amount", group_by="reference_name", )[0].amount - self.assertEqual(flt(total_debit_amount, 2), -500) + # total credit includes the exchange gain/loss amount + self.assertEqual(flt(total_credit_amount, 2), 8500) + + jea_parent = frappe.db.get_all( + "Journal Entry Account", + filters={"account": self.debtors_eur, "docstatus": 1, "reference_name": si.name, "credit": 500}, + fields=["parent"], + )[0] + self.assertEqual( + frappe.db.get_value("Journal Entry", jea_parent.parent, "voucher_type"), "Exchange Gain Or Loss" + ) def test_difference_amount_via_payment_entry(self): # Make Sale Invoice diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 51c5c83f2b3..fa64b2f22e1 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1032,8 +1032,8 @@ class AccountsController(TransactionBase): # "reference_type": self.doctype, # "reference_name": self.name, # "reference_detail_no": arg.idx, - reverse_dr_or_cr + "_in_account_currency": abs(arg.get("difference_amount")), reverse_dr_or_cr: abs(arg.get("difference_amount")), + reverse_dr_or_cr + "_in_account_currency": 0, } ) From 00a26ea6e6ae26de08699682851e2067a0932351 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 11:41:14 +0530 Subject: [PATCH 14/54] refactor(test): payment will have same exch rate - no gain/loss while making payment entry using reference to sales/purchase invoice, it herits the parent docs exchange rate. so, there will be no exchange gain/loss (cherry picked from commit ee2d1fa36e24326aa9f5b11877139857ed3a6f21) --- .../accounts/doctype/payment_request/test_payment_request.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/payment_request/test_payment_request.py b/erpnext/accounts/doctype/payment_request/test_payment_request.py index e17a846dd81..feb2fdffc95 100644 --- a/erpnext/accounts/doctype/payment_request/test_payment_request.py +++ b/erpnext/accounts/doctype/payment_request/test_payment_request.py @@ -144,8 +144,7 @@ class TestPaymentRequest(unittest.TestCase): (d[0], d) for d in [ ["_Test Receivable USD - _TC", 0, 5000, si_usd.name], - [pr.payment_account, 6290.0, 0, None], - ["_Test Exchange Gain/Loss - _TC", 0, 1290, None], + [pr.payment_account, 5000.0, 0, None], ] ) From f6bb6b78db2854c53550f5cf69f82f47f8a7a183 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 12:29:02 +0530 Subject: [PATCH 15/54] refactor: only post on base currency for exchange gain/loss (cherry picked from commit 78bc712756bc9d8966c22cbbce68e5058daa87db) --- erpnext/controllers/accounts_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index fa64b2f22e1..43813882e58 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1144,7 +1144,7 @@ class AccountsController(TransactionBase): "reference_type": self.doctype, "reference_name": self.name, "reference_detail_no": d.idx, - reverse_dr_or_cr + "_in_account_currency": abs(d.exchange_gain_loss), + reverse_dr_or_cr + "_in_account_currency": 0, reverse_dr_or_cr: abs(d.exchange_gain_loss), } ) From 4025442491103d771689a4fe701814f4d737b34e Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 12:42:07 +0530 Subject: [PATCH 16/54] refactor(test): exc gain/loss journal for advance in purchase invoice (cherry picked from commit 5b06bd1af4197b0c6ab8714c65d8f7a578499163) --- .../purchase_invoice/test_purchase_invoice.py | 63 +++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index ab2e3cf103c..f60c83dcf5c 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -1264,10 +1264,11 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): pi.save() pi.submit() + creditors_account = pi.credit_to + expected_gle = [ ["_Test Account Cost for Goods Sold - _TC", 37500.0], - ["_Test Payable USD - _TC", -35000.0], - ["Exchange Gain/Loss - _TC", -2500.0], + ["_Test Payable USD - _TC", -37500.0], ] gl_entries = frappe.db.sql( @@ -1284,6 +1285,31 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): self.assertEqual(expected_gle[i][0], gle.account) self.assertEqual(expected_gle[i][1], gle.balance) + pi.reload() + self.assertEqual(pi.outstanding_amount, 0) + + total_debit_amount = frappe.db.get_all( + "Journal Entry Account", + {"account": creditors_account, "docstatus": 1, "reference_name": pi.name}, + "sum(debit) as amount", + group_by="reference_name", + )[0].amount + self.assertEqual(flt(total_debit_amount, 2), 2500) + jea_parent = frappe.db.get_all( + "Journal Entry Account", + filters={ + "account": creditors_account, + "docstatus": 1, + "reference_name": pi.name, + "debit": 2500, + "debit_in_account_currency": 0, + }, + fields=["parent"], + )[0] + self.assertEqual( + frappe.db.get_value("Journal Entry", jea_parent.parent, "voucher_type"), "Exchange Gain Or Loss" + ) + pi_2 = make_purchase_invoice( supplier="_Test Supplier USD", currency="USD", @@ -1308,10 +1334,12 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): pi_2.save() pi_2.submit() + pi_2.reload() + self.assertEqual(pi_2.outstanding_amount, 0) + expected_gle = [ ["_Test Account Cost for Goods Sold - _TC", 36500.0], - ["_Test Payable USD - _TC", -35000.0], - ["Exchange Gain/Loss - _TC", -1500.0], + ["_Test Payable USD - _TC", -36500.0], ] gl_entries = frappe.db.sql( @@ -1342,12 +1370,39 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): self.assertEqual(expected_gle[i][0], gle.account) self.assertEqual(expected_gle[i][1], gle.balance) + total_debit_amount = frappe.db.get_all( + "Journal Entry Account", + {"account": creditors_account, "docstatus": 1, "reference_name": pi_2.name}, + "sum(debit) as amount", + group_by="reference_name", + )[0].amount + self.assertEqual(flt(total_debit_amount, 2), 1500) + jea_parent_2 = frappe.db.get_all( + "Journal Entry Account", + filters={ + "account": creditors_account, + "docstatus": 1, + "reference_name": pi_2.name, + "debit": 1500, + "debit_in_account_currency": 0, + }, + fields=["parent"], + )[0] + self.assertEqual( + frappe.db.get_value("Journal Entry", jea_parent_2.parent, "voucher_type"), + "Exchange Gain Or Loss", + ) + pi.reload() pi.cancel() + self.assertEqual(frappe.db.get_value("Journal Entry", jea_parent.parent, "docstatus"), 2) + pi_2.reload() pi_2.cancel() + self.assertEqual(frappe.db.get_value("Journal Entry", jea_parent_2.parent, "docstatus"), 2) + pay.reload() pay.cancel() From e20b21373778f9f2df3645424387a2def42cd1eb Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 16:11:03 +0530 Subject: [PATCH 17/54] refactor(test): difference amount no updated for exchange gain/loss (cherry picked from commit 72bc5b3a11528611db8a322d68c0ecc422b570c6) # Conflicts: # erpnext/accounts/test/test_utils.py --- erpnext/accounts/test/test_utils.py | 53 +++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/erpnext/accounts/test/test_utils.py b/erpnext/accounts/test/test_utils.py index 882cd694a32..f72ac783bd3 100644 --- a/erpnext/accounts/test/test_utils.py +++ b/erpnext/accounts/test/test_utils.py @@ -73,6 +73,59 @@ class TestUtils(unittest.TestCase): sorted_vouchers = sort_stock_vouchers_by_posting_date(list(reversed(vouchers))) self.assertEqual(sorted_vouchers, vouchers) +<<<<<<< HEAD +======= + def test_update_reference_in_payment_entry(self): + item = make_item().name + + purchase_invoice = make_purchase_invoice( + item=item, supplier="_Test Supplier USD", currency="USD", conversion_rate=82.32, do_not_submit=1 + ) + purchase_invoice.credit_to = "_Test Payable USD - _TC" + purchase_invoice.submit() + + payment_entry = get_payment_entry(purchase_invoice.doctype, purchase_invoice.name) + payment_entry.paid_amount = 15725 + payment_entry.deductions = [] + payment_entry.save() + + # below is the difference between base_received_amount and base_paid_amount + self.assertEqual(payment_entry.difference_amount, -4855.0) + + payment_entry.target_exchange_rate = 62.9 + payment_entry.save() + + # below is due to change in exchange rate + self.assertEqual(payment_entry.references[0].exchange_gain_loss, -4855.0) + + payment_entry.references = [] + self.assertEqual(payment_entry.difference_amount, 0.0) + payment_entry.submit() + + payment_reconciliation = frappe.new_doc("Payment Reconciliation") + payment_reconciliation.company = payment_entry.company + payment_reconciliation.party_type = "Supplier" + payment_reconciliation.party = purchase_invoice.supplier + payment_reconciliation.receivable_payable_account = payment_entry.paid_to + payment_reconciliation.get_unreconciled_entries() + payment_reconciliation.allocate_entries( + { + "payments": [d.__dict__ for d in payment_reconciliation.payments], + "invoices": [d.__dict__ for d in payment_reconciliation.invoices], + } + ) + for d in payment_reconciliation.invoices: + # Reset invoice outstanding_amount because allocate_entries will zero this value out. + d.outstanding_amount = d.amount + for d in payment_reconciliation.allocation: + d.difference_account = "Exchange Gain/Loss - _TC" + payment_reconciliation.reconcile() + + payment_entry.load_from_db() + self.assertEqual(len(payment_entry.references), 1) + self.assertEqual(payment_entry.difference_amount, 0) + +>>>>>>> 72bc5b3a11 (refactor(test): difference amount no updated for exchange gain/loss) ADDRESS_RECORDS = [ { From 86aead3d45bdf88c081039149f1ab034968555da Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 16:57:38 +0530 Subject: [PATCH 18/54] refactor: remove call for setting deductions in payment entry (cherry picked from commit 1bcb728c850c67f3e479eb402ce1296dc215496b) # Conflicts: # erpnext/accounts/utils.py --- erpnext/accounts/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 034a404564f..38ba5095b70 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -636,6 +636,7 @@ def update_reference_in_payment_entry( new_row.docstatus = 1 new_row.update(reference_details) +<<<<<<< HEAD payment_entry.flags.ignore_validate_update_after_submit = True payment_entry.setup_party_account_field() payment_entry.set_missing_values() @@ -652,6 +653,8 @@ def update_reference_in_payment_entry( payment_entry.set_gain_or_loss(account_details=account_details) +======= +>>>>>>> 1bcb728c85 (refactor: remove call for setting deductions in payment entry) payment_entry.flags.ignore_validate_update_after_submit = True payment_entry.setup_party_account_field() payment_entry.set_missing_values() From 075a7dfe2e15f8cf375d01aa324c4e2c6d93aa57 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 10 Jul 2023 15:28:10 +0530 Subject: [PATCH 19/54] chore: code cleanup (cherry picked from commit cd42b268391113d5d5b10d75a6e2562736e43aae) --- .../payment_reconciliation/payment_reconciliation.py | 6 ------ erpnext/controllers/accounts_controller.py | 8 ++++++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index ab03699785a..1f3d4826adc 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -347,12 +347,6 @@ class PaymentReconciliation(Document): payment_details = self.get_payment_details(row, dr_or_cr) reconciled_entry.append(payment_details) - # if payment_details.difference_amount and row.reference_type not in [ - # "Sales Invoice", - # "Purchase Invoice", - # ]: - # self.make_difference_entry(payment_details) - if entry_list: reconcile_against_document(entry_list, skip_ref_details_update_for_pe) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 43813882e58..e50ab011828 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -966,9 +966,12 @@ class AccountsController(TransactionBase): """ Make Exchange Gain/Loss journal for Invoices and Payments """ - # Cancelling existing exchange gain/loss journals is handled in on_cancel event in accounts/utils.py + # Cancelling existing exchange gain/loss journals is handled during the `on_cancel` event. + # see accounts/utils.py:cancel_exchange_gain_loss_journal() if self.docstatus == 1: if self.get("doctype") == "Journal Entry": + # 'args' is populated with exchange gain/loss account and the amount to be booked. + # These are generated by Sales/Purchase Invoice during reconciliation and advance allocation. if args: for arg in args: if arg.get("difference_amount", 0) != 0 and arg.get("difference_account"): @@ -1029,6 +1032,7 @@ class AccountsController(TransactionBase): "exchange_rate": 1, "cost_center": erpnext.get_default_cost_center(self.company), # TODO: figure out a way to pass reference + # throws 'Journal Entry doesn't have {account} or doesn't have matched account' # "reference_type": self.doctype, # "reference_name": self.name, # "reference_detail_no": arg.idx, @@ -1043,7 +1047,7 @@ class AccountsController(TransactionBase): journal_entry.submit() if self.get("doctype") == "Payment Entry": - # For Payment Entry, exchange_gain_loss field in the `reference` table is the trigger for journal creation + # For Payment Entry, exchange_gain_loss field in the `references` table is the trigger for journal creation gain_loss_to_book = [x for x in self.references if x.exchange_gain_loss != 0] booked = [] if gain_loss_to_book: From 01953bc0e3d4e51b1b2d033857c7384bc94cc184 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jul 2023 12:04:13 +0530 Subject: [PATCH 20/54] refactor: linkage between journal as payment and gain/loss journal (cherry picked from commit f119a1e11553a0357f937bd23a397757f3f5b54f) # Conflicts: # erpnext/accounts/doctype/gl_entry/gl_entry.py --- erpnext/accounts/doctype/gl_entry/gl_entry.py | 11 +++++++++++ .../accounts/doctype/journal_entry/journal_entry.py | 11 ++++++----- erpnext/controllers/accounts_controller.py | 6 +++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/erpnext/accounts/doctype/gl_entry/gl_entry.py b/erpnext/accounts/doctype/gl_entry/gl_entry.py index fa4a66aaacf..26f4f2ba75f 100644 --- a/erpnext/accounts/doctype/gl_entry/gl_entry.py +++ b/erpnext/accounts/doctype/gl_entry/gl_entry.py @@ -58,7 +58,18 @@ class GLEntry(Document): validate_balance_type(self.account, adv_adj) validate_frozen_account(self.account, adv_adj) +<<<<<<< HEAD if frappe.db.get_value("Account", self.account, "account_type") not in [ +======= + if ( + self.voucher_type == "Journal Entry" + and frappe.get_cached_value("Journal Entry", self.voucher_no, "voucher_type") + == "Exchange Gain Or Loss" + ): + return + + if frappe.get_cached_value("Account", self.account, "account_type") not in [ +>>>>>>> f119a1e115 (refactor: linkage between journal as payment and gain/loss journal) "Receivable", "Payable", ]: diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 8c5cc2c921f..90f9e820d93 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -487,11 +487,12 @@ class JournalEntry(AccountsController): ) if not against_entries: - frappe.throw( - _( - "Journal Entry {0} does not have account {1} or already matched against other voucher" - ).format(d.reference_name, d.account) - ) + if self.voucher_type != "Exchange Gain Or Loss": + frappe.throw( + _( + "Journal Entry {0} does not have account {1} or already matched against other voucher" + ).format(d.reference_name, d.account) + ) else: dr_or_cr = "debit" if d.credit > 0 else "credit" valid = False diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index e50ab011828..647a984d2de 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1033,9 +1033,9 @@ class AccountsController(TransactionBase): "cost_center": erpnext.get_default_cost_center(self.company), # TODO: figure out a way to pass reference # throws 'Journal Entry doesn't have {account} or doesn't have matched account' - # "reference_type": self.doctype, - # "reference_name": self.name, - # "reference_detail_no": arg.idx, + "reference_type": self.doctype, + "reference_name": self.name, + "reference_detail_no": arg.idx, reverse_dr_or_cr: abs(arg.get("difference_amount")), reverse_dr_or_cr + "_in_account_currency": 0, } From 7c3fc7eb3bc82942774e161e695de6112b57535f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jul 2023 12:21:10 +0530 Subject: [PATCH 21/54] refactor: cancel gain/loss JE on Journal as payment cancellation (cherry picked from commit 6e18bb6456b3a7a2cbad89b86dcc124978337e4d) --- .../doctype/journal_entry/journal_entry.py | 5 ++- erpnext/accounts/utils.py | 2 +- erpnext/controllers/accounts_controller.py | 11 +++--- .../tests/test_accounts_controller.py | 34 ++++++++++++++++--- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 90f9e820d93..74349626248 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -87,9 +87,8 @@ class JournalEntry(AccountsController): self.update_invoice_discounting() def on_cancel(self): - from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries - - unlink_ref_doc_from_payment_entries(self) + # References for this Journal are removed on the `on_cancel` event in accounts_controller + super(JournalEntry, self).on_cancel() self.ignore_linked_doctypes = ( "GL Entry", "Stock Ledger Entry", diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 38ba5095b70..9da0d7d3399 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -671,7 +671,7 @@ def cancel_exchange_gain_loss_journal(parent_doc: dict | object) -> None: """ Cancel Exchange Gain/Loss for Sales/Purchase Invoice, if they have any. """ - if parent_doc.doctype in ["Sales Invoice", "Purchase Invoice", "Payment Entry"]: + if parent_doc.doctype in ["Sales Invoice", "Purchase Invoice", "Payment Entry", "Journal Entry"]: journals = frappe.db.get_all( "Journal Entry Account", filters={ diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 647a984d2de..c2f723894d4 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -985,10 +985,11 @@ class AccountsController(TransactionBase): party_account_currency = frappe.get_cached_value( "Account", party_account, "account_currency" ) - dr_or_cr = "debit" if arg.get("difference_amount") > 0 else "credit" - if arg.reference_doctype == "Purchase Invoice": - dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + dr_or_cr = "debit" if arg.get("party_type") == "Customer" else "credit" + + # if arg.reference_doctype == "Purchase Invoice": + # dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" @@ -1032,6 +1033,7 @@ class AccountsController(TransactionBase): "exchange_rate": 1, "cost_center": erpnext.get_default_cost_center(self.company), # TODO: figure out a way to pass reference + # TODO: add reference_detail_no field in payment ledger # throws 'Journal Entry doesn't have {account} or doesn't have matched account' "reference_type": self.doctype, "reference_name": self.name, @@ -1157,6 +1159,7 @@ class AccountsController(TransactionBase): journal_entry.save() journal_entry.submit() + # frappe.throw("stopping...") def make_precision_loss_gl_entry(self, gl_entries): round_off_account, round_off_cost_center = get_round_off_account_and_cost_center( @@ -1250,7 +1253,7 @@ class AccountsController(TransactionBase): unlink_ref_doc_from_payment_entries, ) - if self.doctype in ["Sales Invoice", "Purchase Invoice", "Payment Entry"]: + if self.doctype in ["Sales Invoice", "Purchase Invoice", "Payment Entry", "Journal Entry"]: # Cancel Exchange Gain/Loss Journal before unlinking cancel_exchange_gain_loss_journal(self) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 31aa857c8f5..28a569b5246 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -11,6 +11,7 @@ from frappe.utils import add_days, flt, nowdate from erpnext import get_default_cost_center from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry +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.stock.doctype.item.test_item import create_item @@ -20,7 +21,7 @@ def make_customer(customer_name, currency=None): if not frappe.db.exists("Customer", customer_name): customer = frappe.new_doc("Customer") customer.customer_name = customer_name - customer.type = "Individual" + customer.customer_type = "Individual" if currency: customer.default_currency = currency @@ -30,7 +31,22 @@ def make_customer(customer_name, currency=None): return customer_name -class TestAccountsController(FrappeTestCase): +def make_supplier(supplier_name, currency=None): + if not frappe.db.exists("Supplier", supplier_name): + supplier = frappe.new_doc("Supplier") + supplier.supplier_name = supplier_name + supplier.supplier_type = "Individual" + + if currency: + supplier.default_currency = currency + supplier.save() + return supplier.name + else: + return supplier_name + + +# class TestAccountsController(FrappeTestCase): +class TestAccountsController(unittest.TestCase): """ Test Exchange Gain/Loss booking on various scenarios """ @@ -39,11 +55,12 @@ class TestAccountsController(FrappeTestCase): self.create_company() self.create_account() self.create_item() - self.create_customer() + self.create_parties() self.clear_old_entries() def tearDown(self): - frappe.db.rollback() + # frappe.db.rollback() + pass def create_company(self): company_name = "_Test Company MC" @@ -80,9 +97,16 @@ class TestAccountsController(FrappeTestCase): ) self.item = item if isinstance(item, str) else item.item_code + def create_parties(self): + self.create_customer() + self.create_supplier() + def create_customer(self): self.customer = make_customer("_Test MC Customer USD", "USD") + def create_supplier(self): + self.supplier = make_supplier("_Test MC Supplier USD", "USD") + def create_account(self): account_name = "Debtors USD" if not frappe.db.get_value( @@ -215,7 +239,7 @@ class TestAccountsController(FrappeTestCase): return journals def test_01_payment_against_invoice(self): - # Invoice in Foreign Currency + # Sales Invoice in Foreign Currency si = self.create_sales_invoice(qty=1, rate=1) # Payment pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() From 197e5881aa1485f971e80ad2be1d0c80ae9e5ad6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jul 2023 16:34:20 +0530 Subject: [PATCH 22/54] refactor: assert payment ledger outstanding in both currencies (cherry picked from commit 73cc1ba654f39d81b7e1d9769ef6a0a8ceb689fe) --- .../tests/test_accounts_controller.py | 249 +++++++++++------- 1 file changed, 158 insertions(+), 91 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 28a569b5246..fc30c4b8cde 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -5,6 +5,7 @@ import unittest import frappe from frappe import qb +from frappe.query_builder.functions import Sum from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import add_days, flt, nowdate @@ -48,7 +49,15 @@ def make_supplier(supplier_name, currency=None): # class TestAccountsController(FrappeTestCase): class TestAccountsController(unittest.TestCase): """ - Test Exchange Gain/Loss booking on various scenarios + Test Exchange Gain/Loss booking on various scenarios. + Test Cases are numbered for better readbility + + 10 series - Sales Invoice against Payment Entries + 20 series - Sales Invoice against Journals + 30 series - Sales Invoice against Credit Notes + 40 series - Purchase Invoice against Payment Entries + 50 series - Purchase Invoice against Journals + 60 series - Purchase Invoice against Debit Notes """ def setUp(self): @@ -130,7 +139,13 @@ class TestAccountsController(unittest.TestCase): self.debtors_usd = acc.name def create_sales_invoice( - self, qty=1, rate=1, posting_date=nowdate(), do_not_save=False, do_not_submit=False + self, + qty=1, + rate=1, + conversion_rate=80, + posting_date=nowdate(), + do_not_save=False, + do_not_submit=False, ): """ Helper function to populate default values in sales invoice @@ -148,7 +163,7 @@ class TestAccountsController(unittest.TestCase): parent_cost_center=self.cost_center, update_stock=0, currency="USD", - conversion_rate=80, + conversion_rate=conversion_rate, is_pos=0, is_return=0, return_against=None, @@ -238,96 +253,140 @@ class TestAccountsController(unittest.TestCase): ) return journals - def test_01_payment_against_invoice(self): - # Sales Invoice in Foreign Currency - si = self.create_sales_invoice(qty=1, rate=1) - # Payment - pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() - pe.append( - "references", - {"reference_doctype": si.doctype, "reference_name": si.name, "allocated_amount": 1}, + def assert_ledger_outstanding( + self, + voucher_type: str, + voucher_no: str, + outstanding: float, + outstanding_in_account_currency: float, + ) -> None: + """ + Assert outstanding amount based on ledger on both company/base currency and account currency + """ + + ple = qb.DocType("Payment Ledger Entry") + current_outstanding = ( + qb.from_(ple) + .select( + Sum(ple.amount).as_("outstanding"), + Sum(ple.amount_in_account_currency).as_("outstanding_in_account_currency"), + ) + .where( + (ple.against_voucher_type == voucher_type) + & (ple.against_voucher_no == voucher_no) + & (ple.delinked == 0) + ) + .run(as_dict=True)[0] + ) + self.assertEqual(outstanding, current_outstanding.outstanding) + self.assertEqual( + outstanding_in_account_currency, current_outstanding.outstanding_in_account_currency ) - pe = pe.save().submit() - si.reload() - self.assertEqual(si.outstanding_amount, 0) + def test_10_payment_against_sales_invoice(self): + # Sales Invoice in Foreign Currency + rate = 80 + rate_in_account_currency = 1 - # Exchange Gain/Loss Journal should've been created. - exc_je_for_si = self.get_journals_for(si.doctype, si.name) - exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + si = self.create_sales_invoice(qty=1, rate=rate_in_account_currency) - self.assertNotEqual(exc_je_for_si, []) - self.assertEqual(len(exc_je_for_si), 1) - self.assertEqual(len(exc_je_for_pe), 1) - self.assertEqual(exc_je_for_si[0], exc_je_for_pe[0]) + # Test payments with different exchange rates + for exc_rate in [75.9, 83.1, 80.01]: + with self.subTest(exc_rate=exc_rate): + pe = self.create_payment_entry(amount=1, source_exc_rate=exc_rate).save() + pe.append( + "references", + {"reference_doctype": si.doctype, "reference_name": si.name, "allocated_amount": 1}, + ) + pe = pe.save().submit() - # Cancel Payment - pe.cancel() + # Outstanding in both currencies should be '0' + si.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) - si.reload() - self.assertEqual(si.outstanding_amount, 1) + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_si[0], exc_je_for_pe[0]) - # Exchange Gain/Loss Journal should've been cancelled - exc_je_for_si = self.get_journals_for(si.doctype, si.name) - exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + # Cancel Payment + pe.cancel() - self.assertEqual(exc_je_for_si, []) - self.assertEqual(exc_je_for_pe, []) + # outstanding should be same as grand total + si.reload() + self.assertEqual(si.outstanding_amount, rate_in_account_currency) + self.assert_ledger_outstanding(si.doctype, si.name, rate, rate_in_account_currency) - def test_02_advance_against_invoice(self): + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_pe, []) + + def test_11_advance_against_sales_invoice(self): # Advance Payment adv = self.create_payment_entry(amount=1, source_exc_rate=85).save().submit() adv.reload() - # Invoice in Foreign Currency - si = self.create_sales_invoice(qty=1, rate=1, do_not_submit=True) - si.append( - "advances", - { - "doctype": "Sales Invoice Advance", - "reference_type": adv.doctype, - "reference_name": adv.name, - "advance_amount": 1, - "allocated_amount": 1, - "ref_exchange_rate": 85, - "remarks": "Test", - }, - ) - si = si.save() - si = si.submit() + # Sales Invoices in different exchange rates + for exc_rate in [75.9, 83.1, 80.01]: + with self.subTest(exc_rate=exc_rate): + si = self.create_sales_invoice(qty=1, conversion_rate=exc_rate, rate=1, do_not_submit=True) + si.append( + "advances", + { + "doctype": "Sales Invoice Advance", + "reference_type": adv.doctype, + "reference_name": adv.name, + "advance_amount": 1, + "allocated_amount": 1, + "ref_exchange_rate": 85, + "remarks": "Test", + }, + ) + si = si.save() + si = si.submit() - adv.reload() - self.assertEqual(si.outstanding_amount, 0) + # Outstanding in both currencies should be '0' + adv.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) - # Exchange Gain/Loss Journal should've been created. - exc_je_for_si = self.get_journals_for(si.doctype, si.name) - exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) - self.assertNotEqual(exc_je_for_si, []) - self.assertEqual(len(exc_je_for_si), 1) - self.assertEqual(len(exc_je_for_adv), 1) - self.assertEqual(exc_je_for_si, exc_je_for_adv) + # Cancel Invoice + si.cancel() - # Cancel Invoice - si.cancel() + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_adv, []) - # Exchange Gain/Loss Journal should've been cancelled - exc_je_for_si = self.get_journals_for(si.doctype, si.name) - exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) - - self.assertEqual(exc_je_for_si, []) - self.assertEqual(exc_je_for_adv, []) - - def test_03_partial_advance_and_payment_for_invoice(self): + def test_12_partial_advance_and_payment_for_sales_invoice(self): """ - Invoice with partial advance payment, and a normal payment + Sales invoice with partial advance payment, and a normal payment reconciled """ # Partial Advance adv = self.create_payment_entry(amount=1, source_exc_rate=85).save().submit() adv.reload() - # Invoice in Foreign Currency linked with advance - si = self.create_sales_invoice(qty=2, rate=1, do_not_submit=True) + # sales invoice with advance(partial amount) + rate = 80 + rate_in_account_currency = 1 + si = self.create_sales_invoice( + qty=2, conversion_rate=80, rate=rate_in_account_currency, do_not_submit=True + ) si.append( "advances", { @@ -343,19 +402,20 @@ class TestAccountsController(unittest.TestCase): si = si.save() si = si.submit() + # Outstanding should be there in both currencies si.reload() - self.assertEqual(si.outstanding_amount, 1) + self.assertEqual(si.outstanding_amount, 1) # account currency + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) # Exchange Gain/Loss Journal should've been created for the partial advance exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) - self.assertNotEqual(exc_je_for_si, []) self.assertEqual(len(exc_je_for_si), 1) self.assertEqual(len(exc_je_for_adv), 1) self.assertEqual(exc_je_for_si, exc_je_for_adv) - # Payment + # Payment for remaining amount pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() pe.append( "references", @@ -363,13 +423,14 @@ class TestAccountsController(unittest.TestCase): ) pe = pe.save().submit() + # Outstanding in both currencies should be '0' si.reload() self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) # Exchange Gain/Loss Journal should've been created for the payment exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) - self.assertNotEqual(exc_je_for_si, []) # There should be 2 JE's now. One for the advance and one for the payment self.assertEqual(len(exc_je_for_si), 2) @@ -384,21 +445,20 @@ class TestAccountsController(unittest.TestCase): exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) - self.assertEqual(exc_je_for_si, []) self.assertEqual(exc_je_for_pe, []) self.assertEqual(exc_je_for_adv, []) - def test_04_partial_advance_and_payment_for_invoice_with_cancellation(self): + def test_13_partial_advance_and_payment_for_invoice_with_cancellation(self): """ - Invoice with partial advance payment, and a normal payment. Cancel advance and payment. + Invoice with partial advance payment, and a normal payment. Then cancel advance and payment. """ # Partial Advance adv = self.create_payment_entry(amount=1, source_exc_rate=85).save().submit() adv.reload() - # Invoice in Foreign Currency linked with advance - si = self.create_sales_invoice(qty=2, rate=1, do_not_submit=True) + # invoice with advance(partial amount) + si = self.create_sales_invoice(qty=2, conversion_rate=80, rate=1, do_not_submit=True) si.append( "advances", { @@ -414,19 +474,20 @@ class TestAccountsController(unittest.TestCase): si = si.save() si = si.submit() + # Outstanding should be there in both currencies si.reload() - self.assertEqual(si.outstanding_amount, 1) + self.assertEqual(si.outstanding_amount, 1) # account currency + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) # Exchange Gain/Loss Journal should've been created for the partial advance exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) - self.assertNotEqual(exc_je_for_si, []) self.assertEqual(len(exc_je_for_si), 1) self.assertEqual(len(exc_je_for_adv), 1) self.assertEqual(exc_je_for_si, exc_je_for_adv) - # Payment + # Payment(remaining amount) pe = self.create_payment_entry(amount=1, source_exc_rate=75).save() pe.append( "references", @@ -434,13 +495,14 @@ class TestAccountsController(unittest.TestCase): ) pe = pe.save().submit() + # Outstanding should be '0' in both currencies si.reload() self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) # Exchange Gain/Loss Journal should've been created for the payment exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) - self.assertNotEqual(exc_je_for_si, []) # There should be 2 JE's now. One for the advance and one for the payment self.assertEqual(len(exc_je_for_si), 2) @@ -450,21 +512,22 @@ class TestAccountsController(unittest.TestCase): adv.reload() adv.cancel() + # Outstanding should be there in both currencies, since advance is cancelled. si.reload() - self.assertEqual(si.outstanding_amount, 1) + self.assertEqual(si.outstanding_amount, 1) # account currency + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) - # Exchange Gain/Loss Journal for advance should been cancelled self.assertEqual(len(exc_je_for_si), 1) self.assertEqual(len(exc_je_for_pe), 1) self.assertEqual(exc_je_for_adv, []) - def test_05_same_payment_split_against_invoice(self): + def test_14_same_payment_split_against_invoice(self): # Invoice in Foreign Currency - si = self.create_sales_invoice(qty=2, rate=1) + si = self.create_sales_invoice(qty=2, conversion_rate=80, rate=1) # Payment pe = self.create_payment_entry(amount=2, source_exc_rate=75).save() pe.append( @@ -473,13 +536,14 @@ class TestAccountsController(unittest.TestCase): ) pe = pe.save().submit() + # There should be outstanding in both currencies si.reload() self.assertEqual(si.outstanding_amount, 1) + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) # Exchange Gain/Loss Journal should've been created. exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) - self.assertNotEqual(exc_je_for_si, []) self.assertEqual(len(exc_je_for_si), 1) self.assertEqual(len(exc_je_for_pe), 1) @@ -491,32 +555,35 @@ class TestAccountsController(unittest.TestCase): pr.party_type = "Customer" pr.party = self.customer pr.receivable_payable_account = self.debit_usd - pr.get_unreconciled_entries() self.assertEqual(len(pr.invoices), 1) self.assertEqual(len(pr.payments), 1) - - # Test exact payment allocation 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() self.assertEqual(len(pr.invoices), 0) self.assertEqual(len(pr.payments), 0) + # Exc gain/loss journal should have been creaetd for the reconciled amount exc_je_for_si = self.get_journals_for(si.doctype, si.name) exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) self.assertEqual(len(exc_je_for_si), 2) self.assertEqual(len(exc_je_for_pe), 2) self.assertEqual(exc_je_for_si, exc_je_for_pe) + # There should be no outstanding + si.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) + # Cancel Payment pe.reload() pe.cancel() si.reload() self.assertEqual(si.outstanding_amount, 2) + self.assert_ledger_outstanding(si.doctype, si.name, 160.0, 2.0) # Exchange Gain/Loss Journal should've been cancelled exc_je_for_si = self.get_journals_for(si.doctype, si.name) From 8be312b73e155c8f3511e8d798e2562e77545676 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 12 Jul 2023 06:14:17 +0530 Subject: [PATCH 23/54] refactor: dr/cr logic for journals as payments (cherry picked from commit 056724377206f9bdc3e7ac9894fabb0d93bd3176) --- erpnext/controllers/accounts_controller.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index c2f723894d4..a6d7ce43b24 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -986,15 +986,14 @@ class AccountsController(TransactionBase): "Account", party_account, "account_currency" ) - dr_or_cr = "debit" if arg.get("party_type") == "Customer" else "credit" - - # if arg.reference_doctype == "Purchase Invoice": - # dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + if arg.get("difference_amount") > 0: + dr_or_cr = "debit" if arg.get("party_type") == "Customer" else "credit" + else: + dr_or_cr = "credit" if arg.get("party_type") == "Customer" else "debit" reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" gain_loss_account = arg.get("difference_account") - if not gain_loss_account: frappe.throw( _("Please set default Exchange Gain/Loss Account in Company {}").format( From 077d98e0fa20574ca9787f237cd3706db9609d4c Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 12 Jul 2023 06:46:59 +0530 Subject: [PATCH 24/54] refactor: unit tests for journals (cherry picked from commit 5695d6a5a62e634536c51a22e045eb8281a29d9d) --- .../tests/test_accounts_controller.py | 83 +++++++++++++++++-- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index fc30c4b8cde..9e857f04c31 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -216,12 +216,21 @@ class TestAccountsController(unittest.TestCase): return pr def create_journal_entry( - self, acc1=None, acc2=None, amount=0, posting_date=None, cost_center=None + self, + acc1=None, + acc1_exc_rate=None, + acc2_exc_rate=None, + acc2=None, + acc1_amount=0, + acc2_amount=0, + posting_date=None, + cost_center=None, ): je = frappe.new_doc("Journal Entry") je.posting_date = posting_date or nowdate() je.company = self.company je.user_remark = "test" + je.multi_currency = True if not cost_center: cost_center = self.cost_center je.set( @@ -229,15 +238,21 @@ class TestAccountsController(unittest.TestCase): [ { "account": acc1, + "exchange_rate": acc1_exc_rate or 1, "cost_center": cost_center, - "debit_in_account_currency": amount if amount > 0 else 0, - "credit_in_account_currency": abs(amount) if amount < 0 else 0, + "debit_in_account_currency": acc1_amount if acc1_amount > 0 else 0, + "credit_in_account_currency": abs(acc1_amount) if acc1_amount < 0 else 0, + "debit": acc1_amount * acc1_exc_rate if acc1_amount > 0 else 0, + "credit": abs(acc1_amount * acc1_exc_rate) if acc1_amount < 0 else 0, }, { "account": acc2, + "exchange_rate": acc2_exc_rate or 1, "cost_center": cost_center, - "credit_in_account_currency": amount if amount > 0 else 0, - "debit_in_account_currency": abs(amount) if amount < 0 else 0, + "credit_in_account_currency": acc2_amount if acc2_amount > 0 else 0, + "debit_in_account_currency": abs(acc2_amount) if acc2_amount < 0 else 0, + "credit": acc2_amount * acc2_exc_rate if acc2_amount > 0 else 0, + "debit": abs(acc2_amount * acc2_exc_rate) if acc2_amount < 0 else 0, }, ], ) @@ -590,3 +605,61 @@ class TestAccountsController(unittest.TestCase): exc_je_for_pe = self.get_journals_for(pe.doctype, pe.name) self.assertEqual(exc_je_for_si, []) self.assertEqual(exc_je_for_pe, []) + + def test_21_journal_against_sales_invoice(self): + # Invoice in Foreign Currency + si = self.create_sales_invoice(qty=1, conversion_rate=80, rate=1) + # Payment + je = self.create_journal_entry( + acc1=self.debit_usd, + acc1_exc_rate=75, + acc2=self.cash, + acc1_amount=-1, + acc2_amount=-75, + acc2_exc_rate=1, + ) + je.accounts[0].party_type = "Customer" + je.accounts[0].party = self.customer + je = je.save().submit() + + # Reconcile the remaining amount + pr = self.create_payment_reconciliation() + # pr.receivable_payable_account = self.debit_usd + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + 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() + self.assertEqual(len(pr.invoices), 0) + self.assertEqual(len(pr.payments), 0) + + # There should be no outstanding in both currencies + si.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_je = self.get_journals_for(je.doctype, je.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual( + len(exc_je_for_si), 2 + ) # payment also has reference. so, there are 2 journals referencing invoice + self.assertEqual(len(exc_je_for_je), 1) + self.assertIn(exc_je_for_je[0], exc_je_for_si) + + # Cancel Payment + je.reload() + je.cancel() + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) + + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_je = self.get_journals_for(je.doctype, je.name) + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_je, []) From 513721c33836736ddc48a213b787e0c4c7ae9706 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 14 Jul 2023 16:51:42 +0530 Subject: [PATCH 25/54] refactor: handle diff amount in various names (cherry picked from commit f4a65cccc48bd15fd732973030451c93629bc84b) --- erpnext/controllers/accounts_controller.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index a6d7ce43b24..c5bf808a2f1 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -974,7 +974,10 @@ class AccountsController(TransactionBase): # These are generated by Sales/Purchase Invoice during reconciliation and advance allocation. if args: for arg in args: - if arg.get("difference_amount", 0) != 0 and arg.get("difference_account"): + # Advance section uses `exchange_gain_loss` and reconciliation uses `difference_amount` + if ( + arg.get("difference_amount", 0) != 0 or arg.get("exchange_gain_loss", 0) != 0 + ) and arg.get("difference_account"): journal_entry = frappe.new_doc("Journal Entry") journal_entry.voucher_type = "Exchange Gain Or Loss" journal_entry.company = self.company @@ -986,7 +989,8 @@ class AccountsController(TransactionBase): "Account", party_account, "account_currency" ) - if arg.get("difference_amount") > 0: + difference_amount = arg.get("difference_amount") or arg.get("exchange_gain_loss") + if difference_amount > 0: dr_or_cr = "debit" if arg.get("party_type") == "Customer" else "credit" else: dr_or_cr = "credit" if arg.get("party_type") == "Customer" else "debit" @@ -1018,7 +1022,7 @@ class AccountsController(TransactionBase): "reference_type": arg.get("against_voucher_type"), "reference_name": arg.get("against_voucher"), "reference_detail_no": arg.get("idx"), - dr_or_cr: abs(arg.difference_amount), + dr_or_cr: abs(difference_amount), dr_or_cr + "_in_account_currency": 0, } ) @@ -1037,7 +1041,7 @@ class AccountsController(TransactionBase): "reference_type": self.doctype, "reference_name": self.name, "reference_detail_no": arg.idx, - reverse_dr_or_cr: abs(arg.get("difference_amount")), + reverse_dr_or_cr: abs(difference_amount), reverse_dr_or_cr + "_in_account_currency": 0, } ) From 4094fbc3e59540cdee2b33771a9914e4139b6190 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 16 Jul 2023 21:29:19 +0530 Subject: [PATCH 26/54] test: journals against sales invoice (cherry picked from commit f3363e813a353363696169602bae5b0a36ae0376) --- .../tests/test_accounts_controller.py | 264 +++++++++++++++++- 1 file changed, 263 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 9e857f04c31..9a7326ea291 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -606,7 +606,7 @@ class TestAccountsController(unittest.TestCase): self.assertEqual(exc_je_for_si, []) self.assertEqual(exc_je_for_pe, []) - def test_21_journal_against_sales_invoice(self): + def test_20_journal_against_sales_invoice(self): # Invoice in Foreign Currency si = self.create_sales_invoice(qty=1, conversion_rate=80, rate=1) # Payment @@ -663,3 +663,265 @@ class TestAccountsController(unittest.TestCase): exc_je_for_je = self.get_journals_for(je.doctype, je.name) self.assertEqual(exc_je_for_si, []) self.assertEqual(exc_je_for_je, []) + + def test_21_advance_journal_against_sales_invoice(self): + # Advance Payment + adv_exc_rate = 80 + adv = self.create_journal_entry( + acc1=self.debit_usd, + acc1_exc_rate=adv_exc_rate, + acc2=self.cash, + acc1_amount=-1, + acc2_amount=adv_exc_rate * -1, + acc2_exc_rate=1, + ) + adv.accounts[0].party_type = "Customer" + adv.accounts[0].party = self.customer + adv.accounts[0].is_advance = "Yes" + adv = adv.save().submit() + adv.reload() + + # Sales Invoices in different exchange rates + for exc_rate in [75.9, 83.1, 80.01]: + with self.subTest(exc_rate=exc_rate): + si = self.create_sales_invoice(qty=1, conversion_rate=exc_rate, rate=1, do_not_submit=True) + si.append( + "advances", + { + "doctype": "Sales Invoice Advance", + "reference_type": adv.doctype, + "reference_name": adv.name, + "reference_row": adv.accounts[0].name, + "advance_amount": 1, + "allocated_amount": 1, + "ref_exchange_rate": adv_exc_rate, + "remarks": "Test", + }, + ) + si = si.save() + si = si.submit() + + # Outstanding in both currencies should be '0' + adv.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = [x for x in self.get_journals_for(si.doctype, si.name) if x.parent != adv.name] + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) + + # Cancel Invoice + si.cancel() + + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_adv, []) + + def test_22_partial_advance_and_payment_for_invoice_with_cancellation(self): + """ + Invoice with partial advance payment as Journal, and a normal payment. Then cancel advance and payment. + """ + # Partial Advance + adv_exc_rate = 75 + adv = self.create_journal_entry( + acc1=self.debit_usd, + acc1_exc_rate=adv_exc_rate, + acc2=self.cash, + acc1_amount=-1, + acc2_amount=adv_exc_rate * -1, + acc2_exc_rate=1, + ) + adv.accounts[0].party_type = "Customer" + adv.accounts[0].party = self.customer + adv.accounts[0].is_advance = "Yes" + adv = adv.save().submit() + adv.reload() + + # invoice with advance(partial amount) + si = self.create_sales_invoice(qty=3, conversion_rate=80, rate=1, do_not_submit=True) + si.append( + "advances", + { + "doctype": "Sales Invoice Advance", + "reference_type": adv.doctype, + "reference_name": adv.name, + "reference_row": adv.accounts[0].name, + "advance_amount": 1, + "allocated_amount": 1, + "ref_exchange_rate": adv_exc_rate, + "remarks": "Test", + }, + ) + si = si.save() + si = si.submit() + + # Outstanding should be there in both currencies + si.reload() + self.assertEqual(si.outstanding_amount, 2) # account currency + self.assert_ledger_outstanding(si.doctype, si.name, 160.0, 2.0) + + # Exchange Gain/Loss Journal should've been created for the partial advance + exc_je_for_si = [x for x in self.get_journals_for(si.doctype, si.name) if x.parent != adv.name] + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) + + # Payment + adv2_exc_rate = 83 + pay = self.create_journal_entry( + acc1=self.debit_usd, + acc1_exc_rate=adv2_exc_rate, + acc2=self.cash, + acc1_amount=-2, + acc2_amount=adv2_exc_rate * -2, + acc2_exc_rate=1, + ) + pay.accounts[0].party_type = "Customer" + pay.accounts[0].party = self.customer + pay.accounts[0].is_advance = "Yes" + pay = pay.save().submit() + pay.reload() + + # Reconcile the remaining amount + pr = self.create_payment_reconciliation() + # pr.receivable_payable_account = self.debit_usd + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + 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() + self.assertEqual(len(pr.invoices), 0) + self.assertEqual(len(pr.payments), 0) + + # Outstanding should be '0' in both currencies + si.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) + + # Exchange Gain/Loss Journal should've been created for the payment + exc_je_for_si = [ + x + for x in self.get_journals_for(si.doctype, si.name) + if x.parent != adv.name and x.parent != pay.name + ] + exc_je_for_pe = self.get_journals_for(pay.doctype, pay.name) + self.assertNotEqual(exc_je_for_si, []) + # There should be 2 JE's now. One for the advance and one for the payment + self.assertEqual(len(exc_je_for_si), 2) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_si, exc_je_for_pe + exc_je_for_adv) + + adv.reload() + adv.cancel() + + # Outstanding should be there in both currencies, since advance is cancelled. + si.reload() + self.assertEqual(si.outstanding_amount, 1) # account currency + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) + + exc_je_for_si = [ + x + for x in self.get_journals_for(si.doctype, si.name) + if x.parent != adv.name and x.parent != pay.name + ] + exc_je_for_pe = self.get_journals_for(pay.doctype, pay.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + # Exchange Gain/Loss Journal for advance should been cancelled + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_pe), 1) + self.assertEqual(exc_je_for_adv, []) + + def test_23_same_journal_split_against_single_invoice(self): + # Invoice in Foreign Currency + si = self.create_sales_invoice(qty=2, conversion_rate=80, rate=1) + # Payment + je = self.create_journal_entry( + acc1=self.debit_usd, + acc1_exc_rate=75, + acc2=self.cash, + acc1_amount=-2, + acc2_amount=-150, + acc2_exc_rate=1, + ) + je.accounts[0].party_type = "Customer" + je.accounts[0].party = self.customer + je = je.save().submit() + + # Reconcile the first half + pr = self.create_payment_reconciliation() + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + 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})) + difference_amount = pr.calculate_difference_on_allocation_change( + [x.as_dict() for x in pr.payments], [x.as_dict() for x in pr.invoices], 1 + ) + pr.allocation[0].allocated_amount = 1 + pr.allocation[0].difference_amount = difference_amount + pr.reconcile() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + + # There should be outstanding in both currencies + si.reload() + self.assertEqual(si.outstanding_amount, 1) + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = [x for x in self.get_journals_for(si.doctype, si.name) if x.parent != je.name] + exc_je_for_je = self.get_journals_for(je.doctype, je.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_je), 1) + self.assertIn(exc_je_for_je[0], exc_je_for_si) + + # reconcile remaining half + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + 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.allocation[0].allocated_amount = 1 + pr.allocation[0].difference_amount = difference_amount + pr.reconcile() + self.assertEqual(len(pr.invoices), 0) + self.assertEqual(len(pr.payments), 0) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = [x for x in self.get_journals_for(si.doctype, si.name) if x.parent != je.name] + exc_je_for_je = self.get_journals_for(je.doctype, je.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 2) + self.assertEqual(len(exc_je_for_je), 2) + self.assertIn(exc_je_for_je[0], exc_je_for_si) + + si.reload() + self.assertEqual(si.outstanding_amount, 0) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) + + # Cancel Payment + je.reload() + je.cancel() + + si.reload() + self.assertEqual(si.outstanding_amount, 2) + self.assert_ledger_outstanding(si.doctype, si.name, 160.0, 2.0) + + # Exchange Gain/Loss Journal should've been cancelled + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_je = self.get_journals_for(je.doctype, je.name) + self.assertEqual(exc_je_for_si, []) + self.assertEqual(exc_je_for_je, []) From 395f87a0f85a32682e30ca492b78387a009e2474 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 17 Jul 2023 12:29:42 +0530 Subject: [PATCH 27/54] chore(test): fix broken unit test (cherry picked from commit 70dd9d0671e1d77d50c814885c6a6f59508c4f62) # Conflicts: # erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py --- .../accounts/doctype/sales_invoice/test_sales_invoice.py | 7 +++++++ erpnext/controllers/tests/test_accounts_controller.py | 4 +--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 277e584aeaf..2e231608588 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3213,9 +3213,11 @@ class TestSalesInvoice(unittest.TestCase): account.disabled = 0 account.save() + @change_settings("Accounts Settings", {"unlink_payment_on_cancel_of_invoice": 1}) def test_gain_loss_with_advance_entry(self): from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry +<<<<<<< HEAD unlink_enabled = frappe.db.get_value( "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice" ) @@ -3224,6 +3226,8 @@ class TestSalesInvoice(unittest.TestCase): "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice", 1 ) +======= +>>>>>>> 70dd9d0671 (chore(test): fix broken unit test) jv = make_journal_entry("_Test Receivable USD - _TC", "_Test Bank - _TC", -7000, save=False) jv.accounts[0].exchange_rate = 70 @@ -3265,10 +3269,13 @@ class TestSalesInvoice(unittest.TestCase): check_gl_entries(self, si.name, expected_gle, nowdate()) +<<<<<<< HEAD frappe.db.set_value( "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice", unlink_enabled ) +======= +>>>>>>> 70dd9d0671 (chore(test): fix broken unit test) def test_batch_expiry_for_sales_invoice_return(self): from erpnext.controllers.sales_and_purchase_return import make_return_doc from erpnext.stock.doctype.item.test_item import make_item diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 9a7326ea291..eefe202e476 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -37,6 +37,7 @@ def make_supplier(supplier_name, currency=None): supplier = frappe.new_doc("Supplier") supplier.supplier_name = supplier_name supplier.supplier_type = "Individual" + supplier.supplier_group = "All Supplier Groups" if currency: supplier.default_currency = currency @@ -55,9 +56,6 @@ class TestAccountsController(unittest.TestCase): 10 series - Sales Invoice against Payment Entries 20 series - Sales Invoice against Journals 30 series - Sales Invoice against Credit Notes - 40 series - Purchase Invoice against Payment Entries - 50 series - Purchase Invoice against Journals - 60 series - Purchase Invoice against Debit Notes """ def setUp(self): From d9219dc7d21a04d50a041c9d1eb6d813afb9b4e7 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 24 Jul 2023 20:41:05 +0530 Subject: [PATCH 28/54] chore(test): fix broken test case (cherry picked from commit 37895a361cdf7be4704f376eb6ec749af0ab3c90) --- erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 2e231608588..eed33693e9b 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3213,7 +3213,7 @@ class TestSalesInvoice(unittest.TestCase): account.disabled = 0 account.save() - @change_settings("Accounts Settings", {"unlink_payment_on_cancel_of_invoice": 1}) + @change_settings("Accounts Settings", {"unlink_payment_on_cancellation_of_invoice": 1}) def test_gain_loss_with_advance_entry(self): from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry From a9faa927965584977ca252c4107dacba09d212a8 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 25 Jul 2023 10:30:08 +0530 Subject: [PATCH 29/54] chore: type info (cherry picked from commit 6628632fbb15ddcc80f5af201d15976337141fc6) --- erpnext/controllers/accounts_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index c5bf808a2f1..af4c47b56c7 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -962,7 +962,7 @@ class AccountsController(TransactionBase): d.exchange_gain_loss = difference - def make_exchange_gain_loss_journal(self, args=None) -> None: + def make_exchange_gain_loss_journal(self, args: dict = None) -> None: """ Make Exchange Gain/Loss journal for Invoices and Payments """ From 57af6d9c21fb8b98d0db29d2cf7e47f1b07122ca Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 25 Jul 2023 10:30:49 +0530 Subject: [PATCH 30/54] refactor: cr/dr note will be on single exchange rate (cherry picked from commit c87332d5da638c43ff6d0560bf3c26dde81e21cf) # Conflicts: # erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py --- .../doctype/journal_entry/journal_entry.py | 29 +++++++++++-------- .../payment_reconciliation.py | 16 ++++++++-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 74349626248..73f0a4c2d45 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -756,18 +756,23 @@ class JournalEntry(AccountsController): ) ): - # Modified to include the posting date for which to retreive the exchange rate - d.exchange_rate = get_exchange_rate( - self.posting_date, - d.account, - d.account_currency, - self.company, - d.reference_type, - d.reference_name, - d.debit, - d.credit, - d.exchange_rate, - ) + ignore_exchange_rate = False + if self.get("flags") and self.flags.get("ignore_exchange_rate"): + ignore_exchange_rate = True + + if not ignore_exchange_rate: + # Modified to include the posting date for which to retreive the exchange rate + d.exchange_rate = get_exchange_rate( + self.posting_date, + d.account, + d.account_currency, + self.company, + d.reference_type, + d.reference_name, + d.debit, + d.credit, + d.exchange_rate, + ) if not d.exchange_rate: frappe.throw(_("Row {0}: Exchange Rate is mandatory").format(d.idx)) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 1f3d4826adc..e4e5aeb1593 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -634,7 +634,11 @@ def reconcile_dr_cr_note(dr_cr_notes, company): "reference_type": inv.against_voucher_type, "reference_name": inv.against_voucher, "cost_center": erpnext.get_default_cost_center(company), +<<<<<<< HEAD "user_remark": f"{fmt_money(flt(inv.allocated_amount), currency=company_currency)} against {inv.against_voucher}", +======= + "exchange_rate": inv.exchange_rate, +>>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) }, { "account": inv.account, @@ -648,17 +652,23 @@ def reconcile_dr_cr_note(dr_cr_notes, company): "reference_type": inv.voucher_type, "reference_name": inv.voucher_no, "cost_center": erpnext.get_default_cost_center(company), +<<<<<<< HEAD "user_remark": f"{fmt_money(flt(inv.allocated_amount), currency=company_currency)} from {inv.voucher_no}", +======= + "exchange_rate": inv.exchange_rate, +>>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) }, ], } ) - if difference_entry := get_difference_row(inv): - jv.append("accounts", difference_entry) - jv.flags.ignore_mandatory = True +<<<<<<< HEAD jv.remark = None jv.flags.skip_remarks_creation = True jv.is_system_generated = True +======= + jv.flags.ignore_exchange_rate = True +>>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) jv.submit() + jv.make_exchange_gain_loss_journal(args=[inv]) From 1999132c28acb9df46e6ec7c889ba8fe3855f6e1 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 25 Jul 2023 10:51:58 +0530 Subject: [PATCH 31/54] refactor: split make_exchage_gain_loss_journal into smaller function (cherry picked from commit c0b3b069b587cff11969112b01fff08c8df7adf0) --- erpnext/controllers/accounts_controller.py | 219 ++++++++++----------- 1 file changed, 103 insertions(+), 116 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index af4c47b56c7..3f82312ef74 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -962,6 +962,78 @@ class AccountsController(TransactionBase): d.exchange_gain_loss = difference + def create_gain_loss_journal( + self, + party_type, + party, + party_account, + gain_loss_account, + exc_gain_loss, + dr_or_cr, + reverse_dr_or_cr, + ref1_dt, + ref1_dn, + ref1_detail_no, + ref2_dt, + ref2_dn, + ref2_detail_no, + ) -> str: + journal_entry = frappe.new_doc("Journal Entry") + journal_entry.voucher_type = "Exchange Gain Or Loss" + journal_entry.company = self.company + journal_entry.posting_date = nowdate() + journal_entry.multi_currency = 1 + + party_account_currency = frappe.get_cached_value("Account", party_account, "account_currency") + + if not gain_loss_account: + frappe.throw( + _("Please set default Exchange Gain/Loss Account in Company {}").format(self.get("company")) + ) + gain_loss_account_currency = get_account_currency(gain_loss_account) + company_currency = frappe.get_cached_value("Company", self.company, "default_currency") + + if gain_loss_account_currency != self.company_currency: + frappe.throw(_("Currency for {0} must be {1}").format(gain_loss_account, company_currency)) + + journal_account = frappe._dict( + { + "account": party_account, + "party_type": party_type, + "party": party, + "account_currency": party_account_currency, + "exchange_rate": 0, + "cost_center": erpnext.get_default_cost_center(self.company), + "reference_type": ref1_dt, + "reference_name": ref1_dn, + "reference_detail_no": ref1_detail_no, + dr_or_cr: abs(exc_gain_loss), + dr_or_cr + "_in_account_currency": 0, + } + ) + + journal_entry.append("accounts", journal_account) + + journal_account = frappe._dict( + { + "account": gain_loss_account, + "account_currency": gain_loss_account_currency, + "exchange_rate": 1, + "cost_center": erpnext.get_default_cost_center(self.company), + "reference_type": ref2_dt, + "reference_name": ref2_dn, + "reference_detail_no": ref2_detail_no, + reverse_dr_or_cr + "_in_account_currency": 0, + reverse_dr_or_cr: abs(exc_gain_loss), + } + ) + + journal_entry.append("accounts", journal_account) + + journal_entry.save() + journal_entry.submit() + return journal_entry.name + def make_exchange_gain_loss_journal(self, args: dict = None) -> None: """ Make Exchange Gain/Loss journal for Invoices and Payments @@ -972,23 +1044,16 @@ class AccountsController(TransactionBase): if self.get("doctype") == "Journal Entry": # 'args' is populated with exchange gain/loss account and the amount to be booked. # These are generated by Sales/Purchase Invoice during reconciliation and advance allocation. + # and below logic is only for such scenarios if args: for arg in args: # Advance section uses `exchange_gain_loss` and reconciliation uses `difference_amount` if ( arg.get("difference_amount", 0) != 0 or arg.get("exchange_gain_loss", 0) != 0 ) and arg.get("difference_account"): - journal_entry = frappe.new_doc("Journal Entry") - journal_entry.voucher_type = "Exchange Gain Or Loss" - journal_entry.company = self.company - journal_entry.posting_date = nowdate() - journal_entry.multi_currency = 1 party_account = arg.get("account") - party_account_currency = frappe.get_cached_value( - "Account", party_account, "account_currency" - ) - + gain_loss_account = arg.get("difference_account") difference_amount = arg.get("difference_amount") or arg.get("exchange_gain_loss") if difference_amount > 0: dr_or_cr = "debit" if arg.get("party_type") == "Customer" else "credit" @@ -997,60 +1062,22 @@ class AccountsController(TransactionBase): reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" - gain_loss_account = arg.get("difference_account") - if not gain_loss_account: - frappe.throw( - _("Please set default Exchange Gain/Loss Account in Company {}").format( - self.get("company") - ) - ) - - gain_loss_account_currency = get_account_currency(gain_loss_account) - if gain_loss_account_currency != self.company_currency: - frappe.throw( - _("Currency for {0} must be {1}").format(gain_loss_account, self.company_currency) - ) - - journal_account = frappe._dict( - { - "account": party_account, - "party_type": arg.get("party_type"), - "party": arg.get("party"), - "account_currency": party_account_currency, - "exchange_rate": 0, - "cost_center": erpnext.get_default_cost_center(self.company), - "reference_type": arg.get("against_voucher_type"), - "reference_name": arg.get("against_voucher"), - "reference_detail_no": arg.get("idx"), - dr_or_cr: abs(difference_amount), - dr_or_cr + "_in_account_currency": 0, - } + self.create_gain_loss_journal( + arg.get("party_type"), + arg.get("party"), + party_account, + gain_loss_account, + difference_amount, + dr_or_cr, + reverse_dr_or_cr, + arg.get("against_voucher_type"), + arg.get("against_voucher"), + arg.get("idx"), + self.doctype, + self.name, + arg.get("idx"), ) - journal_entry.append("accounts", journal_account) - - journal_account = frappe._dict( - { - "account": gain_loss_account, - "account_currency": gain_loss_account_currency, - "exchange_rate": 1, - "cost_center": erpnext.get_default_cost_center(self.company), - # TODO: figure out a way to pass reference - # TODO: add reference_detail_no field in payment ledger - # throws 'Journal Entry doesn't have {account} or doesn't have matched account' - "reference_type": self.doctype, - "reference_name": self.name, - "reference_detail_no": arg.idx, - reverse_dr_or_cr: abs(difference_amount), - reverse_dr_or_cr + "_in_account_currency": 0, - } - ) - - journal_entry.append("accounts", journal_account) - - journal_entry.save() - journal_entry.submit() - if self.get("doctype") == "Payment Entry": # For Payment Entry, exchange_gain_loss field in the `references` table is the trigger for journal creation gain_loss_to_book = [x for x in self.references if x.exchange_gain_loss != 0] @@ -1087,23 +1114,15 @@ class AccountsController(TransactionBase): ) for d in gain_loss_to_book: + # Filter out References for which Gain/Loss is already booked if d.exchange_gain_loss and ( (d.reference_doctype, d.reference_name, str(d.idx)) not in booked ): - journal_entry = frappe.new_doc("Journal Entry") - journal_entry.voucher_type = "Exchange Gain Or Loss" - journal_entry.company = self.company - journal_entry.posting_date = nowdate() - journal_entry.multi_currency = 1 - if self.payment_type == "Receive": party_account = self.paid_from elif self.payment_type == "Pay": party_account = self.paid_to - party_account_currency = frappe.get_cached_value( - "Account", party_account, "account_currency" - ) dr_or_cr = "debit" if d.exchange_gain_loss > 0 else "credit" if d.reference_doctype == "Purchase Invoice": @@ -1114,54 +1133,22 @@ class AccountsController(TransactionBase): gain_loss_account = frappe.get_cached_value( "Company", self.company, "exchange_gain_loss_account" ) - if not gain_loss_account: - frappe.throw( - _("Please set default Exchange Gain/Loss Account in Company {}").format( - self.get("company") - ) - ) - gain_loss_account_currency = get_account_currency(gain_loss_account) - if gain_loss_account_currency != self.company_currency: - frappe.throw( - _("Currency for {0} must be {1}").format(gain_loss_account, self.company_currency) - ) - journal_account = frappe._dict( - { - "account": party_account, - "party_type": self.party_type, - "party": self.party, - "account_currency": party_account_currency, - "exchange_rate": 0, - "cost_center": erpnext.get_default_cost_center(self.company), - "reference_type": d.reference_doctype, - "reference_name": d.reference_name, - "reference_detail_no": d.idx, - dr_or_cr: abs(d.exchange_gain_loss), - dr_or_cr + "_in_account_currency": 0, - } + self.create_gain_loss_journal( + self.party_type, + self.party, + party_account, + gain_loss_account, + d.exchange_gain_loss, + dr_or_cr, + reverse_dr_or_cr, + d.reference_doctype, + d.reference_name, + d.idx, + self.doctype, + self.name, + d.idx, ) - - journal_entry.append("accounts", journal_account) - - journal_account = frappe._dict( - { - "account": gain_loss_account, - "account_currency": gain_loss_account_currency, - "exchange_rate": 1, - "cost_center": erpnext.get_default_cost_center(self.company), - "reference_type": self.doctype, - "reference_name": self.name, - "reference_detail_no": d.idx, - reverse_dr_or_cr + "_in_account_currency": 0, - reverse_dr_or_cr: abs(d.exchange_gain_loss), - } - ) - - journal_entry.append("accounts", journal_account) - - journal_entry.save() - journal_entry.submit() # frappe.throw("stopping...") def make_precision_loss_gl_entry(self, gl_entries): From 22dbe52586c750b9638fe953c5ef899f6250c7f3 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 16:19:38 +0530 Subject: [PATCH 32/54] refactor: convert class method to standalone function (cherry picked from commit 1ea1bfebc4a2407961d93a6d0c4c6c9f43202689) --- erpnext/accounts/utils.py | 71 ++++++++++++++++++ erpnext/controllers/accounts_controller.py | 85 +++------------------- 2 files changed, 81 insertions(+), 75 deletions(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 9da0d7d3399..d8235cb256d 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -1845,3 +1845,74 @@ class QueryPaymentLedger(object): self.query_for_outstanding() return self.voucher_outstandings + + +def create_gain_loss_journal( + company, + party_type, + party, + party_account, + gain_loss_account, + exc_gain_loss, + dr_or_cr, + reverse_dr_or_cr, + ref1_dt, + ref1_dn, + ref1_detail_no, + ref2_dt, + ref2_dn, + ref2_detail_no, +) -> str: + journal_entry = frappe.new_doc("Journal Entry") + journal_entry.voucher_type = "Exchange Gain Or Loss" + journal_entry.company = company + journal_entry.posting_date = nowdate() + journal_entry.multi_currency = 1 + + party_account_currency = frappe.get_cached_value("Account", party_account, "account_currency") + + if not gain_loss_account: + frappe.throw(_("Please set default Exchange Gain/Loss Account in Company {}").format(company)) + gain_loss_account_currency = get_account_currency(gain_loss_account) + company_currency = frappe.get_cached_value("Company", company, "default_currency") + + if gain_loss_account_currency != company_currency: + frappe.throw(_("Currency for {0} must be {1}").format(gain_loss_account, company_currency)) + + journal_account = frappe._dict( + { + "account": party_account, + "party_type": party_type, + "party": party, + "account_currency": party_account_currency, + "exchange_rate": 0, + "cost_center": erpnext.get_default_cost_center(company), + "reference_type": ref1_dt, + "reference_name": ref1_dn, + "reference_detail_no": ref1_detail_no, + dr_or_cr: abs(exc_gain_loss), + dr_or_cr + "_in_account_currency": 0, + } + ) + + journal_entry.append("accounts", journal_account) + + journal_account = frappe._dict( + { + "account": gain_loss_account, + "account_currency": gain_loss_account_currency, + "exchange_rate": 1, + "cost_center": erpnext.get_default_cost_center(company), + "reference_type": ref2_dt, + "reference_name": ref2_dn, + "reference_detail_no": ref2_detail_no, + reverse_dr_or_cr + "_in_account_currency": 0, + reverse_dr_or_cr: abs(exc_gain_loss), + } + ) + + journal_entry.append("accounts", journal_account) + + journal_entry.save() + journal_entry.submit() + return journal_entry.name diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 3f82312ef74..a61795fd450 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -38,7 +38,12 @@ from erpnext.accounts.party import ( get_party_gle_currency, validate_party_frozen_disabled, ) -from erpnext.accounts.utils import get_account_currency, get_fiscal_years, validate_fiscal_year +from erpnext.accounts.utils import ( + create_gain_loss_journal, + get_account_currency, + get_fiscal_years, + validate_fiscal_year, +) from erpnext.buying.utils import update_last_purchase_rate from erpnext.controllers.print_settings import ( set_print_templates_for_item_table, @@ -962,78 +967,6 @@ class AccountsController(TransactionBase): d.exchange_gain_loss = difference - def create_gain_loss_journal( - self, - party_type, - party, - party_account, - gain_loss_account, - exc_gain_loss, - dr_or_cr, - reverse_dr_or_cr, - ref1_dt, - ref1_dn, - ref1_detail_no, - ref2_dt, - ref2_dn, - ref2_detail_no, - ) -> str: - journal_entry = frappe.new_doc("Journal Entry") - journal_entry.voucher_type = "Exchange Gain Or Loss" - journal_entry.company = self.company - journal_entry.posting_date = nowdate() - journal_entry.multi_currency = 1 - - party_account_currency = frappe.get_cached_value("Account", party_account, "account_currency") - - if not gain_loss_account: - frappe.throw( - _("Please set default Exchange Gain/Loss Account in Company {}").format(self.get("company")) - ) - gain_loss_account_currency = get_account_currency(gain_loss_account) - company_currency = frappe.get_cached_value("Company", self.company, "default_currency") - - if gain_loss_account_currency != self.company_currency: - frappe.throw(_("Currency for {0} must be {1}").format(gain_loss_account, company_currency)) - - journal_account = frappe._dict( - { - "account": party_account, - "party_type": party_type, - "party": party, - "account_currency": party_account_currency, - "exchange_rate": 0, - "cost_center": erpnext.get_default_cost_center(self.company), - "reference_type": ref1_dt, - "reference_name": ref1_dn, - "reference_detail_no": ref1_detail_no, - dr_or_cr: abs(exc_gain_loss), - dr_or_cr + "_in_account_currency": 0, - } - ) - - journal_entry.append("accounts", journal_account) - - journal_account = frappe._dict( - { - "account": gain_loss_account, - "account_currency": gain_loss_account_currency, - "exchange_rate": 1, - "cost_center": erpnext.get_default_cost_center(self.company), - "reference_type": ref2_dt, - "reference_name": ref2_dn, - "reference_detail_no": ref2_detail_no, - reverse_dr_or_cr + "_in_account_currency": 0, - reverse_dr_or_cr: abs(exc_gain_loss), - } - ) - - journal_entry.append("accounts", journal_account) - - journal_entry.save() - journal_entry.submit() - return journal_entry.name - def make_exchange_gain_loss_journal(self, args: dict = None) -> None: """ Make Exchange Gain/Loss journal for Invoices and Payments @@ -1062,7 +995,8 @@ class AccountsController(TransactionBase): reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" - self.create_gain_loss_journal( + create_gain_loss_journal( + self.company, arg.get("party_type"), arg.get("party"), party_account, @@ -1134,7 +1068,8 @@ class AccountsController(TransactionBase): "Company", self.company, "exchange_gain_loss_account" ) - self.create_gain_loss_journal( + create_gain_loss_journal( + self.company, self.party_type, self.party, party_account, From 72a507f888aeac4d48701a7bdc7ceb46b3056d58 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 16:46:50 +0530 Subject: [PATCH 33/54] refactor: create gain/loss on Cr/Dr notes with different exc rates (cherry picked from commit ba1f065765db6fc36358281fb4e4d775f1c1dcb1) --- .../doctype/journal_entry/journal_entry.py | 4 ++- .../payment_reconciliation.py | 27 ++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 73f0a4c2d45..e20575f31a1 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -574,7 +574,9 @@ class JournalEntry(AccountsController): else: party_account = against_voucher[1] - if against_voucher[0] != cstr(d.party) or party_account != d.account: + if ( + against_voucher[0] != cstr(d.party) or party_account != d.account + ) and self.voucher_type != "Exchange Gain Or Loss": frappe.throw( _("Row {0}: Party / Account does not match with {1} / {2} in {3} {4}").format( d.idx, diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index e4e5aeb1593..774d674b5ee 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -14,6 +14,7 @@ from erpnext.accounts.doctype.process_payment_reconciliation.process_payment_rec ) from erpnext.accounts.utils import ( QueryPaymentLedger, + create_gain_loss_journal, get_outstanding_invoices, reconcile_against_document, ) @@ -671,4 +672,28 @@ def reconcile_dr_cr_note(dr_cr_notes, company): jv.flags.ignore_exchange_rate = True >>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) jv.submit() - jv.make_exchange_gain_loss_journal(args=[inv]) + + # make gain/loss journal + if inv.party_type == "Customer": + dr_or_cr = "credit" if inv.difference_amount < 0 else "debit" + else: + dr_or_cr = "debit" if inv.difference_amount < 0 else "credit" + + reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + + create_gain_loss_journal( + company, + inv.party_type, + inv.party, + inv.account, + inv.difference_account, + inv.difference_amount, + dr_or_cr, + reverse_dr_or_cr, + inv.against_voucher_type, + inv.against_voucher, + None, + inv.voucher_type, + inv.voucher_no, + None, + ) From 39c439dc4b8dc7f054644917141ecfd861656e12 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 20:53:07 +0530 Subject: [PATCH 34/54] fix: incorrect gain/loss on allocation change on reconciliation tool (cherry picked from commit 506a5775f9937fc893ae02b287ecd7303487363c) --- .../doctype/payment_reconciliation/payment_reconciliation.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 774d674b5ee..18d986cb7c4 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -261,6 +261,11 @@ class PaymentReconciliation(Document): def calculate_difference_on_allocation_change(self, payment_entry, invoice, allocated_amount): invoice_exchange_map = self.get_invoice_exchange_map(invoice, payment_entry) invoice[0]["exchange_rate"] = invoice_exchange_map.get(invoice[0].get("invoice_number")) + if payment_entry[0].get("reference_type") in ["Sales Invoice", "Purchase Invoice"]: + payment_entry[0]["exchange_rate"] = invoice_exchange_map.get( + payment_entry[0].get("reference_name") + ) + new_difference_amount = self.get_difference_amount( payment_entry[0], invoice[0], allocated_amount ) From 09e9b16b932899122eab5f424c95ff42c74ae37f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:12:14 +0530 Subject: [PATCH 35/54] test: cr notes against invoice (cherry picked from commit e3d2a2c5bdd94364f22828acc40854f6834b66ce) --- .../tests/test_accounts_controller.py | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index eefe202e476..fc4fb9fe9bf 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -51,7 +51,7 @@ def make_supplier(supplier_name, currency=None): class TestAccountsController(unittest.TestCase): """ Test Exchange Gain/Loss booking on various scenarios. - Test Cases are numbered for better readbility + Test Cases are numbered for better organization 10 series - Sales Invoice against Payment Entries 20 series - Sales Invoice against Journals @@ -923,3 +923,44 @@ class TestAccountsController(unittest.TestCase): exc_je_for_je = self.get_journals_for(je.doctype, je.name) self.assertEqual(exc_je_for_si, []) self.assertEqual(exc_je_for_je, []) + + def test_30_cr_note_against_sales_invoice(self): + """ + Reconciling Cr Note against Sales Invoice, both having different exchange rates + """ + # Invoice in Foreign currency + si = self.create_sales_invoice(qty=2, conversion_rate=80, rate=1) + + # Cr Note in Foreign currency of different exchange rate + cr_note = self.create_sales_invoice(qty=-2, conversion_rate=75, rate=1, do_not_save=True) + cr_note.is_return = 1 + cr_note.save().submit() + + # Reconcile the first half + pr = self.create_payment_reconciliation() + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + 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})) + difference_amount = pr.calculate_difference_on_allocation_change( + [x.as_dict() for x in pr.payments], [x.as_dict() for x in pr.invoices], 1 + ) + pr.allocation[0].allocated_amount = 1 + pr.allocation[0].difference_amount = difference_amount + pr.reconcile() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_cr = self.get_journals_for(cr_note.doctype, cr_note.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 2) + self.assertEqual(len(exc_je_for_cr), 2) + self.assertEqual(exc_je_for_cr, exc_je_for_si) + + si.reload() + self.assertEqual(si.outstanding_amount, 1) + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) From 349601b4b971010cfc7d260c644c9ec7b95924a3 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:15:48 +0530 Subject: [PATCH 36/54] fix: cr/dr note should be posted for exc gain/loss (cherry picked from commit 95543225cf402e9f17e05efee80f2dfb199aa4d9) --- .../payment_reconciliation/payment_reconciliation.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 18d986cb7c4..2f55eedd858 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -695,10 +695,10 @@ def reconcile_dr_cr_note(dr_cr_notes, company): inv.difference_amount, dr_or_cr, reverse_dr_or_cr, - inv.against_voucher_type, - inv.against_voucher, - None, inv.voucher_type, inv.voucher_no, None, + inv.against_voucher_type, + inv.against_voucher, + None, ) From c5c440b7bc429e04accecb58a946a1ba98a3dc9f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:24:08 +0530 Subject: [PATCH 37/54] test: assert ledger after cr note cancellation (cherry picked from commit ae424fdfedb49e6018d957eebb11eb4e03d9d410) --- .../tests/test_accounts_controller.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index fc4fb9fe9bf..415e1734a93 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -964,3 +964,19 @@ class TestAccountsController(unittest.TestCase): si.reload() self.assertEqual(si.outstanding_amount, 1) self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) + + cr_note.reload() + cr_note.cancel() + + # Exchange Gain/Loss Journal should've been created. + exc_je_for_si = self.get_journals_for(si.doctype, si.name) + exc_je_for_cr = self.get_journals_for(cr_note.doctype, cr_note.name) + self.assertNotEqual(exc_je_for_si, []) + self.assertEqual(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_cr), 0) + + # The Credit Note JE is still active and is referencing the sales invoice + # So, outstanding stays the same + si.reload() + self.assertEqual(si.outstanding_amount, 1) + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) From 3542df70f68e76cc9c7b806900b723b4258e5e94 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:54:23 +0530 Subject: [PATCH 38/54] fix(test): test case breakage in Github Actions (cherry picked from commit bfa54d533572f33ea5bc83794489293d36949e5d) --- erpnext/accounts/doctype/journal_entry/test_journal_entry.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index f7297d19e0f..e44ebc6afce 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -5,6 +5,7 @@ import unittest import frappe +from frappe.tests.utils import change_settings from frappe.utils import flt, nowdate from erpnext.accounts.doctype.account.test_account import get_inventory_account @@ -13,6 +14,7 @@ from erpnext.exceptions import InvalidAccountCurrency class TestJournalEntry(unittest.TestCase): + @change_settings("Accounts Settings", {"unlink_payment_on_cancellation_of_invoice": 1}) def test_journal_entry_with_against_jv(self): jv_invoice = frappe.copy_doc(test_records[2]) base_jv = frappe.copy_doc(test_records[0]) From 052abcb0756156b224fe6a74603df7970c9dfe48 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 22:32:59 +0530 Subject: [PATCH 39/54] refactor(test): assert ledger outstanding (cherry picked from commit 025091161e47bd2ad77beec068d5263567605425) # Conflicts: # erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py --- .../sales_invoice/test_sales_invoice.py | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index eed33693e9b..6f05cf36f93 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3260,15 +3260,19 @@ class TestSalesInvoice(unittest.TestCase): ) si.save() si.submit() - expected_gle = [ +<<<<<<< HEAD ["_Test Receivable USD - _TC", 7500.0, 500], ["Exchange Gain/Loss - _TC", 500.0, 0.0], ["Sales - _TC", 0.0, 7500.0], +======= + ["_Test Receivable USD - _TC", 7500.0, 0.0, nowdate()], + ["Sales - _TC", 0.0, 7500.0, nowdate()], +>>>>>>> 025091161e (refactor(test): assert ledger outstanding) ] - check_gl_entries(self, si.name, expected_gle, nowdate()) +<<<<<<< HEAD <<<<<<< HEAD frappe.db.set_value( "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice", unlink_enabled @@ -3276,6 +3280,26 @@ class TestSalesInvoice(unittest.TestCase): ======= >>>>>>> 70dd9d0671 (chore(test): fix broken unit test) +======= + si.reload() + self.assertEqual(si.outstanding_amount, 0) + journals = frappe.db.get_all( + "Journal Entry Account", + filters={"reference_type": "Sales Invoice", "reference_name": si.name, "docstatus": 1}, + pluck="parent", + ) + journals = [x for x in journals if x != jv.name] + self.assertEqual(len(journals), 1) + je_type = frappe.get_cached_value("Journal Entry", journals[0], "voucher_type") + self.assertEqual(je_type, "Exchange Gain Or Loss") + ledger_outstanding = frappe.db.get_all( + "Payment Ledger Entry", + filters={"against_voucher_no": si.name, "delinked": 0}, + fields=["sum(amount), sum(amount_in_account_currency)"], + as_list=1, + ) + +>>>>>>> 025091161e (refactor(test): assert ledger outstanding) def test_batch_expiry_for_sales_invoice_return(self): from erpnext.controllers.sales_and_purchase_return import make_return_doc from erpnext.stock.doctype.item.test_item import make_item From 2a61d854d34117b6892a7a75d1108d1f67e24df7 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 05:54:13 +0530 Subject: [PATCH 40/54] chore: use frappetestcase (cherry picked from commit 47bbb37291eba1e2bb8217837417a072f73f5634) --- erpnext/controllers/tests/test_accounts_controller.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 415e1734a93..acda12bf595 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -47,8 +47,7 @@ def make_supplier(supplier_name, currency=None): return supplier_name -# class TestAccountsController(FrappeTestCase): -class TestAccountsController(unittest.TestCase): +class TestAccountsController(FrappeTestCase): """ Test Exchange Gain/Loss booking on various scenarios. Test Cases are numbered for better organization @@ -66,8 +65,7 @@ class TestAccountsController(unittest.TestCase): self.clear_old_entries() def tearDown(self): - # frappe.db.rollback() - pass + frappe.db.rollback() def create_company(self): company_name = "_Test Company MC" From 4c527d6bba1d5980950ff31cafe24b914b9228f2 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 07:52:01 +0530 Subject: [PATCH 41/54] chore: add msgprint for exc JE (cherry picked from commit acc7322874b97830a838d066925aec99b01af129) --- erpnext/controllers/accounts_controller.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index a61795fd450..7afd80b4bcf 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -995,7 +995,7 @@ class AccountsController(TransactionBase): reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" - create_gain_loss_journal( + je = create_gain_loss_journal( self.company, arg.get("party_type"), arg.get("party"), @@ -1011,6 +1011,11 @@ class AccountsController(TransactionBase): self.name, arg.get("idx"), ) + frappe.msgprint( + _("Exchange Gain/Loss amount has been booked through {0}").format( + get_link_to_form("Journal Entry", je) + ) + ) if self.get("doctype") == "Payment Entry": # For Payment Entry, exchange_gain_loss field in the `references` table is the trigger for journal creation @@ -1068,7 +1073,7 @@ class AccountsController(TransactionBase): "Company", self.company, "exchange_gain_loss_account" ) - create_gain_loss_journal( + je = create_gain_loss_journal( self.company, self.party_type, self.party, @@ -1084,7 +1089,11 @@ class AccountsController(TransactionBase): self.name, d.idx, ) - # frappe.throw("stopping...") + frappe.msgprint( + _("Exchange Gain/Loss amount has been booked through {0}").format( + get_link_to_form("Journal Entry", je) + ) + ) def make_precision_loss_gl_entry(self, gl_entries): round_off_account, round_off_cost_center = get_round_off_account_and_cost_center( From 8d32a1f4b32531be9f1d50be4d384fad57475951 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 08:02:46 +0530 Subject: [PATCH 42/54] chore: rename some internal variables (cherry picked from commit d9d685615335778cd36734b1d1bd0c2b4189b690) --- erpnext/accounts/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index d8235cb256d..29747b14b7d 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -682,8 +682,9 @@ def cancel_exchange_gain_loss_journal(parent_doc: dict | object) -> None: fields=["parent"], as_list=1, ) + if journals: - exchange_journals = frappe.db.get_all( + gain_loss_journals = frappe.db.get_all( "Journal Entry", filters={ "name": ["in", [x[0] for x in journals]], @@ -692,7 +693,7 @@ def cancel_exchange_gain_loss_journal(parent_doc: dict | object) -> None: }, as_list=1, ) - for doc in exchange_journals: + for doc in gain_loss_journals: frappe.get_doc("Journal Entry", doc[0]).cancel() From efb293398aae6677b9d5a94a823f681a63fb2c36 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 09:30:38 +0530 Subject: [PATCH 43/54] chore(test): use existing company for unit test (cherry picked from commit 804afaa647b5727c37206fc4207c203652c10d53) --- erpnext/controllers/tests/test_accounts_controller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index acda12bf595..8e5f813d97d 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -68,8 +68,8 @@ class TestAccountsController(FrappeTestCase): frappe.db.rollback() def create_company(self): - company_name = "_Test Company MC" - self.company_abbr = abbr = "_CM" + company_name = "_Test Company" + self.company_abbr = abbr = "_TC" if frappe.db.exists("Company", company_name): company = frappe.get_doc("Company", company_name) else: From ed0881dacb67df9f0ebea5e19a73c0bda88e1861 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 28 Jul 2023 08:12:44 +0530 Subject: [PATCH 44/54] chore: don't make gain/loss journal for base currency transactions (cherry picked from commit 567c0ce1e85a42056d76cfc399b3468df32a576a) --- .../payment_reconciliation.py | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 2f55eedd858..8bdf5dcb759 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -678,27 +678,28 @@ def reconcile_dr_cr_note(dr_cr_notes, company): >>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) jv.submit() - # make gain/loss journal - if inv.party_type == "Customer": - dr_or_cr = "credit" if inv.difference_amount < 0 else "debit" - else: - dr_or_cr = "debit" if inv.difference_amount < 0 else "credit" + if inv.difference_amount != 0: + # make gain/loss journal + if inv.party_type == "Customer": + dr_or_cr = "credit" if inv.difference_amount < 0 else "debit" + else: + dr_or_cr = "debit" if inv.difference_amount < 0 else "credit" - reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" + reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" - create_gain_loss_journal( - company, - inv.party_type, - inv.party, - inv.account, - inv.difference_account, - inv.difference_amount, - dr_or_cr, - reverse_dr_or_cr, - inv.voucher_type, - inv.voucher_no, - None, - inv.against_voucher_type, - inv.against_voucher, - None, - ) + create_gain_loss_journal( + company, + inv.party_type, + inv.party, + inv.account, + inv.difference_account, + inv.difference_amount, + dr_or_cr, + reverse_dr_or_cr, + inv.voucher_type, + inv.voucher_no, + None, + inv.against_voucher_type, + inv.against_voucher, + None, + ) From 61afffc908a148316a37286f08f5408860e203ae Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 28 Jul 2023 08:29:19 +0530 Subject: [PATCH 45/54] chore: cancel gain/loss je while posting reverse gl (cherry picked from commit 46ea81440066af74a3b98f4ab9d5006839a17a4b) --- .../accounts/doctype/journal_entry/journal_entry.py | 3 +++ .../accounts/doctype/payment_entry/payment_entry.py | 12 ++++++++++-- .../accounts/doctype/sales_invoice/sales_invoice.py | 3 ++- erpnext/controllers/stock_controller.py | 3 ++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index e20575f31a1..f6898026134 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -18,6 +18,7 @@ from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category ) from erpnext.accounts.party import get_party_account from erpnext.accounts.utils import ( + cancel_exchange_gain_loss_journal, get_account_currency, get_balance_on, get_stock_accounts, @@ -933,6 +934,8 @@ class JournalEntry(AccountsController): merge_entries=merge_entries, update_outstanding=update_outstanding, ) + if cancel: + cancel_exchange_gain_loss_journal(frappe._dict(doctype=self.doctype, name=self.name)) @frappe.whitelist() def get_balance(self, difference_account=None): diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index b4c39f41063..379903dade3 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -24,7 +24,12 @@ from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category ) from erpnext.accounts.general_ledger import make_gl_entries, process_gl_map from erpnext.accounts.party import get_party_account -from erpnext.accounts.utils import get_account_currency, get_balance_on, get_outstanding_invoices +from erpnext.accounts.utils import ( + cancel_exchange_gain_loss_journal, + get_account_currency, + get_balance_on, + get_outstanding_invoices, +) from erpnext.controllers.accounts_controller import ( AccountsController, get_supplier_block_status, @@ -993,7 +998,10 @@ class PaymentEntry(AccountsController): gl_entries = self.build_gl_map() gl_entries = process_gl_map(gl_entries) make_gl_entries(gl_entries, cancel=cancel, adv_adj=adv_adj) - self.make_exchange_gain_loss_journal() + if cancel: + cancel_exchange_gain_loss_journal(frappe._dict(doctype=self.doctype, name=self.name)) + else: + self.make_exchange_gain_loss_journal() def add_party_gl_entries(self, gl_entries): if self.party_account: diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 9e9cd61e203..ab629913cd4 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -23,7 +23,7 @@ from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category ) from erpnext.accounts.general_ledger import get_round_off_account_and_cost_center from erpnext.accounts.party import get_due_date, get_party_account, get_party_details -from erpnext.accounts.utils import get_account_currency +from erpnext.accounts.utils import cancel_exchange_gain_loss_journal, get_account_currency from erpnext.assets.doctype.asset.depreciation import ( depreciate_asset, get_disposal_account_and_cost_center, @@ -1049,6 +1049,7 @@ class SalesInvoice(SellingController): self.make_exchange_gain_loss_journal() elif self.docstatus == 2: + cancel_exchange_gain_loss_journal(frappe._dict(doctype=self.doctype, name=self.name)) make_reverse_gl_entries(voucher_type=self.doctype, voucher_no=self.name) if update_outstanding == "No": diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 4f0c8a9a54f..e24d8fb661f 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -15,7 +15,7 @@ from erpnext.accounts.general_ledger import ( make_reverse_gl_entries, process_gl_map, ) -from erpnext.accounts.utils import get_fiscal_year +from erpnext.accounts.utils import cancel_exchange_gain_loss_journal, get_fiscal_year from erpnext.controllers.accounts_controller import AccountsController from erpnext.stock import get_warehouse_account_map from erpnext.stock.doctype.inventory_dimension.inventory_dimension import ( @@ -513,6 +513,7 @@ class StockController(AccountsController): make_sl_entries(sl_entries, allow_negative_stock, via_landed_cost_voucher) def make_gl_entries_on_cancel(self): + cancel_exchange_gain_loss_journal(frappe._dict(doctype=self.doctype, name=self.name)) if frappe.db.sql( """select name from `tabGL Entry` where voucher_type=%s and voucher_no=%s""", From 7469018d3e0dd6359ba3dbf2ec3844c32b5c01a9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 14 Aug 2023 10:37:56 +0530 Subject: [PATCH 46/54] chore: resolve merge conflict --- erpnext/accounts/doctype/gl_entry/gl_entry.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/erpnext/accounts/doctype/gl_entry/gl_entry.py b/erpnext/accounts/doctype/gl_entry/gl_entry.py index 26f4f2ba75f..3a564825b55 100644 --- a/erpnext/accounts/doctype/gl_entry/gl_entry.py +++ b/erpnext/accounts/doctype/gl_entry/gl_entry.py @@ -58,9 +58,6 @@ class GLEntry(Document): validate_balance_type(self.account, adv_adj) validate_frozen_account(self.account, adv_adj) -<<<<<<< HEAD - if frappe.db.get_value("Account", self.account, "account_type") not in [ -======= if ( self.voucher_type == "Journal Entry" and frappe.get_cached_value("Journal Entry", self.voucher_no, "voucher_type") @@ -69,7 +66,6 @@ class GLEntry(Document): return if frappe.get_cached_value("Account", self.account, "account_type") not in [ ->>>>>>> f119a1e115 (refactor: linkage between journal as payment and gain/loss journal) "Receivable", "Payable", ]: From f92453ae456b2f2f2346c20ec75ccceb2bc61277 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 14 Aug 2023 10:50:31 +0530 Subject: [PATCH 47/54] chore: resolve merge conflict in `accounts/utils.py` and its tests --- erpnext/accounts/test/test_utils.py | 3 --- erpnext/accounts/utils.py | 19 ------------------- 2 files changed, 22 deletions(-) diff --git a/erpnext/accounts/test/test_utils.py b/erpnext/accounts/test/test_utils.py index f72ac783bd3..0a8c7239861 100644 --- a/erpnext/accounts/test/test_utils.py +++ b/erpnext/accounts/test/test_utils.py @@ -73,8 +73,6 @@ class TestUtils(unittest.TestCase): sorted_vouchers = sort_stock_vouchers_by_posting_date(list(reversed(vouchers))) self.assertEqual(sorted_vouchers, vouchers) -<<<<<<< HEAD -======= def test_update_reference_in_payment_entry(self): item = make_item().name @@ -125,7 +123,6 @@ class TestUtils(unittest.TestCase): self.assertEqual(len(payment_entry.references), 1) self.assertEqual(payment_entry.difference_amount, 0) ->>>>>>> 72bc5b3a11 (refactor(test): difference amount no updated for exchange gain/loss) ADDRESS_RECORDS = [ { diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 29747b14b7d..3e06a36e67e 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -636,25 +636,6 @@ def update_reference_in_payment_entry( new_row.docstatus = 1 new_row.update(reference_details) -<<<<<<< HEAD - payment_entry.flags.ignore_validate_update_after_submit = True - payment_entry.setup_party_account_field() - payment_entry.set_missing_values() - payment_entry.set_amounts() - - if d.difference_amount and d.difference_account: - account_details = { - "account": d.difference_account, - "cost_center": payment_entry.cost_center - or frappe.get_cached_value("Company", payment_entry.company, "cost_center"), - } - if d.difference_amount: - account_details["amount"] = d.difference_amount - - payment_entry.set_gain_or_loss(account_details=account_details) - -======= ->>>>>>> 1bcb728c85 (refactor: remove call for setting deductions in payment entry) payment_entry.flags.ignore_validate_update_after_submit = True payment_entry.setup_party_account_field() payment_entry.set_missing_values() From 946aadb0c00368fb4def4b96f1c0f6ff00d13664 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 14 Aug 2023 10:52:35 +0530 Subject: [PATCH 48/54] chore: resolve conflict in `test_sales_invoice.py` --- .../sales_invoice/test_sales_invoice.py | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 6f05cf36f93..c36a05ac099 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3217,17 +3217,6 @@ class TestSalesInvoice(unittest.TestCase): def test_gain_loss_with_advance_entry(self): from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry -<<<<<<< HEAD - unlink_enabled = frappe.db.get_value( - "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice" - ) - - frappe.db.set_value( - "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice", 1 - ) - -======= ->>>>>>> 70dd9d0671 (chore(test): fix broken unit test) jv = make_journal_entry("_Test Receivable USD - _TC", "_Test Bank - _TC", -7000, save=False) jv.accounts[0].exchange_rate = 70 @@ -3261,26 +3250,11 @@ class TestSalesInvoice(unittest.TestCase): si.save() si.submit() expected_gle = [ -<<<<<<< HEAD - ["_Test Receivable USD - _TC", 7500.0, 500], - ["Exchange Gain/Loss - _TC", 500.0, 0.0], - ["Sales - _TC", 0.0, 7500.0], -======= ["_Test Receivable USD - _TC", 7500.0, 0.0, nowdate()], ["Sales - _TC", 0.0, 7500.0, nowdate()], ->>>>>>> 025091161e (refactor(test): assert ledger outstanding) ] check_gl_entries(self, si.name, expected_gle, nowdate()) -<<<<<<< HEAD -<<<<<<< HEAD - frappe.db.set_value( - "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice", unlink_enabled - ) - -======= ->>>>>>> 70dd9d0671 (chore(test): fix broken unit test) -======= si.reload() self.assertEqual(si.outstanding_amount, 0) journals = frappe.db.get_all( @@ -3299,7 +3273,6 @@ class TestSalesInvoice(unittest.TestCase): as_list=1, ) ->>>>>>> 025091161e (refactor(test): assert ledger outstanding) def test_batch_expiry_for_sales_invoice_return(self): from erpnext.controllers.sales_and_purchase_return import make_return_doc from erpnext.stock.doctype.item.test_item import make_item From b3f4c14a26bf3fb2e5c15f3291fabe1bc2f6e487 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 14 Aug 2023 10:53:42 +0530 Subject: [PATCH 49/54] chore: resolve conflict in `payment_reconciliation.py` backport will merge the better remarks PR https://github.com/frappe/erpnext/pull/36573 wil exchange gain/loss booking refactor --- .../payment_reconciliation.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 8bdf5dcb759..b6708ce24b1 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -640,11 +640,8 @@ def reconcile_dr_cr_note(dr_cr_notes, company): "reference_type": inv.against_voucher_type, "reference_name": inv.against_voucher, "cost_center": erpnext.get_default_cost_center(company), -<<<<<<< HEAD "user_remark": f"{fmt_money(flt(inv.allocated_amount), currency=company_currency)} against {inv.against_voucher}", -======= "exchange_rate": inv.exchange_rate, ->>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) }, { "account": inv.account, @@ -658,24 +655,18 @@ def reconcile_dr_cr_note(dr_cr_notes, company): "reference_type": inv.voucher_type, "reference_name": inv.voucher_no, "cost_center": erpnext.get_default_cost_center(company), -<<<<<<< HEAD "user_remark": f"{fmt_money(flt(inv.allocated_amount), currency=company_currency)} from {inv.voucher_no}", -======= "exchange_rate": inv.exchange_rate, ->>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) }, ], } ) jv.flags.ignore_mandatory = True -<<<<<<< HEAD - jv.remark = None jv.flags.skip_remarks_creation = True - jv.is_system_generated = True -======= jv.flags.ignore_exchange_rate = True ->>>>>>> c87332d5da (refactor: cr/dr note will be on single exchange rate) + jv.is_system_generated = True + jv.remark = None jv.submit() if inv.difference_amount != 0: From 2e6bfa36de244e2944cd2ff6d42fff4e6ba3cbb8 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 5 Aug 2023 14:11:57 +0530 Subject: [PATCH 50/54] fix(test): replace hardcoded reference to adv with dynamic one --- .../tests/test_accounts_controller.py | 65 ++++++++++++------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 8e5f813d97d..0f8e133e0fd 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -347,18 +347,23 @@ class TestAccountsController(FrappeTestCase): for exc_rate in [75.9, 83.1, 80.01]: with self.subTest(exc_rate=exc_rate): si = self.create_sales_invoice(qty=1, conversion_rate=exc_rate, rate=1, do_not_submit=True) + advances = si.get_advance_entries() + self.assertEqual(len(advances), 1) + self.assertEqual(advances[0].reference_name, adv.name) si.append( "advances", { "doctype": "Sales Invoice Advance", - "reference_type": adv.doctype, - "reference_name": adv.name, + "reference_type": advances[0].reference_type, + "reference_name": advances[0].reference_name, + "reference_row": advances[0].reference_row, "advance_amount": 1, "allocated_amount": 1, - "ref_exchange_rate": 85, - "remarks": "Test", + "ref_exchange_rate": advances[0].exchange_rate, + "remarks": advances[0].remarks, }, ) + si = si.save() si = si.submit() @@ -398,16 +403,19 @@ class TestAccountsController(FrappeTestCase): si = self.create_sales_invoice( qty=2, conversion_rate=80, rate=rate_in_account_currency, do_not_submit=True ) + advances = si.get_advance_entries() + self.assertEqual(len(advances), 1) + self.assertEqual(advances[0].reference_name, adv.name) si.append( "advances", { "doctype": "Sales Invoice Advance", - "reference_type": adv.doctype, - "reference_name": adv.name, + "reference_type": advances[0].reference_type, + "reference_name": advances[0].reference_name, "advance_amount": 1, "allocated_amount": 1, - "ref_exchange_rate": 85, - "remarks": "Test", + "ref_exchange_rate": advances[0].exchange_rate, + "remarks": advances[0].remarks, }, ) si = si.save() @@ -470,16 +478,19 @@ class TestAccountsController(FrappeTestCase): # invoice with advance(partial amount) si = self.create_sales_invoice(qty=2, conversion_rate=80, rate=1, do_not_submit=True) + advances = si.get_advance_entries() + self.assertEqual(len(advances), 1) + self.assertEqual(advances[0].reference_name, adv.name) si.append( "advances", { "doctype": "Sales Invoice Advance", - "reference_type": adv.doctype, - "reference_name": adv.name, + "reference_type": advances[0].reference_type, + "reference_name": advances[0].reference_name, "advance_amount": 1, "allocated_amount": 1, - "ref_exchange_rate": 85, - "remarks": "Test", + "ref_exchange_rate": advances[0].exchange_rate, + "remarks": advances[0].remarks, }, ) si = si.save() @@ -678,22 +689,26 @@ class TestAccountsController(FrappeTestCase): adv.reload() # Sales Invoices in different exchange rates - for exc_rate in [75.9, 83.1, 80.01]: + for exc_rate in [75.9, 83.1]: with self.subTest(exc_rate=exc_rate): si = self.create_sales_invoice(qty=1, conversion_rate=exc_rate, rate=1, do_not_submit=True) + advances = si.get_advance_entries() + self.assertEqual(len(advances), 1) + self.assertEqual(advances[0].reference_name, adv.name) si.append( "advances", { "doctype": "Sales Invoice Advance", - "reference_type": adv.doctype, - "reference_name": adv.name, - "reference_row": adv.accounts[0].name, + "reference_type": advances[0].reference_type, + "reference_name": advances[0].reference_name, + "reference_row": advances[0].reference_row, "advance_amount": 1, "allocated_amount": 1, - "ref_exchange_rate": adv_exc_rate, - "remarks": "Test", + "ref_exchange_rate": advances[0].exchange_rate, + "remarks": advances[0].remarks, }, ) + si = si.save() si = si.submit() @@ -741,19 +756,23 @@ class TestAccountsController(FrappeTestCase): # invoice with advance(partial amount) si = self.create_sales_invoice(qty=3, conversion_rate=80, rate=1, do_not_submit=True) + advances = si.get_advance_entries() + self.assertEqual(len(advances), 1) + self.assertEqual(advances[0].reference_name, adv.name) si.append( "advances", { "doctype": "Sales Invoice Advance", - "reference_type": adv.doctype, - "reference_name": adv.name, - "reference_row": adv.accounts[0].name, + "reference_type": advances[0].reference_type, + "reference_name": advances[0].reference_name, + "reference_row": advances[0].reference_row, "advance_amount": 1, "allocated_amount": 1, - "ref_exchange_rate": adv_exc_rate, - "remarks": "Test", + "ref_exchange_rate": advances[0].exchange_rate, + "remarks": advances[0].remarks, }, ) + si = si.save() si = si.submit() From 18cf93d1c8902c9116881fd901c6b0d8afdcacff Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 14 Aug 2023 11:49:46 +0530 Subject: [PATCH 51/54] refactor(test): import missing functions --- erpnext/accounts/test/test_utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/accounts/test/test_utils.py b/erpnext/accounts/test/test_utils.py index 0a8c7239861..3d5e5fc4ec7 100644 --- a/erpnext/accounts/test/test_utils.py +++ b/erpnext/accounts/test/test_utils.py @@ -3,6 +3,8 @@ import unittest import frappe from frappe.test_runner import make_test_objects +from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry +from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.accounts.party import get_party_shipping_address from erpnext.accounts.utils import ( get_future_stock_vouchers, From 33d5250cec18fa42850184021aa1da79cf3f4317 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 15 Aug 2023 18:27:01 +0530 Subject: [PATCH 52/54] chore: add validation for depreciation expense account in asset category (backport #36659) (#36661) chore: add validation for depreciation expense account in asset category (#36659) (cherry picked from commit e0c79d3b53399e336bf6ff45489751b7d6c58f6a) Co-authored-by: Anand Baburajan --- erpnext/assets/doctype/asset_category/asset_category.js | 1 + erpnext/assets/doctype/asset_category/asset_category.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset_category/asset_category.js b/erpnext/assets/doctype/asset_category/asset_category.js index c702687072d..7dde14ea0e6 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.js +++ b/erpnext/assets/doctype/asset_category/asset_category.js @@ -33,6 +33,7 @@ frappe.ui.form.on('Asset Category', { var d = locals[cdt][cdn]; return { "filters": { + "account_type": "Depreciation", "root_type": ["in", ["Expense", "Income"]], "is_group": 0, "company": d.company_name diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index 2e1def98fc3..8d351412ca8 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -53,7 +53,7 @@ class AssetCategory(Document): account_type_map = { "fixed_asset_account": {"account_type": ["Fixed Asset"]}, "accumulated_depreciation_account": {"account_type": ["Accumulated Depreciation"]}, - "depreciation_expense_account": {"root_type": ["Expense", "Income"]}, + "depreciation_expense_account": {"account_type": ["Depreciation"]}, "capital_work_in_progress_account": {"account_type": ["Capital Work in Progress"]}, } for d in self.accounts: From 99777d3fa40e6a3225a33321ede92da759312553 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 16 Aug 2023 08:09:04 +0530 Subject: [PATCH 53/54] fix: re-add permission that was unintentionally removed (#36663) fix: re-add permission that was unintentionally removed Remove `Reversal OF ITC` and re-add permissions. Both of them unintended changes (cherry picked from commit 45662fa646cbd861d89e827bc557a3f2a25964b1) Co-authored-by: ruthra kumar --- .../doctype/journal_entry/journal_entry.json | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.json b/erpnext/accounts/doctype/journal_entry/journal_entry.json index cff84dc9252..2eb54a54d54 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.json +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.json @@ -555,7 +555,45 @@ "name": "Journal Entry", "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", - "permissions": [], + "permissions": [ + { + "amend": 1, + "cancel": 1, + "create": 1, + "delete": 1, + "email": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Accounts User", + "share": 1, + "submit": 1, + "write": 1 + }, + { + "amend": 1, + "cancel": 1, + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "import": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Accounts Manager", + "share": 1, + "submit": 1, + "write": 1 + }, + { + "email": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Auditor" + } + ], "search_fields": "voucher_type,posting_date, due_date, cheque_no", "sort_field": "modified", "sort_order": "DESC", From 1deebe87574115a563b99dd04442f1b599150e09 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 16 Aug 2023 08:09:38 +0530 Subject: [PATCH 54/54] fix: Tax withholding post LDC limit consumed (#36611) * fix: Tax withholding post LDC limit consumed (#36611) * fix: Tax withholding post LDC limit consumed * fix: LDC condition check (cherry picked from commit 985ff9781b9f18f7d2da55acaecab8ebf3c51bb7) # Conflicts: # erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py * chore: resolve conflicts * chore: linting issues --------- Co-authored-by: Deepesh Garg --- .../tax_withholding_category.py | 62 +++++++--------- .../test_tax_withholding_category.py | 73 +++++++++++++++++++ 2 files changed, 100 insertions(+), 35 deletions(-) diff --git a/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py b/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py index e66a886bf9a..d17ca08c408 100644 --- a/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py +++ b/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py @@ -262,14 +262,20 @@ def get_tax_amount(party_type, parties, inv, tax_details, posting_date, pan_no=N if tax_deducted: net_total = inv.tax_withholding_net_total if ldc: - tax_amount = get_tds_amount_from_ldc(ldc, parties, tax_details, posting_date, net_total) + limit_consumed = get_limit_consumed(ldc, parties) + if is_valid_certificate(ldc, posting_date, limit_consumed): + tax_amount = get_lower_deduction_amount( + net_total, limit_consumed, ldc.certificate_limit, ldc.rate, tax_details + ) + else: + tax_amount = net_total * tax_details.rate / 100 if net_total > 0 else 0 else: tax_amount = net_total * tax_details.rate / 100 if net_total > 0 else 0 # once tds is deducted, not need to add vouchers in the invoice voucher_wise_amount = {} else: - tax_amount = get_tds_amount(ldc, parties, inv, tax_details, tax_deducted, vouchers) + tax_amount = get_tds_amount(ldc, parties, inv, tax_details, vouchers) elif party_type == "Customer": if tax_deducted: @@ -416,7 +422,7 @@ def get_deducted_tax(taxable_vouchers, tax_details): return sum(entries) -def get_tds_amount(ldc, parties, inv, tax_details, tax_deducted, vouchers): +def get_tds_amount(ldc, parties, inv, tax_details, vouchers): tds_amount = 0 invoice_filters = {"name": ("in", vouchers), "docstatus": 1, "apply_tds": 1} @@ -496,15 +502,10 @@ def get_tds_amount(ldc, parties, inv, tax_details, tax_deducted, vouchers): net_total += inv.tax_withholding_net_total supp_credit_amt = net_total - cumulative_threshold - if ldc and is_valid_certificate( - ldc.valid_from, - ldc.valid_upto, - inv.get("posting_date") or inv.get("transaction_date"), - tax_deducted, - inv.tax_withholding_net_total, - ldc.certificate_limit, - ): - tds_amount = get_ltds_amount(supp_credit_amt, 0, ldc.certificate_limit, ldc.rate, tax_details) + if ldc and is_valid_certificate(ldc, inv.get("posting_date") or inv.get("transaction_date"), 0): + tds_amount = get_lower_deduction_amount( + supp_credit_amt, 0, ldc.certificate_limit, ldc.rate, tax_details + ) else: tds_amount = supp_credit_amt * tax_details.rate / 100 if supp_credit_amt > 0 else 0 @@ -582,8 +583,7 @@ def get_invoice_total_without_tcs(inv, tax_details): return inv.grand_total - tcs_tax_row_amount -def get_tds_amount_from_ldc(ldc, parties, tax_details, posting_date, net_total): - tds_amount = 0 +def get_limit_consumed(ldc, parties): limit_consumed = frappe.db.get_value( "Purchase Invoice", { @@ -597,37 +597,29 @@ def get_tds_amount_from_ldc(ldc, parties, tax_details, posting_date, net_total): "sum(tax_withholding_net_total)", ) - if is_valid_certificate( - ldc.valid_from, ldc.valid_upto, posting_date, limit_consumed, net_total, ldc.certificate_limit - ): - tds_amount = get_ltds_amount( - net_total, limit_consumed, ldc.certificate_limit, ldc.rate, tax_details - ) - - return tds_amount + return limit_consumed -def get_ltds_amount(current_amount, deducted_amount, certificate_limit, rate, tax_details): - if certificate_limit - flt(deducted_amount) - flt(current_amount) >= 0: +def get_lower_deduction_amount( + current_amount, limit_consumed, certificate_limit, rate, tax_details +): + if certificate_limit - flt(limit_consumed) - flt(current_amount) >= 0: return current_amount * rate / 100 else: - ltds_amount = certificate_limit - flt(deducted_amount) + ltds_amount = certificate_limit - flt(limit_consumed) tds_amount = current_amount - ltds_amount return ltds_amount * rate / 100 + tds_amount * tax_details.rate / 100 -def is_valid_certificate( - valid_from, valid_upto, posting_date, deducted_amount, current_amount, certificate_limit -): - valid = False +def is_valid_certificate(ldc, posting_date, limit_consumed): + available_amount = flt(ldc.certificate_limit) - flt(limit_consumed) + if ( + getdate(ldc.valid_from) <= getdate(posting_date) <= getdate(ldc.valid_upto) + ) and available_amount > 0: + return True - available_amount = flt(certificate_limit) - flt(deducted_amount) - - if (getdate(valid_from) <= getdate(posting_date) <= getdate(valid_upto)) and available_amount > 0: - valid = True - - return valid + return False def normal_round(number): diff --git a/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py b/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py index f8e0e2992f7..0a749f96652 100644 --- a/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py +++ b/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py @@ -4,6 +4,7 @@ import unittest import frappe +from frappe.custom.doctype.custom_field.custom_field import create_custom_fields from frappe.tests.utils import change_settings from frappe.utils import today @@ -18,6 +19,7 @@ class TestTaxWithholdingCategory(unittest.TestCase): # create relevant supplier, etc create_records() create_tax_withholding_category_records() + make_pan_no_field() def tearDown(self): cancel_invoices() @@ -456,6 +458,40 @@ class TestTaxWithholdingCategory(unittest.TestCase): pe2.cancel() pe3.cancel() + def test_lower_deduction_certificate_application(self): + frappe.db.set_value( + "Supplier", + "Test LDC Supplier", + { + "tax_withholding_category": "Test Service Category", + "pan": "ABCTY1234D", + }, + ) + + create_lower_deduction_certificate( + supplier="Test LDC Supplier", + certificate_no="1AE0423AAJ", + tax_withholding_category="Test Service Category", + tax_rate=2, + limit=50000, + ) + + pi1 = create_purchase_invoice(supplier="Test LDC Supplier", rate=35000) + pi1.submit() + self.assertEqual(pi1.taxes[0].tax_amount, 700) + + pi2 = create_purchase_invoice(supplier="Test LDC Supplier", rate=35000) + pi2.submit() + self.assertEqual(pi2.taxes[0].tax_amount, 2300) + + pi3 = create_purchase_invoice(supplier="Test LDC Supplier", rate=35000) + pi3.submit() + self.assertEqual(pi3.taxes[0].tax_amount, 3500) + + pi1.cancel() + pi2.cancel() + pi3.cancel() + def cancel_invoices(): purchase_invoices = frappe.get_all( @@ -615,6 +651,7 @@ def create_records(): "Test TDS Supplier6", "Test TDS Supplier7", "Test TDS Supplier8", + "Test LDC Supplier", ]: if frappe.db.exists("Supplier", name): continue @@ -811,3 +848,39 @@ def create_tax_withholding_category( "accounts": [{"company": "_Test Company", "account": account}], } ).insert() + + +def create_lower_deduction_certificate( + supplier, tax_withholding_category, tax_rate, certificate_no, limit +): + fiscal_year = get_fiscal_year(today(), company="_Test Company") + if not frappe.db.exists("Lower Deduction Certificate", certificate_no): + frappe.get_doc( + { + "doctype": "Lower Deduction Certificate", + "company": "_Test Company", + "supplier": supplier, + "certificate_no": certificate_no, + "tax_withholding_category": tax_withholding_category, + "fiscal_year": fiscal_year[0], + "valid_from": fiscal_year[1], + "valid_upto": fiscal_year[2], + "rate": tax_rate, + "certificate_limit": limit, + } + ).insert() + + +def make_pan_no_field(): + pan_field = { + "Supplier": [ + { + "fieldname": "pan", + "label": "PAN", + "fieldtype": "Data", + "translatable": 0, + } + ] + } + + create_custom_fields(pan_field, update=1)