From 0919234b6653433d7d73ae02a263a58dbcee4238 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Fri, 16 Nov 2018 14:30:05 +0530 Subject: [PATCH 01/17] fix(prepared-report): Serialize Prepared Report data to JSON CSV serialization causes information loss for erpnext reports like P&L Statement, rendering such reports unusable. Also stored files are gzip compressed to reduce space requirement Note: This will cause all previously generated reports to break, this should be fixed --- .../prepared_report/prepared_report.py | 60 ++++--------------- frappe/desk/query_report.py | 9 ++- 2 files changed, 19 insertions(+), 50 deletions(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index a877e5f871..df4f577766 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -5,14 +5,14 @@ from __future__ import unicode_literals import base64 +import gzip import json import frappe from frappe.model.document import Document from frappe.utils.background_jobs import enqueue -from frappe.desk.query_report import generate_report_result, get_columns_dict +from frappe.desk.query_report import generate_report_result from frappe.utils.file_manager import save_file, remove_all -from frappe.utils.csvutils import to_csv, read_csv_content_from_attached_file from frappe.desk.form.load import get_attachments from frappe.utils.file_manager import download_file @@ -36,7 +36,7 @@ class PreparedReport(Document): def run_background(instance): report = frappe.get_doc("Report", instance.ref_report_doctype) result = generate_report_result(report, filters=json.loads(instance.filters), user=instance.owner) - create_csv_file(result['columns'], result['result'], 'Prepared Report', instance.name) + create_json_gz_file(result['result'], 'Prepared Report', instance.name) instance.status = "Completed" instance.columns = json.dumps(result["columns"]) @@ -50,59 +50,23 @@ def run_background(instance): ) -def remove_header_meta(columns): - column_list = [] - columns_header = get_columns_dict(columns) - for idx in range(len(columns)): - column_list.append(columns_header[idx]['label']) - return column_list +def create_json_gz_file(data, dt, dn): + # Storing data in CSV file causes information loss + # Reports like P&L Statement were completely unsuable because of this + json_filename = '{0}.json.gz'.format(frappe.utils.data.format_datetime(frappe.utils.now(), "Y-m-d-H:M")) + encoded_content = frappe.safe_encode(json.dumps(data)) - -def create_csv_file(columns, data, dt, dn): - csv_filename = '{0}.csv'.format(frappe.utils.data.format_datetime(frappe.utils.now(), "Y-m-d-H:M")) - - rows = [] - - if data: - columns_without_meta = remove_header_meta(columns) - - row = data[0] - if type(row) == list: - rows = [tuple(columns_without_meta)] + data - else: - for row in data: - new_row = [] - for col in columns: - key = col.get('fieldname') or col.get('label') - new_row.append(row.get(key, '')) - rows.append(new_row) - - rows = [tuple(columns_without_meta)] + rows - - encoded = base64.b64encode(frappe.safe_encode(to_csv(rows))) - # Call save_file function to upload and attach the file + # GZip compression seems to reduce storage requirements by 80-90% + compressed_content = gzip.compress(encoded_content) save_file( - fname=csv_filename, - content=encoded, + fname=json_filename, + content=compressed_content, dt=dt, dn=dn, folder=None, - decode=True, is_private=False) -@frappe.whitelist() -def get_report_attachment_data(dn): - - doc = frappe.get_doc("Prepared Report", dn) - data = read_csv_content_from_attached_file(doc) - - return { - 'columns': data[0], - 'result': data[1:] - } - - @frappe.whitelist() def download_attachment(dn): attachment = get_attachments("Prepared Report", dn)[0] diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index c5fda2ca66..3c89343af1 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -5,6 +5,7 @@ from __future__ import unicode_literals import frappe import os, json, datetime +import gzip from frappe import _ from frappe.modules import scrub, get_module_path @@ -12,10 +13,10 @@ from frappe.utils import flt, cint, get_html_format, cstr, get_url_to_form from frappe.model.utils import render_include from frappe.translate import send_translations import frappe.desk.reportview -from frappe.utils.csvutils import read_csv_content_from_attached_file from frappe.permissions import get_role_permissions from six import string_types, iteritems from datetime import timedelta +from frappe.utils.file_manager import get_file def get_report_doc(report_name): doc = frappe.get_doc("Report", report_name) @@ -187,7 +188,11 @@ def get_prepared_report_result(report, filters, dn=""): # Get latest doc = frappe.get_doc("Prepared Report", doc_list[0]) - data = read_csv_content_from_attached_file(doc) + # Prepared Report data is stored in a GZip compressed JSON file + attached_file_name = frappe.db.get_value("File", {"attached_to_doctype": doc.doctype, "attached_to_name":doc.name}, "name") + compressed_content = get_file(attached_file_name)[1] + uncompressed_content = gzip.decompress(compressed_content) + data = json.loads(uncompressed_content) if data: latest_report_data = { "columns": json.loads(doc.columns) if doc.columns else data[0], From d76a0136dd815cfdf68be9bb97d51fc381a56c7e Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Sat, 1 Dec 2018 00:25:43 +0530 Subject: [PATCH 02/17] refactor(prepared-report): Do not serialize JSON serialized filters Prepared report filters were being JSON serialized twice. There isn't any apparent need to do this. So don't do it. --- frappe/core/doctype/prepared_report/prepared_report.js | 2 +- frappe/core/doctype/prepared_report/prepared_report.py | 2 +- frappe/desk/query_report.py | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.js b/frappe/core/doctype/prepared_report/prepared_report.js index 002e069874..6a7cf2728c 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.js +++ b/frappe/core/doctype/prepared_report/prepared_report.js @@ -15,7 +15,7 @@ frappe.ui.form.on('Prepared Report', { `); - const filters = JSON.parse(JSON.parse(frm.doc.filters)); + const filters = JSON.parse(frm.doc.filters); Object.keys(filters).forEach(key => { const filter_row = $(` diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index df4f577766..887a766372 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -35,7 +35,7 @@ class PreparedReport(Document): def run_background(instance): report = frappe.get_doc("Report", instance.ref_report_doctype) - result = generate_report_result(report, filters=json.loads(instance.filters), user=instance.owner) + result = generate_report_result(report, filters=instance.filters, user=instance.owner) create_json_gz_file(result['result'], 'Prepared Report', instance.name) instance.status = "Completed" diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 3c89343af1..2e5d077cd1 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -100,7 +100,9 @@ def background_enqueue_run(report_name, filters=None, user=None): frappe.get_doc({ "doctype": "Prepared Report", "report_name": report_name, - "filters": json.dumps(filters), + # This looks like an insanity but, without this it'd be very hard to find Prepared Reports matvhing given condition + # We're ensuring that spacing is consistent. e.g. JS seems to put no spaces after ":", Python on the other hand does. + "filters": json.dumps(json.loads(filters)), "ref_report_doctype": report_name, "report_type": report.report_type, "query": report.query, From 9ced1a2886ea101f980f0ceff2d4c58a5e64c8ff Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Sat, 1 Dec 2018 00:29:40 +0530 Subject: [PATCH 03/17] fix(prepared-report): Show data from report matching filters and owner This fixes permission issues since users can only see the report that they themselves have generated. Also report filters and data won't mismatch. --- frappe/desk/query_report.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 2e5d077cd1..1f65240934 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -169,7 +169,7 @@ def run(report_name, filters=None, user=None): dn = filters.get("prepared_report_name") else: dn = "" - result = get_prepared_report_result(report, filters, dn) + result = get_prepared_report_result(report, filters, dn, user) else: result = generate_report_result(report, filters, user) @@ -178,9 +178,10 @@ def run(report_name, filters=None, user=None): return result -def get_prepared_report_result(report, filters, dn=""): +def get_prepared_report_result(report, filters, dn="", user=None): latest_report_data = {} - doc_list = frappe.get_all("Prepared Report", filters={"status": "Completed", "report_name": report.name}) + # Only look for completed prepared reports with given filters. + doc_list = frappe.get_all("Prepared Report", filters={"status": "Completed", "report_name": report.name, "filters": json.dumps(filters), "owner": user}) doc = None if len(doc_list): if dn: From fe657b2f730e4d271b9f520a4410e67569007a84 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Fri, 30 Nov 2018 18:05:46 +0530 Subject: [PATCH 04/17] fix(prepared-report): Delete all prepared reports Changes made to Prepared Report aren't backwards compatible. Let's get rid of existing prepared reports and files associated with them. --- frappe/patches.txt | 3 ++- frappe/patches/v11_0/delete_all_prepared_reports.py | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 frappe/patches/v11_0/delete_all_prepared_reports.py diff --git a/frappe/patches.txt b/frappe/patches.txt index be104092c8..d8ed1fd82b 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -230,4 +230,5 @@ frappe.patches.v10_0.modify_naming_series_table frappe.patches.v10_0.enhance_security frappe.patches.v11_0.multiple_references_in_events frappe.patches.v11_0.set_allow_self_approval_in_workflow -frappe.patches.v11_0.migrate_report_settings_for_new_listview \ No newline at end of file +frappe.patches.v11_0.migrate_report_settings_for_new_listview +frappe.patches.v11_0.delete_all_prepared_reports \ No newline at end of file diff --git a/frappe/patches/v11_0/delete_all_prepared_reports.py b/frappe/patches/v11_0/delete_all_prepared_reports.py new file mode 100644 index 0000000000..ee4b1dbd08 --- /dev/null +++ b/frappe/patches/v11_0/delete_all_prepared_reports.py @@ -0,0 +1,6 @@ +import frappe + +def execute(): + prepared_reports = frappe.get_all("Prepared Report") + for report in prepared_reports: + frappe.delete_doc("Prepared Report", report.name) From fc48597694cc80a24eef8c0b27674620c8e21f41 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Sat, 1 Dec 2018 00:59:23 +0530 Subject: [PATCH 05/17] fix(prepared-report): Python 2 compatibility fix gzip.compress and gzip.decompress were introduced in Python 3.2 Added gzip_compress and gzip_decompress to frappe.utils to add this functionality to both Python 2 and 3 --- .../prepared_report/prepared_report.py | 5 ++--- frappe/desk/query_report.py | 4 ++-- frappe/utils/__init__.py | 20 +++++++++++++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 887a766372..0f24e2fc99 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -5,7 +5,6 @@ from __future__ import unicode_literals import base64 -import gzip import json import frappe @@ -15,7 +14,7 @@ from frappe.desk.query_report import generate_report_result from frappe.utils.file_manager import save_file, remove_all from frappe.desk.form.load import get_attachments from frappe.utils.file_manager import download_file - +from frappe.utils import gzip_compress class PreparedReport(Document): @@ -57,7 +56,7 @@ def create_json_gz_file(data, dt, dn): encoded_content = frappe.safe_encode(json.dumps(data)) # GZip compression seems to reduce storage requirements by 80-90% - compressed_content = gzip.compress(encoded_content) + compressed_content = gzip_compress(encoded_content) save_file( fname=json_filename, content=compressed_content, diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 1f65240934..8bf6aa3a3d 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -5,7 +5,6 @@ from __future__ import unicode_literals import frappe import os, json, datetime -import gzip from frappe import _ from frappe.modules import scrub, get_module_path @@ -17,6 +16,7 @@ from frappe.permissions import get_role_permissions from six import string_types, iteritems from datetime import timedelta from frappe.utils.file_manager import get_file +from frappe.utils import gzip_decompress def get_report_doc(report_name): doc = frappe.get_doc("Report", report_name) @@ -194,7 +194,7 @@ def get_prepared_report_result(report, filters, dn="", user=None): # Prepared Report data is stored in a GZip compressed JSON file attached_file_name = frappe.db.get_value("File", {"attached_to_doctype": doc.doctype, "attached_to_name":doc.name}, "name") compressed_content = get_file(attached_file_name)[1] - uncompressed_content = gzip.decompress(compressed_content) + uncompressed_content = gzip_decompress(compressed_content) data = json.loads(uncompressed_content) if data: latest_report_data = { diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 403353ee59..b3b46696bb 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -14,6 +14,8 @@ from email.utils import parseaddr, formataddr from frappe.utils.data import * from six.moves.urllib.parse import quote from six import text_type, string_types +import io +from gzip import GzipFile default_fields = ['doctype', 'name', 'owner', 'creation', 'modified', 'modified_by', 'parent', 'parentfield', 'parenttype', 'idx', 'docstatus'] @@ -620,3 +622,21 @@ def call(fn, *args, **kwargs): bench --site erpnext.local execute frappe.utils.call --args '''["frappe.get_all", "Activity Log"]''' --kwargs '''{"fields": ["user", "creation", "full_name"], "filters":{"Operation": "Login", "Status": "Success"}, "limit": "10"}''' """ return json.loads(frappe.as_json(frappe.call(fn, *args, **kwargs))) + +# Following methods are aken as-is from Python 3 codebase +# since gzip.compress and gzip.decompress are not available in Python 2.7 +def gzip_compress(data, compresslevel=9): + """Compress data in one shot and return the compressed string. + Optional argument is the compression level, in range of 0-9. + """ + buf = io.BytesIO() + with GzipFile(fileobj=buf, mode='wb', compresslevel=compresslevel) as f: + f.write(data) + return buf.getvalue() + +def gzip_decompress(data): + """Decompress a gzip compressed string in one shot. + Return the decompressed string. + """ + with GzipFile(fileobj=io.BytesIO(data)) as f: + return f.read() From 0467719466b09c2d1069d2d3231e02b05095ba87 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 15:16:31 +0530 Subject: [PATCH 06/17] fix(prepared-report): Disable filters when showing a Prepared Report doc --- frappe/desk/query_report.py | 1 + .../js/frappe/views/reports/query_report.js | 39 +++++++++++++++---- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 8bf6aa3a3d..14953a7bdc 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -167,6 +167,7 @@ def run(report_name, filters=None, user=None): filters = json.loads(filters) dn = filters.get("prepared_report_name") + filters.pop("prepared_report_name", None) else: dn = "" result = get_prepared_report_result(report, filters, dn, user) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 6c3e699d85..ffe03197ff 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -268,7 +268,21 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { if (data.prepared_report){ this.prepared_report = true; - this.add_prepared_report_buttons(data.doc); + var query_string = frappe.utils.get_query_string(frappe.get_route_str()) + var query_params = frappe.utils.get_query_params(query_string) + // If query_string contains prepared_report_name then set filters + // to match the mentioned prepared report doc and disable editing + if(query_params.prepared_report_name) { + this.render_prepared_report_doc = true + var filters_from_report = JSON.parse(data.doc.filters); + Object.values(this.filters).forEach(function(field) { + if (filters_from_report[field.fieldname]) { + field.set_input(filters_from_report[field.fieldname]); + } + field.input.disabled = true; + }); + } + this.add_prepared_report_buttons(data.doc, ); } this.toggle_message(false); @@ -304,12 +318,23 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { `)); }; - // if - - this.page.set_primary_action( - __("Generate New Report"), - this.generate_background_report.bind(this) - ); + // if query_string has prepared_report_name then change primary action to create + // a new prepared report + if(this.render_prepared_report_doc) { + this.page.set_primary_action( + __("New"), + () => { + this.render_prepared_report_doc = false + frappe.set_route(frappe.get_route()) + this.add_prepared_report_buttons() + } + ); + } else { + this.page.set_primary_action( + __("Rebuild"), + this.generate_background_report.bind(this) + ); + } } generate_background_report() { From 013f6ceffcb21f968d84ca3057df0072dd216737 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 15:57:24 +0530 Subject: [PATCH 07/17] fix(prepared-report): Uncompress Prepared Report data files before downloading --- frappe/core/doctype/prepared_report/prepared_report.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 0f24e2fc99..720a8d8682 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -13,8 +13,8 @@ from frappe.utils.background_jobs import enqueue from frappe.desk.query_report import generate_report_result from frappe.utils.file_manager import save_file, remove_all from frappe.desk.form.load import get_attachments -from frappe.utils.file_manager import download_file -from frappe.utils import gzip_compress +from frappe.utils.file_manager import get_file +from frappe.utils import gzip_compress, gzip_decompress class PreparedReport(Document): @@ -69,4 +69,6 @@ def create_json_gz_file(data, dt, dn): @frappe.whitelist() def download_attachment(dn): attachment = get_attachments("Prepared Report", dn)[0] - download_file(attachment.file_url) + frappe.local.response.filename = attachment.file_name[:-2] + frappe.local.response.filecontent = gzip_decompress(get_file(attachment.name)[1]) + frappe.local.response.type = "binary" From 49e5d4e21faa361af61456b962960b39816f5d34 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 15:58:50 +0530 Subject: [PATCH 08/17] fix(prepared-report): Set prepared_report_name query param in Link to query report Since visiting the query report URL will show most recently generated report that matches given filters, Explicitly mention which Prepared Report is going to be rendered. --- frappe/core/doctype/prepared_report/prepared_report.py | 2 +- frappe/public/js/frappe/views/reports/query_report.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 720a8d8682..ab055fd15e 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -44,7 +44,7 @@ def run_background(instance): frappe.publish_realtime( 'report_generated', - {"report_name": instance.report_name}, + {"report_name": instance.report_name, "name": instance.name}, user=frappe.session.user ) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index ffe03197ff..f2346542b6 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -64,7 +64,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { frappe.realtime.on("report_generated", (data) => { if(data.report_name) { let alert_message = `Report ${this.report_name} generated. - View`; + View`; frappe.show_alert({message: alert_message, indicator: 'orange'}); } }); From dbf9f15f1a9febee3f6d9c3b7c42ab85cded225e Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 16:03:22 +0530 Subject: [PATCH 09/17] fix(prepared-report): Fix typo in a comment --- 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 14953a7bdc..df23208913 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -100,7 +100,7 @@ def background_enqueue_run(report_name, filters=None, user=None): frappe.get_doc({ "doctype": "Prepared Report", "report_name": report_name, - # This looks like an insanity but, without this it'd be very hard to find Prepared Reports matvhing given condition + # This looks like an insanity but, without this it'd be very hard to find Prepared Reports matching given condition # We're ensuring that spacing is consistent. e.g. JS seems to put no spaces after ":", Python on the other hand does. "filters": json.dumps(json.loads(filters)), "ref_report_doctype": report_name, From 304a563c2548bf73cca183315dac50954050aa37 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 6 Dec 2018 17:22:57 +0530 Subject: [PATCH 10/17] style: Linting fixes --- .../js/frappe/views/reports/query_report.js | 18 +++++++++--------- 1 file changed, 9 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 f2346542b6..f483c521db 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -266,15 +266,15 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.hide_status(); - if (data.prepared_report){ + if (data.prepared_report) { this.prepared_report = true; - var query_string = frappe.utils.get_query_string(frappe.get_route_str()) - var query_params = frappe.utils.get_query_params(query_string) + const query_string = frappe.utils.get_query_string(frappe.get_route_str()); + const query_params = frappe.utils.get_query_params(query_string); // If query_string contains prepared_report_name then set filters // to match the mentioned prepared report doc and disable editing if(query_params.prepared_report_name) { - this.render_prepared_report_doc = true - var filters_from_report = JSON.parse(data.doc.filters); + this.render_prepared_report_doc = true; + const filters_from_report = JSON.parse(data.doc.filters); Object.values(this.filters).forEach(function(field) { if (filters_from_report[field.fieldname]) { field.set_input(filters_from_report[field.fieldname]); @@ -282,7 +282,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { field.input.disabled = true; }); } - this.add_prepared_report_buttons(data.doc, ); + this.add_prepared_report_buttons(data.doc); } this.toggle_message(false); @@ -324,9 +324,9 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.page.set_primary_action( __("New"), () => { - this.render_prepared_report_doc = false - frappe.set_route(frappe.get_route()) - this.add_prepared_report_buttons() + this.render_prepared_report_doc = false; + frappe.set_route(frappe.get_route()); + this.add_prepared_report_buttons(); } ); } else { From 7660ac8f98b588aedb37b155031c205138b7ad06 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 6 Dec 2018 17:24:54 +0530 Subject: [PATCH 11/17] fix: Use frappe.as_json instead of json.dumps --- frappe/core/doctype/prepared_report/prepared_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index ab055fd15e..64c26f27d1 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -53,7 +53,7 @@ def create_json_gz_file(data, dt, dn): # Storing data in CSV file causes information loss # Reports like P&L Statement were completely unsuable because of this json_filename = '{0}.json.gz'.format(frappe.utils.data.format_datetime(frappe.utils.now(), "Y-m-d-H:M")) - encoded_content = frappe.safe_encode(json.dumps(data)) + encoded_content = frappe.safe_encode(frappe.as_json(data)) # GZip compression seems to reduce storage requirements by 80-90% compressed_content = gzip_compress(encoded_content) From 28e36b201d1fc019513fd9a9114de9d3d059e8d0 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 17:24:31 +0530 Subject: [PATCH 12/17] fix(prepared-report): Do not lose the first row in Prepared Report data --- 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 df23208913..79744ae135 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -200,7 +200,7 @@ def get_prepared_report_result(report, filters, dn="", user=None): if data: latest_report_data = { "columns": json.loads(doc.columns) if doc.columns else data[0], - "result": data[1:] + "result": data } latest_report_data.update({ From 521d5a982cf4a2c746072fc9b41ac6e1b05027c9 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 19:48:01 +0530 Subject: [PATCH 13/17] fix(prepared-report): UX Improvements - Update primary action according to the Report's status - Automatically refresh report view if report generation is finished - Correctly translate message strings Co-authored-by: Faris Ansari --- frappe/desk/query_report.py | 1 + .../js/frappe/views/reports/query_report.js | 63 ++++++++++++++----- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 79744ae135..2a511e83cd 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -111,6 +111,7 @@ def background_enqueue_run(report_name, filters=None, user=None): track_instance.insert(ignore_permissions=True) frappe.db.commit() return { + "name": track_instance.name, "redirect_url": get_url_to_form("Prepared Report", track_instance.name) } diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index f483c521db..44bb57c99e 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -63,9 +63,17 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { setup_events() { frappe.realtime.on("report_generated", (data) => { if(data.report_name) { - let alert_message = `Report ${this.report_name} generated. - View`; - frappe.show_alert({message: alert_message, indicator: 'orange'}); + this.prepared_report_action = "Rebuild"; + // If generated report and currently active Prepared Report has same fiters + // then refresh the Prepared Report + // Otherwise show alert with the link to the Prepared Report + if(data.name == this.prepared_report_doc_name) { + this.refresh(); + } else { + let alert_message = `Report ${this.report_name} generated. + View`; + frappe.show_alert({message: alert_message, indicator: 'orange'}); + } } }); } @@ -93,6 +101,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.page_title = __(this.report_name); this.menu_items = this.get_menu_items(); this.datatable = null; + this.prepared_report_action = "New"; frappe.run_serially([ () => this.get_report_doc(), @@ -273,7 +282,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { // If query_string contains prepared_report_name then set filters // to match the mentioned prepared report doc and disable editing if(query_params.prepared_report_name) { - this.render_prepared_report_doc = true; + this.prepared_report_action = "Edit"; const filters_from_report = JSON.parse(data.doc.filters); Object.values(this.filters).forEach(function(field) { if (filters_from_report[field.fieldname]) { @@ -311,25 +320,38 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { +"dn="+encodeURIComponent(doc.name))); }); - this.show_status(__(` - This report was generated - on ${frappe.datetime.convert_to_user_tz(doc.report_end_time)}. - See all past reports. - `)); + const part1 = __('This report was generated {0}.', [frappe.datetime.comment_when(doc.report_end_time)]); + const part2 = __('To get the updated report, click on {0}.', [__('Rebuild')]); + const part3 = __('See all past reports.'); + + this.show_status(` + + ${part1} + ${part2} + ${part3} + + `); }; - // if query_string has prepared_report_name then change primary action to create - // a new prepared report - if(this.render_prepared_report_doc) { + // Three cases + // 1. First time with given filters, no data. + // 2. Showing data from specific report + // 3. Showing data from an old report without specific report name + if(this.prepared_report_action == "New") { this.page.set_primary_action( - __("New"), + __("Generate New Report"), () => { - this.render_prepared_report_doc = false; - frappe.set_route(frappe.get_route()); - this.add_prepared_report_buttons(); + this.generate_background_report(); } ); - } else { + } else if(this.prepared_report_action == "Edit") { + this.page.set_primary_action( + __("Edit"), + () => { + frappe.set_route(frappe.get_route()); + } + ); + } else if(this.prepared_report_action == "Rebuild"){ this.page.set_primary_action( __("Rebuild"), this.generate_background_report.bind(this) @@ -352,6 +374,8 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { callback: resolve })).then(r => { const data = r.message; + // Rememeber the name of Prepared Report doc + this.prepared_report_doc_name = data.name let alert_message = `Report initiated. You can track its status here`; frappe.show_alert({message: alert_message, indicator: 'orange'}); @@ -923,6 +947,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { reset_report_view() { this.hide_status(); this.toggle_nothing_to_show(true); + this.refresh(); } toggle_loading(flag) { @@ -937,6 +962,10 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { Please set the appropriate filters and then generate a new one.`); } this.toggle_message(flag, message); + if(flag){ + this.prepared_report_action = "New"; + } + this.add_prepared_report_buttons(); } toggle_message(flag, message) { From 8006fe4e83d9e8ec101a157ae58693215f7563e7 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Thu, 6 Dec 2018 21:20:46 +0530 Subject: [PATCH 14/17] style: Linting fixes --- frappe/public/js/frappe/views/reports/query_report.js | 2 +- 1 file changed, 1 insertion(+), 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 44bb57c99e..4fc6d4da69 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -375,7 +375,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { })).then(r => { const data = r.message; // Rememeber the name of Prepared Report doc - this.prepared_report_doc_name = data.name + this.prepared_report_doc_name = data.name; let alert_message = `Report initiated. You can track its status here`; frappe.show_alert({message: alert_message, indicator: 'orange'}); From a4e30abac2a88a0e8b3761791daa282fe7b91151 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 7 Dec 2018 15:30:18 +0530 Subject: [PATCH 15/17] fix: Update Datatable to 1.6.1 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 3003271177..eed4c72e05 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "awesomplete": "^1.1.2", "cookie": "^0.3.1", "express": "^4.16.2", - "frappe-datatable": "^1.6.0", + "frappe-datatable": "^1.6.1", "frappe-gantt": "^0.1.0", "fuse.js": "^3.2.0", "highlight.js": "^9.12.0", diff --git a/yarn.lock b/yarn.lock index d8190b4b90..3475604959 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1219,10 +1219,10 @@ forwarded@~0.1.2: resolved "https://registry.yarnpkg.com/forwarded/-/forwarded-0.1.2.tgz#98c23dab1175657b8c0573e8ceccd91b0ff18c84" integrity sha1-mMI9qxF1ZXuMBXPozszZGw/xjIQ= -frappe-datatable@^1.6.0: - version "1.6.0" - resolved "https://registry.yarnpkg.com/frappe-datatable/-/frappe-datatable-1.6.0.tgz#c86c2c7fc054a500c70cdf6c6362b5fed63c6fe6" - integrity sha512-40iwguZr0w+X0MV/yTzsYDoKlH7b/GuOq6o2lEOrbVT3p4dZYpalxQmO8mkM9gKjNjSgGMVL1r6Se8peq80Qqg== +frappe-datatable@^1.6.1: + version "1.6.1" + resolved "https://registry.yarnpkg.com/frappe-datatable/-/frappe-datatable-1.6.1.tgz#e57850923b5f307fd02328c522c5abe35a17dd89" + integrity sha512-u2l4I2Pwu4jTLSBF7vW7EDwzcRdfrlKbgaUCyhDUYlOhWl0sRt6rO2ZmIhU7znSYz7TNkKkAt7hkI+x+Mg6ONw== dependencies: hyperlist "^1.0.0-beta" lodash "^4.17.5" From 5d3c7391452ba103f4e675372c421533fb398cfc Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 7 Dec 2018 15:55:43 +0530 Subject: [PATCH 16/17] fix(Quill): Use pre tag instead of div (#6585) --- frappe/public/js/frappe/form/controls/text_editor.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/public/js/frappe/form/controls/text_editor.js b/frappe/public/js/frappe/form/controls/text_editor.js index 41de01e6f7..9e27dcf0f8 100644 --- a/frappe/public/js/frappe/form/controls/text_editor.js +++ b/frappe/public/js/frappe/form/controls/text_editor.js @@ -5,6 +5,10 @@ const Block = Quill.import('blots/block'); Block.tagName = 'DIV'; Quill.register(Block, true); +const CodeBlockContainer = Quill.import('formats/code-block-container'); +CodeBlockContainer.tagName = 'PRE'; +Quill.register(CodeBlockContainer, true); + // table const Table = Quill.import('formats/table-container'); const superCreate = Table.create.bind(Table); From dc2cc5c177d75f66041299b722c0e8d2ea931416 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Fri, 7 Dec 2018 17:53:24 +0600 Subject: [PATCH 17/17] bumped to version 11.0.3-beta.34 --- frappe/hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/hooks.py b/frappe/hooks.py index 8a81c3d35a..1a55e20736 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -12,7 +12,7 @@ source_link = "https://github.com/frappe/frappe" app_license = "MIT" develop_version = '12.x.x-develop' -staging_version = '11.0.3-beta.33' +staging_version = '11.0.3-beta.34' app_email = "info@frappe.io"