From e059aa385fb2bfa005add29aa5d9fe10f55f55dd Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Thu, 9 Nov 2023 12:08:05 +0530 Subject: [PATCH 1/9] feat: add check to include filters in popup --- .../js/frappe/views/reports/query_report.js | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index b92c3166f5..e03f227055 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -1475,21 +1475,34 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { return; } - let extra_fields = null; + let extra_fields = []; + if (this.tree_report) { - extra_fields = [ - { - label: __("Include indentation"), - fieldname: "include_indentation", - fieldtype: "Check", - }, - ]; + extra_fields.push({ + label: __("Include indentation"), + fieldname: "include_indentation", + fieldtype: "Check", + }); + } + + if (this.filters.length > 0) { + extra_fields.push({ + label: __("Include filters"), + fieldname: "include_filters", + fieldtype: "Check", + }); } this.export_dialog = frappe.report_utils.get_export_dialog( __(this.report_name), extra_fields, - ({ file_format, include_indentation, csv_delimiter, csv_quoting }) => { + ({ + file_format, + include_indentation, + include_filters, + csv_delimiter, + csv_quoting, + }) => { this.make_access_log("Export", file_format); let filters = this.get_filter_values(true); @@ -1515,6 +1528,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { csv_delimiter, csv_quoting, include_indentation, + include_filters, }; open_url_post(frappe.request.url, args); From a52d1870dc8e803f28c1410860ec5b54a9560c69 Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Thu, 9 Nov 2023 12:09:42 +0530 Subject: [PATCH 2/9] feat: add filter values while building report xlsx data --- frappe/desk/query_report.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 7ca483d806..0a25dd3b8f 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -318,6 +318,7 @@ def export_query(): file_format_type = form_params.file_format_type custom_columns = frappe.parse_json(form_params.custom_columns or "[]") include_indentation = form_params.include_indentation + include_filters = form_params.include_filters visible_idx = form_params.visible_idx if isinstance(visible_idx, str): @@ -327,6 +328,7 @@ def export_query(): report_name, form_params.filters, custom_columns=custom_columns, are_default_filters=False ) data = frappe._dict(data) + data.filters = form_params.filters if not data.columns: frappe.respond_as_web_page( _("No data to export"), @@ -335,7 +337,9 @@ def export_query(): return format_duration_fields(data) - xlsx_data, column_widths = build_xlsx_data(data, visible_idx, include_indentation) + xlsx_data, column_widths = build_xlsx_data( + data, visible_idx, include_indentation, include_filters=include_filters + ) if file_format_type == "CSV": content = get_csv_bytes(xlsx_data, csv_params) @@ -360,7 +364,9 @@ def format_duration_fields(data: frappe._dict) -> None: row[index] = format_duration(row[index]) -def build_xlsx_data(data, visible_idx, include_indentation, ignore_visible_idx=False): +def build_xlsx_data( + data, visible_idx, include_indentation, include_filters=False, ignore_visible_idx=False +): EXCEL_TYPES = ( str, bool, @@ -380,17 +386,30 @@ def build_xlsx_data(data, visible_idx, include_indentation, ignore_visible_idx=F # Note: converted for faster lookups visible_idx = set(visible_idx) - result = [[]] + result = [] column_widths = [] + if cint(include_filters): + filter_data = [] + filters = data.filters + for filter_name, filter_value in filters.items(): + if filter_value in ["", None, []]: + continue + filter_value = ", ".join(filter_value) if isinstance(filter_value, list) else cstr(filter_value) + filter_data.append([cstr(filter_name) + ": ", filter_value]) + filter_data.append([]) + result += filter_data + + column_data = [] for column in data.columns: if column.get("hidden"): continue - result[0].append(_(column.get("label"))) + column_data.append(_(column.get("label"))) column_width = cint(column.get("width", 0)) # to convert into scale accepted by openpyxl column_width /= 10 column_widths.append(column_width) + result.append(column_data) # build table from result for row_idx, row in enumerate(data.result): From fdfe94a8baf99e4117b66b80516bbe322784d982 Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Thu, 30 Nov 2023 15:06:30 +0530 Subject: [PATCH 3/9] refactor: fetch labels instead of fieldnames for applied filters --- frappe/public/js/frappe/views/reports/query_report.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index e03f227055..72060d8b1c 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -1512,6 +1512,12 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { filters ); } + let applied_filters = Object.fromEntries( + Object.entries(filters).map(([key, value]) => [ + frappe.query_report.get_filter(key).df.label, + value, + ]) + ); const visible_idx = this.datatable.bodyRenderer.visibleRowIndices; if (visible_idx.length + 1 === this.data.length) { @@ -1524,6 +1530,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { custom_columns: this.custom_columns.length ? this.custom_columns : [], file_format_type: file_format, filters: filters, + applied_filters: applied_filters, visible_idx, csv_delimiter, csv_quoting, From 8a4d85a5ade879f4d6d5de8cfa4177e494d250ee Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Thu, 30 Nov 2023 15:07:30 +0530 Subject: [PATCH 4/9] fix: parse applied filter json before building data --- frappe/desk/query_report.py | 5 +++-- frappe/desk/reportview.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 0a25dd3b8f..cb5294b5ef 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -328,7 +328,8 @@ def export_query(): report_name, form_params.filters, custom_columns=custom_columns, are_default_filters=False ) data = frappe._dict(data) - data.filters = form_params.filters + data.filters = form_params.applied_filters + if not data.columns: frappe.respond_as_web_page( _("No data to export"), @@ -396,7 +397,7 @@ def build_xlsx_data( if filter_value in ["", None, []]: continue filter_value = ", ".join(filter_value) if isinstance(filter_value, list) else cstr(filter_value) - filter_data.append([cstr(filter_name) + ": ", filter_value]) + filter_data.append([cstr(filter_name), filter_value]) filter_data.append([]) result += filter_data diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 8f2f7f8dca..5874fd37ac 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -217,6 +217,8 @@ def clean_params(data): def parse_json(data): if (filters := data.get("filters")) and isinstance(filters, str): data["filters"] = json.loads(filters) + if (applied_filters := data.get("applied_filters")) and isinstance(applied_filters, str): + data["applied_filters"] = json.loads(applied_filters) if (or_filters := data.get("or_filters")) and isinstance(or_filters, str): data["or_filters"] = json.loads(or_filters) if (fields := data.get("fields")) and isinstance(fields, str): From 54d3679b9de24a3a178228776a762a4919b1ea52 Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Fri, 1 Dec 2023 13:07:39 +0530 Subject: [PATCH 5/9] fix: format filters with multiple values --- frappe/desk/query_report.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index cb5294b5ef..59c9114ec9 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -396,7 +396,11 @@ def build_xlsx_data( for filter_name, filter_value in filters.items(): if filter_value in ["", None, []]: continue - filter_value = ", ".join(filter_value) if isinstance(filter_value, list) else cstr(filter_value) + filter_value = ( + ", ".join(map(lambda x: cstr(x), filter_value)) + if isinstance(filter_value, list) + else cstr(filter_value) + ) filter_data.append([cstr(filter_name), filter_value]) filter_data.append([]) result += filter_data From 4e382fb217908131887a12cd563a7edb30a81312 Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Fri, 1 Dec 2023 13:09:31 +0530 Subject: [PATCH 6/9] test: build_xlsx_data with filters --- frappe/tests/test_query_report.py | 50 +++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index 0a22c0f1f2..8d47a846bb 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -22,18 +22,7 @@ class TestQueryReport(FrappeTestCase): """Test exporting report using rows with multiple datatypes (list, dict)""" # Create mock data - data = frappe._dict() - data.columns = [ - {"label": "Column A", "fieldname": "column_a", "fieldtype": "Float"}, - {"label": "Column B", "fieldname": "column_b", "width": 100, "fieldtype": "Float"}, - {"label": "Column C", "fieldname": "column_c", "width": 150, "fieldtype": "Duration"}, - ] - data.result = [ - [1.0, 3.0, 600], - {"column_a": 22.1, "column_b": 21.8, "column_c": 86412}, - {"column_b": 5.1, "column_c": 53234, "column_a": 11.1}, - [3.0, 1.5, 333], - ] + data = create_mock_data() # Define the visible rows visible_idx = [0, 2, 3] @@ -54,6 +43,27 @@ class TestQueryReport(FrappeTestCase): for cell in row: self.assertIsInstance(cell, (int, float)) + def test_xlsx_data_with_filters(self): + """Test building xlsx data along with filters""" + + # Create mock data + data = create_mock_data() + data.filters = {"Label 1": "Filter Value", "Label 2": None, "Label 3": list(range(5))} + + # Define the visible rows + visible_idx = [0, 2, 3] + + # Build the result + xlsx_data, column_widths = build_xlsx_data( + data, visible_idx, include_indentation=False, include_filters=True + ) + + # Check if unset filters are skipped | Rows - 2 filters + 1 empty + 1 column + 3 data + self.assertEqual(len(xlsx_data), 7) + + # Check filter formatting + self.assertListEqual(xlsx_data[:2], [["Label 1", "Filter Value"], ["Label 3", "0, 1, 2, 3, 4"]]) + def test_xlsx_export_with_composite_cell_value(self): """Test excel export using rows with composite cell value""" @@ -236,3 +246,19 @@ data = columns, result except Exception as e: raise e frappe.db.rollback() + + +def create_mock_data(): + data = frappe._dict() + data.columns = [ + {"label": "Column A", "fieldname": "column_a", "fieldtype": "Float"}, + {"label": "Column B", "fieldname": "column_b", "width": 100, "fieldtype": "Float"}, + {"label": "Column C", "fieldname": "column_c", "width": 150, "fieldtype": "Duration"}, + ] + data.result = [ + [1.0, 3.0, 600], + {"column_a": 22.1, "column_b": 21.8, "column_c": 86412}, + {"column_b": 5.1, "column_c": 53234, "column_a": 11.1}, + [3.0, 1.5, 333], + ] + return data From 4a7c60fb5a98197e11517aa0c02ac3e6f589765a Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Fri, 15 Dec 2023 17:02:09 +0530 Subject: [PATCH 7/9] refactor: use simple condition for filter values --- frappe/desk/query_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 59c9114ec9..bc12f52955 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -394,7 +394,7 @@ def build_xlsx_data( filter_data = [] filters = data.filters for filter_name, filter_value in filters.items(): - if filter_value in ["", None, []]: + if not filter_value: continue filter_value = ( ", ".join(map(lambda x: cstr(x), filter_value)) From 801b9e1140f19b2a5ec6f00549c091fc661303ce Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Fri, 15 Dec 2023 17:51:22 +0530 Subject: [PATCH 8/9] fix: labels for boolean filters --- frappe/public/js/frappe/views/reports/query_report.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 72060d8b1c..5ac979f029 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -1512,10 +1512,13 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { filters ); } + let boolean_labels = { 1: __("Yes"), 0: __("No") }; let applied_filters = Object.fromEntries( Object.entries(filters).map(([key, value]) => [ frappe.query_report.get_filter(key).df.label, - value, + frappe.query_report.get_filter(key).df.fieldtype == "Check" + ? boolean_labels[value] + : value, ]) ); From 806708650cfdc952d15d6b559422d8781a31b8dc Mon Sep 17 00:00:00 2001 From: Gursheen Anand Date: Mon, 18 Dec 2023 12:15:51 +0530 Subject: [PATCH 9/9] chore: linter issue --- frappe/desk/query_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index bc12f52955..161e45c75e 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -397,7 +397,7 @@ def build_xlsx_data( if not filter_value: continue filter_value = ( - ", ".join(map(lambda x: cstr(x), filter_value)) + ", ".join([cstr(x) for x in filter_value]) if isinstance(filter_value, list) else cstr(filter_value) )