Merge pull request #36911 from ruthra-kumar/deduplicate_gain_loss_journal_creation
fix: deduplicate gain/loss JE creation for journals as payment
diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py
index 7ef5278..4ef35fd 100644
--- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py
+++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py
@@ -116,7 +116,7 @@
"Journal Entry" as reference_type, t1.name as reference_name,
t1.posting_date, t1.remark as remarks, t2.name as reference_row,
{dr_or_cr} as amount, t2.is_advance, t2.exchange_rate,
- t2.account_currency as currency
+ t2.account_currency as currency, t2.cost_center as cost_center
from
`tabJournal Entry` t1, `tabJournal Entry Account` t2
where
@@ -209,6 +209,7 @@
"amount": -(inv.outstanding_in_account_currency),
"posting_date": inv.posting_date,
"currency": inv.currency,
+ "cost_center": inv.cost_center,
}
)
)
@@ -357,6 +358,7 @@
"allocated_amount": allocated_amount,
"difference_amount": pay.get("difference_amount"),
"currency": inv.get("currency"),
+ "cost_center": pay.get("cost_center"),
}
)
@@ -431,6 +433,7 @@
"allocated_amount": flt(row.get("allocated_amount")),
"difference_amount": flt(row.get("difference_amount")),
"difference_account": row.get("difference_account"),
+ "cost_center": row.get("cost_center"),
}
)
@@ -603,7 +606,7 @@
inv.dr_or_cr: abs(inv.allocated_amount),
"reference_type": inv.against_voucher_type,
"reference_name": inv.against_voucher,
- "cost_center": erpnext.get_default_cost_center(company),
+ "cost_center": inv.cost_center or erpnext.get_default_cost_center(company),
"exchange_rate": inv.exchange_rate,
"user_remark": f"{fmt_money(flt(inv.allocated_amount), currency=company_currency)} against {inv.against_voucher}",
},
@@ -618,7 +621,7 @@
),
"reference_type": inv.voucher_type,
"reference_name": inv.voucher_no,
- "cost_center": erpnext.get_default_cost_center(company),
+ "cost_center": inv.cost_center or erpnext.get_default_cost_center(company),
"exchange_rate": inv.exchange_rate,
"user_remark": f"{fmt_money(flt(inv.allocated_amount), currency=company_currency)} from {inv.voucher_no}",
},
@@ -657,4 +660,5 @@
inv.against_voucher_type,
inv.against_voucher,
None,
+ inv.cost_center,
)
diff --git a/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json b/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json
index 0f7e47a..ec718aa 100644
--- a/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json
+++ b/erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json
@@ -22,7 +22,8 @@
"column_break_7",
"difference_account",
"exchange_rate",
- "currency"
+ "currency",
+ "cost_center"
],
"fields": [
{
@@ -144,11 +145,17 @@
"fieldtype": "Float",
"label": "Exchange Rate",
"read_only": 1
+ },
+ {
+ "fieldname": "cost_center",
+ "fieldtype": "Link",
+ "label": "Cost Center",
+ "options": "Cost Center"
}
],
"istable": 1,
"links": [],
- "modified": "2022-12-24 21:01:14.882747",
+ "modified": "2023-09-03 07:52:33.684217",
"modified_by": "Administrator",
"module": "Accounts",
"name": "Payment Reconciliation Allocation",
diff --git a/erpnext/accounts/doctype/payment_reconciliation_payment/payment_reconciliation_payment.json b/erpnext/accounts/doctype/payment_reconciliation_payment/payment_reconciliation_payment.json
index d300ea9..17f3900 100644
--- a/erpnext/accounts/doctype/payment_reconciliation_payment/payment_reconciliation_payment.json
+++ b/erpnext/accounts/doctype/payment_reconciliation_payment/payment_reconciliation_payment.json
@@ -16,7 +16,8 @@
"sec_break1",
"remark",
"currency",
- "exchange_rate"
+ "exchange_rate",
+ "cost_center"
],
"fields": [
{
@@ -98,11 +99,17 @@
"fieldtype": "Float",
"hidden": 1,
"label": "Exchange Rate"
+ },
+ {
+ "fieldname": "cost_center",
+ "fieldtype": "Link",
+ "label": "Cost Center",
+ "options": "Cost Center"
}
],
"istable": 1,
"links": [],
- "modified": "2022-11-08 18:18:36.268760",
+ "modified": "2023-09-03 07:43:29.965353",
"modified_by": "Administrator",
"module": "Accounts",
"name": "Payment Reconciliation Payment",
diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py
index f61d506..06d05cb 100644
--- a/erpnext/accounts/utils.py
+++ b/erpnext/accounts/utils.py
@@ -474,10 +474,12 @@
# update ref in advance entry
if voucher_type == "Journal Entry":
- update_reference_in_journal_entry(entry, doc, do_not_save=True)
+ referenced_row = update_reference_in_journal_entry(entry, doc, do_not_save=False)
# 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)
+ # referenced_row is used to deduplicate gain/loss journal
+ entry.update({"referenced_row": referenced_row})
+ doc.make_exchange_gain_loss_journal([entry])
else:
update_reference_in_payment_entry(
entry, doc, do_not_save=True, skip_ref_details_update_for_pe=skip_ref_details_update_for_pe
@@ -627,6 +629,8 @@
if not do_not_save:
journal_entry.save(ignore_permissions=True)
+ return new_row.name
+
def update_reference_in_payment_entry(
d, payment_entry, do_not_save=False, skip_ref_details_update_for_pe=False
@@ -1750,6 +1754,7 @@
ple.posting_date,
ple.due_date,
ple.account_currency.as_("currency"),
+ ple.cost_center.as_("cost_center"),
Sum(ple.amount).as_("amount"),
Sum(ple.amount_in_account_currency).as_("amount_in_account_currency"),
)
@@ -1812,6 +1817,7 @@
).as_("paid_amount_in_account_currency"),
Table("vouchers").due_date,
Table("vouchers").currency,
+ Table("vouchers").cost_center.as_("cost_center"),
)
.where(Criterion.all(filter_on_outstanding_amount))
)
@@ -1895,12 +1901,14 @@
ref2_dt,
ref2_dn,
ref2_detail_no,
+ cost_center,
) -> 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
+ journal_entry.is_system_generated = True
party_account_currency = frappe.get_cached_value("Account", party_account, "account_currency")
@@ -1919,7 +1927,7 @@
"party": party,
"account_currency": party_account_currency,
"exchange_rate": 0,
- "cost_center": erpnext.get_default_cost_center(company),
+ "cost_center": cost_center or erpnext.get_default_cost_center(company),
"reference_type": ref1_dt,
"reference_name": ref1_dn,
"reference_detail_no": ref1_detail_no,
@@ -1935,7 +1943,7 @@
"account": gain_loss_account,
"account_currency": gain_loss_account_currency,
"exchange_rate": 1,
- "cost_center": erpnext.get_default_cost_center(company),
+ "cost_center": cost_center or erpnext.get_default_cost_center(company),
"reference_type": ref2_dt,
"reference_name": ref2_dn,
"reference_detail_no": ref2_detail_no,
diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py
index aa2f9f2..fd2be2c 100644
--- a/erpnext/controllers/accounts_controller.py
+++ b/erpnext/controllers/accounts_controller.py
@@ -1023,6 +1023,44 @@
)
)
+ def gain_loss_journal_already_booked(
+ self,
+ gain_loss_account,
+ exc_gain_loss,
+ ref2_dt,
+ ref2_dn,
+ ref2_detail_no,
+ ) -> bool:
+ """
+ Check if gain/loss is booked
+ """
+ if res := frappe.db.get_all(
+ "Journal Entry Account",
+ filters={
+ "docstatus": 1,
+ "account": gain_loss_account,
+ "reference_type": ref2_dt, # this will be Journal Entry
+ "reference_name": ref2_dn,
+ "reference_detail_no": ref2_detail_no,
+ },
+ pluck="parent",
+ ):
+ # deduplicate
+ res = list({x for x in res})
+ if exc_vouchers := frappe.db.get_all(
+ "Journal Entry",
+ filters={"name": ["in", res], "voucher_type": "Exchange Gain Or Loss"},
+ fields=["voucher_type", "total_debit", "total_credit"],
+ ):
+ booked_voucher = exc_vouchers[0]
+ if (
+ booked_voucher.total_debit == exc_gain_loss
+ and booked_voucher.total_credit == exc_gain_loss
+ and booked_voucher.voucher_type == "Exchange Gain Or Loss"
+ ):
+ return True
+ return False
+
def make_exchange_gain_loss_journal(self, args: dict = None) -> None:
"""
Make Exchange Gain/Loss journal for Invoices and Payments
@@ -1051,27 +1089,35 @@
reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit"
- je = create_gain_loss_journal(
- self.company,
- arg.get("party_type"),
- arg.get("party"),
- party_account,
+ if not self.gain_loss_journal_already_booked(
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"),
- )
- frappe.msgprint(
- _("Exchange Gain/Loss amount has been booked through {0}").format(
- get_link_to_form("Journal Entry", je)
+ arg.get("referenced_row"),
+ ):
+ je = create_gain_loss_journal(
+ self.company,
+ 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("referenced_row"),
+ arg.get("cost_center"),
)
- )
+ 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
@@ -1144,6 +1190,7 @@
self.doctype,
self.name,
d.idx,
+ self.cost_center,
)
frappe.msgprint(
_("Exchange Gain/Loss amount has been booked through {0}").format(
diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py
index 0f8e133..391258f 100644
--- a/erpnext/controllers/tests/test_accounts_controller.py
+++ b/erpnext/controllers/tests/test_accounts_controller.py
@@ -55,6 +55,7 @@
10 series - Sales Invoice against Payment Entries
20 series - Sales Invoice against Journals
30 series - Sales Invoice against Credit Notes
+ 40 series - Company default Cost center is unset
"""
def setUp(self):
@@ -941,6 +942,60 @@
self.assertEqual(exc_je_for_si, [])
self.assertEqual(exc_je_for_je, [])
+ def test_24_journal_against_multiple_invoices(self):
+ si1 = self.create_sales_invoice(qty=1, conversion_rate=80, rate=1)
+ si2 = 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=-2,
+ acc2_amount=-150,
+ acc2_exc_rate=1,
+ )
+ je.accounts[0].party_type = "Customer"
+ je.accounts[0].party = self.customer
+ je = je.save().submit()
+
+ pr = self.create_payment_reconciliation()
+ pr.get_unreconciled_entries()
+ self.assertEqual(len(pr.invoices), 2)
+ 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)
+
+ si1.reload()
+ si2.reload()
+
+ self.assertEqual(si1.outstanding_amount, 0)
+ self.assertEqual(si2.outstanding_amount, 0)
+ self.assert_ledger_outstanding(si1.doctype, si1.name, 0.0, 0.0)
+ self.assert_ledger_outstanding(si2.doctype, si2.name, 0.0, 0.0)
+
+ # Exchange Gain/Loss Journal should've been created
+ # remove payment JE from list
+ exc_je_for_si1 = [x for x in self.get_journals_for(si1.doctype, si1.name) if x.parent != je.name]
+ exc_je_for_si2 = [x for x in self.get_journals_for(si2.doctype, si2.name) if x.parent != je.name]
+ exc_je_for_je = [x for x in self.get_journals_for(je.doctype, je.name) if x.parent != je.name]
+ self.assertEqual(len(exc_je_for_si1), 1)
+ self.assertEqual(len(exc_je_for_si2), 1)
+ self.assertEqual(len(exc_je_for_je), 2)
+
+ si1.cancel()
+ # Gain/Loss JE of si1 should've been cancelled
+ exc_je_for_si1 = [x for x in self.get_journals_for(si1.doctype, si1.name) if x.parent != je.name]
+ exc_je_for_si2 = [x for x in self.get_journals_for(si2.doctype, si2.name) if x.parent != je.name]
+ exc_je_for_je = [x for x in self.get_journals_for(je.doctype, je.name) if x.parent != je.name]
+ self.assertEqual(len(exc_je_for_si1), 0)
+ self.assertEqual(len(exc_je_for_si2), 1)
+ self.assertEqual(len(exc_je_for_je), 1)
+
def test_30_cr_note_against_sales_invoice(self):
"""
Reconciling Cr Note against Sales Invoice, both having different exchange rates
@@ -997,3 +1052,139 @@
si.reload()
self.assertEqual(si.outstanding_amount, 1)
self.assert_ledger_outstanding(si.doctype, si.name, 80.0, 1.0)
+
+ def test_40_cost_center_from_payment_entry(self):
+ """
+ Gain/Loss JE should inherit cost center from payment if company default is unset
+ """
+ # remove default cost center
+ cc = frappe.db.get_value("Company", self.company, "cost_center")
+ frappe.db.set_value("Company", self.company, "cost_center", None)
+
+ rate_in_account_currency = 1
+ si = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_submit=True)
+ si.cost_center = None
+ si.save().submit()
+
+ pe = get_payment_entry(si.doctype, si.name)
+ pe.source_exchange_rate = 75
+ pe.received_amount = 75
+ pe.cost_center = self.cost_center
+ pe = pe.save().submit()
+
+ # 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])
+
+ self.assertEqual(
+ [self.cost_center, self.cost_center],
+ frappe.db.get_all(
+ "Journal Entry Account", filters={"parent": exc_je_for_si[0].parent}, pluck="cost_center"
+ ),
+ )
+ frappe.db.set_value("Company", self.company, "cost_center", cc)
+
+ def test_41_cost_center_from_journal_entry(self):
+ """
+ Gain/Loss JE should inherit cost center from payment if company default is unset
+ """
+ # remove default cost center
+ cc = frappe.db.get_value("Company", self.company, "cost_center")
+ frappe.db.set_value("Company", self.company, "cost_center", None)
+
+ rate_in_account_currency = 1
+ si = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_submit=True)
+ si.cost_center = None
+ si.save().submit()
+
+ 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.accounts[0].cost_center = self.cost_center
+ je = je.save().submit()
+
+ # Reconcile
+ 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}))
+ 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 = [x for x in self.get_journals_for(je.doctype, je.name) if x.parent != 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.assertEqual(exc_je_for_si[0], exc_je_for_je[0])
+
+ self.assertEqual(
+ [self.cost_center, self.cost_center],
+ frappe.db.get_all(
+ "Journal Entry Account", filters={"parent": exc_je_for_si[0].parent}, pluck="cost_center"
+ ),
+ )
+ frappe.db.set_value("Company", self.company, "cost_center", cc)
+
+ def test_42_cost_center_from_cr_note(self):
+ """
+ Gain/Loss JE should inherit cost center from payment if company default is unset
+ """
+ # remove default cost center
+ cc = frappe.db.get_value("Company", self.company, "cost_center")
+ frappe.db.set_value("Company", self.company, "cost_center", None)
+
+ rate_in_account_currency = 1
+ si = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_submit=True)
+ si.cost_center = None
+ si.save().submit()
+
+ cr_note = self.create_sales_invoice(qty=-1, conversion_rate=75, rate=1, do_not_save=True)
+ cr_note.cost_center = self.cost_center
+ cr_note.is_return = 1
+ cr_note.save().submit()
+
+ # Reconcile
+ 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}))
+ 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 = self.get_journals_for(si.doctype, si.name)
+ exc_je_for_cr_note = 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_note), 2)
+ self.assertEqual(exc_je_for_si, exc_je_for_cr_note)
+
+ for x in exc_je_for_si + exc_je_for_cr_note:
+ with self.subTest(x=x):
+ self.assertEqual(
+ [self.cost_center, self.cost_center],
+ frappe.db.get_all("Journal Entry Account", filters={"parent": x.parent}, pluck="cost_center"),
+ )
+
+ frappe.db.set_value("Company", self.company, "cost_center", cc)