From 1f06f2e3a0232f597e2365b3979fa46c841079f2 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Thu, 18 Jun 2026 14:16:41 +0530 Subject: [PATCH] refactor(sales_invoice): simplify SalesInvoiceGLComposer GL builders Extract two cross-cutting helpers used across the GL methods: - _amount_in_account_currency: the repeated 'base if account is in company currency else transaction amount' ternary (customer, tax, item, POS and write-off entries). - _return_aware_against_voucher: the return/self-outstanding against_voucher rule duplicated in the customer and POS entries. Pull the per-item income and discount rows into their own builders and flatten make_item_gl_entries with a guard clause. Complexity drops: make_customer C11->B7, make_pos C12->B7, make_item C14->B10, make_discount C13->B10, make_tax->A3. No behaviour change; entry dicts and amounts are identical (full Sales Invoice suite green). stock_delivered_but_not_billed_gl_entries is left for a coverage-first pass (it is the least-tested GL branch). --- .../sales_invoice/services/gl_composer.py | 239 +++++++++--------- 1 file changed, 124 insertions(+), 115 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/services/gl_composer.py b/erpnext/accounts/doctype/sales_invoice/services/gl_composer.py index 9b5d8e2f187..099a0d625d5 100644 --- a/erpnext/accounts/doctype/sales_invoice/services/gl_composer.py +++ b/erpnext/accounts/doctype/sales_invoice/services/gl_composer.py @@ -93,54 +93,7 @@ class SalesInvoiceGLComposer(BaseGLComposer): if enable_discount_accounting: for item in doc.get("items"): if item.get("discount_amount") and item.get("discount_account"): - discount_amount = item.discount_amount * item.qty - income_account = ( - item.income_account - if (not item.enable_deferred_revenue or doc.is_return) - else item.deferred_revenue_account - ) - - account_currency = get_account_currency(item.discount_account) - gl_entries.append( - doc.get_gl_dict( - { - "account": item.discount_account, - "against": doc.customer, - "debit": flt( - discount_amount * doc.get("conversion_rate"), - item.precision("discount_amount"), - ), - "debit_in_transaction_currency": flt( - discount_amount, item.precision("discount_amount") - ), - "cost_center": item.cost_center, - "project": item.project, - }, - account_currency, - item=item, - ) - ) - - account_currency = get_account_currency(income_account) - gl_entries.append( - doc.get_gl_dict( - { - "account": income_account, - "against": doc.customer, - "credit": flt( - discount_amount * doc.get("conversion_rate"), - item.precision("discount_amount"), - ), - "credit_in_transaction_currency": flt( - discount_amount, item.precision("discount_amount") - ), - "cost_center": item.cost_center, - "project": item.project or doc.project, - }, - account_currency, - item=item, - ) - ) + self._append_item_discount_gl_entries(item, gl_entries) if ( (enable_discount_accounting or doc.get("is_cash_or_non_trade_discount")) @@ -159,6 +112,53 @@ class SalesInvoiceGLComposer(BaseGLComposer): ) ) + def _append_item_discount_gl_entries(self, item, gl_entries) -> None: + doc = self.doc + discount_amount = item.discount_amount * item.qty + income_account = ( + item.income_account + if (not item.enable_deferred_revenue or doc.is_return) + else item.deferred_revenue_account + ) + + account_currency = get_account_currency(item.discount_account) + gl_entries.append( + doc.get_gl_dict( + { + "account": item.discount_account, + "against": doc.customer, + "debit": flt( + discount_amount * doc.get("conversion_rate"), + item.precision("discount_amount"), + ), + "debit_in_transaction_currency": flt(discount_amount, item.precision("discount_amount")), + "cost_center": item.cost_center, + "project": item.project, + }, + account_currency, + item=item, + ) + ) + + account_currency = get_account_currency(income_account) + gl_entries.append( + doc.get_gl_dict( + { + "account": income_account, + "against": doc.customer, + "credit": flt( + discount_amount * doc.get("conversion_rate"), + item.precision("discount_amount"), + ), + "credit_in_transaction_currency": flt(discount_amount, item.precision("discount_amount")), + "cost_center": item.cost_center, + "project": item.project or doc.project, + }, + account_currency, + item=item, + ) + ) + def stock_delivered_but_not_billed_gl_entries(self, gl_entries): doc = self.doc if doc.update_stock or not cint(erpnext.is_perpetual_inventory_enabled(doc.company)): @@ -250,10 +250,6 @@ class SalesInvoiceGLComposer(BaseGLComposer): ) if grand_total and not doc.is_internal_transfer(): - against_voucher = doc.name - if doc.is_return and doc.return_against and not doc.update_outstanding_for_self: - against_voucher = doc.return_against - # Did not use base_grand_total to book rounding loss gle gl_entries.append( self.get_gl_dict( @@ -264,11 +260,11 @@ class SalesInvoiceGLComposer(BaseGLComposer): "due_date": doc.due_date, "against": doc.against_income_account, "debit": base_grand_total, - "debit_in_account_currency": base_grand_total - if doc.party_account_currency == doc.company_currency - else grand_total, + "debit_in_account_currency": self._amount_in_account_currency( + doc.party_account_currency, base_grand_total, grand_total + ), "debit_in_transaction_currency": grand_total, - "against_voucher": against_voucher, + "against_voucher": self._return_aware_against_voucher(), "against_voucher_type": doc.doctype, "cost_center": doc.cost_center, "project": doc.project, @@ -296,10 +292,10 @@ class SalesInvoiceGLComposer(BaseGLComposer): "account": tax.account_head, "against": doc.customer, "credit": flt(base_amount, tax.precision("tax_amount_after_discount_amount")), - "credit_in_account_currency": ( - flt(base_amount, tax.precision("base_tax_amount_after_discount_amount")) - if account_currency == doc.company_currency - else flt(amount, tax.precision("tax_amount_after_discount_amount")) + "credit_in_account_currency": self._amount_in_account_currency( + account_currency, + flt(base_amount, tax.precision("base_tax_amount_after_discount_amount")), + flt(amount, tax.precision("tax_amount_after_discount_amount")), ), "credit_in_transaction_currency": flt( amount, tax.precision("tax_amount_after_discount_amount") @@ -341,53 +337,57 @@ class SalesInvoiceGLComposer(BaseGLComposer): ) for item in doc.get("items"): - if ( + if not ( flt(item.base_net_amount, item.precision("base_net_amount")) or item.is_fixed_asset or enable_discount_accounting ): - # Do not book income for transfer within same company - if doc.is_internal_transfer(): - continue + continue - if item.is_fixed_asset and item.asset: - self.get_gl_entries_for_fixed_asset(item, gl_entries) - else: - income_account = ( - item.income_account - if (not item.enable_deferred_revenue or doc.is_return) - else item.deferred_revenue_account - ) + # Do not book income for transfer within same company + if doc.is_internal_transfer(): + continue - amount, base_amount = tax_service.get_amount_and_base_amount( - item, enable_discount_accounting - ) - - account_currency = get_account_currency(income_account) - gl_entries.append( - self.get_gl_dict( - { - "account": income_account, - "against": doc.customer, - "credit": flt(base_amount, item.precision("base_net_amount")), - "credit_in_account_currency": ( - flt(base_amount, item.precision("base_net_amount")) - if account_currency == doc.company_currency - else flt(amount, item.precision("net_amount")) - ), - "credit_in_transaction_currency": flt(amount, item.precision("net_amount")), - "cost_center": item.cost_center, - "project": item.project or doc.project, - }, - account_currency, - item=item, - ) - ) + if item.is_fixed_asset and item.asset: + self.get_gl_entries_for_fixed_asset(item, gl_entries) + else: + self._append_item_income_gl_entry(item, gl_entries, tax_service, enable_discount_accounting) # expense account gl entries if cint(doc.update_stock) and erpnext.is_perpetual_inventory_enabled(doc.company): gl_entries += super(SalesInvoice, doc).get_gl_entries() + def _append_item_income_gl_entry(self, item, gl_entries, tax_service, enable_discount_accounting) -> None: + doc = self.doc + income_account = ( + item.income_account + if (not item.enable_deferred_revenue or doc.is_return) + else item.deferred_revenue_account + ) + + amount, base_amount = tax_service.get_amount_and_base_amount(item, enable_discount_accounting) + + account_currency = get_account_currency(income_account) + gl_entries.append( + self.get_gl_dict( + { + "account": income_account, + "against": doc.customer, + "credit": flt(base_amount, item.precision("base_net_amount")), + "credit_in_account_currency": self._amount_in_account_currency( + account_currency, + flt(base_amount, item.precision("base_net_amount")), + flt(amount, item.precision("net_amount")), + ), + "credit_in_transaction_currency": flt(amount, item.precision("net_amount")), + "cost_center": item.cost_center, + "project": item.project or doc.project, + }, + account_currency, + item=item, + ) + ) + def get_gl_entries_for_fixed_asset(self, item, gl_entries): doc = self.doc asset = frappe.get_cached_doc("Asset", item.asset) @@ -461,10 +461,6 @@ class SalesInvoiceGLComposer(BaseGLComposer): if skip_change_gl_entries and payment_mode.account == doc.account_for_change_amount: payment_mode.base_amount -= flt(doc.change_amount) - against_voucher = doc.name - if doc.is_return and doc.return_against and not doc.update_outstanding_for_self: - against_voucher = doc.return_against - if payment_mode.base_amount: # POS, make payment entries gl_entries.append( @@ -475,11 +471,11 @@ class SalesInvoiceGLComposer(BaseGLComposer): "party": doc.customer, "against": payment_mode.account, "credit": payment_mode.base_amount, - "credit_in_account_currency": payment_mode.base_amount - if doc.party_account_currency == doc.company_currency - else payment_mode.amount, + "credit_in_account_currency": self._amount_in_account_currency( + doc.party_account_currency, payment_mode.base_amount, payment_mode.amount + ), "credit_in_transaction_currency": payment_mode.amount, - "against_voucher": against_voucher, + "against_voucher": self._return_aware_against_voucher(), "against_voucher_type": doc.doctype, "cost_center": doc.cost_center, }, @@ -495,9 +491,11 @@ class SalesInvoiceGLComposer(BaseGLComposer): "account": payment_mode.account, "against": doc.customer, "debit": payment_mode.base_amount, - "debit_in_account_currency": payment_mode.base_amount - if payment_mode_account_currency == doc.company_currency - else payment_mode.amount, + "debit_in_account_currency": self._amount_in_account_currency( + payment_mode_account_currency, + payment_mode.base_amount, + payment_mode.amount, + ), "debit_in_transaction_currency": payment_mode.amount, "cost_center": doc.cost_center, }, @@ -570,10 +568,10 @@ class SalesInvoiceGLComposer(BaseGLComposer): "party": doc.customer, "against": doc.write_off_account, "credit": flt(doc.base_write_off_amount, doc.precision("base_write_off_amount")), - "credit_in_account_currency": ( - flt(doc.base_write_off_amount, doc.precision("base_write_off_amount")) - if doc.party_account_currency == doc.company_currency - else flt(doc.write_off_amount, doc.precision("write_off_amount")) + "credit_in_account_currency": self._amount_in_account_currency( + doc.party_account_currency, + flt(doc.base_write_off_amount, doc.precision("base_write_off_amount")), + flt(doc.write_off_amount, doc.precision("write_off_amount")), ), "credit_in_transaction_currency": flt( doc.write_off_amount, doc.precision("write_off_amount") @@ -593,10 +591,10 @@ class SalesInvoiceGLComposer(BaseGLComposer): "account": doc.write_off_account, "against": doc.customer, "debit": flt(doc.base_write_off_amount, doc.precision("base_write_off_amount")), - "debit_in_account_currency": ( - flt(doc.base_write_off_amount, doc.precision("base_write_off_amount")) - if write_off_account_currency == doc.company_currency - else flt(doc.write_off_amount, doc.precision("write_off_amount")) + "debit_in_account_currency": self._amount_in_account_currency( + write_off_account_currency, + flt(doc.base_write_off_amount, doc.precision("base_write_off_amount")), + flt(doc.write_off_amount, doc.precision("write_off_amount")), ), "debit_in_transaction_currency": flt( doc.write_off_amount, doc.precision("write_off_amount") @@ -659,3 +657,14 @@ class SalesInvoiceGLComposer(BaseGLComposer): item=doc, ) ) + + def _amount_in_account_currency(self, account_currency, base_amount, transaction_amount): + """Base amount when the account is in company currency, else the transaction amount.""" + return base_amount if account_currency == self.doc.company_currency else transaction_amount + + def _return_aware_against_voucher(self) -> str: + """Settle against the original invoice for returns not kept on their own outstanding.""" + doc = self.doc + if doc.is_return and doc.return_against and not doc.update_outstanding_for_self: + return doc.return_against + return doc.name