From 81cd7873d343d893a6e2a3b41107f17e03eedcd8 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Apr 2023 09:46:54 +0530 Subject: [PATCH 01/45] refactor: book exchange gain/loss through journal --- .../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 ed18feaf57..105c4767fa 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -9,7 +9,7 @@ erpnext.accounts.taxes.setup_tax_filters("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 38d8b8fcad..eea7f4d650 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -143,6 +143,7 @@ class PaymentEntry(AccountsController): "Repost Payment Ledger", "Repost Payment Ledger Items", ) + super(PaymentEntry, self).on_cancel() self.make_gl_entries(cancel=1) self.make_advance_gl_entries(cancel=1) self.update_outstanding_amounts() @@ -808,10 +809,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): @@ -1002,6 +1018,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 25d94c55d3..df777f03be 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -363,11 +363,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 974a876429..fa18d8fc0e 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -1029,6 +1029,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) @@ -1054,7 +1056,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 e354663151..0b3f45ad76 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -614,9 +614,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 "account": d.account, } @@ -655,11 +653,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 79404894cd..ee7dfb737a 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.custom import ConstantColumn from frappe.query_builder.functions import Abs, Sum @@ -968,67 +968,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 update_against_document_in_jv(self): """ @@ -1090,9 +1142,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 5e1cd1f22701b7675422b05b3616253d9a3a28db Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 15 Jun 2023 16:55:56 +0530 Subject: [PATCH 02/45] test: different scenarios for exchange booking --- .../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 0000000000..31aa857c8f --- /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 7e94a1c51b428202820858f72a7e4a864cde0e9c Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 11:34:11 +0530 Subject: [PATCH 03/45] refactor: replace with new method in purchase invoice --- 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 230a8b3c58..1f9555a3c3 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -536,6 +536,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) @@ -580,7 +581,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 0587338435a6ffeeb59669ff20dbd9779b9ac740 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 12:32:21 +0530 Subject: [PATCH 04/45] chore: patch to update property setter for Journal Entry Accounts --- 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 0f4238c16b..641d7550e3 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -320,6 +320,7 @@ erpnext.patches.v15_0.update_gpa_and_ndb_for_assdeprsch erpnext.patches.v14_0.create_accounting_dimensions_for_closing_balance erpnext.patches.v14_0.update_closing_balances #14-07-2023 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 execute:frappe.delete_doc_if_exists("Report", "Tax Detail") 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 0000000000..48b6bcf755 --- /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 13febcac811507c7c61bc116ca797857d0b5baf5 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 14:01:48 +0530 Subject: [PATCH 05/45] refactor: add new reference type in journal entry account --- .../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 47ad19e0f9..3ba8cea94b 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 34b5e849a290ee02d9b653286dfbe6590d35a800 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 16 Jun 2023 14:07:44 +0530 Subject: [PATCH 06/45] chore: fix logic for purchase invoice and some typos --- 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 eea7f4d650..44e3e898d2 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -811,8 +811,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 ee7dfb737a..e60719c5c0 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -972,7 +972,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] @@ -1027,6 +1027,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( @@ -1080,7 +1084,6 @@ class AccountsController(TransactionBase): journal_entry.save() journal_entry.submit() - # frappe.throw("stopping...") def update_against_document_in_jv(self): """ From c1184585eda2e37b74718b95d541fa0419511bd9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 17:51:05 +0530 Subject: [PATCH 07/45] refactor: helper method --- .../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 c6e93f3f7a..afd03c6bd4 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 92ae9c220110ddcb32d90a1bb89f0b85e72ff7d0 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 09:58:18 +0530 Subject: [PATCH 08/45] 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. --- 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 44e3e898d2..89241ebfe0 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -400,7 +400,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( @@ -413,7 +413,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( @@ -2005,7 +2005,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) >= ( @@ -2145,7 +2144,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 4ff53e106271e0562e2ba5604802a41c24c999c4 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 10:23:23 +0530 Subject: [PATCH 09/45] refactor: assert exchange gain/loss amount in reference table --- .../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 afd03c6bd4..997d52bed6 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 00a2e42a47fb064afbb31b27653a54d12b6c8097 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 19 Jun 2023 11:26:49 +0530 Subject: [PATCH 10/45] refactor(test): exc gain/loss booked through journal --- .../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 997d52bed6..5f3267427a 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 7b516f84636e7219ac17d972c80a8286c385e954 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 26 Jun 2023 17:34:28 +0530 Subject: [PATCH 11/45] refactor: exc booking logic for Journal Entry --- 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 0b3f45ad76..cfd0133700 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 e60719c5c0..5597b515f6 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -968,13 +968,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 ee3ce82ea82df9dd2910e4d29a5c2c4f885be393 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 26 Jun 2023 21:43:20 +0530 Subject: [PATCH 12/45] chore: remove debugging statements and fixing failing unit tests --- 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 5597b515f6..2548bdc760 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -977,26 +977,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( @@ -1014,14 +1013,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, } @@ -1039,8 +1038,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 389cadf15715b1483986297b38ec2dbb268d2b26 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 11:16:52 +0530 Subject: [PATCH 13/45] refactor(test): assert Exc journal when reconciling Journa to invoic --- .../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 2ac7df0e39..1d843abde1 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 2548bdc760..5abff417bf 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1038,8 +1038,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 ee2d1fa36e24326aa9f5b11877139857ed3a6f21 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 11:41:14 +0530 Subject: [PATCH 14/45] 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 --- .../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 e17a846dd8..feb2fdffc9 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 78bc712756bc9d8966c22cbbce68e5058daa87db Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 12:29:02 +0530 Subject: [PATCH 15/45] refactor: only post on base currency for exchange gain/loss --- 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 5abff417bf..f72ae81a53 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1150,7 +1150,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 5b06bd1af4197b0c6ab8714c65d8f7a578499163 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 12:42:07 +0530 Subject: [PATCH 16/45] refactor(test): exc gain/loss journal for advance in purchase invoice --- .../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 8c96480478..974c881306 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -1273,10 +1273,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( @@ -1293,6 +1294,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", @@ -1317,10 +1343,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( @@ -1351,12 +1379,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 72bc5b3a11528611db8a322d68c0ecc422b570c6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 16:11:03 +0530 Subject: [PATCH 17/45] refactor(test): difference amount no updated for exchange gain/loss --- erpnext/accounts/test/test_utils.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/test/test_utils.py b/erpnext/accounts/test/test_utils.py index 3aca60eae5..3cb5e42e7a 100644 --- a/erpnext/accounts/test/test_utils.py +++ b/erpnext/accounts/test/test_utils.py @@ -80,18 +80,27 @@ class TestUtils(unittest.TestCase): item = make_item().name purchase_invoice = make_purchase_invoice( - item=item, supplier="_Test Supplier USD", currency="USD", conversion_rate=82.32 + 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.target_exchange_rate = 62.9 payment_entry.paid_amount = 15725 payment_entry.deductions = [] - payment_entry.insert() + 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) - self.assertEqual(payment_entry.difference_amount, -4855.00) payment_entry.references = [] + self.assertEqual(payment_entry.difference_amount, 0.0) payment_entry.submit() payment_reconciliation = frappe.new_doc("Payment Reconciliation") From 1bcb728c850c67f3e479eb402ce1296dc215496b Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 27 Jun 2023 16:57:38 +0530 Subject: [PATCH 18/45] refactor: remove call for setting deductions in payment entry --- erpnext/accounts/utils.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index cfd0133700..49a6367784 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -639,17 +639,6 @@ def update_reference_in_payment_entry( new_row.docstatus = 1 new_row.update(reference_details) - 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) - payment_entry.flags.ignore_validate_update_after_submit = True payment_entry.setup_party_account_field() payment_entry.set_missing_values() From cd42b268391113d5d5b10d75a6e2562736e43aae Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 10 Jul 2023 15:28:10 +0530 Subject: [PATCH 19/45] chore: code cleanup --- .../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 df777f03be..d574cd79b8 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -363,12 +363,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 f72ae81a53..611eca621e 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -972,9 +972,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"): @@ -1035,6 +1038,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, @@ -1049,7 +1053,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 f119a1e11553a0357f937bd23a397757f3f5b54f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jul 2023 12:04:13 +0530 Subject: [PATCH 20/45] refactor: linkage between journal as payment and gain/loss journal --- erpnext/accounts/doctype/gl_entry/gl_entry.py | 7 +++++++ .../accounts/doctype/journal_entry/journal_entry.py | 11 ++++++----- erpnext/controllers/accounts_controller.py | 6 +++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/erpnext/accounts/doctype/gl_entry/gl_entry.py b/erpnext/accounts/doctype/gl_entry/gl_entry.py index f07a4fa3bc..7af40c46cb 100644 --- a/erpnext/accounts/doctype/gl_entry/gl_entry.py +++ b/erpnext/accounts/doctype/gl_entry/gl_entry.py @@ -58,6 +58,13 @@ class GLEntry(Document): validate_balance_type(self.account, adv_adj) validate_frozen_account(self.account, adv_adj) + 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 [ "Receivable", "Payable", diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index ea4a2d4b19..0115fd7f7a 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -499,11 +499,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 611eca621e..0b3b41de9f 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1039,9 +1039,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 6e18bb6456b3a7a2cbad89b86dcc124978337e4d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jul 2023 12:21:10 +0530 Subject: [PATCH 21/45] refactor: cancel gain/loss JE on Journal as payment cancellation --- .../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 0115fd7f7a..e6b8b5d281 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 49a6367784..53d9e21c35 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -655,7 +655,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 0b3b41de9f..a126dfe6b3 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -991,10 +991,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" @@ -1038,6 +1039,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, @@ -1163,6 +1165,7 @@ class AccountsController(TransactionBase): journal_entry.save() journal_entry.submit() + # frappe.throw("stopping...") def update_against_document_in_jv(self): """ @@ -1229,7 +1232,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 31aa857c8f..28a569b524 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 73cc1ba654f39d81b7e1d9769ef6a0a8ceb689fe Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jul 2023 16:34:20 +0530 Subject: [PATCH 22/45] refactor: assert payment ledger outstanding in both currencies --- .../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 28a569b524..fc30c4b8cd 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 056724377206f9bdc3e7ac9894fabb0d93bd3176 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 12 Jul 2023 06:14:17 +0530 Subject: [PATCH 23/45] refactor: dr/cr logic for journals as payments --- 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 a126dfe6b3..dfc3114fa0 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -992,15 +992,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 5695d6a5a62e634536c51a22e045eb8281a29d9d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 12 Jul 2023 06:46:59 +0530 Subject: [PATCH 24/45] refactor: unit tests for journals --- .../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 fc30c4b8cd..9e857f04c3 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 f4a65cccc48bd15fd732973030451c93629bc84b Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 14 Jul 2023 16:51:42 +0530 Subject: [PATCH 25/45] refactor: handle diff amount in various names --- 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 dfc3114fa0..2ce1eb8c15 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -980,7 +980,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 @@ -992,7 +995,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" @@ -1024,7 +1028,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, } ) @@ -1043,7 +1047,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 f3363e813a353363696169602bae5b0a36ae0376 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 16 Jul 2023 21:29:19 +0530 Subject: [PATCH 26/45] test: journals against sales invoice --- .../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 9e857f04c3..9a7326ea29 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 70dd9d0671e1d77d50c814885c6a6f59508c4f62 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 17 Jul 2023 12:29:42 +0530 Subject: [PATCH 27/45] chore(test): fix broken unit test --- .../doctype/sales_invoice/test_sales_invoice.py | 11 +---------- erpnext/controllers/tests/test_accounts_controller.py | 4 +--- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 41e55546a8..6ddf305228 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3213,15 +3213,10 @@ 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 - unlink_enabled = frappe.db.get_value( - "Accounts Settings", "Accounts Settings", "unlink_payment_on_cancel_of_invoice" - ) - - frappe.db.set_single_value("Accounts Settings", "unlink_payment_on_cancel_of_invoice", 1) - jv = make_journal_entry("_Test Receivable USD - _TC", "_Test Bank - _TC", -7000, save=False) jv.accounts[0].exchange_rate = 70 @@ -3264,10 +3259,6 @@ class TestSalesInvoice(unittest.TestCase): check_gl_entries(self, si.name, expected_gle, nowdate()) - frappe.db.set_single_value( - "Accounts Settings", "unlink_payment_on_cancel_of_invoice", unlink_enabled - ) - 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 9a7326ea29..eefe202e47 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 37895a361cdf7be4704f376eb6ec749af0ab3c90 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 24 Jul 2023 20:41:05 +0530 Subject: [PATCH 28/45] chore(test): fix broken test case --- 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 6ddf305228..2f193972a7 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 6628632fbb15ddcc80f5af201d15976337141fc6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 25 Jul 2023 10:30:08 +0530 Subject: [PATCH 29/45] chore: type info --- 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 2ce1eb8c15..40432f70f9 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -968,7 +968,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 c87332d5da638c43ff6d0560bf3c26dde81e21cf Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 25 Jul 2023 10:30:49 +0530 Subject: [PATCH 30/45] refactor: cr/dr note will be on single exchange rate --- .../doctype/journal_entry/journal_entry.py | 29 +++++++++++-------- .../payment_reconciliation.py | 7 +++-- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index e6b8b5d281..0c23d772d5 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -768,18 +768,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 d574cd79b8..2c11ef5120 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -650,6 +650,7 @@ 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), + "exchange_rate": inv.exchange_rate, }, { "account": inv.account, @@ -663,13 +664,13 @@ 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), + "exchange_rate": inv.exchange_rate, }, ], } ) - if difference_entry := get_difference_row(inv): - jv.append("accounts", difference_entry) - jv.flags.ignore_mandatory = True + jv.flags.ignore_exchange_rate = True jv.submit() + jv.make_exchange_gain_loss_journal(args=[inv]) From c0b3b069b587cff11969112b01fff08c8df7adf0 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 25 Jul 2023 10:51:58 +0530 Subject: [PATCH 31/45] refactor: split make_exchage_gain_loss_journal into smaller function --- 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 40432f70f9..6bf9d299d2 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -968,6 +968,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 @@ -978,23 +1050,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" @@ -1003,60 +1068,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] @@ -1093,23 +1120,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": @@ -1120,54 +1139,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 update_against_document_in_jv(self): From 1ea1bfebc4a2407961d93a6d0c4c6c9f43202689 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 16:19:38 +0530 Subject: [PATCH 32/45] refactor: convert class method to standalone function --- 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 53d9e21c35..fa889c0f65 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -1836,3 +1836,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 6bf9d299d2..b9fc0826cc 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, @@ -968,78 +973,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 @@ -1068,7 +1001,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, @@ -1140,7 +1074,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 ba1f065765db6fc36358281fb4e4d775f1c1dcb1 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 16:46:50 +0530 Subject: [PATCH 33/45] refactor: create gain/loss on Cr/Dr notes with different exc rates --- .../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 0c23d772d5..daa6355a44 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -586,7 +586,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 2c11ef5120..5937a29ff5 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, ) @@ -673,4 +674,28 @@ def reconcile_dr_cr_note(dr_cr_notes, company): jv.flags.ignore_mandatory = True jv.flags.ignore_exchange_rate = True 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 506a5775f9937fc893ae02b287ecd7303487363c Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 20:53:07 +0530 Subject: [PATCH 34/45] fix: incorrect gain/loss on allocation change on reconciliation tool --- .../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 5937a29ff5..36f362210d 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -277,6 +277,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 e3d2a2c5bdd94364f22828acc40854f6834b66ce Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:12:14 +0530 Subject: [PATCH 35/45] test: cr notes against invoice --- .../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 eefe202e47..fc4fb9fe9b 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 95543225cf402e9f17e05efee80f2dfb199aa4d9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:15:48 +0530 Subject: [PATCH 36/45] fix: cr/dr note should be posted for exc gain/loss --- .../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 36f362210d..59abecd0b9 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -697,10 +697,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 ae424fdfedb49e6018d957eebb11eb4e03d9d410 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:24:08 +0530 Subject: [PATCH 37/45] test: assert ledger after cr note cancellation --- .../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 fc4fb9fe9b..415e1734a9 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 bfa54d533572f33ea5bc83794489293d36949e5d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 21:54:23 +0530 Subject: [PATCH 38/45] fix(test): test case breakage in Github Actions --- 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 e7aca79d08..a6e920b7ef 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 025091161e47bd2ad77beec068d5263567605425 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 26 Jul 2023 22:32:59 +0530 Subject: [PATCH 39/45] refactor(test): assert ledger outstanding --- .../sales_invoice/test_sales_invoice.py | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 2f193972a7..0de43bab2d 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3249,16 +3249,30 @@ class TestSalesInvoice(unittest.TestCase): ) si.save() si.submit() - expected_gle = [ - ["_Test Exchange Gain/Loss - _TC", 500.0, 0.0, nowdate()], ["_Test Receivable USD - _TC", 7500.0, 0.0, nowdate()], - ["_Test Receivable USD - _TC", 0.0, 500.0, nowdate()], ["Sales - _TC", 0.0, 7500.0, nowdate()], ] - check_gl_entries(self, si.name, expected_gle, nowdate()) + 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, + ) + 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 47bbb37291eba1e2bb8217837417a072f73f5634 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 05:54:13 +0530 Subject: [PATCH 40/45] chore: use frappetestcase --- 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 415e1734a9..acda12bf59 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 acc7322874b97830a838d066925aec99b01af129 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 07:52:01 +0530 Subject: [PATCH 41/45] chore: add msgprint for exc JE --- 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 b9fc0826cc..37a18d80e9 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1001,7 +1001,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"), @@ -1017,6 +1017,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 @@ -1074,7 +1079,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, @@ -1090,7 +1095,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 update_against_document_in_jv(self): """ From d9d685615335778cd36734b1d1bd0c2b4189b690 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 08:02:46 +0530 Subject: [PATCH 42/45] chore: rename some internal variables --- 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 fa889c0f65..61359a6671 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -666,8 +666,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]], @@ -676,7 +677,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 804afaa647b5727c37206fc4207c203652c10d53 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 27 Jul 2023 09:30:38 +0530 Subject: [PATCH 43/45] chore(test): use existing company for unit test --- 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 acda12bf59..8e5f813d97 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 567c0ce1e85a42056d76cfc399b3468df32a576a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 28 Jul 2023 08:12:44 +0530 Subject: [PATCH 44/45] chore: don't make gain/loss journal for base currency transactions --- .../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 59abecd0b9..ea06e0ec9a 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -680,27 +680,28 @@ def reconcile_dr_cr_note(dr_cr_notes, company): jv.flags.ignore_exchange_rate = True 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 46ea81440066af74a3b98f4ab9d5006839a17a4b Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 28 Jul 2023 08:29:19 +0530 Subject: [PATCH 45/45] chore: cancel gain/loss je while posting reverse gl --- .../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 daa6355a44..1e1b3ba642 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, @@ -942,6 +943,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 89241ebfe0..dec7f2b777 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -28,7 +28,12 @@ from erpnext.accounts.general_ledger import ( 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, @@ -1018,7 +1023,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 fa18d8fc0e..e6bec93050 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, @@ -1032,6 +1032,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 caf4b6f18b..d669abe910 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 ( @@ -534,6 +534,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""",