From 985b9a4ac4580da2195783f0a78e861c843d5901 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Thu, 14 Sep 2023 11:32:31 +0530 Subject: [PATCH 1/9] fix: fixed the conflict between fieldname in General Ledger Report --- frappe/desk/query_report.py | 17 +++++++++++-- .../js/frappe/views/reports/query_report.js | 25 ++++++++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index dd07dced40d..218a3080336 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -84,8 +84,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: @@ -101,6 +101,13 @@ def generate_report_result( report_custom_columns = [ column for column in columns if column["fieldname"] not in report_column_names ] + # for column in report_custom_columns: + # column["fieldname"] = column["fieldname"].split("-")[0] + + # for column in columns: + # if (len(column.get("fieldname").split("-")) > 1): + # print(column.get("fieldname")) + # column["fieldname"] = column["fieldname"].split("-")[0] if report_custom_columns: result = add_custom_column_data(report_custom_columns, result) @@ -111,6 +118,7 @@ def generate_report_result( if cint(report.add_total_row) and result and not skip_total_row: result = add_total_row(result, columns, is_tree=is_tree, parent_field=parent_field) + # breakpoint() return { "result": result, "columns": columns, @@ -223,6 +231,9 @@ def run( def add_custom_column_data(custom_columns, result): + for column in custom_columns: + column["fieldname"] = column["fieldname"].split("-")[0] + custom_column_data = get_data_for_custom_report(custom_columns, result) for column in custom_columns: @@ -240,6 +251,9 @@ def add_custom_column_data(custom_columns, result): # possible if the row is empty if not row_reference: continue + + if custom_column_data.get(key).get(row_reference) is None: + column["fieldname"] = column.get("fieldname") + "-" + frappe.unscrub(key[0]) row[column.get("fieldname")] = custom_column_data.get(key).get(row_reference) return result @@ -501,7 +515,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 diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 39c3431df89..842056438ba 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -933,7 +933,10 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { render_datatable() { let data = this.data; + console.log(this.data); let columns = this.columns.filter((col) => !col.hidden); + // columns = this. + // debugger if (this.raw_data.add_total_row && !this.report_settings.tree) { data = data.slice(); @@ -1681,7 +1684,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { (column) => column.label === values.insert_after ); custom_columns.push({ - fieldname: df.fieldname, + fieldname: df.fieldname + "-" + frappe.scrub(values.doctype), fieldtype: df.fieldtype, label: df.label, insert_after_index: insert_after_index, @@ -1703,14 +1706,15 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { }, callback: (r) => { const custom_data = r.message; + console.log(r.message); const link_field = this.doctype_field_map[values.doctype].fieldname; - + console.log(link_field, values.field); this.add_custom_column( custom_columns, custom_data, link_field, - values.field, + values, insert_after_index ); d.hide(); @@ -1791,13 +1795,22 @@ 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 + ) { + console.log(custom_column); 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]]; + console.log(row); + row[column_field + "-" + frappe.scrub(new_column_data.doctype)] = + custom_data[row[link_field]]; }); this.render_datatable(); From d3178527b3b5e56d2abf59e4d87e2bd41bca7c6e Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Thu, 14 Sep 2023 13:23:00 +0530 Subject: [PATCH 2/9] fix: works for multiple rows now after saving --- frappe/desk/query_report.py | 18 +++++++-------- .../js/frappe/views/reports/query_report.js | 23 +++++++++++-------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 218a3080336..a90a3911dc6 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -101,13 +101,6 @@ def generate_report_result( report_custom_columns = [ column for column in columns if column["fieldname"] not in report_column_names ] - # for column in report_custom_columns: - # column["fieldname"] = column["fieldname"].split("-")[0] - - # for column in columns: - # if (len(column.get("fieldname").split("-")) > 1): - # print(column.get("fieldname")) - # column["fieldname"] = column["fieldname"].split("-")[0] if report_custom_columns: result = add_custom_column_data(report_custom_columns, result) @@ -118,7 +111,6 @@ def generate_report_result( if cint(report.add_total_row) and result and not skip_total_row: result = add_total_row(result, columns, is_tree=is_tree, parent_field=parent_field) - # breakpoint() return { "result": result, "columns": columns, @@ -231,7 +223,11 @@ def run( def add_custom_column_data(custom_columns, result): + doctype_name_from_custom_field = [] for column in custom_columns: + doctype_name_from_custom_field.append(frappe.unscrub(column["fieldname"].split("-")[1])) if len( + column["fieldname"].split("-") + ) > 1 else None column["fieldname"] = column["fieldname"].split("-")[0] custom_column_data = get_data_for_custom_report(custom_columns, result) @@ -251,8 +247,10 @@ def add_custom_column_data(custom_columns, result): # possible if the row is empty if not row_reference: continue - - if custom_column_data.get(key).get(row_reference) is None: + if ( + custom_column_data.get(key).get(row_reference) is None + and key[0] in doctype_name_from_custom_field + ): column["fieldname"] = column.get("fieldname") + "-" + frappe.unscrub(key[0]) row[column.get("fieldname")] = custom_column_data.get(key).get(row_reference) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 842056438ba..cd01fecba74 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -933,10 +933,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { render_datatable() { let data = this.data; - console.log(this.data); let columns = this.columns.filter((col) => !col.hidden); - // columns = this. - // debugger if (this.raw_data.add_total_row && !this.report_settings.tree) { data = data.slice(); @@ -1683,8 +1680,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 + "-" + frappe.scrub(values.doctype), + 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, @@ -1706,10 +1708,8 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { }, callback: (r) => { const custom_data = r.message; - console.log(r.message); const link_field = this.doctype_field_map[values.doctype].fieldname; - console.log(link_field, values.field); this.add_custom_column( custom_columns, custom_data, @@ -1802,15 +1802,18 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { new_column_data, insert_after_index ) { - console.log(custom_column); 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) => { - console.log(row); - row[column_field + "-" + frappe.scrub(new_column_data.doctype)] = - 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(); From 4bbed3287eb95ed97c14f658d62fed9236f79676 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Fri, 15 Sep 2023 12:19:21 +0530 Subject: [PATCH 3/9] chore: code cleanup --- frappe/desk/query_report.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index a90a3911dc6..1b323a52351 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -247,11 +247,8 @@ def add_custom_column_data(custom_columns, result): # possible if the row is empty if not row_reference: continue - if ( - custom_column_data.get(key).get(row_reference) is None - and key[0] in doctype_name_from_custom_field - ): - column["fieldname"] = column.get("fieldname") + "-" + frappe.unscrub(key[0]) + if key[0] in doctype_name_from_custom_field: + column["fieldname"] = column.get("id") row[column.get("fieldname")] = custom_column_data.get(key).get(row_reference) return result From 7ae5ffca198c5fdbdfd2461345c7ba8f414a11f3 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 18 Sep 2023 18:24:43 +0530 Subject: [PATCH 4/9] chore: code cleanup --- frappe/desk/query_report.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 1b323a52351..08b800a14fb 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -223,11 +223,12 @@ def run( def add_custom_column_data(custom_columns, result): - doctype_name_from_custom_field = [] + doctype_names_from_custom_field = [] for column in custom_columns: - doctype_name_from_custom_field.append(frappe.unscrub(column["fieldname"].split("-")[1])) if len( - column["fieldname"].split("-") - ) > 1 else None + 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) @@ -247,7 +248,7 @@ def add_custom_column_data(custom_columns, result): # possible if the row is empty if not row_reference: continue - if key[0] in doctype_name_from_custom_field: + 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) From 34cd9435566dcac6580c47d6b6bb8bc6ca1c4b9d Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Sun, 24 Sep 2023 01:31:41 +0530 Subject: [PATCH 5/9] test: added test for conflicting column names --- frappe/tests/test_query_report.py | 135 +++++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index f93ab81c8b1..2dc9071dea4 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -1,14 +1,29 @@ # Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import json +import os + 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, + generate_report_result, + get_data_for_custom_field, + get_data_for_custom_report, + run, +) +from frappe.tests.ui_test_helpers import create_doctype from frappe.tests.utils import FrappeTestCase from frappe.utils.xlsxutils import make_xlsx class TestQueryReport(FrappeTestCase): + def tearDown(self): + frappe.db.rollback() + def test_xlsx_data_with_multiple_datatypes(self): """Test exporting report using rows with multiple datatypes (list, dict)""" @@ -110,3 +125,121 @@ 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() + data = report.get_data() + + 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() From 0c2c6f2aecb2389940f37e454c56ed85feb78d15 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Sun, 24 Sep 2023 02:36:04 +0530 Subject: [PATCH 6/9] chore: code cleanup --- frappe/tests/test_query_report.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index 2dc9071dea4..688285b0626 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -7,14 +7,7 @@ import frappe import frappe.utils from frappe.core.doctype.doctype.test_doctype import new_doctype -from frappe.desk.query_report import ( - build_xlsx_data, - export_query, - generate_report_result, - get_data_for_custom_field, - get_data_for_custom_report, - run, -) +from frappe.desk.query_report import build_xlsx_data, export_query, run from frappe.tests.ui_test_helpers import create_doctype from frappe.tests.utils import FrappeTestCase from frappe.utils.xlsxutils import make_xlsx From 32e3198c83a9c268fb72776b1de2589293b35e27 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Sun, 24 Sep 2023 11:34:06 +0530 Subject: [PATCH 7/9] test: code cleanup --- frappe/tests/test_query_report.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index 688285b0626..72d2ecca270 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -1,14 +1,10 @@ # Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -import json -import os - import frappe import frappe.utils 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.ui_test_helpers import create_doctype from frappe.tests.utils import FrappeTestCase from frappe.utils.xlsxutils import make_xlsx From 9ae56289df19149ef69bb95f001035d16960612f Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 25 Sep 2023 12:31:18 +0530 Subject: [PATCH 8/9] test: code cleanup --- frappe/tests/test_query_report.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index 72d2ecca270..6c00bbc41eb 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -190,7 +190,6 @@ def test_report_for_duplicate_column_names(self): data = columns, result """ report.save() - data = report.get_data() custom_columns = [ { From 9368f0aee437445b7aaa92156633c4ecd353477e Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Tue, 26 Sep 2023 12:02:25 +0530 Subject: [PATCH 9/9] fix: enable server script while testing --- frappe/tests/test_query_report.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index 6c00bbc41eb..0a22c0f1f29 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -10,6 +10,11 @@ class TestQueryReport(FrappeTestCase): + @classmethod + def setUpClass(cls) -> None: + cls.enable_safe_exec() + return super().setUpClass() + def tearDown(self): frappe.db.rollback()