Merge pull request #52933 from frappe/mergify/bp/version-16-hotfix/pr-52824

refactor: separate construction of chart related data from `get_columns()` (backport #52824)

erpnext/accounts/report/balance_sheet/balance_sheet.py

@@ -102,7 +102,7 @@ def execute(filters=None):
 		filters.periodicity, period_list, filters.accumulated_values, company=filters.company
 	)
 
-	chart = get_chart_data(filters, columns, asset, liability, equity, currency)
+	chart = get_chart_data(filters, period_list, asset, liability, equity, currency)
 
 	report_summary, primitive_summary = get_report_summary(
 		period_list, asset, liability, equity, provisional_profit_loss, currency, filters
@@ -231,18 +231,19 @@ def get_report_summary(
 	], (net_asset - net_liability + net_equity)
 
 
-def get_chart_data(filters, columns, asset, liability, equity, currency):
-	labels = [d.get("label") for d in columns[4:]]
+def get_chart_data(filters, chart_columns, asset, liability, equity, currency):
+	labels = [col.get("label") for col in chart_columns]
 
 	asset_data, liability_data, equity_data = [], [], []
 
-	for p in columns[4:]:
+	for col in chart_columns:
+		key = col.get("key") or col.get("fieldname")
 		if asset:
-			asset_data.append(asset[-2].get(p.get("fieldname")))
+			asset_data.append(asset[-2].get(key))
 		if liability:
-			liability_data.append(liability[-2].get(p.get("fieldname")))
+			liability_data.append(liability[-2].get(key))
 		if equity:
-			equity_data.append(equity[-2].get(p.get("fieldname")))
+			equity_data.append(equity[-2].get(key))
 
 	datasets = []
 	if asset_data:

erpnext/accounts/report/cash_flow/cash_flow.py

@@ -145,7 +145,7 @@ def execute(filters=None):
 		True,
 	)
 
-	chart = get_chart_data(columns, data, company_currency)
+	chart = get_chart_data(period_list, data, company_currency)
 
 	report_summary = get_report_summary(summary_data, company_currency)
 
@@ -417,12 +417,12 @@ def get_report_summary(summary_data, currency):
 	return report_summary
 
 
-def get_chart_data(columns, data, currency):
-	labels = [d.get("label") for d in columns[2:]]
+def get_chart_data(period_list, data, currency):
+	labels = [period.get("label") for period in period_list]
 	datasets = [
 		{
 			"name": section.get("section").replace("'", ""),
-			"values": [section.get(d.get("fieldname")) for d in columns[2:]],
+			"values": [section.get(period.get("key")) for period in period_list],
 		}
 		for section in data
 		if section.get("parent_section") is None and section.get("currency")

erpnext/accounts/report/consolidated_financial_statement/consolidated_financial_statement.py

@@ -48,22 +48,25 @@ def execute(filters=None):
 		return columns, data, message, chart
 
 	fiscal_year = get_fiscal_year_data(filters.get("from_fiscal_year"), filters.get("to_fiscal_year"))
-	companies_column, companies = get_companies(filters)
-	columns = get_columns(companies_column, filters)
+	company_list, companies = get_companies(filters)
+	company_columns = get_company_columns(company_list, filters)
+	columns = get_columns(company_columns)
 
 	if filters.get("report") == "Balance Sheet":
 		data, message, chart, report_summary = get_balance_sheet_data(
-			fiscal_year, companies, columns, filters
+			fiscal_year, companies, company_columns, filters
 		)
 	elif filters.get("report") == "Profit and Loss Statement":
-		data, message, chart, report_summary = get_profit_loss_data(fiscal_year, companies, columns, filters)
+		data, message, chart, report_summary = get_profit_loss_data(
+			fiscal_year, companies, company_columns, filters
+		)
 	else:
 		data, report_summary = get_cash_flow_data(fiscal_year, companies, filters)
 
 	return columns, data, message, chart, report_summary
 
 
-def get_balance_sheet_data(fiscal_year, companies, columns, filters):
+def get_balance_sheet_data(fiscal_year, companies, company_columns, filters):
 	asset = get_data(companies, "Asset", "Debit", fiscal_year, filters=filters)
 
 	liability = get_data(companies, "Liability", "Credit", fiscal_year, filters=filters)
@@ -116,7 +119,7 @@ def get_balance_sheet_data(fiscal_year, companies, columns, filters):
 		True,
 	)
 
-	chart = get_chart_data(filters, columns, asset, liability, equity, company_currency)
+	chart = get_chart_data(filters, company_columns, asset, liability, equity, company_currency)
 
 	return data, message, chart, report_summary
 
@@ -164,7 +167,7 @@ def get_root_account_name(root_type, company):
 		return root_account[0][0]
 
 
-def get_profit_loss_data(fiscal_year, companies, columns, filters):
+def get_profit_loss_data(fiscal_year, companies, company_columns, filters):
 	income, expense, net_profit_loss = get_income_expense_data(companies, fiscal_year, filters)
 	company_currency = get_company_currency(filters)
 
@@ -174,7 +177,7 @@ def get_profit_loss_data(fiscal_year, companies, columns, filters):
 	if net_profit_loss:
 		data.append(net_profit_loss)
 
-	chart = get_pl_chart_data(filters, columns, income, expense, net_profit_loss, company_currency)
+	chart = get_pl_chart_data(filters, company_columns, income, expense, net_profit_loss, company_currency)
 
 	report_summary, primitive_summary = get_pl_summary(
 		companies, "", income, expense, net_profit_loss, company_currency, filters, True
@@ -280,7 +283,30 @@ def get_account_type_based_data(account_type, companies, fiscal_year, filters):
 	return data
 
 
-def get_columns(companies, filters):
+def get_company_columns(companies, filters):
+	company_columns = []
+	for company in companies:
+		apply_currency_formatter = 1 if not filters.presentation_currency else 0
+		currency = filters.presentation_currency
+		if not currency:
+			currency = erpnext.get_company_currency(company)
+
+		company_columns.append(
+			{
+				"fieldname": company,
+				"label": f"{company} ({currency})",
+				"fieldtype": "Currency",
+				"options": "currency",
+				"width": 150,
+				"apply_currency_formatter": apply_currency_formatter,
+				"company_name": company,
+			}
+		)
+
+	return company_columns
+
+
+def get_columns(company_columns):
 	columns = [
 		{
 			"fieldname": "account",
@@ -298,23 +324,7 @@ def get_columns(companies, filters):
 		},
 	]
 
-	for company in companies:
-		apply_currency_formatter = 1 if not filters.presentation_currency else 0
-		currency = filters.presentation_currency
-		if not currency:
-			currency = erpnext.get_company_currency(company)
-
-		columns.append(
-			{
-				"fieldname": company,
-				"label": f"{company} ({currency})",
-				"fieldtype": "Currency",
-				"options": "currency",
-				"width": 150,
-				"apply_currency_formatter": apply_currency_formatter,
-				"company_name": company,
-			}
-		)
+	columns.extend(company_columns)
 
 	return columns
 

erpnext/accounts/report/profit_and_loss_statement/profit_and_loss_statement.py

@@ -68,7 +68,7 @@ def execute(filters=None):
 	currency = filters.presentation_currency or frappe.get_cached_value(
 		"Company", filters.company, "default_currency"
 	)
-	chart = get_chart_data(filters, columns, income, expense, net_profit_loss, currency)
+	chart = get_chart_data(filters, period_list, income, expense, net_profit_loss, currency)
 
 	report_summary, primitive_summary = get_report_summary(
 		period_list, filters.periodicity, income, expense, net_profit_loss, currency, filters
@@ -162,18 +162,19 @@ def get_net_profit_loss(income, expense, period_list, company, currency=None, co
 		return net_profit_loss
 
 
-def get_chart_data(filters, columns, income, expense, net_profit_loss, currency):
-	labels = [d.get("label") for d in columns[4:]]
+def get_chart_data(filters, chart_columns, income, expense, net_profit_loss, currency):
+	labels = [col.get("label") for col in chart_columns]
 
 	income_data, expense_data, net_profit = [], [], []
 
-	for p in columns[4:]:
+	for col in chart_columns:
+		key = col.get("key") or col.get("fieldname")
 		if income:
-			income_data.append(income[-2].get(p.get("fieldname")))
+			income_data.append(income[-2].get(key))
 		if expense:
-			expense_data.append(expense[-2].get(p.get("fieldname")))
+			expense_data.append(expense[-2].get(key))
 		if net_profit_loss:
-			net_profit.append(net_profit_loss.get(p.get("fieldname")))
+			net_profit.append(net_profit_loss.get(key))
 
 	datasets = []
 	if income_data:

erpnext/manufacturing/report/production_analytics/production_analytics.py

@@ -6,24 +6,25 @@ import frappe
 from frappe import _, scrub
 from frappe.utils import getdate, today
 
-from erpnext.stock.report.stock_analytics.stock_analytics import get_period, get_period_date_ranges
+from erpnext.stock.report.stock_analytics.stock_analytics import (
+	get_period,
+	get_period_columns,
+	get_period_date_ranges,
+)
 
 WORK_ORDER_STATUS_LIST = ["Not Started", "Overdue", "Pending", "Completed", "Closed", "Stopped"]
 
 
 def execute(filters=None):
-	columns = get_columns(filters)
-	data, chart = get_data(filters, columns)
+	period_columns = get_period_columns(filters)
+	columns = get_columns(period_columns)
+	data, chart = get_data(filters, period_columns)
 	return columns, data, None, chart
 
 
-def get_columns(filters):
+def get_columns(period_columns):
 	columns = [{"label": _("Status"), "fieldname": "status", "fieldtype": "Data", "width": 140}]
-	ranges = get_period_date_ranges(filters)
-
-	for _dummy, end_date in ranges:
-		period = get_period(end_date, filters)
-		columns.append({"label": _(period), "fieldname": scrub(period), "fieldtype": "Float", "width": 120})
+	columns.extend(period_columns)
 
 	return columns
 
@@ -49,7 +50,7 @@ def get_work_orders(filters):
 	)
 
 
-def get_data(filters, columns):
+def get_data(filters, period_columns):
 	ranges = build_ranges(filters)
 	period_labels = [scrub(pd) for _fd, _td, pd in ranges]
 	periodic_data = {status: {pd: 0 for pd in period_labels} for status in WORK_ORDER_STATUS_LIST}
@@ -84,7 +85,7 @@ def get_data(filters, columns):
 			row[scrub(period)] = periodic_data[status].get(scrub(period), 0)
 		data.append(row)
 
-	chart = get_chart_data(periodic_data, columns)
+	chart = get_chart_data(periodic_data, period_columns)
 	return data, chart
 
 
@@ -103,9 +104,9 @@ def build_ranges(filters):
 	return ranges
 
 
-def get_chart_data(periodic_data, columns):
-	period_labels = [d.get("label") for d in columns[1:]]
-	period_fieldnames = [d.get("fieldname") for d in columns[1:]]
+def get_chart_data(periodic_data, period_columns):
+	period_labels = [col.get("label") for col in period_columns]
+	period_fieldnames = [col.get("fieldname") for col in period_columns]
 
 	datasets = []
 	for status in WORK_ORDER_STATUS_LIST:

erpnext/stock/report/stock_analytics/stock_analytics.py

@@ -16,14 +16,29 @@ from erpnext.stock.utils import is_reposting_item_valuation_in_progress
 def execute(filters=None):
 	is_reposting_item_valuation_in_progress()
 	filters = frappe._dict(filters or {})
-	columns = get_columns(filters)
+	period_columns = get_period_columns(filters)
+	columns = get_columns(period_columns)
 	data = get_data(filters)
-	chart = get_chart_data(columns)
+	chart = get_chart_data(period_columns)
 
 	return columns, data, None, chart
 
 
-def get_columns(filters):
+def get_period_columns(filters):
+	period_columns = []
+	ranges = get_period_date_ranges(filters)
+
+	for _dummy, end_date in ranges:
+		period = get_period(end_date, filters)
+
+		period_columns.append(
+			{"label": _(period), "fieldname": scrub(period), "fieldtype": "Float", "width": 120}
+		)
+
+	return period_columns
+
+
+def get_columns(period_columns):
 	columns = [
 		{"label": _("Item"), "options": "Item", "fieldname": "name", "fieldtype": "Link", "width": 140},
 		{
@@ -44,12 +59,7 @@ def get_columns(filters):
 		{"label": _("UOM"), "fieldname": "uom", "fieldtype": "Data", "width": 120},
 	]
 
-	ranges = get_period_date_ranges(filters)
-
-	for _dummy, end_date in ranges:
-		period = get_period(end_date, filters)
-
-		columns.append({"label": _(period), "fieldname": scrub(period), "fieldtype": "Float", "width": 120})
+	columns.extend(period_columns)
 
 	return columns
 
@@ -249,8 +259,8 @@ def get_data(filters):
 	return data
 
 
-def get_chart_data(columns):
-	labels = [d.get("label") for d in columns[5:]]
+def get_chart_data(period_columns):
+	labels = [col.get("label") for col in period_columns]
 	chart = {"data": {"labels": labels, "datasets": []}}
 	chart["type"] = "line"