Merge pull request #36911 from ruthra-kumar/deduplicate_gain_loss_journal_creation

fix: deduplicate gain/loss JE creation for journals as payment

erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py

@@ -116,7 +116,7 @@ class PaymentReconciliation(Document):
 				"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 @@ class PaymentReconciliation(Document):
 								"amount": -(inv.outstanding_in_account_currency),
 								"posting_date": inv.posting_date,
 								"currency": inv.currency,
+								"cost_center": inv.cost_center,
 							}
 						)
 					)
@@ -357,6 +358,7 @@ class PaymentReconciliation(Document):
 				"allocated_amount": allocated_amount,
 				"difference_amount": pay.get("difference_amount"),
 				"currency": inv.get("currency"),
+				"cost_center": pay.get("cost_center"),
 			}
 		)
 
@@ -431,6 +433,7 @@ class PaymentReconciliation(Document):
 				"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 @@ def reconcile_dr_cr_note(dr_cr_notes, company):
 						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 @@ 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),
+						"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 @@ def reconcile_dr_cr_note(dr_cr_notes, company):
 				inv.against_voucher_type,
 				inv.against_voucher,
 				None,
+				inv.cost_center,
 			)

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",

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",

erpnext/accounts/utils.py

@@ -474,10 +474,12 @@ 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)
+				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 @@ def update_reference_in_journal_entry(d, journal_entry, do_not_save=False):
 	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 @@ class QueryPaymentLedger(object):
 				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 @@ class QueryPaymentLedger(object):
 				).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 @@ def create_gain_loss_journal(
 	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 @@ def create_gain_loss_journal(
 			"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 @@ def create_gain_loss_journal(
 			"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,

erpnext/controllers/accounts_controller.py

@@ -1023,6 +1023,44 @@ class AccountsController(TransactionBase):
 				)
 			)
 
+	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,6 +1089,13 @@ class AccountsController(TransactionBase):
 
 							reverse_dr_or_cr = "debit" if dr_or_cr == "credit" else "credit"
 
+							if not self.gain_loss_journal_already_booked(
+								gain_loss_account,
+								difference_amount,
+								self.doctype,
+								self.name,
+								arg.get("referenced_row"),
+							):
 								je = create_gain_loss_journal(
 									self.company,
 									arg.get("party_type"),
@@ -1065,7 +1110,8 @@ class AccountsController(TransactionBase):
 									arg.get("idx"),
 									self.doctype,
 									self.name,
-								arg.get("idx"),
+									arg.get("referenced_row"),
+									arg.get("cost_center"),
 								)
 								frappe.msgprint(
 									_("Exchange Gain/Loss amount has been booked through {0}").format(
@@ -1144,6 +1190,7 @@ class AccountsController(TransactionBase):
 							self.doctype,
 							self.name,
 							d.idx,
+							self.cost_center,
 						)
 						frappe.msgprint(
 							_("Exchange Gain/Loss amount has been booked through {0}").format(

erpnext/controllers/tests/test_accounts_controller.py

@@ -55,6 +55,7 @@ class TestAccountsController(FrappeTestCase):
 	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 @@ class TestAccountsController(FrappeTestCase):
 		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 @@ class TestAccountsController(FrappeTestCase):
 		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)