Skip to content

Commit

Permalink
Merge pull request frappe#22408 from RitvikSardana/develop-ritvik-add…
Browse files Browse the repository at this point in the history
…-column-fix
  • Loading branch information
shariquerik committed Sep 26, 2023
2 parents 2056b0a + 9368f0a commit 9bafe1f
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 8 deletions.
13 changes: 11 additions & 2 deletions frappe/desk/query_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ def generate_report_result(
columns, result, message, chart, report_summary, skip_total_row = ljust_list(res, 6)
columns = [get_column_as_dict(col) for col in (columns or [])]
report_column_names = [col["fieldname"] for col in columns]

# convert to list of dicts

result = normalize_result(result, columns)

if report.custom_columns:
Expand Down Expand Up @@ -230,6 +230,14 @@ def run(


def add_custom_column_data(custom_columns, result):
doctype_names_from_custom_field = []
for column in custom_columns:
if len(column["fieldname"].split("-")) > 1:
# length greater than 1, means that the column is a custom field with confilicting fieldname
doctype_name = frappe.unscrub(column["fieldname"].split("-")[1])
doctype_names_from_custom_field.append(doctype_name)
column["fieldname"] = column["fieldname"].split("-")[0]

custom_column_data = get_data_for_custom_report(custom_columns, result)

for column in custom_columns:
Expand All @@ -247,6 +255,8 @@ def add_custom_column_data(custom_columns, result):
# possible if the row is empty
if not row_reference:
continue
if key[0] in doctype_names_from_custom_field:
column["fieldname"] = column.get("id")
row[column.get("fieldname")] = custom_column_data.get(key).get(row_reference)

return result
Expand Down Expand Up @@ -508,7 +518,6 @@ def get_data_for_custom_report(columns, result):
names = list(set(names))

doc_field_value_map[(doctype, fieldname)] = get_data_for_custom_field(doctype, fieldname, names)

return doc_field_value_map


Expand Down
26 changes: 21 additions & 5 deletions frappe/public/js/frappe/views/reports/query_report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1689,8 +1689,13 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
const insert_after_index = this.columns.findIndex(
(column) => column.label === values.insert_after
);

custom_columns.push({
fieldname: df.fieldname,
fieldname: this.columns
.map((column) => column.fieldname)
.includes(df.fieldname)
? df.fieldname + "-" + frappe.scrub(values.doctype)
: df.fieldname,
fieldtype: df.fieldtype,
label: df.label,
insert_after_index: insert_after_index,
Expand All @@ -1714,12 +1719,11 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
const custom_data = r.message;
const link_field =
this.doctype_field_map[values.doctype].fieldname;

this.add_custom_column(
custom_columns,
custom_data,
link_field,
values.field,
values,
insert_after_index
);
d.hide();
Expand Down Expand Up @@ -1800,13 +1804,25 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
}
}

add_custom_column(custom_column, custom_data, link_field, column_field, insert_after_index) {
add_custom_column(
custom_column,
custom_data,
link_field,
new_column_data,
insert_after_index
) {
const column = this.prepare_columns(custom_column);
const column_field = new_column_data.field;

this.columns.splice(insert_after_index + 1, 0, column[0]);

this.data.forEach((row) => {
row[column_field] = custom_data[row[link_field]];
if (column[0].fieldname.includes("-")) {
row[column_field + "-" + frappe.scrub(new_column_data.doctype)] =
custom_data[row[link_field]];
} else {
row[column_field] = custom_data[row[link_field]];
}
});

this.render_datatable();
Expand Down
128 changes: 127 additions & 1 deletion frappe/tests/test_query_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,21 @@

import frappe
import frappe.utils
from frappe.desk.query_report import build_xlsx_data, export_query
from frappe.core.doctype.doctype.test_doctype import new_doctype
from frappe.desk.query_report import build_xlsx_data, export_query, run
from frappe.tests.utils import FrappeTestCase
from frappe.utils.xlsxutils import make_xlsx


class TestQueryReport(FrappeTestCase):
@classmethod
def setUpClass(cls) -> None:
cls.enable_safe_exec()
return super().setUpClass()

def tearDown(self):
frappe.db.rollback()

def test_xlsx_data_with_multiple_datatypes(self):
"""Test exporting report using rows with multiple datatypes (list, dict)"""

Expand Down Expand Up @@ -110,3 +119,120 @@ def test_csv(self):
self.assertIn(column, row)

frappe.delete_doc("Report", REPORT_NAME, delete_permanently=True)

def test_report_for_duplicate_column_names(self):
"""Test report with duplicate column names"""

try:
fields = [
{"label": "First Name", "fieldname": "first_name", "fieldtype": "Data"},
{"label": "Last Name", "fieldname": "last_name", "fieldtype": "Data"},
]
docA = frappe.get_doc(
{
"doctype": "DocType",
"name": "Doc A",
"module": "Core",
"custom": 1,
"autoname": "field:first_name",
"fields": fields,
"permissions": [{"role": "System Manager"}],
}
).insert(ignore_if_duplicate=True)

docB = frappe.get_doc(
{
"doctype": "DocType",
"name": "Doc B",
"module": "Core",
"custom": 1,
"autoname": "field:last_name",
"fields": fields,
"permissions": [{"role": "System Manager"}],
}
).insert(ignore_if_duplicate=True)

for i in range(1, 3):
frappe.get_doc({"doctype": "Doc A", "first_name": f"John{i}", "last_name": "Doe"}).insert()
frappe.get_doc({"doctype": "Doc B", "last_name": f"Doe{i}", "first_name": "John"}).insert()

if not frappe.db.exists("Report", "Doc A Report"):
report = frappe.get_doc(
{
"doctype": "Report",
"ref_doctype": "Doc A",
"report_name": "Doc A Report",
"report_type": "Script Report",
"is_standard": "No",
}
).insert(ignore_permissions=True)
else:
report = frappe.get_doc("Report", "Doc A Report")

report.report_script = """
result = [["Ritvik","Sardana", "Doe1"],["Shariq","Ansari", "Doe2"]]
columns = [{
"label": "First Name",
"fieldname": "first_name",
"fieldtype": "Data",
"width": 180,
},
{
"label": "Last Name",
"fieldname": "last_name",
"fieldtype": "Data",
"width": 180,
},
{
"label": "Linked Field",
"fieldname": "linked_field",
"fieldtype": "Link",
"options": "Doc B",
"width": 180,
},
]
data = columns, result
"""
report.save()

custom_columns = [
{
"fieldname": "first_name-Doc_B",
"fieldtype": "Data",
"label": "First Name",
"insert_after_index": 1,
"link_field": {"fieldname": "linked_field", "names": {}},
"doctype": "Doc B",
"width": 100,
"id": "first_name-Doc_B",
"name": "First Name",
"editable": False,
"compareValue": None,
},
]

response = run(
"Doc A Report",
filters={"user": "Administrator", "doctype": "Doc A"},
custom_columns=custom_columns,
)

self.assertListEqual(
["first_name", "last_name", "first_name-Doc_B", "linked_field"],
[d["fieldname"] for d in response["columns"]],
)

self.assertDictEqual(
{
"first_name": "Ritvik",
"last_name": "Sardana",
"linked_field": "Doe1",
"first_name-Doc_B": "John",
},
response["result"][0],
)

except Exception as e:
raise e
frappe.db.rollback()

0 comments on commit 9bafe1f

Please sign in to comment.