Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: link cash flow rows and fix summary linking #42524

Merged
merged 1 commit into from
Aug 26, 2024
Merged

fix: link cash flow rows and fix summary linking #42524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion erpnext/accounts/report/cash_flow/cash_flow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) 2013, Frappe Technologies Pvt. Ltd. and contributors
// For license information, please see license.txt

frappe.query_reports["Cash Flow"] = $.extend({}, erpnext.financial_statements);
frappe.query_reports["Cash Flow"] = $.extend(erpnext.financial_statements, {
name_field: "section",
parent_field: "parent_section",
});

erpnext.utils.add_dimensions("Cash Flow", 10);

Expand Down
58 changes: 34 additions & 24 deletions erpnext/accounts/report/cash_flow/cash_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def execute(filters=None):
company=filters.company,
)

cash_flow_accounts = get_cash_flow_accounts()
cash_flow_sections = get_cash_flow_accounts()

# compute net profit / loss
income = get_data(
Expand Down Expand Up @@ -60,46 +60,55 @@ def execute(filters=None):
summary_data = {}
company_currency = frappe.get_cached_value("Company", filters.company, "default_currency")

for cash_flow_account in cash_flow_accounts:
for cash_flow_section in cash_flow_sections:
section_data = []
data.append(
{
"account_name": cash_flow_account["section_header"],
"parent_account": None,
"section_name": "'" + cash_flow_section["section_header"] + "'",
"parent_section": None,
"indent": 0.0,
"account": cash_flow_account["section_header"],
"section": cash_flow_section["section_header"],
}
)

if len(data) == 1:
# add first net income in operations section
if net_profit_loss:
net_profit_loss.update(
{"indent": 1, "parent_account": cash_flow_accounts[0]["section_header"]}
{"indent": 1, "parent_section": cash_flow_sections[0]["section_header"]}
)
data.append(net_profit_loss)
section_data.append(net_profit_loss)

for account in cash_flow_account["account_types"]:
account_data = get_account_type_based_data(
filters.company, account["account_type"], period_list, filters.accumulated_values, filters
for row in cash_flow_section["account_types"]:
row_data = get_account_type_based_data(
filters.company, row["account_type"], period_list, filters.accumulated_values, filters
)
account_data.update(
accounts = frappe.get_all(
"Account",
filters={
"account_type": row["account_type"],
"is_group": 0,
},
pluck="name",
)
row_data.update(
{
"account_name": account["label"],
"account": account["label"],
"section_name": row["label"],
"section": row["label"],
"indent": 1,
"parent_account": cash_flow_account["section_header"],
"accounts": accounts,
"parent_section": cash_flow_section["section_header"],
"currency": company_currency,
}
)
data.append(account_data)
section_data.append(account_data)
data.append(row_data)
section_data.append(row_data)

add_total_row_account(
data,
section_data,
cash_flow_account["section_footer"],
cash_flow_section["section_footer"],
period_list,
company_currency,
summary_data,
Expand All @@ -109,7 +118,7 @@ def execute(filters=None):
add_total_row_account(
data, data, _("Net Change in Cash"), period_list, company_currency, summary_data, filters
)
columns = get_columns(filters.periodicity, period_list, filters.accumulated_values, filters.company)
columns = get_columns(filters.periodicity, period_list, filters.accumulated_values, filters.company, True)

chart = get_chart_data(columns, data)

Expand Down Expand Up @@ -217,8 +226,8 @@ def get_start_date(period, accumulated_values, company):

def add_total_row_account(out, data, label, period_list, currency, summary_data, filters, consolidated=False):
total_row = {
"account_name": "'" + _("{0}").format(label) + "'",
"account": "'" + _("{0}").format(label) + "'",
"section_name": "'" + _("{0}").format(label) + "'",
"section": "'" + _("{0}").format(label) + "'",
"currency": currency,
}

Expand All @@ -229,7 +238,7 @@ def add_total_row_account(out, data, label, period_list, currency, summary_data,
period_list = get_filtered_list_for_consolidated_report(filters, period_list)

for row in data:
if row.get("parent_account"):
if row.get("parent_section"):
for period in period_list:
key = period if consolidated else period["key"]
total_row.setdefault(key, 0.0)
Expand All @@ -254,13 +263,14 @@ def get_report_summary(summary_data, currency):

def get_chart_data(columns, data):
labels = [d.get("label") for d in columns[2:]]
print(data)
datasets = [
{
"name": account.get("account").replace("'", ""),
"values": [account.get(d.get("fieldname")) for d in columns[2:]],
"name": section.get("section").replace("'", ""),
"values": [section.get(d.get("fieldname")) for d in columns[2:]],
}
for account in data
if account.get("parent_account") is None and account.get("currency")
for section in data
if section.get("parent_section") is None and section.get("currency")
]
datasets = datasets[:-1]

Expand Down
10 changes: 5 additions & 5 deletions erpnext/accounts/report/financial_statements.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ def filter_out_zero_value_rows(data, parent_children_map, show_zero_values=False

def add_total_row(out, root_type, balance_must_be, period_list, company_currency):
total_row = {
"account_name": _("Total {0} ({1})").format(_(root_type), _(balance_must_be)),
"account": _("Total {0} ({1})").format(_(root_type), _(balance_must_be)),
"account_name": "'" + _("Total {0} ({1})").format(_(root_type), _(balance_must_be)) + "'",
"account": "'" + _("Total {0} ({1})").format(_(root_type), _(balance_must_be)) + "'",
"currency": company_currency,
"opening_balance": 0.0,
}
Expand Down Expand Up @@ -613,11 +613,11 @@ def get_cost_centers_with_children(cost_centers):
return list(set(all_cost_centers))


def get_columns(periodicity, period_list, accumulated_values=1, company=None):
def get_columns(periodicity, period_list, accumulated_values=1, company=None, cash_flow=False):
columns = [
{
"fieldname": "account",
"label": _("Account"),
"fieldname": "stub",
"label": _("Account") if not cash_flow else _("Section"),
"fieldtype": "Link",
"options": "Account",
"width": 300,
Expand Down
15 changes: 8 additions & 7 deletions erpnext/public/js/financial_statements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ erpnext.financial_statements = {

return value;
} else if (frappe.query_report.get_filter_value("selected_view") == "Margin" && data) {
if (column.fieldname == "account" && data.account_name == __("Income")) {
if (column.fieldname == "stub" && data.account_name == __("Income")) {
Copy link
Member

@ruthra-kumar ruthra-kumar Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing fieldname to 'stub' breaks export as the statements still generates with 'account' as fieldname. In hindsight, I don't see any benefit to changing this.

Copy link
Collaborator Author

@blaggacao blaggacao Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to fix this via a pre-export hook, where necessary?

In the javascript code account is misleading, at best: it's the table stub column which may contain anything, including, but not exclusively, an account.

I can't remember if there was a reason exceeding the purpose of type clarification, but that's a strong reason, imo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the javascript code account is misleading

Not sure how this is misleading. Any Examples?

Copy link
Collaborator Author

@blaggacao blaggacao Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

account is misleading in that its contents are not limited to account strings. In reality, though, it's the table's stub column, which indicates what the row to the right contains, including:

  • accounts
  • summarizing lines (e.g. "Net Operating Capital Gain")
  • component descriptions / title / section separators

That is true for prepared display data.

The origin source data may still be based off accounts. When that transition happens, it should be reflected by the correct name.

I don't quite understand the export use case, but as far as I can see, if it works with origin data, it should handle an account column, however if it works with prepared data it should handle the stub column. If it works with the prepared data and still needs the account column for other reasons that looks like a type mismatch, maybe it should work with the origin data instead.

//Taking the total income from each column (for all the financial years) as the base (100%)
this.baseData = row;
}
Expand All @@ -52,16 +52,17 @@ erpnext.financial_statements = {
}
}

if (data && column.fieldname == "account") {
value = data.account_name || value;
if (data && column.fieldname == "stub") {
// first column
value = data.section_name || data.account_name || value;

if (filter && filter?.text && filter?.type == "contains") {
if (!value.toLowerCase().includes(filter.text)) {
return value;
}
}

if (data.account) {
if (data.account || data.accounts) {
column.link_onclick =
"erpnext.financial_statements.open_general_ledger(" + JSON.stringify(data) + ")";
}
Expand All @@ -70,7 +71,7 @@ erpnext.financial_statements = {

value = default_formatter(value, row, column, data);

if (data && !data.parent_account) {
if (data && !data.parent_account && !data.parent_section) {
value = $(`<span>${value}</span>`);

var $value = $(value).css("font-weight", "bold");
Expand All @@ -84,13 +85,13 @@ erpnext.financial_statements = {
return value;
},
open_general_ledger: function (data) {
if (!data.account) return;
if (!data.account && !data.accounts) return;
let project = $.grep(frappe.query_report.filters, function (e) {
return e.df.fieldname == "project";
});

frappe.route_options = {
account: data.account,
account: data.account || data.accounts,
company: frappe.query_report.get_filter_value("company"),
from_date: data.from_date || data.year_start_date,
to_date: data.to_date || data.year_end_date,
Expand Down
Loading