From 4e533682ba2ee3d822016ded2222bcbe4453a70a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:26:12 +0530 Subject: [PATCH 01/12] feat: get_traceback with context --- frappe/__init__.py | 4 ++-- frappe/utils/__init__.py | 14 ++++++++++---- requirements.txt | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 97e605394b..257d02a09a 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -354,11 +354,11 @@ def cache() -> "RedisWrapper": return redis_server -def get_traceback(): +def get_traceback(with_context=False): """Returns error traceback.""" from frappe.utils import get_traceback - return get_traceback() + return get_traceback(with_context=with_context) def errprint(msg): diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 3e62589664..d0dfe44760 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -18,6 +18,7 @@ from typing import Generator, Iterable from urllib.parse import quote, urlparse from redis.exceptions import ConnectionError +from traceback_with_variables import iter_exc_lines from werkzeug.test import Client import frappe @@ -255,7 +256,7 @@ def get_gravatar(email): return gravatar_url -def get_traceback() -> str: +def get_traceback(with_context=False) -> str: """ Returns the traceback of the Exception """ @@ -264,10 +265,15 @@ def get_traceback() -> str: if not any([exc_type, exc_value, exc_tb]): return "" - trace_list = traceback.format_exception(exc_type, exc_value, exc_tb) - bench_path = get_bench_path() + "/" + if with_context: + trace_list = iter_exc_lines() + tb = "\n".join(trace_list) + else: + trace_list = traceback.format_exception(exc_type, exc_value, exc_tb) + tb = "".join(cstr(t) for t in trace_list) - return "".join(cstr(t) for t in trace_list).replace(bench_path, "") + bench_path = get_bench_path() + "/" + return tb.replace(bench_path, "") def log(event, details): diff --git a/requirements.txt b/requirements.txt index c77ab1d424..495574e05b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -63,6 +63,7 @@ semantic-version~=2.8.5 sqlparse~=0.4.1 stripe~=2.56.0 terminaltables~=3.1.0 +traceback-with-variables~=2.0.4 urllib3~=1.26.4 Werkzeug~=2.0.3 Whoosh~=2.7.4 From 5ecc9fe4ffec55a70fde9f4196ce74c15630f758 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:27:01 +0530 Subject: [PATCH 02/12] refactor(log_error): de-clutter & log context with traceback --- frappe/__init__.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 257d02a09a..4653845c3e 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -2069,7 +2069,6 @@ def logger( def log_error(title=None, message=None, reference_doctype=None, reference_name=None): """Log error to Error Log""" - # Parameter ALERT: # the title and message may be swapped # the better API for this is log_error(title, message), and used in many cases this way @@ -2082,20 +2081,15 @@ def log_error(title=None, message=None, reference_doctype=None, reference_name=N else: traceback = message - if not traceback: - traceback = get_traceback() - - if not title: - title = "Error" + title = title or "Error" + traceback = as_unicode(traceback or get_traceback(with_context=True)) return get_doc( - dict( - doctype="Error Log", - error=as_unicode(traceback), - method=title, - reference_doctype=reference_doctype, - reference_name=reference_name, - ) + doctype="Error Log", + error=traceback, + method=title, + reference_doctype=reference_doctype, + reference_name=reference_name, ).insert(ignore_permissions=True) From c691537e61b5d2d7f0151e4b13a7842d083bdbbb Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:28:49 +0530 Subject: [PATCH 03/12] chore: Add typing for ease of development --- frappe/database/database.py | 2 +- frappe/model/base_document.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index d4677a1295..edc57ae543 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1066,7 +1066,7 @@ class Database(object): now_datetime() - relativedelta(minutes=minutes), )[0][0] - def get_db_table_columns(self, table): + def get_db_table_columns(self, table) -> List[str]: """Returns list of column names from given table.""" columns = frappe.cache().hget("table_columns", table) if columns is None: diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 02eb2ab38c..f8d60d0763 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import datetime import json +from typing import Dict, List import frappe from frappe import _ @@ -252,7 +253,7 @@ class BaseDocument(object): def get_valid_dict( self, sanitize=True, convert_dates_to_str=False, ignore_nulls=False, ignore_virtual=False - ): + ) -> Dict: d = frappe._dict() for fieldname in self.meta.get_valid_columns(): d[fieldname] = self.get(fieldname) @@ -329,7 +330,7 @@ class BaseDocument(object): if key not in self.__dict__: self.__dict__[key] = None - def get_valid_columns(self): + def get_valid_columns(self) -> List[str]: if self.doctype not in frappe.local.valid_columns: if self.doctype in DOCTYPES_FOR_DOCTYPE: from frappe.model.meta import get_table_columns @@ -342,7 +343,7 @@ class BaseDocument(object): return frappe.local.valid_columns[self.doctype] - def is_new(self): + def is_new(self) -> bool: return self.get("__islocal") @property @@ -359,7 +360,7 @@ class BaseDocument(object): no_default_fields=False, convert_dates_to_str=False, no_child_table_fields=False, - ): + ) -> Dict: doc = self.get_valid_dict(convert_dates_to_str=convert_dates_to_str) doc["doctype"] = self.doctype From 418dcdd2f445d39da4fa01bce94f19f973d27d50 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:32:28 +0530 Subject: [PATCH 04/12] fix!: Use event as a differentiator for frappe.utils.log --- frappe/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index d0dfe44760..13e6911634 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -277,7 +277,7 @@ def get_traceback(with_context=False) -> str: def log(event, details): - frappe.logger().info(details) + frappe.logger(event).info(details) def dict_to_str(args, sep="&"): From dfef7192da8e99a0285c6a445806f452f69b2365 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 21 Apr 2022 15:27:59 +0530 Subject: [PATCH 05/12] refactor: remove duplicate code from db.get_descendants (#16699) --- frappe/database/database.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index d4677a1295..978667a7e9 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1146,18 +1146,13 @@ class Database(object): return frappe.db.is_missing_column(e) def get_descendants(self, doctype, name): - """Return descendants of the current record""" - node_location_indexes = self.get_value(doctype, name, ("lft", "rgt")) - if node_location_indexes: - lft, rgt = node_location_indexes - return self.sql_list( - """select name from `tab{doctype}` - where lft > {lft} and rgt < {rgt}""".format( - doctype=doctype, lft=lft, rgt=rgt - ) - ) - else: - # when document does not exist + """Return descendants of the group node in tree""" + from frappe.utils.nestedset import get_descendants_of + + try: + return get_descendants_of(doctype, name, ignore_permissions=True) + except Exception: + # Can only happen if document doesn't exists - kept for backward compatibility return [] def is_missing_table_or_column(self, e): From 9823e51512e3d4870f6bece76c36d69123724d3c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 16:00:24 +0530 Subject: [PATCH 06/12] feat(safe_exec): Allow new_doc, get_last_doc, rename_doc, delte_doc * rename_doc points to the unwhitelisted method which supports ignore_permissions check * Allowed other safe utils for better DX --- frappe/utils/safe_exec.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index aa6fa8b67f..2e7ef5da21 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -15,6 +15,8 @@ import frappe.utils.data from frappe import _ from frappe.frappeclient import FrappeClient from frappe.handler import execute_cmd +from frappe.model.delete_doc import delete_doc +from frappe.model.rename_doc import rename_doc from frappe.modules import scrub from frappe.utils.background_jobs import enqueue, get_jobs from frappe.website.utils import get_next_link, get_shade, get_toc @@ -110,12 +112,15 @@ def get_safe_globals(): errprint=frappe.errprint, qb=frappe.qb, get_meta=frappe.get_meta, + new_doc=frappe.new_doc, get_doc=frappe.get_doc, + get_last_doc=frappe.get_last_doc, get_cached_doc=frappe.get_cached_doc, get_list=frappe.get_list, get_all=frappe.get_all, get_system_settings=frappe.get_system_settings, - rename_doc=frappe.rename_doc, + rename_doc=rename_doc, + delete_doc=delete_doc, utils=datautils, get_url=frappe.utils.get_url, render_template=frappe.render_template, From e565deb88e07d952ec5f410e82c6fa8ccc826e6e Mon Sep 17 00:00:00 2001 From: KrutikaBhatt <65107474+KrutikaBhatt@users.noreply.github.com> Date: Thu, 21 Apr 2022 16:00:56 +0530 Subject: [PATCH 07/12] fix(UI): Duration Filter overlapping issue (#16599) --- frappe/public/scss/common/controls.scss | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/public/scss/common/controls.scss b/frappe/public/scss/common/controls.scss index fcc924650e..9df89f23e1 100644 --- a/frappe/public/scss/common/controls.scss +++ b/frappe/public/scss/common/controls.scss @@ -343,11 +343,10 @@ textarea.form-control { .duration-picker { position: absolute; z-index: 999; - border-radius: var(--border-radius); box-shadow: var(--shadow-sm); background: var(--popover-bg); - + width: max-content; &:after, &:before { border: solid transparent; @@ -466,4 +465,4 @@ button.data-pill { top: 0; right: 0; cursor: pointer; -} \ No newline at end of file +} From b6683db57e477df7949ad2658e060c495e8cd447 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 16:09:08 +0530 Subject: [PATCH 08/12] refactor: frappe.rename_doc definition Use explicit naming of args, kwargs and don't accept cmd and ignore_permissions explicitly --- frappe/__init__.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 97e605394b..4e85eb3afc 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1210,18 +1210,35 @@ def reload_doc(module, dt=None, dn=None, force=False, reset_permissions=False): @whitelist() -def rename_doc(*args, **kwargs): +def rename_doc( + doctype: str, + old: str, + new: str, + force: bool = False, + merge: bool = False, + *, + ignore_if_exists: bool = False, + show_alert: bool = True, + rebuild_search: bool = True, +) -> str: """ Renames a doc(dt, old) to doc(dt, new) and updates all linked fields of type "Link" Calls `frappe.model.rename_doc.rename_doc` """ - kwargs.pop("ignore_permissions", None) - kwargs.pop("cmd", None) from frappe.model.rename_doc import rename_doc - return rename_doc(*args, **kwargs) + return rename_doc( + doctype=doctype, + old=old, + new=new, + force=force, + merge=merge, + ignore_if_exists=ignore_if_exists, + show_alert=show_alert, + rebuild_search=rebuild_search, + ) def get_module(modulename): From ab1f893e41c91843512c1b92dac548eb4dfe08d2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 16:46:49 +0530 Subject: [PATCH 09/12] feat: Add get_mapped_doc in safe_exec under frappe --- frappe/utils/safe_exec.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index 2e7ef5da21..fc53243021 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -16,6 +16,7 @@ from frappe import _ from frappe.frappeclient import FrappeClient from frappe.handler import execute_cmd from frappe.model.delete_doc import delete_doc +from frappe.model.mapper import get_mapped_doc from frappe.model.rename_doc import rename_doc from frappe.modules import scrub from frappe.utils.background_jobs import enqueue, get_jobs @@ -114,6 +115,7 @@ def get_safe_globals(): get_meta=frappe.get_meta, new_doc=frappe.new_doc, get_doc=frappe.get_doc, + get_mapped_doc=get_mapped_doc, get_last_doc=frappe.get_last_doc, get_cached_doc=frappe.get_cached_doc, get_list=frappe.get_list, From 7d9695a0d2aafcc0e15c20d06a64f43d3d96ee54 Mon Sep 17 00:00:00 2001 From: chillaranand Date: Fri, 22 Apr 2022 11:10:54 +0530 Subject: [PATCH 10/12] chore: Bump wrapt to 1.14.0 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c77ab1d424..099e49a0b0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -66,7 +66,7 @@ terminaltables~=3.1.0 urllib3~=1.26.4 Werkzeug~=2.0.3 Whoosh~=2.7.4 -wrapt~=1.12.1 +wrapt~=1.14.0 xlrd~=2.0.1 zxcvbn-python~=4.4.24 tenacity~=8.0.1 From a245cb51a24f044ce9e332c7193c16abfa21d325 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 22 Apr 2022 13:32:31 +0530 Subject: [PATCH 11/12] feat: configurable auto email reports limit (#16684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The previous limit was 3 per user which is way too less, no known reason to restrict this other than hogging of system with too many reports. Bumped default limit to 20. - site config is not easily discoverable or editable, added config in system settings. - Moved auto email report background job form daily queue to `daily_long` queue. closes https://github.com/frappe/frappe/issues/16681 Screenshot 2022-04-20 at 12 33 06 PM `no-docs` (error message is sufficient to explain to user what to do without referring docs) ref: ISS-21-22-10245 ISS-21-22-07742 ISS-20-21-10850 ISS-20-21-10112 and many more times. This is such a stupid validation 🤦 --- .../system_settings/system_settings.json | 16 ++++++++++++++-- .../auto_email_report/auto_email_report.py | 17 +++++++++++------ frappe/hooks.py | 2 +- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 61410fb1a8..0c9b87e618 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -68,6 +68,8 @@ "prepared_report_section", "enable_prepared_report_auto_deletion", "prepared_report_expiry_period", + "column_break_64", + "max_auto_email_report_per_user", "system_updates_section", "disable_system_update_notification" ], @@ -445,7 +447,7 @@ "collapsible": 1, "fieldname": "prepared_report_section", "fieldtype": "Section Break", - "label": "Prepared Report" + "label": "Reports" }, { "default": "Frappe", @@ -485,12 +487,22 @@ "fieldtype": "Select", "label": "First Day of the Week", "options": "Sunday\nMonday\nTuesday\nWednesday\nThursday\nFriday\nSaturday" + }, + { + "fieldname": "column_break_64", + "fieldtype": "Column Break" + }, + { + "default": "20", + "fieldname": "max_auto_email_report_per_user", + "fieldtype": "Int", + "label": "Max auto email report per user" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2022-01-04 11:28:34.881192", + "modified": "2022-04-21 09:11:35.218721", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/email/doctype/auto_email_report/auto_email_report.py b/frappe/email/doctype/auto_email_report/auto_email_report.py index 7a9af6149a..9f897a1308 100644 --- a/frappe/email/doctype/auto_email_report/auto_email_report.py +++ b/frappe/email/doctype/auto_email_report/auto_email_report.py @@ -12,6 +12,7 @@ from frappe.model.document import Document from frappe.model.naming import append_number_if_name_exists from frappe.utils import ( add_to_date, + cint, format_time, get_link_to_form, get_url_to_report, @@ -51,14 +52,18 @@ class AutoEmailReport(Document): self.email_to = "\n".join(valid) def validate_report_count(self): - """check that there are only 3 enabled reports per user""" - count = frappe.db.sql( - "select count(*) from `tabAuto Email Report` where user=%s and enabled=1", self.user - )[0][0] - max_reports_per_user = frappe.local.conf.max_reports_per_user or 3 + count = frappe.db.count("Auto Email Report", {"user": self.user, "enabled": 1}) + + max_reports_per_user = ( + cint(frappe.local.conf.max_reports_per_user) # kept for backward compatibilty + or cint(frappe.db.get_single_value("System Settings", "max_auto_email_report_per_user")) + or 20 + ) if count > max_reports_per_user + (-1 if self.flags.in_insert else 0): - frappe.throw(_("Only {0} emailed reports are allowed per user").format(max_reports_per_user)) + msg = _("Only {0} emailed reports are allowed per user.").format(max_reports_per_user) + msg += " " + _("To allow more reports update limit in System Settings.") + frappe.throw(msg, title=_("Report limit reached")) def validate_report_format(self): """check if user has select correct report format""" diff --git a/frappe/hooks.py b/frappe/hooks.py index d3de3877ba..f7a67dc7ec 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -226,7 +226,6 @@ scheduler_events = { "frappe.sessions.clear_expired_sessions", "frappe.email.doctype.notification.notification.trigger_daily_alerts", "frappe.utils.scheduler.restrict_scheduler_events_if_dormant", - "frappe.email.doctype.auto_email_report.auto_email_report.send_daily", "frappe.website.doctype.personal_data_deletion_request.personal_data_deletion_request.remove_unverified_record", "frappe.desk.form.document_follow.send_daily_updates", "frappe.social.doctype.energy_point_settings.energy_point_settings.allocate_review_points", @@ -241,6 +240,7 @@ scheduler_events = { "frappe.integrations.doctype.dropbox_settings.dropbox_settings.take_backups_daily", "frappe.utils.change_log.check_for_update", "frappe.integrations.doctype.s3_backup_settings.s3_backup_settings.take_backups_daily", + "frappe.email.doctype.auto_email_report.auto_email_report.send_daily", "frappe.integrations.doctype.google_drive.google_drive.daily_backup", ], "weekly_long": [ From e1db9bf653eb63b56ba1814fb0f584b1cf2de858 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Fri, 22 Apr 2022 15:37:00 +0530 Subject: [PATCH 12/12] fix: Grid layout broken for some grids (#16702) --- frappe/public/js/frappe/form/grid_row.js | 36 +++++++++++++++--------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 4bba8ae7ad..a1c3dce91f 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -289,19 +289,23 @@ export default class GridRow { var me = this; if(this.doc && !this.grid.df.in_place_edit) { // remove row - if(!this.open_form_button) { - this.open_form_button = $(` -
- ${frappe.utils.icon('edit', 'xs')} - -
- `) - .appendTo($('
').appendTo(this.row)) - .on('click', function() { - me.toggle_view(); return false; - }); + if (!this.open_form_button) { + this.open_form_button = $('
').appendTo(this.row); - if(this.is_too_small()) { + if (!this.configure_columns) { + this.open_form_button = $(` +
+ ${frappe.utils.icon('edit', 'xs')} + +
+ `) + .appendTo(this.open_form_button) + .on('click', function() { + me.toggle_view(); return false; + }); + } + + if (this.is_too_small()) { // narrow this.open_form_button.css({'margin-right': '-2px'}); } @@ -310,7 +314,9 @@ export default class GridRow { } add_column_configure_button() { - if (this.configure_columns) { + if (this.grid.df.in_place_edit && !this.frm) return; + + if (this.configure_columns && this.frm) { this.configure_columns_button = $(`
${frappe.utils.icon('setting-gear', 'sm', '', 'filter: opacity(0.5)')} @@ -320,6 +326,10 @@ export default class GridRow { .on('click', () => { this.configure_dialog_for_columns_selector(); }); + } else if (this.configure_columns && !this.frm) { + this.configure_columns_button = $(` +
+ `).appendTo(this.row); } }