From b6cd4330f21b21ada5362b1759e0debb8d97ed11 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 17 Aug 2020 17:09:04 +0530 Subject: [PATCH 01/11] feat(reports): add columns and filters via configuration --- frappe/core/doctype/doctype/doctype.json | 2 +- frappe/core/doctype/report/report.js | 20 -- frappe/core/doctype/report/report.json | 52 +++- frappe/core/doctype/report/report.py | 269 ++++++++++-------- frappe/core/doctype/report/test_report.py | 38 +++ frappe/core/doctype/report_column/__init__.py | 0 .../doctype/report_column/report_column.json | 61 ++++ .../doctype/report_column/report_column.py | 10 + frappe/core/doctype/report_filter/__init__.py | 0 .../doctype/report_filter/report_filter.json | 71 +++++ .../doctype/report_filter/report_filter.py | 10 + .../js/frappe/views/reports/query_report.js | 13 +- 12 files changed, 395 insertions(+), 151 deletions(-) create mode 100644 frappe/core/doctype/report_column/__init__.py create mode 100644 frappe/core/doctype/report_column/report_column.json create mode 100644 frappe/core/doctype/report_column/report_column.py create mode 100644 frappe/core/doctype/report_filter/__init__.py create mode 100644 frappe/core/doctype/report_filter/report_filter.json create mode 100644 frappe/core/doctype/report_filter/report_filter.py diff --git a/frappe/core/doctype/doctype/doctype.json b/frappe/core/doctype/doctype/doctype.json index b890f743cc..6cdf2e40ac 100644 --- a/frappe/core/doctype/doctype/doctype.json +++ b/frappe/core/doctype/doctype/doctype.json @@ -609,7 +609,7 @@ "link_fieldname": "reference_doctype" } ], - "modified": "2020-08-06 12:59:32.369093", + "modified": "2020-08-06 12:59:32.369095", "modified_by": "Administrator", "module": "Core", "name": "DocType", diff --git a/frappe/core/doctype/report/report.js b/frappe/core/doctype/report/report.js index c410e9aa1a..14e9b3a901 100644 --- a/frappe/core/doctype/report/report.js +++ b/frappe/core/doctype/report/report.js @@ -42,26 +42,6 @@ frappe.ui.form.on('Report', { } }, - report_type: function(frm) { - frm.set_intro(""); - switch(frm.doc.report_type) { - case "Report Builder": - frm.set_intro(__("Report Builder reports are managed directly by the report builder. Nothing to do.")); - break; - case "Query Report": - frm.set_intro(__("Write a SELECT query. Note result is not paged (all data is sent in one go).") - + __("To format columns, give column labels in the query.") + "
" - + __("[Label]:[Field Type]/[Options]:[Width]") + "

" - + __("Example:") + "
" - + "Employee:Link/Employee:200" + "
" - + "Rate:Currency:120" + "
") - break; - case "Script Report": - frm.set_intro(__("Write a Python file in the same folder where this is saved and return column and result.")); - break; - } - }, - set_doctype_roles: function(frm) { return frm.call('set_doctype_roles').then(() => { frm.refresh_field('roles'); diff --git a/frappe/core/doctype/report/report.json b/frappe/core/doctype/report/report.json index 40d2417a56..5b3593e658 100644 --- a/frappe/core/doctype/report/report.json +++ b/frappe/core/doctype/report/report.json @@ -1,4 +1,5 @@ { + "actions": [], "autoname": "field:report_name", "creation": "2013-03-09 15:45:57", "doctype": "DocType", @@ -17,10 +18,15 @@ "disabled", "disable_prepared_report", "prepared_report", + "filters_section", + "filters", + "columns_section", + "columns", "section_break_6", "query", - "javascript", "report_script", + "client_code_section", + "javascript", "json", "permission_rules", "roles" @@ -94,7 +100,8 @@ }, { "fieldname": "section_break_6", - "fieldtype": "Section Break" + "fieldtype": "Section Break", + "label": "Query / Script" }, { "depends_on": "eval:doc.report_type==\"Query Report\"", @@ -142,15 +149,50 @@ "read_only": 1 }, { - "depends_on": "eval:doc.report_type==\"Script Report\" && doc.is_standard===\"No\"", - "description": "output in the form of `data = [columns, result]`", + "depends_on": "eval:(doc.report_type===\"Script Report\" \n|| doc.report_type==\"Query Report\") \n&& doc.is_standard===\"No\"", + "description": "Filters will be accessible via filters.

Send output as result = [result], or for old style data = [columns], [result]", "fieldname": "report_script", "fieldtype": "Code", "label": "Script" + }, + { + "collapsible": 1, + "collapsible_depends_on": "filters", + "fieldname": "filters_section", + "fieldtype": "Section Break", + "label": "Filters" + }, + { + "fieldname": "filters", + "fieldtype": "Table", + "label": "Filters", + "options": "Report Filter" + }, + { + "collapsible": 1, + "collapsible_depends_on": "columns", + "fieldname": "columns_section", + "fieldtype": "Section Break", + "label": "Columns" + }, + { + "fieldname": "columns", + "fieldtype": "Table", + "label": "Columns", + "options": "Report Column" + }, + { + "collapsible": 1, + "collapsible_depends_on": "javascript", + "fieldname": "client_code_section", + "fieldtype": "Section Break", + "label": "Client Code" } ], "idx": 1, - "modified": "2019-10-09 15:43:08.577610", + "index_web_pages_for_search": 1, + "links": [], + "modified": "2020-08-17 16:49:28.474274", "modified_by": "Administrator", "module": "Core", "name": "Report", diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index dd695d484e..bbdab54686 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -51,6 +51,9 @@ class Report(Document): def on_trash(self): delete_custom_role('report', self.name) + def get_columns(self): + return [d.as_dict(no_default_fields = True) for d in self.columns] + def set_doctype_roles(self): if not self.get('roles') and self.is_standard == 'No': meta = frappe.get_meta(self.ref_doctype) @@ -99,8 +102,8 @@ class Report(Document): if not self.query.lower().startswith("select"): frappe.throw(_("Query must be a SELECT"), title=_('Report Document Error')) - result = [list(t) for t in frappe.db.sql(self.query, filters)] - columns = [cstr(c[0]) for c in frappe.db.get_description()] + result = [list(t) for t in frappe.db.sql(self.query, filters, debug=True)] + columns = self.get_columns() or [cstr(c[0]) for c in frappe.db.get_description()] return [columns, result] @@ -134,135 +137,155 @@ class Report(Document): def execute_script(self, filters): # server script - loc = {"filters": frappe._dict(filters), 'data':[]} + loc = {"filters": frappe._dict(filters), 'data':None, 'result':None} safe_exec(self.report_script, None, loc) - return loc['data'] + if loc['data']: + return loc['data'] + else: + return self.get_columns(), loc['result'] def get_data(self, filters=None, limit=None, user=None, as_dict=False, ignore_prepared_report=False): - columns = [] - out = [] - if self.report_type in ('Query Report', 'Script Report', 'Custom Report'): - # query and script reports - data = frappe.desk.query_report.run(self.name, - filters=filters, user=user, ignore_prepared_report=ignore_prepared_report) - - for d in data.get('columns'): - if isinstance(d, dict): - col = frappe._dict(d) - if not col.fieldname: - col.fieldname = col.label - columns.append(col) - else: - fieldtype, options = "Data", None - parts = d.split(':') - if len(parts) > 1: - if parts[1]: - fieldtype, options = parts[1], None - if fieldtype and '/' in fieldtype: - fieldtype, options = fieldtype.split('/') - - columns.append(frappe._dict(label=parts[0], fieldtype=fieldtype, fieldname=parts[0], options=options)) - - out += data.get('result') + columns, result = self.run_query_report(filters, user, ignore_prepared_report) else: - # standard report - params = json.loads(self.json) - - if params.get('fields'): - columns = params.get('fields') - elif params.get('columns'): - columns = params.get('columns') - else: - columns = [['name', self.ref_doctype]] - for df in frappe.get_meta(self.ref_doctype).fields: - if df.in_list_view: - columns.append([df.fieldname, self.ref_doctype]) - - _filters = params.get('filters') or [] - - if filters: - for key, value in iteritems(filters): - condition, _value = '=', value - if isinstance(value, (list, tuple)): - condition, _value = value - _filters.append([key, condition, _value]) - - def _format(parts): - # sort by is saved as DocType.fieldname, covert it to sql - return '`tab{0}`.`{1}`'.format(*parts) - - if params.get('sort_by'): - order_by = _format(params.get('sort_by').split('.')) + ' ' + params.get('sort_order') - elif params.get('order_by'): - order_by = params.get('order_by') - else: - order_by = _format([self.ref_doctype, 'modified']) + ' desc' - - if params.get('sort_by_next'): - order_by += ', ' + _format(params.get('sort_by_next').split('.')) + ' ' + params.get('sort_order_next') - - group_by = None - if params.get('group_by'): - group_by_args = frappe._dict(params['group_by']) - group_by = group_by_args['group_by'] - order_by = '_aggregate_column desc' - - result = frappe.get_list(self.ref_doctype, - fields = [ - get_group_by_field(group_by_args, c[1]) if c[0] == '_aggregate_column' and group_by_args - else _format([c[1], c[0]]) - for c in columns - ], - filters=_filters, - order_by = order_by, - as_list=True, - limit=limit, - group_by=group_by, - user=user) - - _columns = [] - - for (fieldname, doctype) in columns: - meta = frappe.get_meta(doctype) - - if meta.get_field(fieldname): - field = meta.get_field(fieldname) - else: - if fieldname == '_aggregate_column': - label = get_group_by_column_label(group_by_args, meta) - else: - label = meta.get_label(fieldname) - - field = frappe._dict(fieldname=fieldname, label=label) - # since name is the primary key for a document, it will always be a Link datatype - if fieldname == "name": - field.fieldtype = "Link" - field.options = doctype - - _columns.append(field) - columns = _columns - - out = out + [list(d) for d in result] - - if params.get('add_totals_row'): - out = append_totals_row(out) + columns, result = self.run_standard_report(filters, limit, user) if as_dict: - data = [] - for row in out: - if isinstance(row, (list, tuple)): - _row = frappe._dict() - for i, val in enumerate(row): - _row[columns[i].get('fieldname')] = val - elif isinstance(row, dict): - # no need to convert from dict to dict - _row = frappe._dict(row) - data.append(_row) - else: - data = out - return columns, data + result = self.build_data_dict(result) + return columns, result + + def run_query_report(self, filters, user, ignore_prepared_report=False): + columns, result = [], [] + data = frappe.desk.query_report.run(self.name, + filters=filters, user=user, ignore_prepared_report=ignore_prepared_report) + + for d in data.get('columns'): + if isinstance(d, dict): + col = frappe._dict(d) + if not col.fieldname: + col.fieldname = col.label + columns.append(col) + else: + fieldtype, options = "Data", None + parts = d.split(':') + if len(parts) > 1: + if parts[1]: + fieldtype, options = parts[1], None + if fieldtype and '/' in fieldtype: + fieldtype, options = fieldtype.split('/') + + columns.append(frappe._dict(label=parts[0], fieldtype=fieldtype, fieldname=parts[0], options=options)) + + result += data.get('result') + + return columns, result + + def run_standard_report(self, filters, limit, user): + params = json.loads(self.json) + columns = self.get_standard_report_columns(params) + result = [] + + _result = frappe.get_list(self.ref_doctype, + fields = [Report._format([c[1], c[0]]) for c in columns], + filters = self.get_standard_report_filters(params, filters), + order_by = self.get_standard_report_order_by(params), + as_list = True, + limit = limit, + user = user) + + columns = self.build_standard_report_columns(columns) + + result = result + [list(d) for d in _result] + + if params.get('add_totals_row'): + result = append_totals_row(out) + + return columns, result + + @staticmethod + def _format(parts): + # sort by is saved as DocType.fieldname, covert it to sql + return '`tab{0}`.`{1}`'.format(*parts) + + def get_standard_report_columns(self, params): + if params.get('fields'): + columns = params.get('fields') + elif params.get('columns'): + columns = params.get('columns') + elif params.get('fields'): + columns = params.get('fields') + else: + columns = [['name', self.ref_doctype]] + for df in frappe.get_meta(self.ref_doctype).fields: + if df.in_list_view: + columns.append([df.fieldname, self.ref_doctype]) + + return columns + + def get_standard_report_filters(self, params, filters): + _filters = params.get('filters') or [] + + if filters: + for key, value in iteritems(filters): + condition, _value = '=', value + if isinstance(value, (list, tuple)): + condition, _value = value + _filters.append([key, condition, _value]) + + return _filters + + def get_standard_report_order_by(self, params): + if params.get('sort_by'): + order_by = Report._format(params.get('sort_by').split('.')) + ' ' + params.get('sort_order') + + elif params.get('order_by'): + order_by = params.get('order_by') + else: + order_by = Report._format([self.ref_doctype, 'modified']) + ' desc' + + if params.get('sort_by_next'): + order_by += ', ' + Report._format(params.get('sort_by_next').split('.')) + ' ' + params.get('sort_order_next') + + return order_by + + def build_standard_report_columns(self, columns): + _columns = [] + + for (fieldname, doctype) in columns: + meta = frappe.get_meta(doctype) + + if meta.get_field(fieldname): + field = meta.get_field(fieldname) + else: + if fieldname == '_aggregate_column': + label = get_group_by_column_label(group_by_args, meta) + else: + label = meta.get_label(fieldname) + + field = frappe._dict(fieldname=fieldname, label=label) + + # since name is the primary key for a document, it will always be a Link datatype + if fieldname == "name": + field.fieldtype = "Link" + field.options = doctype + + _columns.append(field) + return _columns + + def build_data_dict(self, result): + data = [] + for row in result: + if isinstance(row, (list, tuple)): + _row = frappe._dict() + for i, val in enumerate(row): + _row[columns[i].get('fieldname')] = val + elif isinstance(row, dict): + # no need to convert from dict to dict + _row = frappe._dict(row) + data.append(_row) + + return data @Document.whitelist def toggle_disable(self, disable): diff --git a/frappe/core/doctype/report/test_report.py b/frappe/core/doctype/report/test_report.py index b8b18205b4..805b903300 100644 --- a/frappe/core/doctype/report/test_report.py +++ b/frappe/core/doctype/report/test_report.py @@ -111,3 +111,41 @@ data = [ # check values self.assertTrue('System User' in [d.get('type') for d in data[1]]) + def test_script_report_with_columns(self): + report_name = 'Test Script Report With Columns' + + if frappe.db.exists("Report", report_name): + frappe.delete_doc('Report', report_name) + + report = frappe.get_doc({ + 'doctype': 'Report', + 'ref_doctype': 'User', + 'report_name': report_name, + 'report_type': 'Script Report', + 'is_standard': 'No', + 'columns': [ + dict(fieldname='type', label='Type', fieldtype='Data'), + dict(fieldname='value', label='Value', fieldtype='Int'), + ] + }).insert(ignore_permissions=True) + + report.report_script = ''' +totals = {} +for user in frappe.get_all('User', fields = ['name', 'user_type', 'creation']): + if not user.user_type in totals: + totals[user.user_type] = 0 + totals[user.user_type] = totals[user.user_type] + 1 + +result = [ + {"type":key, "value": value} for key, value in totals.items() + ] +''' + + report.save() + data = report.get_data() + + # check columns + self.assertEqual(data[0][0]['label'], 'Type') + + # check values + self.assertTrue('System User' in [d.get('type') for d in data[1]]) diff --git a/frappe/core/doctype/report_column/__init__.py b/frappe/core/doctype/report_column/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/report_column/report_column.json b/frappe/core/doctype/report_column/report_column.json new file mode 100644 index 0000000000..53b5dff9b6 --- /dev/null +++ b/frappe/core/doctype/report_column/report_column.json @@ -0,0 +1,61 @@ +{ + "actions": [], + "creation": "2020-01-14 11:28:37.583656", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "fieldname", + "label", + "fieldtype", + "options", + "width" + ], + "fields": [ + { + "fieldname": "fieldname", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Fieldname", + "reqd": 1 + }, + { + "fieldname": "label", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Label", + "reqd": 1 + }, + { + "fieldname": "fieldtype", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Fieldtype", + "options": "Check\nCurrency\nData\nDate\nDatetime\nDynamic Link\nFloat\nFold\nInt\nLink\nSelect\nTime", + "reqd": 1 + }, + { + "fieldname": "options", + "fieldtype": "Data", + "label": "Options" + }, + { + "fieldname": "width", + "fieldtype": "Int", + "label": "Width" + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2020-08-17 14:32:17.174796", + "modified_by": "Administrator", + "module": "Core", + "name": "Report Column", + "owner": "Administrator", + "permissions": [], + "quick_entry": 1, + "sort_field": "modified", + "sort_order": "DESC", + "track_changes": 1 +} \ No newline at end of file diff --git a/frappe/core/doctype/report_column/report_column.py b/frappe/core/doctype/report_column/report_column.py new file mode 100644 index 0000000000..69c88b7bda --- /dev/null +++ b/frappe/core/doctype/report_column/report_column.py @@ -0,0 +1,10 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020, Frappe Technologies and contributors +# For license information, please see license.txt + +from __future__ import unicode_literals +# import frappe +from frappe.model.document import Document + +class ReportColumn(Document): + pass diff --git a/frappe/core/doctype/report_filter/__init__.py b/frappe/core/doctype/report_filter/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/report_filter/report_filter.json b/frappe/core/doctype/report_filter/report_filter.json new file mode 100644 index 0000000000..9d277db11d --- /dev/null +++ b/frappe/core/doctype/report_filter/report_filter.json @@ -0,0 +1,71 @@ +{ + "actions": [], + "creation": "2020-01-14 11:38:58.016498", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "fieldname", + "label", + "fieldtype", + "mandatory", + "options", + "wildcard_filter" + ], + "fields": [ + { + "fieldname": "fieldname", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Fieldname", + "reqd": 1 + }, + { + "fieldname": "label", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Label", + "reqd": 1 + }, + { + "fieldname": "fieldtype", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Fieldtype", + "options": "Check\nCurrency\nData\nDate\nDatetime\nDynamic Link\nFloat\nFold\nInt\nLink\nSelect\nTime", + "reqd": 1 + }, + { + "default": "0", + "fieldname": "mandatory", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Mandatory" + }, + { + "fieldname": "options", + "fieldtype": "Data", + "label": "Options" + }, + { + "default": "0", + "description": "Will add \"%\" before and after the query", + "fieldname": "wildcard_filter", + "fieldtype": "Check", + "label": "Wildcard Filter" + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2020-08-17 16:15:46.937267", + "modified_by": "Administrator", + "module": "Core", + "name": "Report Filter", + "owner": "Administrator", + "permissions": [], + "quick_entry": 1, + "sort_field": "modified", + "sort_order": "DESC", + "track_changes": 1 +} \ No newline at end of file diff --git a/frappe/core/doctype/report_filter/report_filter.py b/frappe/core/doctype/report_filter/report_filter.py new file mode 100644 index 0000000000..d85a1a5a65 --- /dev/null +++ b/frappe/core/doctype/report_filter/report_filter.py @@ -0,0 +1,10 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020, Frappe Technologies and contributors +# For license information, please see license.txt + +from __future__ import unicode_literals +# import frappe +from frappe.model.document import Document + +class ReportFilter(Document): + pass diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index e0b9019ca3..706cde13b7 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -366,6 +366,12 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.report_settings.html_format = settings.html_format; this.report_settings.execution_time = settings.execution_time || 0; frappe.query_reports[this.report_name] = this.report_settings; + + if (this.report_doc.filters && !this.report_settings.filters) { + // add configured filters + this.report_settings.filters = this.report_doc.filters; + } + resolve(); }); }).catch(reject); @@ -1109,8 +1115,11 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { .map(f => { var v = f.get_value(); // hidden fields dont have $input - if(f.df.hidden) v = f.value; - if(v === '%') v = null; + if (f.df.hidden) v = f.value; + if (v === '%') v = null; + if (f.df.wildcard_filter) { + v = `%${v}%`; + } return { [f.df.fieldname]: v }; From e3f686276e87d82fed74b428dc26db9850fea325 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 17 Aug 2020 17:26:27 +0530 Subject: [PATCH 02/11] fix(minor): pass columns to build_data_dict in report.py --- frappe/core/doctype/report/report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index bbdab54686..790df3da3a 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -151,7 +151,7 @@ class Report(Document): columns, result = self.run_standard_report(filters, limit, user) if as_dict: - result = self.build_data_dict(result) + result = self.build_data_dict(result, columns) return columns, result @@ -273,7 +273,7 @@ class Report(Document): _columns.append(field) return _columns - def build_data_dict(self, result): + def build_data_dict(self, result, columns): data = [] for row in result: if isinstance(row, (list, tuple)): From 6bd38fd00ad733b80e8558152585d73e390b05c7 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 17 Aug 2020 22:27:07 +0530 Subject: [PATCH 03/11] fix(minor): report.py --- frappe/core/doctype/report/report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 790df3da3a..b9584ba122 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -199,7 +199,7 @@ class Report(Document): result = result + [list(d) for d in _result] if params.get('add_totals_row'): - result = append_totals_row(out) + result = append_totals_row(result) return columns, result From a15e54aec4b095ff42b16551cc58515c674d6e16 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 18 Aug 2020 15:23:21 +0530 Subject: [PATCH 04/11] fix: set is_query_report for custom reports --- frappe/desk/desktop.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/desk/desktop.py b/frappe/desk/desktop.py index ae9d070976..148ae87249 100644 --- a/frappe/desk/desktop.py +++ b/frappe/desk/desktop.py @@ -47,11 +47,11 @@ class Workspace: self.allowed_pages = get_allowed_pages(cache=True) self.allowed_reports = get_allowed_reports(cache=True) - + if not minimal: self.onboarding_doc = self.get_onboarding_doc() self.onboarding = None - + self.table_counts = get_table_with_counts() self.restricted_doctypes = frappe.cache().get_value("domain_restricted_doctypes") or build_domain_restriced_doctype_cache() self.restricted_pages = frappe.cache().get_value("domain_restricted_pages") or build_domain_restriced_page_cache() @@ -59,7 +59,7 @@ class Workspace: def is_page_allowed(self): cards = self.doc.cards + get_custom_reports_and_doctypes(self.doc.module) + self.extended_cards shortcuts = self.doc.shortcuts + self.extended_shortcuts - + for section in cards: links = loads(section.links) if isinstance(section.links, string_types) else section.links for item in links: @@ -195,7 +195,7 @@ class Workspace: 'docs_url': self.onboarding_doc.documentation_url, 'items': self.get_onboarding_steps() } - + @handle_not_exist def get_cards(self): cards = self.doc.cards @@ -303,7 +303,7 @@ class Workspace: if self.is_item_allowed(item.link_to, item.type) and _in_active_domains(item): if item.type == "Report": report = self.allowed_reports.get(item.link_to, {}) - if report.get("report_type") in ["Query Report", "Script Report"]: + if report.get("report_type") in ["Query Report", "Script Report", "Custom Report"]: new_item['is_query_report'] = 1 else: new_item['ref_doctype'] = report.get('ref_doctype') @@ -358,7 +358,7 @@ def get_desk_sidebar_items(flatten=False, cache=True): _cache = frappe.cache() if cache: pages = _cache.get_value("desk_sidebar_items", user=frappe.session.user) - + if not pages or not cache: # don't get domain restricted pages blocked_modules = frappe.get_doc('User', frappe.session.user).get_blocked_modules() @@ -377,7 +377,7 @@ def get_desk_sidebar_items(flatten=False, cache=True): order_by = "pin_to_top desc, pin_to_bottom asc, name asc" all_pages = frappe.get_all("Desk Page", fields=["name", "category"], filters=filters, order_by=order_by, ignore_permissions=True) pages = [] - + # Filter Page based on Permission for page in all_pages: try: From 5aea22eb337821b9b62df1849c6cf23b676b9b4e Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 18 Aug 2020 17:05:19 +0530 Subject: [PATCH 05/11] fix(minor): report.py: adapt recent group_by changes --- frappe/core/doctype/report/report.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index b9584ba122..04bf535e79 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -185,16 +185,20 @@ class Report(Document): params = json.loads(self.json) columns = self.get_standard_report_columns(params) result = [] + order_by, group_by_args = self.get_standard_report_order_by(params) _result = frappe.get_list(self.ref_doctype, - fields = [Report._format([c[1], c[0]]) for c in columns], + fields = [ + get_group_by_field(group_by_args, c[1]) if c[0] == '_aggregate_column' and group_by_args + else _format([c[1], c[0]]) + ], filters = self.get_standard_report_filters(params, filters), - order_by = self.get_standard_report_order_by(params), + order_by = order_by, as_list = True, limit = limit, user = user) - columns = self.build_standard_report_columns(columns) + columns = self.build_standard_report_columns(columns, group_by_args) result = result + [list(d) for d in _result] @@ -236,6 +240,7 @@ class Report(Document): return _filters def get_standard_report_order_by(self, params): + group_by_args = None if params.get('sort_by'): order_by = Report._format(params.get('sort_by').split('.')) + ' ' + params.get('sort_order') @@ -247,9 +252,15 @@ class Report(Document): if params.get('sort_by_next'): order_by += ', ' + Report._format(params.get('sort_by_next').split('.')) + ' ' + params.get('sort_order_next') - return order_by + group_by = None + if params.get('group_by'): + group_by_args = frappe._dict(params['group_by']) + group_by = group_by_args['group_by'] + order_by = '_aggregate_column desc' - def build_standard_report_columns(self, columns): + return order_by, group_by_args + + def build_standard_report_columns(self, columns, group_by_args): _columns = [] for (fieldname, doctype) in columns: From 6100169233d510920f8d230e3ba0d7f92ac06bbd Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 18 Aug 2020 17:16:31 +0530 Subject: [PATCH 06/11] fix(minor): report.py: adapt recent group_by changes --- frappe/core/doctype/report/report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 04bf535e79..d65d0db1ce 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -190,7 +190,7 @@ class Report(Document): _result = frappe.get_list(self.ref_doctype, fields = [ get_group_by_field(group_by_args, c[1]) if c[0] == '_aggregate_column' and group_by_args - else _format([c[1], c[0]]) + else Report._format([c[1], c[0]]) for c in columns ], filters = self.get_standard_report_filters(params, filters), order_by = order_by, From 9786485f7036d7a632f6340aa7328559db4c0826 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 18 Aug 2020 17:35:00 +0530 Subject: [PATCH 07/11] fix(minor): report.py: adapt recent group_by changes --- frappe/core/doctype/report/report.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index d65d0db1ce..8b7a03aa28 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -185,7 +185,7 @@ class Report(Document): params = json.loads(self.json) columns = self.get_standard_report_columns(params) result = [] - order_by, group_by_args = self.get_standard_report_order_by(params) + order_by, group_by, group_by_args = self.get_standard_report_order_by(params) _result = frappe.get_list(self.ref_doctype, fields = [ @@ -194,6 +194,7 @@ class Report(Document): ], filters = self.get_standard_report_filters(params, filters), order_by = order_by, + group_by = group_by, as_list = True, limit = limit, user = user) @@ -258,7 +259,7 @@ class Report(Document): group_by = group_by_args['group_by'] order_by = '_aggregate_column desc' - return order_by, group_by_args + return order_by, group_by, group_by_args def build_standard_report_columns(self, columns, group_by_args): _columns = [] From b83e0b9abd63014cd9c3b331518c79885381547d Mon Sep 17 00:00:00 2001 From: Mangesh-Khairnar Date: Tue, 18 Aug 2020 17:54:27 +0530 Subject: [PATCH 08/11] fix: add an explicit check for schedule sending (#11291) --- frappe/email/doctype/newsletter/newsletter.json | 15 +++++++++------ frappe/email/doctype/newsletter/newsletter.py | 3 ++- .../email/doctype/newsletter/test_newsletter.py | 1 + 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index 1ec64826da..4804b3d6fa 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -8,7 +8,7 @@ "engine": "InnoDB", "field_order": [ "send_from", - "column_break_2", + "schedule_sending", "schedule_send", "recipients", "email_group", @@ -114,19 +114,22 @@ "label": "Recipients" }, { + "depends_on": "eval: doc.schedule_sending", "fieldname": "schedule_send", "fieldtype": "Datetime", "label": "Schedule Send" }, - { - "fieldname": "column_break_2", - "fieldtype": "Column Break" - }, { "default": "0", "fieldname": "send_attachments", "fieldtype": "Check", "label": "Send Attachments" + }, + { + "default": "0", + "fieldname": "schedule_sending", + "fieldtype": "Check", + "label": "Schedule Sending" } ], "has_web_view": 1, @@ -136,7 +139,7 @@ "is_published_field": "published", "links": [], "max_attachments": 3, - "modified": "2020-07-21 16:25:17.687476", + "modified": "2020-08-17 18:11:59.541686", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 48688afdb6..849c21f768 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -271,7 +271,8 @@ def send_scheduled_email(): """Send scheduled newsletter to the recipients.""" scheduled_newsletter = frappe.get_all('Newsletter', filters = { 'schedule_send': ('<=', now_datetime()), - 'email_sent': 0 + 'email_sent': 0, + 'schedule_sending': 1 }, fields = ['name'], ignore_ifnull=True) for newsletter in scheduled_newsletter: send_newsletter(newsletter.name) diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index 5a13f99e56..bb339165d3 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/frappe/email/doctype/newsletter/test_newsletter.py @@ -69,6 +69,7 @@ class TestNewsletter(unittest.TestCase): "send_from": "Test Sender ", "message": "Testing my news.", "published": published, + "schedule_sending": bool(schedule_send), "schedule_send": schedule_send }).insert(ignore_permissions=True) From a4dfac507b130b986460ff9848610fbe6052e429 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Wed, 19 Aug 2020 14:57:59 +0530 Subject: [PATCH 09/11] fix(minor): use get_list in client.get_value for tighter checks --- frappe/client.py | 12 +++++++++--- frappe/tests/test_api.py | 32 ++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 045e28dd8f..a813e3ec55 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -75,17 +75,23 @@ def get_value(doctype, fieldname, filters=None, as_dict=True, debug=False, paren filters = get_safe_filters(filters) try: - fieldname = json.loads(fieldname) + fields = json.loads(fieldname) except (TypeError, ValueError): # name passed, not json - pass + fields = [fieldname] # check whether the used filters were really parseable and usable # and did not just result in an empty string or dict if not filters: filters = None - return frappe.db.get_value(doctype, filters, fieldname, as_dict=as_dict, debug=debug) + value = frappe.get_list(doctype, filters=filters, fields=fields, debug=debug, limit=1) + if as_dict: + value = value[0] if value else {} + else: + value = value[0].fieldname + + return value @frappe.whitelist() def get_single_value(doctype, field): diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py index 2472f3191d..0edc1d4759 100644 --- a/frappe/tests/test_api.py +++ b/frappe/tests/test_api.py @@ -4,18 +4,14 @@ from __future__ import unicode_literals import unittest, frappe, os from frappe.core.doctype.user.user import generate_keys -from frappe.frappeclient import FrappeClient +from frappe.frappeclient import FrappeClient, FrappeException from frappe.utils.data import get_url import requests import base64 class TestAPI(unittest.TestCase): - def test_insert_many(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) - frappe.db.sql("delete from `tabNote` where title in ('Sing','a','song','of','sixpence')") - frappe.db.commit() - + def insert_docs(self): server.insert_many([ {"doctype": "Note", "public": True, "title": "Sing"}, {"doctype": "Note", "public": True, "title": "a"}, @@ -24,6 +20,13 @@ class TestAPI(unittest.TestCase): {"doctype": "Note", "public": True, "title": "sixpence"}, ]) + def test_insert_many(self, server): + server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + frappe.db.sql("delete from `tabNote` where title in ('Sing','a','song','of','sixpence')") + frappe.db.commit() + + self.insert_docs(server) + self.assertTrue(frappe.db.get_value('Note', {'title': 'Sing'})) self.assertTrue(frappe.db.get_value('Note', {'title': 'a'})) self.assertTrue(frappe.db.get_value('Note', {'title': 'song'})) @@ -41,6 +44,8 @@ class TestAPI(unittest.TestCase): def test_list_docs(self): server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + self.insert_docs(server) + doc_list = server.get_list("Note") self.assertTrue(len(doc_list)) @@ -56,6 +61,21 @@ class TestAPI(unittest.TestCase): doc = server.get_doc("Note", "get_this") self.assertTrue(doc) + def test_get_value(self): + server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + frappe.db.sql("delete from `tabNote` where title = 'get_value'") + frappe.db.commit() + + test_content = "test get value" + + server.insert_many([ + {"doctype": "Note", "public": True, "title": "get_value", "content": test_content}, + ]) + self.assertEqual(server.get_value("Note", "content", {"title": "get_value"}).get('content'), test_content) + + self.assertRaises(FrappeException, server.get_value, "Note", "(select (password) from(__Auth) order by name desc limit 1)", {"title": "get_value"}) + + def test_update_doc(self): server = FrappeClient(get_url(), "Administrator", "admin", verify=False) frappe.db.sql("delete from `tabNote` where title in ('Sing','sing')") From 89717b00606d8efbccef6b7ad0cbd540f827e7b2 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Wed, 19 Aug 2020 15:03:17 +0530 Subject: [PATCH 10/11] fix(tests): test_insert_many --- frappe/tests/test_api.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py index 0edc1d4759..3e559ac0f3 100644 --- a/frappe/tests/test_api.py +++ b/frappe/tests/test_api.py @@ -11,7 +11,11 @@ import requests import base64 class TestAPI(unittest.TestCase): - def insert_docs(self): + def test_insert_many(self): + server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + frappe.db.sql("delete from `tabNote` where title in ('Sing','a','song','of','sixpence')") + frappe.db.commit() + server.insert_many([ {"doctype": "Note", "public": True, "title": "Sing"}, {"doctype": "Note", "public": True, "title": "a"}, @@ -20,13 +24,6 @@ class TestAPI(unittest.TestCase): {"doctype": "Note", "public": True, "title": "sixpence"}, ]) - def test_insert_many(self, server): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) - frappe.db.sql("delete from `tabNote` where title in ('Sing','a','song','of','sixpence')") - frappe.db.commit() - - self.insert_docs(server) - self.assertTrue(frappe.db.get_value('Note', {'title': 'Sing'})) self.assertTrue(frappe.db.get_value('Note', {'title': 'a'})) self.assertTrue(frappe.db.get_value('Note', {'title': 'song'})) @@ -44,8 +41,6 @@ class TestAPI(unittest.TestCase): def test_list_docs(self): server = FrappeClient(get_url(), "Administrator", "admin", verify=False) - self.insert_docs(server) - doc_list = server.get_list("Note") self.assertTrue(len(doc_list)) From c72b68d0fb41938bdad915becf3374ab8ce92478 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 19 Aug 2020 11:53:13 +0200 Subject: [PATCH 11/11] fix: remove obsolete argument --- frappe/website/website_generator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/website/website_generator.py b/frappe/website/website_generator.py index c406679e54..fc08abeed9 100644 --- a/frappe/website/website_generator.py +++ b/frappe/website/website_generator.py @@ -154,7 +154,7 @@ class WebsiteGenerator(Document): # Check if the route is changed if old_doc and old_doc.route != self.route: # Remove the route from index if the route has changed - remove_document_from_index("web_routes", old_doc.route) + remove_document_from_index(old_doc.route) def update_website_search_index(self): """ @@ -169,4 +169,4 @@ class WebsiteGenerator(Document): frappe.enqueue(update_index_for_path, path=self.route) elif self.route: # If the website is not published - remove_document_from_index("web_routes", self.route) + remove_document_from_index(self.route)