feat(accounts): standardize additional columns implementation for sales/purchase reports (#36000)

erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py

@@ -15,20 +15,21 @@ from erpnext.accounts.report.item_wise_sales_register.item_wise_sales_register i
 	get_group_by_conditions,
 	get_tax_accounts,
 )
+from erpnext.accounts.report.utils import get_query_columns, get_values_for_columns
 
 
 def execute(filters=None):
 	return _execute(filters)
 
 
-def _execute(filters=None, additional_table_columns=None, additional_query_columns=None):
+def _execute(filters=None, additional_table_columns=None):
 	if not filters:
 		filters = {}
 	columns = get_columns(additional_table_columns, filters)
 
 	company_currency = erpnext.get_company_currency(filters.company)
 
-	item_list = get_items(filters, additional_query_columns)
+	item_list = get_items(filters, get_query_columns(additional_table_columns))
 	aii_account_map = get_aii_accounts()
 	if item_list:
 		itemised_tax, tax_columns = get_tax_accounts(
@@ -79,28 +80,20 @@ def _execute(filters=None, additional_table_columns=None, additional_query_colum
 			"posting_date": d.posting_date,
 			"supplier": d.supplier,
 			"supplier_name": d.supplier_name,
+			**get_values_for_columns(additional_table_columns, d),
+			"credit_to": d.credit_to,
+			"mode_of_payment": d.mode_of_payment,
+			"project": d.project,
+			"company": d.company,
+			"purchase_order": d.purchase_order,
+			"purchase_receipt": purchase_receipt,
+			"expense_account": expense_account,
+			"stock_qty": d.stock_qty,
+			"stock_uom": d.stock_uom,
+			"rate": d.base_net_amount / d.stock_qty if d.stock_qty else d.base_net_amount,
+			"amount": d.base_net_amount,
 		}
 
-		if additional_query_columns:
-			for col in additional_query_columns:
-				row.update({col: d.get(col)})
-
-		row.update(
-			{
-				"credit_to": d.credit_to,
-				"mode_of_payment": d.mode_of_payment,
-				"project": d.project,
-				"company": d.company,
-				"purchase_order": d.purchase_order,
-				"purchase_receipt": purchase_receipt,
-				"expense_account": expense_account,
-				"stock_qty": d.stock_qty,
-				"stock_uom": d.stock_uom,
-				"rate": d.base_net_amount / d.stock_qty if d.stock_qty else d.base_net_amount,
-				"amount": d.base_net_amount,
-			}
-		)
-
 		total_tax = 0
 		for tax in tax_columns:
 			item_tax = itemised_tax.get(d.name, {}).get(tax, {})
@@ -317,11 +310,6 @@ def get_conditions(filters):
 def get_items(filters, additional_query_columns):
 	conditions = get_conditions(filters)
 
-	if additional_query_columns:
-		additional_query_columns = ", " + ", ".join(additional_query_columns)
-	else:
-		additional_query_columns = ""
-
 	return frappe.db.sql(
 		"""
 		select
@@ -340,11 +328,10 @@ def get_items(filters, additional_query_columns):
 		from `tabPurchase Invoice`, `tabPurchase Invoice Item`, `tabItem`
 		where `tabPurchase Invoice`.name = `tabPurchase Invoice Item`.`parent` and
 			`tabItem`.name = `tabPurchase Invoice Item`.`item_code` and
-			`tabPurchase Invoice`.docstatus = 1 %s
+			`tabPurchase Invoice`.docstatus = 1 {1}
 	""".format(
-			additional_query_columns
-		)
-		% (conditions),
+			additional_query_columns, conditions
+		),
 		filters,
 		as_dict=1,
 	)

erpnext/accounts/report/item_wise_sales_register/item_wise_sales_register.py

@@ -9,6 +9,7 @@ from frappe.utils import cstr, flt
 from frappe.utils.xlsxutils import handle_html
 
 from erpnext.accounts.report.sales_register.sales_register import get_mode_of_payments
+from erpnext.accounts.report.utils import get_query_columns, get_values_for_columns
 from erpnext.selling.report.item_wise_sales_history.item_wise_sales_history import (
 	get_customer_details,
 )
@@ -18,19 +19,14 @@ def execute(filters=None):
 	return _execute(filters)
 
 
-def _execute(
-	filters=None,
-	additional_table_columns=None,
-	additional_query_columns=None,
-	additional_conditions=None,
-):
+def _execute(filters=None, additional_table_columns=None, additional_conditions=None):
 	if not filters:
 		filters = {}
 	columns = get_columns(additional_table_columns, filters)
 
 	company_currency = frappe.get_cached_value("Company", filters.get("company"), "default_currency")
 
-	item_list = get_items(filters, additional_query_columns, additional_conditions)
+	item_list = get_items(filters, get_query_columns(additional_table_columns), additional_conditions)
 	if item_list:
 		itemised_tax, tax_columns = get_tax_accounts(item_list, columns, company_currency)
 
@@ -79,30 +75,22 @@ def _execute(
 			"customer": d.customer,
 			"customer_name": customer_record.customer_name,
 			"customer_group": customer_record.customer_group,
+			**get_values_for_columns(additional_table_columns, d),
+			"debit_to": d.debit_to,
+			"mode_of_payment": ", ".join(mode_of_payments.get(d.parent, [])),
+			"territory": d.territory,
+			"project": d.project,
+			"company": d.company,
+			"sales_order": d.sales_order,
+			"delivery_note": d.delivery_note,
+			"income_account": d.unrealized_profit_loss_account
+			if d.is_internal_customer == 1
+			else d.income_account,
+			"cost_center": d.cost_center,
+			"stock_qty": d.stock_qty,
+			"stock_uom": d.stock_uom,
 		}
 
-		if additional_query_columns:
-			for col in additional_query_columns:
-				row.update({col: d.get(col)})
-
-		row.update(
-			{
-				"debit_to": d.debit_to,
-				"mode_of_payment": ", ".join(mode_of_payments.get(d.parent, [])),
-				"territory": d.territory,
-				"project": d.project,
-				"company": d.company,
-				"sales_order": d.sales_order,
-				"delivery_note": d.delivery_note,
-				"income_account": d.unrealized_profit_loss_account
-				if d.is_internal_customer == 1
-				else d.income_account,
-				"cost_center": d.cost_center,
-				"stock_qty": d.stock_qty,
-				"stock_uom": d.stock_uom,
-			}
-		)
-
 		if d.stock_uom != d.uom and d.stock_qty:
 			row.update({"rate": (d.base_net_rate * d.qty) / d.stock_qty, "amount": d.base_net_amount})
 		else:
@@ -394,11 +382,6 @@ def get_group_by_conditions(filters, doctype):
 def get_items(filters, additional_query_columns, additional_conditions=None):
 	conditions = get_conditions(filters, additional_conditions)
 
-	if additional_query_columns:
-		additional_query_columns = ", " + ", ".join(additional_query_columns)
-	else:
-		additional_query_columns = ""
-
 	return frappe.db.sql(
 		"""
 		select
@@ -424,7 +407,7 @@ def get_items(filters, additional_query_columns, additional_conditions=None):
 			`tabItem`.name = `tabSales Invoice Item`.`item_code` and
 			`tabSales Invoice`.docstatus = 1 {1}
 		""".format(
-			additional_query_columns or "", conditions
+			additional_query_columns, conditions
 		),
 		filters,
 		as_dict=1,

erpnext/accounts/report/purchase_register/purchase_register.py

@@ -10,17 +10,18 @@ from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import (
 	get_accounting_dimensions,
 	get_dimension_with_children,
 )
+from erpnext.accounts.report.utils import get_query_columns, get_values_for_columns
 
 
 def execute(filters=None):
 	return _execute(filters)
 
 
-def _execute(filters=None, additional_table_columns=None, additional_query_columns=None):
+def _execute(filters=None, additional_table_columns=None):
 	if not filters:
 		filters = {}
 
-	invoice_list = get_invoices(filters, additional_query_columns)
+	invoice_list = get_invoices(filters, get_query_columns(additional_table_columns))
 	columns, expense_accounts, tax_accounts, unrealized_profit_loss_accounts = get_columns(
 		invoice_list, additional_table_columns
 	)
@@ -47,13 +48,12 @@ def _execute(filters=None, additional_table_columns=None, additional_query_colum
 		purchase_receipt = list(set(invoice_po_pr_map.get(inv.name, {}).get("purchase_receipt", [])))
 		project = list(set(invoice_po_pr_map.get(inv.name, {}).get("project", [])))
 
-		row = [inv.name, inv.posting_date, inv.supplier, inv.supplier_name]
-
-		if additional_query_columns:
-			for col in additional_query_columns:
-				row.append(inv.get(col))
-
-		row += [
+		row = [
+			inv.name,
+			inv.posting_date,
+			inv.supplier,
+			inv.supplier_name,
+			*get_values_for_columns(additional_table_columns, inv).values(),
 			supplier_details.get(inv.supplier),  # supplier_group
 			inv.tax_id,
 			inv.credit_to,
@@ -244,9 +244,6 @@ def get_conditions(filters):
 
 
 def get_invoices(filters, additional_query_columns):
-	if additional_query_columns:
-		additional_query_columns = ", " + ", ".join(additional_query_columns)
-
 	conditions = get_conditions(filters)
 	return frappe.db.sql(
 		"""
@@ -255,11 +252,10 @@ def get_invoices(filters, additional_query_columns):
 			remarks, base_net_total, base_grand_total, outstanding_amount,
 			mode_of_payment {0}
 		from `tabPurchase Invoice`
-		where docstatus = 1 %s
+		where docstatus = 1 {1}
 		order by posting_date desc, name desc""".format(
-			additional_query_columns or ""
-		)
-		% conditions,
+			additional_query_columns, conditions
+		),
 		filters,
 		as_dict=1,
 	)

erpnext/accounts/report/sales_register/sales_register.py

@@ -11,17 +11,18 @@ from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import (
 	get_accounting_dimensions,
 	get_dimension_with_children,
 )
+from erpnext.accounts.report.utils import get_query_columns, get_values_for_columns
 
 
 def execute(filters=None):
 	return _execute(filters)
 
 
-def _execute(filters, additional_table_columns=None, additional_query_columns=None):
+def _execute(filters, additional_table_columns=None):
 	if not filters:
 		filters = frappe._dict({})
 
-	invoice_list = get_invoices(filters, additional_query_columns)
+	invoice_list = get_invoices(filters, get_query_columns(additional_table_columns))
 	columns, income_accounts, tax_accounts, unrealized_profit_loss_accounts = get_columns(
 		invoice_list, additional_table_columns
 	)
@@ -54,30 +55,22 @@ def _execute(filters, additional_table_columns=None, additional_query_columns=No
 			"posting_date": inv.posting_date,
 			"customer": inv.customer,
 			"customer_name": inv.customer_name,
+			**get_values_for_columns(additional_table_columns, inv),
+			"customer_group": inv.get("customer_group"),
+			"territory": inv.get("territory"),
+			"tax_id": inv.get("tax_id"),
+			"receivable_account": inv.debit_to,
+			"mode_of_payment": ", ".join(mode_of_payments.get(inv.name, [])),
+			"project": inv.project,
+			"owner": inv.owner,
+			"remarks": inv.remarks,
+			"sales_order": ", ".join(sales_order),
+			"delivery_note": ", ".join(delivery_note),
+			"cost_center": ", ".join(cost_center),
+			"warehouse": ", ".join(warehouse),
+			"currency": company_currency,
 		}
 
-		if additional_query_columns:
-			for col in additional_query_columns:
-				row.update({col: inv.get(col)})
-
-		row.update(
-			{
-				"customer_group": inv.get("customer_group"),
-				"territory": inv.get("territory"),
-				"tax_id": inv.get("tax_id"),
-				"receivable_account": inv.debit_to,
-				"mode_of_payment": ", ".join(mode_of_payments.get(inv.name, [])),
-				"project": inv.project,
-				"owner": inv.owner,
-				"remarks": inv.remarks,
-				"sales_order": ", ".join(sales_order),
-				"delivery_note": ", ".join(delivery_note),
-				"cost_center": ", ".join(cost_center),
-				"warehouse": ", ".join(warehouse),
-				"currency": company_currency,
-			}
-		)
-
 		# map income values
 		base_net_total = 0
 		for income_acc in income_accounts:
@@ -402,9 +395,6 @@ def get_conditions(filters):
 
 
 def get_invoices(filters, additional_query_columns):
-	if additional_query_columns:
-		additional_query_columns = ", " + ", ".join(additional_query_columns)
-
 	conditions = get_conditions(filters)
 	return frappe.db.sql(
 		"""
@@ -413,10 +403,10 @@ def get_invoices(filters, additional_query_columns):
 		base_net_total, base_grand_total, base_rounded_total, outstanding_amount,
 		is_internal_customer, represents_company, company {0}
 		from `tabSales Invoice`
-		where docstatus = 1 %s order by posting_date desc, name desc""".format(
-			additional_query_columns or ""
-		)
-		% conditions,
+		where docstatus = 1 {1}
+		order by posting_date desc, name desc""".format(
+			additional_query_columns, conditions
+		),
 		filters,
 		as_dict=1,
 	)

erpnext/accounts/report/utils.py

@@ -1,5 +1,5 @@
 import frappe
-from frappe.utils import flt, formatdate, get_datetime_str
+from frappe.utils import flt, formatdate, get_datetime_str, get_table_name
 
 from erpnext import get_company_currency, get_default_company
 from erpnext.accounts.doctype.fiscal_year.fiscal_year import get_from_and_to_date
@@ -151,3 +151,32 @@ def get_invoiced_item_gross_margin(
 		result = sum(d.gross_profit for d in result)
 
 	return result
+
+
+def get_query_columns(report_columns):
+	if not report_columns:
+		return ""
+
+	columns = []
+	for column in report_columns:
+		fieldname = column["fieldname"]
+
+		if doctype := column.get("_doctype"):
+			columns.append(f"`{get_table_name(doctype)}`.`{fieldname}`")
+		else:
+			columns.append(fieldname)
+
+	return ", " + ", ".join(columns)
+
+
+def get_values_for_columns(report_columns, report_row):
+	values = {}
+
+	if not report_columns:
+		return values
+
+	for column in report_columns:
+		fieldname = column["fieldname"]
+		values[fieldname] = report_row.get(fieldname)
+
+	return values