From 199a64937b4d504b1ff3b151d913e47ad7ff5361 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 8 May 2024 09:55:55 +0530 Subject: [PATCH 01/15] chore: remove validation on payment entry (cherry picked from commit e7740033cad4bfbe0ca35ca05795635b2db4257e) --- .../doctype/payment_entry/payment_entry.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 69075e5cf6c..e4f6a735183 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -163,22 +163,6 @@ class PaymentEntry(AccountsController): alert=True, ) - def validate_advance_account_currency(self): - if self.book_advance_payments_in_separate_party_account is True: - company_currency = frappe.get_cached_value("Company", self.company, "default_currency") - if self.payment_type == "Receive" and self.paid_from_account_currency != company_currency: - frappe.throw( - _("Booking advances in foreign currency account: {0} ({1}) is not yet supported.").format( - frappe.bold(self.paid_from), frappe.bold(self.paid_from_account_currency) - ) - ) - if self.payment_type == "Pay" and self.paid_to_account_currency != company_currency: - frappe.throw( - _("Booking advances in foreign currency account: {0} ({1}) is not yet supported.").format( - frappe.bold(self.paid_to), frappe.bold(self.paid_to_account_currency) - ) - ) - def on_cancel(self): self.ignore_linked_doctypes = ( "GL Entry", From 5d2f296ca8c350f28d6b650dad13a6f9b581f6f2 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 9 May 2024 11:06:06 +0530 Subject: [PATCH 02/15] refactor: convert amount to base currency for advances (cherry picked from commit c9ede1ffbe25d6250b52910372c1d7cfa82096c5) --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index e4f6a735183..79ed5639cf2 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1250,7 +1250,7 @@ class PaymentEntry(AccountsController): dr_or_cr, account = self.get_dr_and_account_for_advances(invoice) args_dict["account"] = account - args_dict[dr_or_cr] = invoice.allocated_amount + args_dict[dr_or_cr] = self.calculate_base_allocated_amount_for_reference(invoice) args_dict[dr_or_cr + "_in_account_currency"] = invoice.allocated_amount args_dict.update( { @@ -1269,7 +1269,7 @@ class PaymentEntry(AccountsController): args_dict[dr_or_cr + "_in_account_currency"] = 0 dr_or_cr = "debit" if dr_or_cr == "credit" else "credit" args_dict["account"] = self.party_account - args_dict[dr_or_cr] = invoice.allocated_amount + args_dict[dr_or_cr] = self.calculate_base_allocated_amount_for_reference(invoice) args_dict[dr_or_cr + "_in_account_currency"] = invoice.allocated_amount args_dict.update( { From 47071cec5df815534e72ef4730349d8cd8d0d749 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 20 May 2024 16:00:39 +0530 Subject: [PATCH 03/15] refactor: for advances uses the party account in references table (cherry picked from commit 7dce6e03c7f8452fa9ca942b38a0cf1bd2264d3e) --- erpnext/controllers/accounts_controller.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 40ce8b6bc57..f576ecc1b11 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1430,10 +1430,13 @@ class AccountsController(TransactionBase): if d.exchange_gain_loss and ( (d.reference_doctype, d.reference_name, str(d.idx)) not in booked ): - if self.payment_type == "Receive": - party_account = self.paid_from - elif self.payment_type == "Pay": - party_account = self.paid_to + if self.book_advance_payments_in_separate_party_account: + party_account = d.account + else: + if self.payment_type == "Receive": + party_account = self.paid_from + elif self.payment_type == "Pay": + party_account = self.paid_to dr_or_cr = "debit" if d.exchange_gain_loss > 0 else "credit" From 6ebd4ba2ccf8de2045f6d9d7691c97b9a11e61e1 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 28 May 2024 08:44:19 +0530 Subject: [PATCH 04/15] refactor(test): simpler create_account helper method (cherry picked from commit 475e0ddeeef3700e2c75910886fad3e44f58eb6d) --- .../tests/test_accounts_controller.py | 98 +++++++++++-------- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index f91acb2e4ea..37d98549a4d 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -55,6 +55,7 @@ class TestAccountsController(FrappeTestCase): 40 series - Company default Cost center is unset 50 series - Journals against Journals 60 series - Journals against Payment Entries + 70 series - Advances in Separate party account. Both Party and Advance account are in Foreign currency. 90 series - Dimension inheritence """ @@ -114,47 +115,64 @@ class TestAccountsController(FrappeTestCase): self.supplier = make_supplier("_Test MC Supplier 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 + accounts = [ + frappe._dict( + { + "attribute_name": "debtors_usd", + "name": "Debtors USD", + "account_type": "Receivable", + "account_currency": "USD", + "parent_account": "Accounts Receivable - " + self.company_abbr, + } + ), + frappe._dict( + { + "attribute_name": "creditors_usd", + "name": "Creditors USD", + "account_type": "Payable", + "account_currency": "USD", + "parent_account": "Accounts Payable - " + self.company_abbr, + } + ), + # Advance accounts under Asset and Liability header + frappe._dict( + { + "attribute_name": "debtors_advance_usd", + "name": "Advance Received USD", + "account_type": "Receivable", + "account_currency": "USD", + "parent_account": "Current Liabilities - " + self.company_abbr, + } + ), + frappe._dict( + { + "attribute_name": "creditors_advance_usd", + "name": "Advance Paid USD", + "account_type": "Payable", + "account_currency": "USD", + "parent_account": "Current Assets - " + self.company_abbr, + } + ), + ] - account_name = "Creditors 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 Payable - " + self.company_abbr - acc.company = self.company - acc.account_currency = "USD" - acc.account_type = "Payable" - 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.creditors_usd = acc.name + for x in accounts: + if not frappe.db.get_value("Account", filters={"account_name": x.name, "company": self.company}): + acc = frappe.new_doc("Account") + acc.account_name = x.name + acc.parent_account = x.parent_account + acc.company = self.company + acc.account_currency = x.account_currency + acc.account_type = x.account_type + acc.insert() + else: + name = frappe.db.get_value( + "Account", + filters={"account_name": x.name, "company": self.company}, + fieldname="name", + pluck=True, + ) + acc = frappe.get_doc("Account", name) + setattr(self, x.attribute_name, acc.name) def create_sales_invoice( self, From 259d7cde39daf7249f19a2d083842c8c0e9e71e9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 28 May 2024 11:20:07 +0530 Subject: [PATCH 05/15] test: exc gain/loss booking on advances under asset/liability (cherry picked from commit 827d67d02f745346bd32bed5428cc33a7baba745) --- .../tests/test_accounts_controller.py | 67 ++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 37d98549a4d..8a59f0b070c 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -137,7 +137,7 @@ class TestAccountsController(FrappeTestCase): # Advance accounts under Asset and Liability header frappe._dict( { - "attribute_name": "debtors_advance_usd", + "attribute_name": "advance_received_usd", "name": "Advance Received USD", "account_type": "Receivable", "account_currency": "USD", @@ -146,7 +146,7 @@ class TestAccountsController(FrappeTestCase): ), frappe._dict( { - "attribute_name": "creditors_advance_usd", + "attribute_name": "advance_paid_usd", "name": "Advance Paid USD", "account_type": "Payable", "account_currency": "USD", @@ -174,6 +174,18 @@ class TestAccountsController(FrappeTestCase): acc = frappe.get_doc("Account", name) setattr(self, x.attribute_name, acc.name) + def enable_advances_under_asset_and_liability(self): + company = frappe.get_doc("Company", self.company) + company.book_advance_payments_in_separate_party_account = 1 + company.default_advance_received_account = self.advance_received_usd + company.default_advance_paid_account = self.advance_paid_usd + company.save() + + def disable_advances_under_asset_and_liability(self): + company = frappe.get_doc("Company", self.company) + company.book_advance_payments_in_separate_party_account = 0 + company.save() + def create_sales_invoice( self, qty=1, @@ -1716,3 +1728,54 @@ class TestAccountsController(FrappeTestCase): # Exchange Gain/Loss Journal should've been cancelled exc_je_for_je1 = self.get_journals_for(je1.doctype, je1.name) self.assertEqual(exc_je_for_je1, []) + + def test_70_advance_payment_against_sales_invoice_in_foreign_currency(self): + """ + Customer advance booked under Liability + """ + self.enable_advances_under_asset_and_liability() + + adv = self.create_payment_entry(amount=1, source_exc_rate=83) + adv.save() # explicit 'save' is needed to trigger set_liability_account() + self.assertEqual(adv.paid_from, self.advance_received_usd) + adv.submit() + + si = self.create_sales_invoice(qty=1, conversion_rate=80, rate=1, do_not_submit=True) + si.debit_to = self.debtors_usd + si.save().submit() + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) + + pr = self.create_payment_reconciliation() + pr.receivable_payable_account = self.debtors_usd + pr.default_advance_account = self.advance_received_usd + pr.get_unreconciled_entries() + self.assertEqual(pr.invoices[0].invoice_number, si.name) + self.assertEqual(pr.payments[0].reference_name, adv.name) + + # Allocate and Reconcile + 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) + self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0) + + # Exc Gain/Loss journal should've been creatad + 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(len(exc_je_for_si), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_si, exc_je_for_adv) + + adv.reload() + adv.cancel() + si.reload() + self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0) + # Exc 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(len(exc_je_for_si), 0) + self.assertEqual(len(exc_je_for_adv), 0) + + self.disable_advances_under_asset_and_liability() From 88e1c28e7d070fe028c0e034a2ef483f55c0c664 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 29 May 2024 08:17:02 +0530 Subject: [PATCH 06/15] test: advance against purchase invoice (cherry picked from commit 90c84822d01a9b8ab2aba303846702f08b33c63a) --- .../tests/test_accounts_controller.py | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 8a59f0b070c..d2144adde0f 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -10,6 +10,7 @@ from frappe.utils import add_days, getdate, nowdate 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 @@ -248,6 +249,48 @@ class TestAccountsController(FrappeTestCase): payment.posting_date = posting_date return payment + def create_purchase_invoice( + self, + qty=1, + rate=1, + conversion_rate=80, + posting_date=None, + do_not_save=False, + do_not_submit=False, + ): + """ + Helper function to populate default values in purchase invoice + """ + if posting_date is None: + posting_date = nowdate() + + pinv = make_purchase_invoice( + posting_date=posting_date, + qty=qty, + rate=rate, + company=self.company, + supplier=self.supplier, + item_code=self.item, + item_name=self.item, + cost_center=self.cost_center, + warehouse=self.warehouse, + parent_cost_center=self.cost_center, + update_stock=0, + currency="USD", + conversion_rate=conversion_rate, + is_pos=0, + is_return=0, + income_account=self.income_account, + expense_account=self.expense_account, + do_not_save=True, + ) + pinv.credit_to = self.creditors_usd + if not do_not_save: + pinv.save() + if not do_not_submit: + pinv.submit() + return pinv + def clear_old_entries(self): doctype_list = [ "GL Entry", @@ -1779,3 +1822,72 @@ class TestAccountsController(FrappeTestCase): self.assertEqual(len(exc_je_for_adv), 0) self.disable_advances_under_asset_and_liability() + + def test_71_advance_payment_against_purchase_invoice_in_foreign_currency(self): + """ + Supplier advance booked under Asset + """ + self.enable_advances_under_asset_and_liability() + + usd_amount = 1 + inr_amount = 85 + exc_rate = 85 + adv = create_payment_entry( + company=self.company, + payment_type="Pay", + party_type="Supplier", + party=self.supplier, + paid_from=self.cash, + paid_to=self.advance_paid_usd, + paid_amount=inr_amount, + ) + adv.source_exchange_rate = 1 + adv.target_exchange_rate = exc_rate + adv.received_amount = usd_amount + adv.paid_amount = exc_rate * usd_amount + adv.posting_date = nowdate() + adv.save() + # Make sure that advance account is still set + self.assertEqual(adv.paid_to, self.advance_paid_usd) + adv.submit() + + pi = self.create_purchase_invoice(qty=1, conversion_rate=83, rate=1) + self.assertEqual(pi.credit_to, self.creditors_usd) + self.assert_ledger_outstanding(pi.doctype, pi.name, 83.0, 1.0) + + pr = self.create_payment_reconciliation() + pr.party_type = "Supplier" + pr.party = self.supplier + pr.receivable_payable_account = self.creditors_usd + pr.default_advance_account = self.advance_paid_usd + pr.get_unreconciled_entries() + self.assertEqual(pr.invoices[0].invoice_number, pi.name) + self.assertEqual(pr.payments[0].reference_name, adv.name) + + # Allocate and Reconcile + 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) + self.assert_ledger_outstanding(pi.doctype, pi.name, 0.0, 0.0) + + # Exc Gain/Loss journal should've been creatad + exc_je_for_pi = self.get_journals_for(pi.doctype, pi.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertEqual(len(exc_je_for_pi), 1) + self.assertEqual(len(exc_je_for_adv), 1) + self.assertEqual(exc_je_for_pi, exc_je_for_adv) + + adv.reload() + adv.cancel() + pi.reload() + self.assert_ledger_outstanding(pi.doctype, pi.name, 83.0, 1.0) + # Exc Gain/Loss journal should've been cancelled + exc_je_for_pi = self.get_journals_for(pi.doctype, pi.name) + exc_je_for_adv = self.get_journals_for(adv.doctype, adv.name) + self.assertEqual(len(exc_je_for_pi), 0) + self.assertEqual(len(exc_je_for_adv), 0) + + self.disable_advances_under_asset_and_liability() From 78ad3f6cdc8689cc6c297eb8ca4d1adcedc17b6a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 29 May 2024 11:26:10 +0530 Subject: [PATCH 07/15] refactor: validation to force accounts to be on same currency (cherry picked from commit 0f0b4d88bcd5e298154748a58c20589c5aa9f531) --- erpnext/buying/doctype/supplier/supplier.py | 1 + erpnext/selling/doctype/customer/customer.py | 1 + erpnext/utilities/transaction_base.py | 57 ++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/erpnext/buying/doctype/supplier/supplier.py b/erpnext/buying/doctype/supplier/supplier.py index 0df17fdd4b9..ff5385dd961 100644 --- a/erpnext/buying/doctype/supplier/supplier.py +++ b/erpnext/buying/doctype/supplier/supplier.py @@ -138,6 +138,7 @@ class Supplier(TransactionBase): validate_party_accounts(self) self.validate_internal_supplier() self.add_role_for_user() + self.validate_currency_for_receivable_payable_and_advance_account() @frappe.whitelist() def get_supplier_group_details(self): diff --git a/erpnext/selling/doctype/customer/customer.py b/erpnext/selling/doctype/customer/customer.py index f5185c2ff5b..8ce67cc659a 100644 --- a/erpnext/selling/doctype/customer/customer.py +++ b/erpnext/selling/doctype/customer/customer.py @@ -144,6 +144,7 @@ class Customer(TransactionBase): self.validate_default_bank_account() self.validate_internal_customer() self.add_role_for_user() + self.validate_currency_for_receivable_payable_and_advance_account() # set loyalty program tier if frappe.db.exists("Customer", self.name): diff --git a/erpnext/utilities/transaction_base.py b/erpnext/utilities/transaction_base.py index 3b7812f96c2..9559c9170cf 100644 --- a/erpnext/utilities/transaction_base.py +++ b/erpnext/utilities/transaction_base.py @@ -168,6 +168,63 @@ class TransactionBase(StatusUpdater): if len(child_table_values) > 1: self.set(default_field, None) + def validate_currency_for_receivable_payable_and_advance_account(self): + if self.doctype in ["Customer", "Supplier"]: + account_type = "Receivable" if self.doctype == "Customer" else "Payable" + for x in self.accounts: + company_default_currency = frappe.get_cached_value("Company", x.company, "default_currency") + receivable_payable_account_currency = None + advance_account_currency = None + + if x.account: + receivable_payable_account_currency = frappe.get_cached_value( + "Account", x.account, "account_currency" + ) + + if x.advance_account: + advance_account_currency = frappe.get_cached_value( + "Account", x.advance_account, "account_currency" + ) + if receivable_payable_account_currency and ( + receivable_payable_account_currency != self.default_currency + and receivable_payable_account_currency != company_default_currency + ): + frappe.throw( + _( + "{0} Account must be in either customer billing currency: {1} or Company default currency: {2}" + ).format( + account_type, + frappe.bold(self.default_currency), + frappe.bold(company_default_currency), + ) + ) + + if advance_account_currency and ( + advance_account_currency != self.default_currency + and advance_account_currency != company_default_currency + ): + frappe.throw( + _( + "Advance Account must be in either customer billing currency: {0} or Company default currency: {1}" + ).format(frappe.bold(self.default_currency), frappe.bold(company_default_currency)) + ) + + if ( + receivable_payable_account_currency + and advance_account_currency + and receivable_payable_account_currency != advance_account_currency + ): + frappe.throw( + _( + "Both {0} Account: {1} and Advance Account: {2} must be of same currency for company: {3}" + ).format( + account_type, + frappe.bold(x.account), + frappe.bold(x.advance_account), + frappe.bold(x.company), + ) + ) + def delete_events(ref_type, ref_name): events = ( From 4bde34539996757f04f1c4f23184b32d22d5ddb7 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 10 Jun 2024 20:06:21 +0530 Subject: [PATCH 08/15] refactor: validation in customer group (cherry picked from commit 4f9a2281754ad64c845739a29c5225059f1cc11d) --- .../doctype/customer_group/customer_group.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/erpnext/setup/doctype/customer_group/customer_group.py b/erpnext/setup/doctype/customer_group/customer_group.py index 0b783c0c56e..6d95d3bcb24 100644 --- a/erpnext/setup/doctype/customer_group/customer_group.py +++ b/erpnext/setup/doctype/customer_group/customer_group.py @@ -38,6 +38,53 @@ class CustomerGroup(NestedSet): def validate(self): if not self.parent_customer_group: self.parent_customer_group = get_root_of("Customer Group") + self.validate_currency_for_receivable_and_advance_account() + + def validate_currency_for_receivable_and_advance_account(self): + for x in self.accounts: + company_default_currency = frappe.get_cached_value("Company", x.company, "default_currency") + receivable_account_currency = None + advance_account_currency = None + + if x.account: + receivable_account_currency = frappe.get_cached_value( + "Account", x.account, "account_currency" + ) + + if x.advance_account: + advance_account_currency = frappe.get_cached_value( + "Account", x.advance_account, "account_currency" + ) + + if receivable_account_currency and receivable_account_currency != company_default_currency: + frappe.throw( + _("Receivable Account: {0} must be in Company default currency: {1}").format( + frappe.bold(x.account), + frappe.bold(company_default_currency), + ) + ) + + if advance_account_currency and advance_account_currency != company_default_currency: + frappe.throw( + _("Advance Account: {0} must be in Company default currency: {1}").format( + frappe.bold(x.advance_account), frappe.bold(company_default_currency) + ) + ) + + if ( + receivable_account_currency + and advance_account_currency + and receivable_account_currency != advance_account_currency + ): + frappe.throw( + _( + "Both Receivable Account: {0} and Advance Account: {1} must be of same currency for company: {2}" + ).format( + frappe.bold(x.account), + frappe.bold(x.advance_account), + frappe.bold(x.company), + ) + ) def on_update(self): self.validate_name_with_customer() From 545d0b9730295407a29990b5b9adc0ce9240b51d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 10 Jun 2024 20:08:28 +0530 Subject: [PATCH 09/15] refactor: validation in Supplier Group (cherry picked from commit 107b614518444339a9674287fa6cb2c788716fa8) --- .../doctype/supplier_group/supplier_group.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/erpnext/setup/doctype/supplier_group/supplier_group.py b/erpnext/setup/doctype/supplier_group/supplier_group.py index b639b962509..fa0c6beac49 100644 --- a/erpnext/setup/doctype/supplier_group/supplier_group.py +++ b/erpnext/setup/doctype/supplier_group/supplier_group.py @@ -3,6 +3,7 @@ import frappe +from frappe import _ from frappe.utils.nestedset import NestedSet, get_root_of @@ -32,6 +33,51 @@ class SupplierGroup(NestedSet): def validate(self): if not self.parent_supplier_group: self.parent_supplier_group = get_root_of("Supplier Group") + self.validate_currency_for_payable_and_advance_account() + + def validate_currency_for_payable_and_advance_account(self): + for x in self.accounts: + company_default_currency = frappe.get_cached_value("Company", x.company, "default_currency") + payable_account_currency = None + advance_account_currency = None + + if x.account: + payable_account_currency = frappe.get_cached_value("Account", x.account, "account_currency") + + if x.advance_account: + advance_account_currency = frappe.get_cached_value( + "Account", x.advance_account, "account_currency" + ) + + if payable_account_currency and payable_account_currency != company_default_currency: + frappe.throw( + _("Payable Account: {0} must be in Company default currency: {1}").format( + frappe.bold(x.account), + frappe.bold(company_default_currency), + ) + ) + + if advance_account_currency and advance_account_currency != company_default_currency: + frappe.throw( + _("Advance Account: {0} must be in Company default currency: {1}").format( + frappe.bold(x.advance_account), frappe.bold(company_default_currency) + ) + ) + + if ( + payable_account_currency + and advance_account_currency + and payable_account_currency != advance_account_currency + ): + frappe.throw( + _( + "Both Payable Account: {0} and Advance Account: {1} must be of same currency for company: {2}" + ).format( + frappe.bold(x.account), + frappe.bold(x.advance_account), + frappe.bold(x.company), + ) + ) def on_update(self): NestedSet.on_update(self) From 2bd10d388ffe7cddb5614e37f644fc0369d8b060 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 11 Jun 2024 10:18:59 +0530 Subject: [PATCH 10/15] refactor: better error messages (cherry picked from commit 83ff94b9b87772251f663f3be59c44cadb2c3a67) --- erpnext/utilities/transaction_base.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/erpnext/utilities/transaction_base.py b/erpnext/utilities/transaction_base.py index 9559c9170cf..6fab5380c38 100644 --- a/erpnext/utilities/transaction_base.py +++ b/erpnext/utilities/transaction_base.py @@ -191,9 +191,11 @@ class TransactionBase(StatusUpdater): ): frappe.throw( _( - "{0} Account must be in either customer billing currency: {1} or Company default currency: {2}" + "{0} Account: {1} ({2}) must be in either customer billing currency: {3} or Company default currency: {4}" ).format( account_type, + frappe.bold(x.account), + frappe.bold(receivable_payable_account_currency), frappe.bold(self.default_currency), frappe.bold(company_default_currency), ) @@ -205,8 +207,12 @@ class TransactionBase(StatusUpdater): ): frappe.throw( _( - "Advance Account must be in either customer billing currency: {0} or Company default currency: {1}" - ).format(frappe.bold(self.default_currency), frappe.bold(company_default_currency)) + "Advance Account: {0} must be in either customer billing currency: {1} or Company default currency: {2}" + ).format( + frappe.bold(x.advance_account), + frappe.bold(self.default_currency), + frappe.bold(company_default_currency), + ) ) if ( From d1679d4663748694c79070587c66e1f951366705 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 17 Jun 2024 07:30:13 +0530 Subject: [PATCH 11/15] chore: fix test data (cherry picked from commit 07d59443b7e679e3dd3498f5837a800983540616) --- erpnext/buying/doctype/supplier/test_records.json | 1 + erpnext/selling/doctype/customer/test_records.json | 1 + 2 files changed, 2 insertions(+) diff --git a/erpnext/buying/doctype/supplier/test_records.json b/erpnext/buying/doctype/supplier/test_records.json index 1aa63fb5ba0..1bb9899cc85 100644 --- a/erpnext/buying/doctype/supplier/test_records.json +++ b/erpnext/buying/doctype/supplier/test_records.json @@ -35,6 +35,7 @@ "doctype": "Supplier", "supplier_name": "_Test Supplier USD", "supplier_group": "_Test Supplier Group", + "default_currency": "USD", "accounts": [{ "company": "_Test Company", "account": "_Test Payable USD - _TC" diff --git a/erpnext/selling/doctype/customer/test_records.json b/erpnext/selling/doctype/customer/test_records.json index 61cb36b0fae..6040f4dd75b 100644 --- a/erpnext/selling/doctype/customer/test_records.json +++ b/erpnext/selling/doctype/customer/test_records.json @@ -47,6 +47,7 @@ "customer_type": "Individual", "doctype": "Customer", "territory": "_Test Territory", + "default_currency": "USD", "accounts": [{ "company": "_Test Company", "account": "_Test Receivable USD - _TC" From a1ebd162841339ae19c0dc620156749716b4bc52 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 17 Jun 2024 08:45:54 +0530 Subject: [PATCH 12/15] chore: remove dead code (cherry picked from commit 7e318c013257ccb673e5065fac32e25e463be96d) --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 79ed5639cf2..a2882206b4a 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -76,7 +76,6 @@ class PaymentEntry(AccountsController): self.setup_party_account_field() self.set_missing_values() self.set_liability_account() - self.validate_advance_account_currency() self.set_missing_ref_details(force=True) self.validate_payment_type() self.validate_party_details() From 3b15708f18e695a93f740f03ddb8b1153abc6208 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 18 Jun 2024 14:35:35 +0530 Subject: [PATCH 13/15] fix(test): incorrect field for customer default billing currency (cherry picked from commit c696d13a5edbd62606efd266db9cd95bf18ed7d6) --- erpnext/accounts/doctype/subscription/test_subscription.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/subscription/test_subscription.py b/erpnext/accounts/doctype/subscription/test_subscription.py index cb4f0200478..ba4963f78ed 100644 --- a/erpnext/accounts/doctype/subscription/test_subscription.py +++ b/erpnext/accounts/doctype/subscription/test_subscription.py @@ -565,7 +565,7 @@ def create_parties(): if not frappe.db.exists("Customer", "_Test Subscription Customer"): customer = frappe.new_doc("Customer") customer.customer_name = "_Test Subscription Customer" - customer.billing_currency = "USD" + customer.default_currency = "USD" customer.append("accounts", {"company": "_Test Company", "account": "_Test Receivable USD - _TC"}) customer.insert() From 6dbe820416fdc91b91ed947f691acf12633b935f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 18 Jun 2024 15:10:25 +0530 Subject: [PATCH 14/15] refactor(test): enfore use of customer/supplier master While using advance accounts in foreign currency, always use Customer/Supplier master to maintain them (cherry picked from commit 64e63887bed71db1cb6e880ee2ae7f2e88d34a6c) --- .../tests/test_accounts_controller.py | 42 +++++++++++++++---- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index d2144adde0f..3f6830c2021 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -175,17 +175,43 @@ class TestAccountsController(FrappeTestCase): acc = frappe.get_doc("Account", name) setattr(self, x.attribute_name, acc.name) - def enable_advances_under_asset_and_liability(self): + def setup_advance_accounts_in_party_master(self): company = frappe.get_doc("Company", self.company) company.book_advance_payments_in_separate_party_account = 1 - company.default_advance_received_account = self.advance_received_usd - company.default_advance_paid_account = self.advance_paid_usd company.save() - def disable_advances_under_asset_and_liability(self): + customer = frappe.get_doc("Customer", self.customer) + customer.append( + "accounts", + { + "company": self.company, + "account": self.debtors_usd, + "advance_account": self.advance_received_usd, + }, + ) + customer.save() + + supplier = frappe.get_doc("Supplier", self.supplier) + supplier.append( + "accounts", + { + "company": self.company, + "account": self.creditors_usd, + "advance_account": self.advance_paid_usd, + }, + ) + supplier.save() + + def remove_advance_accounts_from_party_master(self): company = frappe.get_doc("Company", self.company) company.book_advance_payments_in_separate_party_account = 0 company.save() + customer = frappe.get_doc("Customer", self.customer) + customer.accounts = [] + customer.save() + supplier = frappe.get_doc("Supplier", self.supplier) + supplier.accounts = [] + supplier.save() def create_sales_invoice( self, @@ -1776,7 +1802,7 @@ class TestAccountsController(FrappeTestCase): """ Customer advance booked under Liability """ - self.enable_advances_under_asset_and_liability() + self.setup_advance_accounts_in_party_master() adv = self.create_payment_entry(amount=1, source_exc_rate=83) adv.save() # explicit 'save' is needed to trigger set_liability_account() @@ -1821,13 +1847,13 @@ class TestAccountsController(FrappeTestCase): self.assertEqual(len(exc_je_for_si), 0) self.assertEqual(len(exc_je_for_adv), 0) - self.disable_advances_under_asset_and_liability() + self.remove_advance_accounts_from_party_master() def test_71_advance_payment_against_purchase_invoice_in_foreign_currency(self): """ Supplier advance booked under Asset """ - self.enable_advances_under_asset_and_liability() + self.setup_advance_accounts_in_party_master() usd_amount = 1 inr_amount = 85 @@ -1890,4 +1916,4 @@ class TestAccountsController(FrappeTestCase): self.assertEqual(len(exc_je_for_pi), 0) self.assertEqual(len(exc_je_for_adv), 0) - self.disable_advances_under_asset_and_liability() + self.remove_advance_accounts_from_party_master() From c45ce75f57bf475152f7ac0af020717919c8758a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 22 Jun 2024 19:32:07 +0530 Subject: [PATCH 15/15] refactor(test): make and use a different party for subscription (cherry picked from commit 3fabf4aaa48943f29429931cf0d3e9a3c08a65e5) --- .../accounts/doctype/subscription/test_subscription.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/subscription/test_subscription.py b/erpnext/accounts/doctype/subscription/test_subscription.py index ba4963f78ed..af3916ae469 100644 --- a/erpnext/accounts/doctype/subscription/test_subscription.py +++ b/erpnext/accounts/doctype/subscription/test_subscription.py @@ -476,7 +476,7 @@ class TestSubscription(FrappeTestCase): start_date="2021-01-01", submit_invoice=0, generate_new_invoices_past_due_date=1, - party="_Test Subscription Customer", + party="_Test Subscription Customer John Doe", ) # create invoices for the first two moths @@ -569,6 +569,12 @@ def create_parties(): customer.append("accounts", {"company": "_Test Company", "account": "_Test Receivable USD - _TC"}) customer.insert() + if not frappe.db.exists("Customer", "_Test Subscription Customer John Doe"): + customer = frappe.new_doc("Customer") + customer.customer_name = "_Test Subscription Customer John Doe" + customer.append("accounts", {"company": "_Test Company", "account": "_Test Receivable - _TC"}) + customer.insert() + def reset_settings(): settings = frappe.get_single("Subscription Settings")