refactor(journal_entry): tidy reference-validator and GL-composer services

Add return type hints and option-A docstrings to JournalEntryReferenceValidator,
and split JournalEntryGLComposer.compose into _set_transaction_currency and
_gl_row helpers. Behaviour preserved.

erpnext/accounts/doctype/journal_entry/services/gl_composer.py

@@ -18,86 +18,88 @@ class JournalEntryGLComposer(BaseGLComposer):
 	from the first foreign-currency row (mirroring the former build_gl_map).
 	"""
 
-	def compose(self):
-		doc = self.doc
-		gl_map = []
-
-		company_currency = erpnext.get_company_currency(doc.company)
-		doc.transaction_currency = company_currency
-		doc.transaction_exchange_rate = 1
-		if doc.multi_currency:
-			for row in doc.get("accounts"):
-				if row.account_currency != company_currency:
-					# Journal assumes the first foreign currency as transaction currency
-					doc.transaction_currency = row.account_currency
-					doc.transaction_exchange_rate = row.exchange_rate
-					break
-
+	def compose(self) -> list:
+		"""Project the Journal Entry's non-zero account rows into GL dicts."""
+		self._set_transaction_currency()
 		advance_doctypes = get_advance_payment_doctypes()
 
-		for d in doc.get("accounts"):
-			if d.debit or d.credit or (doc.voucher_type == "Exchange Gain Or Loss"):
-				r = [d.user_remark, doc.remark]
-				r = [x for x in r if x]
-				remarks = "\n".join(r)
-
-				row = {
-					"account": d.account,
-					"party_type": d.party_type,
-					"due_date": doc.due_date,
-					"party": d.party,
-					"against": d.against_account,
-					"debit": flt(d.debit, d.precision("debit")),
-					"credit": flt(d.credit, d.precision("credit")),
-					"account_currency": d.account_currency,
-					"debit_in_account_currency": flt(
-						d.debit_in_account_currency, d.precision("debit_in_account_currency")
-					),
-					"credit_in_account_currency": flt(
-						d.credit_in_account_currency, d.precision("credit_in_account_currency")
-					),
-					"transaction_currency": doc.transaction_currency,
-					"transaction_exchange_rate": doc.transaction_exchange_rate,
-					"debit_in_transaction_currency": flt(
-						d.debit_in_account_currency, d.precision("debit_in_account_currency")
-					)
-					if doc.transaction_currency == d.account_currency
-					else flt(d.debit, d.precision("debit")) / doc.transaction_exchange_rate,
-					"credit_in_transaction_currency": flt(
-						d.credit_in_account_currency, d.precision("credit_in_account_currency")
-					)
-					if doc.transaction_currency == d.account_currency
-					else flt(d.credit, d.precision("credit")) / doc.transaction_exchange_rate,
-					"against_voucher_type": d.reference_type,
-					"against_voucher": d.reference_name,
-					"remarks": remarks,
-					"voucher_detail_no": d.reference_detail_no,
-					"cost_center": d.cost_center,
-					"project": d.project,
-					"finance_book": doc.finance_book,
-					"advance_voucher_type": d.advance_voucher_type,
-					"advance_voucher_no": d.advance_voucher_no,
-				}
-
-				if d.reference_type in advance_doctypes:
-					row.update(
-						{
-							"against_voucher_type": doc.doctype,
-							"against_voucher": doc.name,
-							"advance_voucher_type": d.reference_type,
-							"advance_voucher_no": d.reference_name,
-						}
-					)
-
-				# set flag to skip party validation
-				account_type = frappe.get_cached_value("Account", d.account, "account_type")
-				if account_type in ["Receivable", "Payable"] and doc.party_not_required:
-					frappe.flags.party_not_required = True
-
-				gl_map.append(
-					self.get_gl_dict(
-						row,
-						item=d,
-					)
-				)
+		gl_map = []
+		for d in self.doc.get("accounts"):
+			if d.debit or d.credit or self.doc.voucher_type == "Exchange Gain Or Loss":
+				gl_map.append(self.get_gl_dict(self._gl_row(d, advance_doctypes), item=d))
 		return gl_map
+
+	def _set_transaction_currency(self) -> None:
+		"""Company currency, or the first foreign-currency row, becomes the transaction currency."""
+		doc = self.doc
+		doc.transaction_currency = erpnext.get_company_currency(doc.company)
+		doc.transaction_exchange_rate = 1
+		if not doc.multi_currency:
+			return
+
+		for row in doc.get("accounts"):
+			if row.account_currency != doc.transaction_currency:
+				# Journal assumes the first foreign currency as transaction currency
+				doc.transaction_currency = row.account_currency
+				doc.transaction_exchange_rate = row.exchange_rate
+				break
+
+	def _gl_row(self, d, advance_doctypes: list) -> dict:
+		"""Build the GL dict for a single account row."""
+		doc = self.doc
+		remarks = "\n".join(x for x in [d.user_remark, doc.remark] if x)
+
+		row = {
+			"account": d.account,
+			"party_type": d.party_type,
+			"due_date": doc.due_date,
+			"party": d.party,
+			"against": d.against_account,
+			"debit": flt(d.debit, d.precision("debit")),
+			"credit": flt(d.credit, d.precision("credit")),
+			"account_currency": d.account_currency,
+			"debit_in_account_currency": flt(
+				d.debit_in_account_currency, d.precision("debit_in_account_currency")
+			),
+			"credit_in_account_currency": flt(
+				d.credit_in_account_currency, d.precision("credit_in_account_currency")
+			),
+			"transaction_currency": doc.transaction_currency,
+			"transaction_exchange_rate": doc.transaction_exchange_rate,
+			"debit_in_transaction_currency": flt(
+				d.debit_in_account_currency, d.precision("debit_in_account_currency")
+			)
+			if doc.transaction_currency == d.account_currency
+			else flt(d.debit, d.precision("debit")) / doc.transaction_exchange_rate,
+			"credit_in_transaction_currency": flt(
+				d.credit_in_account_currency, d.precision("credit_in_account_currency")
+			)
+			if doc.transaction_currency == d.account_currency
+			else flt(d.credit, d.precision("credit")) / doc.transaction_exchange_rate,
+			"against_voucher_type": d.reference_type,
+			"against_voucher": d.reference_name,
+			"remarks": remarks,
+			"voucher_detail_no": d.reference_detail_no,
+			"cost_center": d.cost_center,
+			"project": d.project,
+			"finance_book": doc.finance_book,
+			"advance_voucher_type": d.advance_voucher_type,
+			"advance_voucher_no": d.advance_voucher_no,
+		}
+
+		if d.reference_type in advance_doctypes:
+			row.update(
+				{
+					"against_voucher_type": doc.doctype,
+					"against_voucher": doc.name,
+					"advance_voucher_type": d.reference_type,
+					"advance_voucher_no": d.reference_name,
+				}
+			)
+
+		# set flag to skip party validation
+		account_type = frappe.get_cached_value("Account", d.account, "account_type")
+		if account_type in ["Receivable", "Payable"] and doc.party_not_required:
+			frappe.flags.party_not_required = True
+
+		return row

erpnext/accounts/doctype/journal_entry/services/reference_validator.py

@@ -29,10 +29,11 @@ class JournalEntryReferenceValidator:
 	orders and invoices.
 	"""
 
-	def __init__(self, doc):
+	def __init__(self, doc) -> None:
 		self.doc = doc
 
-	def validate(self):
+	def validate(self) -> None:
+		"""Validate every reference-bearing row, then the referenced orders and invoices."""
 		self.doc.reference_totals = {}
 		self.doc.reference_types = {}
 		self.doc.reference_accounts = {}
@@ -47,23 +48,24 @@ class JournalEntryReferenceValidator:
 		self._validate_orders()
 		self._validate_invoices()
 
-	def _normalize_reference_fields(self, row):
+	def _normalize_reference_fields(self, row) -> None:
 		if not row.reference_type:
 			row.reference_name = None
 		if not row.reference_name:
 			row.reference_type = None
 
-	def _has_party_reference(self, row):
+	def _has_party_reference(self, row) -> bool:
 		return bool(
 			row.reference_type and row.reference_name and row.reference_type in REFERENCE_PARTY_ACCOUNT_FIELDS
 		)
 
-	def _reference_amount_field(self, row):
+	def _reference_amount_field(self, row) -> str:
 		if row.reference_type in ("Sales Order", "Sales Invoice"):
 			return "credit_in_account_currency"
 		return "debit_in_account_currency"
 
-	def _validate_order_direction(self, row):
+	def _validate_order_direction(self, row) -> None:
+		"""An order can only be linked on the side that records an advance."""
 		if row.reference_type == "Sales Order" and flt(row.debit) > 0:
 			frappe.throw(
 				_("Row {0}: Debit entry can not be linked with a {1}").format(row.idx, row.reference_type)
@@ -73,7 +75,8 @@ class JournalEntryReferenceValidator:
 				_("Row {0}: Credit entry can not be linked with a {1}").format(row.idx, row.reference_type)
 			)
 
-	def _register_reference(self, row):
+	def _register_reference(self, row) -> None:
+		"""Aggregate the row's amount, type and account onto the per-reference lookups."""
 		if row.reference_name not in self.doc.reference_totals:
 			self.doc.reference_totals[row.reference_name] = 0.0
 		if self.doc.voucher_type not in ("Deferred Revenue", "Deferred Expense"):
@@ -81,7 +84,8 @@ class JournalEntryReferenceValidator:
 		self.doc.reference_types[row.reference_name] = row.reference_type
 		self.doc.reference_accounts[row.reference_name] = row.account
 
-	def _validate_reference_party_and_account(self, row):
+	def _validate_reference_party_and_account(self, row) -> None:
+		"""Reject a missing reference, then check party/account against the linked document."""
 		party_fields = REFERENCE_PARTY_ACCOUNT_FIELDS[row.reference_type]
 		against_voucher = frappe.db.get_value(
 			row.reference_type, row.reference_name, [scrub(f) for f in party_fields]
@@ -94,7 +98,7 @@ class JournalEntryReferenceValidator:
 		elif row.reference_type in ("Sales Order", "Purchase Order"):
 			self._validate_order_party(row, against_voucher)
 
-	def _validate_invoice_party_and_account(self, row, against_voucher, party_fields):
+	def _validate_invoice_party_and_account(self, row, against_voucher, party_fields) -> None:
 		party_account, against_party = self._resolve_invoice_party_account(row, against_voucher)
 		if self.doc.voucher_type == "Exchange Gain Or Loss":
 			return
@@ -105,7 +109,9 @@ class JournalEntryReferenceValidator:
 				)
 			)
 
-	def _resolve_invoice_party_account(self, row, against_voucher):
+	def _resolve_invoice_party_account(self, row, against_voucher) -> tuple:
+		"""Expected (party_account, party) for an invoice row, honouring deferred booking
+		and invoice-discounting accounts."""
 		if self.doc.voucher_type in ("Deferred Revenue", "Deferred Expense") and row.reference_detail_no:
 			debit_or_credit = "Debit" if row.debit else "Credit"
 			party_account = get_deferred_booking_accounts(
@@ -120,7 +126,7 @@ class JournalEntryReferenceValidator:
 			party_account = against_voucher[1]
 		return party_account, against_voucher[0]
 
-	def _validate_order_party(self, row, against_voucher):
+	def _validate_order_party(self, row, against_voucher) -> None:
 		if against_voucher != row.party:
 			frappe.throw(
 				_("Row {0}: {1} {2} does not match with {3}").format(
@@ -128,8 +134,8 @@ class JournalEntryReferenceValidator:
 				)
 			)
 
-	def _validate_orders(self):
-		"""Validate totals, closed and docstatus for orders"""
+	def _validate_orders(self) -> None:
+		"""Validate totals, closed and docstatus for referenced orders."""
 		for reference_name, total in self.doc.reference_totals.items():
 			reference_type = self.doc.reference_types[reference_name]
 			account = self.doc.reference_accounts[reference_name]
@@ -140,7 +146,7 @@ class JournalEntryReferenceValidator:
 			self._validate_order_status(order, reference_type, reference_name)
 			self._validate_order_advance_total(order, account, total, reference_type, reference_name)
 
-	def _validate_order_status(self, order, reference_type, reference_name):
+	def _validate_order_status(self, order, reference_type, reference_name) -> None:
 		if order.docstatus != 1:
 			frappe.throw(_("{0} {1} is not submitted").format(reference_type, reference_name))
 		if flt(order.per_billed) >= 100:
@@ -148,7 +154,8 @@ class JournalEntryReferenceValidator:
 		if cstr(order.status) == "Closed":
 			frappe.throw(_("{0} {1} is closed").format(reference_type, reference_name))
 
-	def _validate_order_advance_total(self, order, account, total, reference_type, reference_name):
+	def _validate_order_advance_total(self, order, account, total, reference_type, reference_name) -> None:
+		"""The advance paid against an order cannot exceed its grand total."""
 		account_currency = get_account_currency(account)
 		if account_currency == self.doc.company_currency:
 			voucher_total = order.base_grand_total
@@ -167,8 +174,8 @@ class JournalEntryReferenceValidator:
 				)
 			)
 
-	def _validate_invoices(self):
-		"""Validate totals and docstatus for invoices"""
+	def _validate_invoices(self) -> None:
+		"""Validate totals and docstatus for referenced invoices."""
 		if self.doc.voucher_type in ("Debit Note", "Credit Note"):
 			return
 		for reference_name, total in self.doc.reference_totals.items():
@@ -178,7 +185,8 @@ class JournalEntryReferenceValidator:
 			invoice = frappe.get_doc(reference_type, reference_name)
 			self._validate_invoice_outstanding(invoice, total, reference_type, reference_name)
 
-	def _validate_invoice_outstanding(self, invoice, total, reference_type, reference_name):
+	def _validate_invoice_outstanding(self, invoice, total, reference_type, reference_name) -> None:
+		"""Payment booked against an invoice cannot exceed its outstanding amount."""
 		if invoice.docstatus != 1:
 			frappe.throw(_("{0} {1} is not submitted").format(reference_type, reference_name))