From 94b2d856fcfcb3a2fc27064efd821020d530bfd5 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Thu, 19 Mar 2020 21:44:45 +0530 Subject: [PATCH 01/52] feat(ldap): allow resetting ldap password from user settings currently, there is no way to reset password for those logging in through ldap. i understand that this shouldn't really be handled by erpnext, but there are people that have requested resetting the ldap password from with the instance itself, and hence, we'll let that happen now. Signed-off-by: Chinmay D. Pai --- frappe/core/doctype/user/user.js | 42 +++++++++++++++++++ .../doctype/ldap_settings/ldap_settings.py | 42 +++++++++++++++++-- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index c4873ee40e..46332e1893 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -97,6 +97,48 @@ frappe.ui.form.on('User', { }); }, __("Password")); + frappe.db.get_single_value("LDAP Settings", "enabled").then((value) => { + if (value === 1 && frm.name != "Administrator") { + frm.add_custom_button(__("Reset LDAP Password"), function(){ + const d = new frappe.ui.Dialog({ + title: __("Reset LDAP Password"), + fields: [ + { + label: __("New Password"), + fieldtype: "Password", + fieldname: "new_password", + reqd: 1 + }, + { + label: __("Confirm New Password"), + fieldtype: "Password", + fieldname: "confirm_password", + reqd: 1 + }, + { + label: __("Logout All Sessions"), + fieldtype: "Check", + fieldname: "logout_sessions" + } + ], + primary_action: (values) => { + d.hide(); + if(values.new_password !== values.confirm_password) { + frappe.throw(__("Passwords do not match!")); + } + frappe.call( + "frappe.integrations.doctype.ldap_settings.ldap_settings.reset_password", { + user: frm.doc.email, + password: values.new_password, + logout: values.logout_sessions + }).then(() => done()); + } + }); + d.show(); + }, __("Password")); + } + }); + frm.add_custom_button(__("Reset OTP Secret"), function() { frappe.call({ method: "frappe.core.doctype.user.user.reset_otp_secret", diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py index c0f12df04a..63e0b8ff55 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py @@ -4,7 +4,7 @@ from __future__ import unicode_literals import frappe -from frappe import _ +from frappe import _, safe_encode from frappe.model.document import Document @@ -19,7 +19,7 @@ class LDAPSettings(Document): else: frappe.throw(_("LDAP Search String needs to end with a placeholder, eg sAMAccountName={0}")) - def connect_to_ldap(self, base_dn, password): + def connect_to_ldap(self, base_dn, password, read_only=True): try: import ldap3 import ssl @@ -44,7 +44,7 @@ class LDAPSettings(Document): user=base_dn, password=password, auto_bind=bind_type, - read_only=True, + read_only=read_only, raise_exceptions=True) return conn @@ -170,6 +170,34 @@ class LDAPSettings(Document): else: frappe.throw(_("Invalid username or password")) + def reset_password(self, user, password, logout_sessions=False): + from ldap3 import HASHED_SALTED_SHA, MODIFY_REPLACE + from ldap3.utils.hashed import hashed + + search_filter = "({0}={1})".format(self.ldap_email_field, user) + + conn = self.connect_to_ldap(self.base_dn, self.get_password(raise_exception=False), + read_only=False) + + if conn.search( + search_base=self.organizational_unit, + search_filter=search_filter, + attributes=self.get_ldap_attributes() + ): + if conn.entries and conn.entries[0]: + entry_dn = conn.entries[0].entry_dn + hashed_password = hashed(HASHED_SALTED_SHA, safe_encode(password)) + changes = {'userPassword': [(MODIFY_REPLACE, [hashed_password])]} + if conn.modify(entry_dn, changes=changes): + if logout_sessions: + from frappe.sessions import clear_sessions + clear_sessions(user=user, force=True) + frappe.msgprint(_("Password changed successfully.")) + else: + frappe.throw(_("Failed to change password.")) + else: + frappe.throw(_("LDAP User does not exist!")) + def convert_ldap_entry_to_dict(self, user_entry): # support multiple email values @@ -211,3 +239,11 @@ def login(): # because of a GET request! frappe.db.commit() + + +@frappe.whitelist() +def reset_password(user, password, logout): + ldap = frappe.get_doc("LDAP Settings") + if not ldap.enabled: + frappe.throw(_("LDAP is not enabled.")) + ldap.reset_password(user, password, logout_sessions=int(logout)) From bc77455d132704f8838ebfa50aee9ff01026661b Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Thu, 19 Mar 2020 21:49:58 +0530 Subject: [PATCH 02/52] chore: correct indentation for ldap exceptions Signed-off-by: Chinmay D. Pai --- frappe/integrations/doctype/ldap_settings/ldap_settings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py index 63e0b8ff55..f9dce6800c 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py @@ -196,7 +196,9 @@ class LDAPSettings(Document): else: frappe.throw(_("Failed to change password.")) else: - frappe.throw(_("LDAP User does not exist!")) + frappe.throw(_("No Entry for the User {0} found within LDAP!").format(user)) + else: + frappe.throw(_("LDAP User does not exist!")) def convert_ldap_entry_to_dict(self, user_entry): From 5fbf1f8c08249a3c6d724b9832efa5f6264af31a Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Thu, 19 Mar 2020 21:53:39 +0530 Subject: [PATCH 03/52] chore: make the exception read slightly better Signed-off-by: Chinmay D. Pai --- frappe/integrations/doctype/ldap_settings/ldap_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py index f9dce6800c..558f7117c0 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py @@ -198,7 +198,7 @@ class LDAPSettings(Document): else: frappe.throw(_("No Entry for the User {0} found within LDAP!").format(user)) else: - frappe.throw(_("LDAP User does not exist!")) + frappe.throw(_("No LDAP User found for email: {0}").format(user)) def convert_ldap_entry_to_dict(self, user_entry): From 3b8d76e5a7dc6798c67afef38d38970c489bf602 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Thu, 19 Mar 2020 22:02:02 +0530 Subject: [PATCH 04/52] chore: check the correct variable for docname Signed-off-by: Chinmay D. Pai --- frappe/core/doctype/user/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index 46332e1893..0f474c85f2 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -98,7 +98,7 @@ frappe.ui.form.on('User', { }, __("Password")); frappe.db.get_single_value("LDAP Settings", "enabled").then((value) => { - if (value === 1 && frm.name != "Administrator") { + if (value === 1 && frm.doc.name != "Administrator") { frm.add_custom_button(__("Reset LDAP Password"), function(){ const d = new frappe.ui.Dialog({ title: __("Reset LDAP Password"), From 15801f870880e9b5a7457fb42a988db0325f7da8 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Thu, 19 Mar 2020 22:15:01 +0530 Subject: [PATCH 05/52] chore: codacy fixes Signed-off-by: Chinmay D. Pai --- frappe/core/doctype/user/user.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index 0f474c85f2..de4dc0c272 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -99,7 +99,7 @@ frappe.ui.form.on('User', { frappe.db.get_single_value("LDAP Settings", "enabled").then((value) => { if (value === 1 && frm.doc.name != "Administrator") { - frm.add_custom_button(__("Reset LDAP Password"), function(){ + frm.add_custom_button(__("Reset LDAP Password"), function() { const d = new frappe.ui.Dialog({ title: __("Reset LDAP Password"), fields: [ @@ -123,7 +123,7 @@ frappe.ui.form.on('User', { ], primary_action: (values) => { d.hide(); - if(values.new_password !== values.confirm_password) { + if (values.new_password !== values.confirm_password) { frappe.throw(__("Passwords do not match!")); } frappe.call( From 7b7d55860477225e85bfa315eede31980a831274 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 20 Mar 2020 18:10:54 +0530 Subject: [PATCH 06/52] feat: add validate_phone api to window to validate phone numbers --- frappe/public/js/frappe/utils/datatype.js | 4 ++++ frappe/public/js/frappe/utils/utils.js | 3 +++ 2 files changed, 7 insertions(+) diff --git a/frappe/public/js/frappe/utils/datatype.js b/frappe/public/js/frappe/utils/datatype.js index 0f73526e04..16f87b872f 100644 --- a/frappe/public/js/frappe/utils/datatype.js +++ b/frappe/public/js/frappe/utils/datatype.js @@ -44,6 +44,10 @@ window.validate_email = function(txt) { return frappe.utils.validate_type(txt, "email"); }; +window.validate_phone = function(txt) { + return frappe.utils.validate_type(txt, "phone"); +}; + window.nth = function(number) { number = cint(number); var s = 'th'; diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 278c80897e..1af37c1f60 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -232,6 +232,9 @@ Object.assign(frappe.utils, { var regExp; switch ( type ) { + case "phone": + regExp = /^([0-9\ \+\_\-\,\.\*\#\(\)]){1,20}$/; + break; case "number": regExp = /^-?(?:\d+|\d{1,3}(?:,\d{3})+)?(?:\.\d+)?$/; break; From 80c4167a0783132cf9ad55a75fec56652f817730 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 23 Mar 2020 18:10:54 +0530 Subject: [PATCH 07/52] feat: server validations for data field options --- frappe/core/doctype/doctype/doctype.py | 14 +++++++++++++- frappe/exceptions.py | 1 + frappe/model/__init__.py | 1 + frappe/model/meta.py | 3 +++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index da1b184cc1..43b375e999 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -15,7 +15,7 @@ import frappe import frappe.website.render from frappe import _ from frappe.utils import now, cint -from frappe.model import no_value_fields, default_fields, data_fieldtypes, table_fields +from frappe.model import no_value_fields, default_fields, data_fieldtypes, table_fields, data_field_options from frappe.model.document import Document from frappe.custom.doctype.property_setter.property_setter import make_property_setter from frappe.custom.doctype.custom_field.custom_field import create_custom_field @@ -942,6 +942,17 @@ def validate_fields(meta): if hasattr(field, 'fetch_from') and getattr(field, 'fetch_from'): field.fetch_from = field.fetch_from.strip('\n').strip() + + def validate_data_field_type(docfield): + if docfield.fieldtype == "Data": + if docfield.options and (docfield.options not in data_field_options): + docfield_label = frappe.bold(docfield.label) + data_field_str = "
  • " + "
  • ".join(data_field_options) + "
" + text = "{0} is an Invalid Data field.{1} Only Options allowed for Data field are: {2}" + message = _(text).format(docfield_label, "

", data_field_str) + frappe.msgprint(message, raise_exception=True) + + fields = meta.get("fields") fieldname_list = [d.fieldname for d in fields] @@ -972,6 +983,7 @@ def validate_fields(meta): check_table_multiselect_option(d) scrub_options_in_select(d) scrub_fetch_from(d) + validate_data_field_type(d) check_fold(fields) check_search_fields(meta, fields) diff --git a/frappe/exceptions.py b/frappe/exceptions.py index 3d63f4b2b4..732fc39e9a 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -78,6 +78,7 @@ class TimestampMismatchError(ValidationError): pass class EmptyTableError(ValidationError): pass class LinkExistsError(ValidationError): pass class InvalidEmailAddressError(ValidationError): pass +class InvalidPhoneNumberError(ValidationError): pass class TemplateNotFoundError(ValidationError): pass class UniqueValidationError(ValidationError): pass class AppNotInstalledError(ValidationError): pass diff --git a/frappe/model/__init__.py b/frappe/model/__init__.py index 1fe92d7a67..7af987f4bc 100644 --- a/frappe/model/__init__.py +++ b/frappe/model/__init__.py @@ -48,6 +48,7 @@ table_fields = ('Table', 'Table MultiSelect') core_doctypes_list = ('DocType', 'DocField', 'DocPerm', 'DocType Action', 'DocType Link', 'User', 'Role', 'Has Role', 'Page', 'Module Def', 'Print Format', 'Report', 'Customize Form', 'Customize Form Field', 'Property Setter', 'Custom Field', 'Custom Script') +data_field_options = ('Email', 'Phone') def copytables(srctype, src, srcfield, tartype, tar, tarfield, srcfields, tarfields=[]): if not tarfields: diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 1938a4a96c..47db0829a1 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -128,6 +128,9 @@ class Meta(Document): def get_link_fields(self): return self.get("fields", {"fieldtype": "Link", "options":["!=", "[Select]"]}) + def get_data_fields(self): + return self.get("fields", {"fieldtype": "Data"}) + def get_dynamic_link_fields(self): if not hasattr(self, '_dynamic_link_fields'): self._dynamic_link_fields = self.get("fields", {"fieldtype": "Dynamic Link"}) From d199d8027a86a3926a81f0d083c4f21da709f74a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 24 Mar 2020 08:10:54 +0530 Subject: [PATCH 08/52] feat: added server validations for phone and email for fieldtype "Data" and options set to "Email" or "Phone", updating documents will trigger data validations for email and phone numbers will be made --- frappe/model/base_document.py | 12 ++++++++++++ frappe/model/document.py | 2 ++ frappe/utils/__init__.py | 17 +++++++++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 569cea9d5f..17eb4164a6 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -544,6 +544,18 @@ class BaseDocument(object): frappe.throw(_('{0} {1} cannot be "{2}". It should be one of "{3}"').format(prefix, label, value, comma_options)) + def _validate_data_fields(self): + # data_field options defined in frappe.model.data_field_options + for data_field in self.meta.get_data_fields(): + data = self.get(data_field.fieldname) + + if data_field.options == "Email": + for email_address in frappe.utils.split_emails(data): + frappe.utils.validate_email_address(email_address, throw=True) + + if data_field.options == "Phone": + frappe.utils.validate_phone_number(data, throw=True) + def _validate_constants(self): if frappe.flags.in_import or self.is_new() or self.flags.ignore_validate_constants: return diff --git a/frappe/model/document.py b/frappe/model/document.py index 41f946efd9..38407851ff 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -468,6 +468,7 @@ class Document(BaseDocument): def _validate(self): self._validate_mandatory() + self._validate_data_fields() self._validate_selects() self._validate_length() self._extract_images_from_text_editor() @@ -477,6 +478,7 @@ class Document(BaseDocument): children = self.get_all_children() for d in children: + d._validate_data_fields() d._validate_selects() d._validate_length() d._extract_images_from_text_editor() diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 82e6ea1b45..ccaa47432f 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -3,7 +3,7 @@ # util __init__.py -from __future__ import unicode_literals, print_function +from __future__ import unicode_literals, print_function, annotations from werkzeug.test import Client import os, re, sys, json, hashlib, requests, traceback from .html_utils import sanitize_html @@ -81,6 +81,19 @@ def validate_email_add(email_str, throw=False): """ return validate_email_address(email_str, throw=False) +def validate_phone_number(phone_number: str, throw: bool = False): + """Returns True if valid phone number""" + if not phone_number: + return False + + phone_number = phone_number.strip() + match = re.match("([0-9\ \+\_\-\,\.\*\#\(\)]){1,20}$", phone_number) + + if not match and throw: + frappe.throw(frappe._("{0} is not a valid Phone Number").format(phone_number), frappe.InvalidPhoneNumberError) + + return bool(match) + def validate_email_address(email_str, throw=False): """Validates the email string""" email = email_str = (email_str or "").strip() @@ -691,4 +704,4 @@ def get_html_for_route(route): set_request(method='GET', path=route) response = render.render() html = frappe.safe_decode(response.get_data()) - return html \ No newline at end of file + return html From 2575006816c7420567eca0641108f9c2c23a1188 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 25 Mar 2020 15:53:43 +0530 Subject: [PATCH 09/52] feat: validations for Email and Phone Data options added set_invalid control for docfields. On event of docfield.invalid is set, in a given form, set_invalid will be executed which changes the textbox colour to red --- .../js/frappe/form/controls/base_control.js | 1 + .../js/frappe/form/controls/base_input.js | 3 ++ frappe/public/js/frappe/form/controls/data.js | 45 ++++--------------- 3 files changed, 13 insertions(+), 36 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/base_control.js b/frappe/public/js/frappe/form/controls/base_control.js index 2adb5435e3..c1ba41ab16 100644 --- a/frappe/public/js/frappe/form/controls/base_control.js +++ b/frappe/public/js/frappe/form/controls/base_control.js @@ -152,6 +152,7 @@ frappe.ui.form.Control = Class.extend({ () => me.set_model_value(value), () => { me.set_mandatory && me.set_mandatory(value); + me.set_invalid && me.set_invalid(); if(me.df.change || me.df.onchange) { // onchange event specified in df diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index 8a8ac271c7..0dbaaeb63c 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -179,6 +179,9 @@ frappe.ui.form.ControlInput = frappe.ui.form.Control.extend({ set_mandatory: function(value) { this.$wrapper.toggleClass("has-error", (this.df.reqd && is_null(value)) ? true : false); }, + set_invalid: function () { + this.$wrapper.toggleClass("has-error", (this.df.invalid ? true : false)); + }, set_bold: function() { if(this.$input) { this.$input.toggleClass("bold", !!(this.df.bold || this.df.reqd)); diff --git a/frappe/public/js/frappe/form/controls/data.js b/frappe/public/js/frappe/form/controls/data.js index 6dc8c3d387..41dee09115 100644 --- a/frappe/public/js/frappe/form/controls/data.js +++ b/frappe/public/js/frappe/form/controls/data.js @@ -87,56 +87,29 @@ frappe.ui.form.ControlData = frappe.ui.form.ControlInput.extend({ return val==null ? "" : val; }, validate: function(v) { + if (!v){ + return ''; + } if(this.df.is_filter) { return v; } if(this.df.options == 'Phone') { - if(v+''=='') { - return ''; - } - var v1 = ''; - // phone may start with + and must only have numbers later, '-' and ' ' are stripped - v = v.replace(/ /g, '').replace(/-/g, '').replace(/\(/g, '').replace(/\)/g, ''); - - // allow initial +,0,00 - if(v && v.substr(0,1)=='+') { - v1 = '+'; v = v.substr(1); - } - if(v && v.substr(0,2)=='00') { - v1 += '00'; v = v.substr(2); - } - if(v && v.substr(0,1)=='0') { - v1 += '0'; v = v.substr(1); - } - v1 += cint(v) + ''; - return v1; + this.df.invalid = !validate_phone(v) + return v; } else if(this.df.options == 'Email') { - if(v+''=='') { - return ''; - } - var email_list = frappe.utils.split_emails(v); if (!email_list) { - // invalid email return ''; } else { - var invalid_email = false; + let emph = false; email_list.forEach(function(email) { if (!validate_email(email)) { - frappe.msgprint(__("Invalid Email: {0}", [email])); - invalid_email = true; + emph = emph || true; } }); - - if (invalid_email) { - // at least 1 invalid email - return ''; - } else { - // all good - return v; - } + this.df.invalid = emph; + return v; } - } else { return v; } From c199a3555b4c18e9e7db834d16ecabc8dd5fc475 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 25 Mar 2020 15:59:01 +0530 Subject: [PATCH 10/52] chore: remove deprecated validate_email_add function --- frappe/utils/__init__.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index ccaa47432f..61d2cf9d82 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -75,12 +75,6 @@ def extract_email_id(email): email_id = email_id.decode("utf-8", "ignore") return email_id -def validate_email_add(email_str, throw=False): - """ - validate_email_add will be renamed to the validate_email_address in v12 - """ - return validate_email_address(email_str, throw=False) - def validate_phone_number(phone_number: str, throw: bool = False): """Returns True if valid phone number""" if not phone_number: From dd0d21f04a0c4d2418bc6cd37e9df8e3e1877179 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 25 Mar 2020 16:02:01 +0530 Subject: [PATCH 11/52] style: rename variable emph => email_invalid --- frappe/public/js/frappe/form/controls/data.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/data.js b/frappe/public/js/frappe/form/controls/data.js index 41dee09115..516cd87a1a 100644 --- a/frappe/public/js/frappe/form/controls/data.js +++ b/frappe/public/js/frappe/form/controls/data.js @@ -101,13 +101,13 @@ frappe.ui.form.ControlData = frappe.ui.form.ControlInput.extend({ if (!email_list) { return ''; } else { - let emph = false; + let email_invalid = false; email_list.forEach(function(email) { if (!validate_email(email)) { - emph = emph || true; + email_invalid = true; } }); - this.df.invalid = emph; + this.df.invalid = email_invalid; return v; } } else { From f7111e20bec5d75b7d24b0ae36ff2502a7da3610 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 26 Mar 2020 12:10:54 +0530 Subject: [PATCH 12/52] fix: Improper ValidationError string returning empty string if parseaddr returns None shows " is not a valid Email Address" eg: if 'email@' is passed as value, email.utils.parseaddr will return a (None, None) tuple --- frappe/utils/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 61d2cf9d82..5d0e9ab24b 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -105,15 +105,15 @@ def validate_email_address(email_str, throw=False): _valid = False else: - e = extract_email_id(e) - match = re.match("[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", e.lower()) if e else None + email_id = extract_email_id(e) + match = re.match("[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", email_id.lower()) if email_id else None if not match: _valid = False else: matched = match.group(0) if match: - match = matched==e.lower() + match = matched==email_id.lower() if not _valid: if throw: From 2c5bc5abcf67cfb8c75346bf3e31f20366703f78 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 25 Mar 2020 17:22:12 +0530 Subject: [PATCH 13/52] fix: remove type annotations --- frappe/utils/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 5d0e9ab24b..649d3bf72c 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -3,7 +3,7 @@ # util __init__.py -from __future__ import unicode_literals, print_function, annotations +from __future__ import unicode_literals, print_function from werkzeug.test import Client import os, re, sys, json, hashlib, requests, traceback from .html_utils import sanitize_html @@ -75,7 +75,7 @@ def extract_email_id(email): email_id = email_id.decode("utf-8", "ignore") return email_id -def validate_phone_number(phone_number: str, throw: bool = False): +def validate_phone_number(phone_number, throw=False): """Returns True if valid phone number""" if not phone_number: return False From 5627981192cb8a2553bcd6a5b7af00089011f34b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 25 Mar 2020 19:50:48 +0530 Subject: [PATCH 14/52] fix: safe check of options in docfield --- frappe/model/base_document.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 17eb4164a6..466824d1d2 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -548,12 +548,13 @@ class BaseDocument(object): # data_field options defined in frappe.model.data_field_options for data_field in self.meta.get_data_fields(): data = self.get(data_field.fieldname) + data_field_options = data_field.get("options") - if data_field.options == "Email": + if data_field_options == "Email": for email_address in frappe.utils.split_emails(data): frappe.utils.validate_email_address(email_address, throw=True) - if data_field.options == "Phone": + if data_field_options == "Phone": frappe.utils.validate_phone_number(data, throw=True) def _validate_constants(self): From 1bfb1a3d37daaab255aab1b6c8d9e6ec2b578eea Mon Sep 17 00:00:00 2001 From: prssanna Date: Thu, 26 Mar 2020 14:50:57 +0530 Subject: [PATCH 15/52] refactor(Dashboard): refactor dashboard permissions --- frappe/core/page/dashboard/dashboard.json | 2 +- frappe/desk/doctype/dashboard/dashboard.json | 23 +++++++++- .../dashboard_chart/dashboard_chart.json | 32 ++++++++++++- .../dashboard_chart/dashboard_chart.py | 45 +++++++++++++++++++ frappe/hooks.py | 2 + frappe/permissions.py | 2 +- 6 files changed, 102 insertions(+), 4 deletions(-) diff --git a/frappe/core/page/dashboard/dashboard.json b/frappe/core/page/dashboard/dashboard.json index 891dcb26f8..58fda5a34c 100644 --- a/frappe/core/page/dashboard/dashboard.json +++ b/frappe/core/page/dashboard/dashboard.json @@ -4,7 +4,7 @@ "docstatus": 0, "doctype": "Page", "idx": 0, - "modified": "2019-01-08 19:19:48.073410", + "modified": "2020-03-26 13:30:44.603948", "modified_by": "Administrator", "module": "Core", "name": "dashboard", diff --git a/frappe/desk/doctype/dashboard/dashboard.json b/frappe/desk/doctype/dashboard/dashboard.json index 239f35bea8..c177ee70ac 100644 --- a/frappe/desk/doctype/dashboard/dashboard.json +++ b/frappe/desk/doctype/dashboard/dashboard.json @@ -34,7 +34,7 @@ } ], "links": [], - "modified": "2020-01-26 20:00:10.069817", + "modified": "2020-03-25 21:09:37.080132", "modified_by": "Administrator", "module": "Desk", "name": "Dashboard", @@ -51,6 +51,27 @@ "role": "System Manager", "share": 1, "write": 1 + }, + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Dashboard Manager", + "share": 1, + "write": 1 + }, + { + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "All", + "share": 1 } ], "quick_entry": 1, diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.json b/frappe/desk/doctype/dashboard_chart/dashboard_chart.json index 0a017a0de2..8c14ea130d 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.json +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.json @@ -215,7 +215,7 @@ } ], "links": [], - "modified": "2020-03-13 19:19:37.162771", + "modified": "2020-03-26 13:41:11.126000", "modified_by": "Administrator", "module": "Desk", "name": "Dashboard Chart", @@ -232,6 +232,36 @@ "role": "System Manager", "share": 1, "write": 1 + }, + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Dashboard Manager", + "share": 1, + "write": 1 + }, + { + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Dashboard User", + "share": 1 + }, + { + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "All", + "share": 1 } ], "sort_field": "modified", diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index f01c976b9c..634b33cd97 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -10,8 +10,53 @@ import json from frappe.core.page.dashboard.dashboard import cache_source, get_from_date_from_timespan from frappe.utils import nowdate, add_to_date, getdate, get_last_day, formatdate, get_datetime from frappe.model.naming import append_number_if_name_exists +from frappe.boot import get_allowed_reports from frappe.model.document import Document + +def get_permission_query_conditions(user): + + if not user: + user = frappe.session.user + + if user == 'Administrator': + return + + roles = frappe.get_roles(user) + if "System Manager" in roles or "Dashboard Manager" in roles or "Dashboard User" in roles: + return None + + allowed_doctypes = tuple(frappe.permissions.get_doctypes_with_read()) + allowed_reports = tuple([key.encode('UTF8') for key in get_allowed_reports()]) + + return ''' + `tabDashboard Chart`.`chart_type` = 'Custom' + or `tabDashboard Chart`.`document_type` in {allowed_doctypes} + or `tabDashboard Chart`.`report_name` in {allowed_reports} + '''.format( + allowed_doctypes=allowed_doctypes, + allowed_reports=allowed_reports + ) + + +def has_permission(doc, ptype, user): + roles = frappe.get_roles(user) + if "System Manager" in roles or "Dashboard Manager" in roles or "Dashboard User" in roles: + return True + + if doc.chart_type == 'Custom': + return True + elif doc.chart_type == 'Report': + allowed_reports = tuple([key.encode('UTF8') for key in get_allowed_reports()]) + if doc.report_name in allowed_reports: + return True + else: + allowed_doctypes = tuple(frappe.permissions.get_doctypes_with_read()) + if doc.document_type in allowed_doctypes: + return True + + return False + @frappe.whitelist() @cache_source def get(chart_name = None, chart = None, no_cache = None, filters = None, from_date = None, diff --git a/frappe/hooks.py b/frappe/hooks.py index c44c05fdf4..733cec7a08 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -87,6 +87,7 @@ permission_query_conditions = { "ToDo": "frappe.desk.doctype.todo.todo.get_permission_query_conditions", "User": "frappe.core.doctype.user.user.get_permission_query_conditions", "Notification Log": "frappe.desk.doctype.notification_log.notification_log.get_permission_query_conditions", + "Dashboard Chart": "frappe.desk.doctype.dashboard_chart.dashboard_chart.get_permission_query_conditions", "Notification Settings": "frappe.desk.doctype.notification_settings.notification_settings.get_permission_query_conditions", "Note": "frappe.desk.doctype.note.note.get_permission_query_conditions", "Kanban Board": "frappe.desk.doctype.kanban_board.kanban_board.get_permission_query_conditions", @@ -101,6 +102,7 @@ has_permission = { "ToDo": "frappe.desk.doctype.todo.todo.has_permission", "User": "frappe.core.doctype.user.user.has_permission", "Note": "frappe.desk.doctype.note.note.has_permission", + "Dashboard Chart": "frappe.desk.doctype.dashboard_chart.dashboard_chart.has_permission", "Kanban Board": "frappe.desk.doctype.kanban_board.kanban_board.has_permission", "Contact": "frappe.contacts.address_and_contact.has_permission", "Address": "frappe.contacts.address_and_contact.has_permission", diff --git a/frappe/permissions.py b/frappe/permissions.py index a0d1677fac..9835724ce0 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -307,7 +307,7 @@ def has_controller_permissions(doc, ptype, user=None): return None def get_doctypes_with_read(): - return list(set([p.parent for p in get_valid_perms()])) + return list(set([p.parent.encode('UTF8') for p in get_valid_perms()])) def get_valid_perms(doctype=None, user=None): '''Get valid permissions for the current user from DocPerm and Custom DocPerm''' From 99f0c082b73970ac7b73e7cc1491919b84bb267d Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 24 Mar 2020 19:03:01 +0530 Subject: [PATCH 16/52] fix: export custom columns in excel --- frappe/desk/query_report.py | 15 +++++++++++---- .../js/frappe/views/reports/query_report.js | 13 +++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index d210af02fd..5597de9bbe 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -42,7 +42,7 @@ def get_report_doc(report_name): return doc -def generate_report_result(report, filters=None, user=None): +def generate_report_result(report, custom_columns, filters=None, user=None): status = None if not user: user = frappe.session.user @@ -80,6 +80,11 @@ def generate_report_result(report, filters=None, user=None): if report.custom_columns: columns = json.loads(report.custom_columns) result = add_data_to_custom_columns(columns, result) + elif custom_columns: + result = add_data_to_custom_columns(custom_columns, result) + + for custom_column in custom_columns: + columns.insert(custom_column['insert_after_index'] + 1, custom_column) if result: result = get_filtered_data(report.ref_doctype, columns, result, user) @@ -161,7 +166,7 @@ def get_script(report_name): @frappe.whitelist() @frappe.read_only() -def run(report_name, filters=None, user=None, ignore_prepared_report=False): +def run(report_name, filters=None, user=None, ignore_prepared_report=False, custom_columns=None): report = get_report_doc(report_name) if not user: @@ -183,7 +188,7 @@ def run(report_name, filters=None, user=None, ignore_prepared_report=False): dn = "" result = get_prepared_report_result(report, filters, dn, user) else: - result = generate_report_result(report, filters, user) + result = generate_report_result(report, custom_columns, filters, user) result["add_total_row"] = report.add_total_row and not result.get('skip_total_row', False) @@ -294,6 +299,8 @@ def export_query(): if isinstance(data.get("file_format_type"), string_types): file_format_type = data["file_format_type"] + custom_columns = frappe.parse_json(data["custom_columns"]) + include_indentation = data["include_indentation"] if isinstance(data.get("visible_idx"), string_types): visible_idx = json.loads(data.get("visible_idx")) @@ -301,7 +308,7 @@ def export_query(): visible_idx = None if file_format_type == "Excel": - data = run(report_name, filters) + data = run(report_name, filters, custom_columns=custom_columns) data = frappe._dict(data) if not data.columns: frappe.respond_as_web_page(_("No data to export"), diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 1d7065e70d..85d628adf9 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -616,6 +616,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { prepare_report_data(data) { this.raw_data = data; this.columns = this.prepare_columns(data.columns); + this.custom_columns = []; this.data = this.prepare_data(data.result); this.linked_doctypes = this.get_linked_doctypes(); this.tree_report = this.data.some(d => 'indent' in d); @@ -1110,6 +1111,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { const args = { cmd: 'frappe.desk.query_report.export_query', report_name: this.report_name, + custom_columns: this.custom_columns.length? this.custom_columns: [], file_format_type: file_format, filters: filters, visible_idx, @@ -1275,16 +1277,20 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { primary_action: (values) => { const custom_columns = []; let df = frappe.meta.get_docfield(values.doctype, values.field); + const insert_after_index = this.columns + .findIndex(column => column.label === values.insert_after) custom_columns.push({ fieldname: df.fieldname, fieldtype: df.fieldtype, label: df.label, + insert_after_index: insert_after_index, link_field: this.doctype_field_map[values.doctype], doctype: values.doctype, options: df.fieldtype === "Link" ? df.options : undefined, width: 100 }); + this.custom_columns = this.custom_columns.concat(custom_columns); frappe.call({ method: 'frappe.desk.query_report.get_data_for_custom_field', args: { @@ -1294,7 +1300,8 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { callback: (r) => { const custom_data = r.message; const link_field = this.doctype_field_map[values.doctype]; - this.add_custom_column(custom_columns, custom_data, link_field, values.field, values.insert_after); + + this.add_custom_column(custom_columns, custom_data, link_field, values.field, insert_after_index); d.hide(); } }); @@ -1369,11 +1376,9 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { } } - add_custom_column(custom_column, custom_data, link_field, column_field, insert_after) { + add_custom_column(custom_column, custom_data, link_field, column_field, insert_after_index) { const column = this.prepare_columns(custom_column); - const insert_after_index = this.columns - .findIndex(column => column.label === insert_after); this.columns.splice(insert_after_index + 1, 0, column[0]); this.data.forEach(row => { From 5651fa2874f188afbf3ddb362671f9caf8c81d4a Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 24 Mar 2020 19:09:44 +0530 Subject: [PATCH 17/52] fix: fix argument order --- frappe/desk/query_report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 5597de9bbe..994f816c8a 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -42,7 +42,7 @@ def get_report_doc(report_name): return doc -def generate_report_result(report, custom_columns, filters=None, user=None): +def generate_report_result(report, filters=None, user=None, custom_columns=None): status = None if not user: user = frappe.session.user @@ -188,7 +188,7 @@ def run(report_name, filters=None, user=None, ignore_prepared_report=False, cust dn = "" result = get_prepared_report_result(report, filters, dn, user) else: - result = generate_report_result(report, custom_columns, filters, user) + result = generate_report_result(report, filters, user, custom_columns) result["add_total_row"] = report.add_total_row and not result.get('skip_total_row', False) From 5deb86f46c272447f483ac3f368ea03fa3e54c4b Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 24 Mar 2020 19:55:28 +0530 Subject: [PATCH 18/52] fix: codacy --- 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 85d628adf9..2276bc07d6 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -1278,7 +1278,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { const custom_columns = []; let df = frappe.meta.get_docfield(values.doctype, values.field); const insert_after_index = this.columns - .findIndex(column => column.label === values.insert_after) + .findIndex(column => column.label === values.insert_after); custom_columns.push({ fieldname: df.fieldname, fieldtype: df.fieldtype, From 708248baf9ef71cf953f5f0026d9372abeadf9e4 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Fri, 27 Mar 2020 16:04:11 +0530 Subject: [PATCH 19/52] fix: avoid sqli inside global web search Signed-off-by: Chinmay D. Pai --- frappe/utils/global_search.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/frappe/utils/global_search.py b/frappe/utils/global_search.py index 4b50745a74..5abb29bab3 100644 --- a/frappe/utils/global_search.py +++ b/frappe/utils/global_search.py @@ -499,22 +499,29 @@ def web_search(text, scope=None, start=0, limit=20): common_query = ''' SELECT `doctype`, `name`, `content`, `title`, `route` FROM `__global_search` WHERE {conditions} - LIMIT {limit} OFFSET {start}''' + LIMIT %(limit)s OFFSET %(start)s}''' - scope_condition = '`route` like "{}%" AND '.format(scope) if scope else '' + scope_condition = '`route` like "%(scope)s" AND ' if scope else '' published_condition = '`published` = 1 AND ' mariadb_conditions = postgres_conditions = ' '.join([published_condition, scope_condition]) # https://mariadb.com/kb/en/library/full-text-index-overview/#in-boolean-mode text = '"{}"'.format(text) - mariadb_conditions += 'MATCH(`content`) AGAINST ({} IN BOOLEAN MODE)'.format(frappe.db.escape(text)) - postgres_conditions += 'TO_TSVECTOR("content") @@ PLAINTO_TSQUERY({})'.format(frappe.db.escape(text)) + mariadb_conditions += 'MATCH(`content`) AGAINST (%(text)s IN BOOLEAN MODE)' + postgres_conditions += 'TO_TSVECTOR("content") @@ PLAINTO_TSQUERY(%(text)s)' + + values = { + "scope": "".join([scope, "%"]) if scope else '', + "limit": limit, + "start": start, + "text": frappe.db.escape(text) + } result = frappe.db.multisql({ - 'mariadb': common_query.format(conditions=mariadb_conditions, limit=limit, start=start), - 'postgres': common_query.format(conditions=postgres_conditions, limit=limit, start=start) - }, as_dict=True) - tmp_result=[] + 'mariadb': common_query.format(conditions=mariadb_conditions), + 'postgres': common_query.format(conditions=postgres_conditions) + }, values=values, as_dict=True) + tmp_result = [] for i in result: if i in results or not results: tmp_result.append(i) From 063def052e4b2bcdded9b36541e328f270399d48 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Fri, 27 Mar 2020 18:54:18 +0530 Subject: [PATCH 20/52] chore: remove trailing closing bracket Signed-off-by: Chinmay D. Pai --- frappe/utils/global_search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/global_search.py b/frappe/utils/global_search.py index 5abb29bab3..a5fcca8bb8 100644 --- a/frappe/utils/global_search.py +++ b/frappe/utils/global_search.py @@ -499,7 +499,7 @@ def web_search(text, scope=None, start=0, limit=20): common_query = ''' SELECT `doctype`, `name`, `content`, `title`, `route` FROM `__global_search` WHERE {conditions} - LIMIT %(limit)s OFFSET %(start)s}''' + LIMIT %(limit)s OFFSET %(start)s''' scope_condition = '`route` like "%(scope)s" AND ' if scope else '' published_condition = '`published` = 1 AND ' From e57a5d347dab403ab7225985c5211df6ef211216 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 26 Mar 2020 02:17:54 +0530 Subject: [PATCH 21/52] chore: update .eslintrc globals style: updated data.js spacing format --- .eslintrc | 1 + frappe/public/js/frappe/form/controls/data.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.eslintrc b/.eslintrc index 7e469f7672..e79571f556 100644 --- a/.eslintrc +++ b/.eslintrc @@ -78,6 +78,7 @@ "has_common": true, "has_words": true, "validate_email": true, + "validate_phone": true, "get_number_format": true, "format_number": true, "format_currency": true, diff --git a/frappe/public/js/frappe/form/controls/data.js b/frappe/public/js/frappe/form/controls/data.js index 516cd87a1a..512c6fbcb0 100644 --- a/frappe/public/js/frappe/form/controls/data.js +++ b/frappe/public/js/frappe/form/controls/data.js @@ -87,7 +87,7 @@ frappe.ui.form.ControlData = frappe.ui.form.ControlInput.extend({ return val==null ? "" : val; }, validate: function(v) { - if (!v){ + if (!v) { return ''; } if(this.df.is_filter) { From 11aefbd8ed4461973d926b3d665c4706aad8c548 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 27 Mar 2020 06:27:34 +0530 Subject: [PATCH 22/52] fix: dont validate email if owner and email_data is in STANDARD_USERS --- frappe/model/base_document.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 466824d1d2..0880c56a22 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -7,6 +7,7 @@ from six import iteritems, string_types import frappe import datetime from frappe import _ +from frappe.core.doctype.user.user import STANDARD_USERS from frappe.model import default_fields, table_fields from frappe.model.naming import set_new_name from frappe.model.utils.link_count import notify_link_count @@ -551,6 +552,8 @@ class BaseDocument(object): data_field_options = data_field.get("options") if data_field_options == "Email": + if (self.owner in STANDARD_USERS) and (data in STANDARD_USERS): + return for email_address in frappe.utils.split_emails(data): frappe.utils.validate_email_address(email_address, throw=True) From cf9440439e758e4b7a1697a224d3a6ad936ea1a1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 28 Mar 2020 08:07:14 +0530 Subject: [PATCH 23/52] fix: translatable string for data field options --- frappe/core/doctype/doctype/doctype.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 43b375e999..d5b293be68 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -946,11 +946,11 @@ def validate_fields(meta): def validate_data_field_type(docfield): if docfield.fieldtype == "Data": if docfield.options and (docfield.options not in data_field_options): - docfield_label = frappe.bold(docfield.label) - data_field_str = "
  • " + "
  • ".join(data_field_options) + "
" - text = "{0} is an Invalid Data field.{1} Only Options allowed for Data field are: {2}" - message = _(text).format(docfield_label, "

", data_field_str) - frappe.msgprint(message, raise_exception=True) + df_str = frappe.bold(_(docfield.label)) + text_str = _("{0} is an invalid Data field.").format(df_str) + "
" * 2 + _("Only Options allowed for Data field are:") + "
" + df_options_str = "
  • " + "
  • ".join([_(x) for x in data_field_options]) + "
" + + frappe.msgprint(text_str + df_options_str, raise_exception=True) fields = meta.get("fields") From a8e1e1c9763ba242ae9217973a7c46cc38383277 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 Mar 2020 10:28:59 +0530 Subject: [PATCH 24/52] style: remove extra spaces for consistency --- frappe/core/doctype/doctype/doctype.py | 60 -------------------------- 1 file changed, 60 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index d5b293be68..de340d65b0 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -96,7 +96,6 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_('Standard DocType cannot have default print format, use Customize Form')) - def set_default_in_list_view(self): '''Set default in-list-view for first 4 mandatory fields''' if not [d.fieldname for d in self.fields if d.in_list_view]: @@ -107,14 +106,12 @@ class DocType(Document): cnt += 1 if cnt == 4: break - def set_default_translatable(self): '''Ensure that non-translatable never will be translatable''' for d in self.fields: if d.translatable and not supports_translation(d.fieldtype): d.translatable = 0 - def check_developer_mode(self): """Throw exception if not developer mode or via patch""" if frappe.flags.in_patch or frappe.flags.in_test: @@ -123,7 +120,6 @@ class DocType(Document): if not frappe.conf.get("developer_mode") and not self.custom: frappe.throw(_("Not in Developer Mode! Set in site_config.json or make 'Custom' DocType."), CannotCreateStandardDoctypeError) - def setup_fields_to_fetch(self): '''Setup query to update values for newly set fetch values''' try: @@ -168,21 +164,18 @@ class DocType(Document): ) ) - def update_fields_to_fetch(self): '''Update fetch values based on queries setup''' if self.flags.update_fields_to_fetch_queries and not self.issingle: for query in self.flags.update_fields_to_fetch_queries: frappe.db.sql(query) - def validate_document_type(self): if self.document_type=="Transaction": self.document_type = "Document" if self.document_type=="Master": self.document_type = "Setup" - def validate_website(self): """Ensure that website generator has field 'route'""" if self.has_web_view: @@ -193,7 +186,6 @@ class DocType(Document): # clear website cache frappe.website.render.clear_cache() - def change_modified_of_parent(self): """Change the timestamp of parent DocType if the current one is a child to clear caches.""" if frappe.flags.in_import: @@ -203,7 +195,6 @@ class DocType(Document): for p in parent_list: frappe.db.sql('UPDATE `tabDocType` SET modified=%s WHERE `name`=%s', (now(), p.parent)) - def scrub_field_names(self): """Sluggify fieldnames if not set from Label.""" restricted = ('name','parent','creation','modified','modified_by', @@ -233,7 +224,6 @@ class DocType(Document): # unique is automatically an index if d.unique: d.search_index = 0 - def validate_series(self, autoname=None, name=None): """Validate if `autoname` property is correctly set.""" if not autoname: autoname = self.autoname @@ -270,7 +260,6 @@ class DocType(Document): if used_in: frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) - def on_update(self): """Update database schema, make controller templates if `custom` is not set and clear cache.""" self.delete_duplicate_custom_fields() @@ -324,7 +313,6 @@ class DocType(Document): dt = {0} and fieldname in ({1}) '''.format('%s', ', '.join(['%s'] * len(fields))), tuple([self.name] + fields), as_dict=True) - def sync_global_search(self): '''If global search settings are changed, rebuild search properties for this table''' global_search_fields_before_update = [d.fieldname for d in @@ -342,7 +330,6 @@ class DocType(Document): frappe.enqueue('frappe.utils.global_search.rebuild_for_doctype', now=now, doctype=self.name) - def set_base_class_for_controller(self): '''Updates the controller class to subclass from `WebsiteGenertor`, if it is a subclass of `Document`''' @@ -362,14 +349,12 @@ class DocType(Document): with open(controller_path, 'w') as f: f.write(code) - def run_module_method(self, method): from frappe.modules import load_doctype_module module = load_doctype_module(self.name, self.module) if hasattr(module, method): getattr(module, method)() - def before_rename(self, old, new, merge=False): """Throw exception if merge. DocTypes cannot be merged.""" if not self.custom and frappe.session.user != "Administrator": @@ -385,7 +370,6 @@ class DocType(Document): if not self.custom and not frappe.flags.in_test and not frappe.flags.in_patch: self.rename_files_and_folders(old, new) - def after_rename(self, old, new, merge=False): """Change table name using `RENAME TABLE` if table exists. Or update `doctype` property for Single type.""" @@ -396,7 +380,6 @@ class DocType(Document): else: frappe.db.sql("rename table `tab%s` to `tab%s`" % (old, new)) - def rename_files_and_folders(self, old, new): # move files new_path = get_doc_path(self.module, 'doctype', new) @@ -413,7 +396,6 @@ class DocType(Document): self.rename_inside_controller(new, old, new_path) frappe.msgprint(_('Renamed files and replaced code in controllers, please check!')) - def rename_inside_controller(self, new, old, new_path): for fname in ('{}.js', '{}.py', '{}_list.js', '{}_calendar.js', 'test_{}.py', 'test_{}.js'): fname = os.path.join(new_path, fname.format(frappe.scrub(new))) @@ -439,7 +421,6 @@ class DocType(Document): if not (self.issingle and self.istable): self.preserve_naming_series_options_in_property_setter() - def preserve_naming_series_options_in_property_setter(self): """Preserve naming_series as property setter if it does not exist""" naming_series = self.get("fields", {"fieldname": "naming_series"}) @@ -459,7 +440,6 @@ class DocType(Document): if naming_series[0].default: make_property_setter(self.name, "naming_series", "default", naming_series[0].default, "Text", validate_fields_for_doctype=False) - def before_export(self, docdict): # remove null and empty fields def remove_null_fields(o): @@ -504,7 +484,6 @@ class DocType(Document): except ValueError: pass - @staticmethod def prepare_for_import(docdict): # set order of fields from field_order @@ -527,19 +506,16 @@ class DocType(Document): if "field_order" in docdict: del docdict["field_order"] - def export_doc(self): """Export to standard folder `[module]/doctype/[name]/[name].json`.""" from frappe.modules.export_file import export_to_files export_to_files(record_list=[['DocType', self.name]], create_init=True) - def import_doc(self): """Import from standard folder `[module]/doctype/[name]/[name].json`.""" from frappe.modules.import_module import import_from_files import_from_files(record_list=[[self.module, 'doctype', self.name]]) - def make_controller_template(self): """Make boilerplate controller template.""" make_boilerplate("controller._py", self) @@ -556,7 +532,6 @@ class DocType(Document): make_boilerplate('templates/controller.html', self.as_dict()) make_boilerplate('templates/controller_row.html', self.as_dict()) - def make_amendable(self): """If is_submittable is set, add amended_from docfields.""" if self.is_submittable: @@ -572,7 +547,6 @@ class DocType(Document): "no_copy": 1 }) - def make_repeatable(self): """If allow_auto_repeat is set, add auto_repeat custom field.""" if self.allow_auto_repeat: @@ -641,14 +615,12 @@ class DocType(Document): }) self.nsm_parent_field = parent_field_name - def get_max_idx(self): """Returns the highest `idx`""" max_idx = frappe.db.sql("""select max(idx) from `tabDocField` where parent = %s""", self.name) return max_idx and max_idx[0][0] or 0 - def validate_name(self, name=None): if not name: name = self.name @@ -668,7 +640,6 @@ def validate_fields_for_doctype(doctype): doc.delete_duplicate_custom_fields() validate_fields(frappe.get_meta(doctype, cached=False)) - # this is separate because it is also called via custom field def validate_fields(meta): """Validate doctype fields. Checks @@ -692,29 +663,24 @@ def validate_fields(meta): def check_illegal_characters(fieldname): validate_column_name(fieldname) - def check_invalid_fieldnames(docname, fieldname): invalid_fields = ('doctype',) if fieldname in invalid_fields: frappe.throw(_("{0}: Fieldname cannot be one of {1}") .format(docname, ", ".join([frappe.bold(d) for d in invalid_fields]))) - def check_unique_fieldname(docname, fieldname): duplicates = list(filter(None, map(lambda df: df.fieldname==fieldname and str(df.idx) or None, fields))) if len(duplicates) > 1: frappe.throw(_("{0}: Fieldname {1} appears multiple times in rows {2}").format(docname, fieldname, ", ".join(duplicates)), UniqueFieldnameError) - def check_fieldname_length(fieldname): validate_column_length(fieldname) - def check_illegal_mandatory(docname, d): if (d.fieldtype in no_value_fields) and d.fieldtype not in table_fields and d.reqd: frappe.throw(_("{0}: Field {1} of type {2} cannot be mandatory").format(docname, d.label, d.fieldtype), IllegalMandatoryError) - def check_link_table_options(docname, d): if frappe.flags.in_patch: return if d.fieldtype in ("Link",) + table_fields: @@ -733,28 +699,23 @@ def validate_fields(meta): # fix case d.options = options - def check_hidden_and_mandatory(docname, d): if d.hidden and d.reqd and not d.default: frappe.throw(_("{0}: Field {1} in row {2} cannot be hidden and mandatory without default").format(docname, d.label, d.idx), HiddenAndMandatoryWithoutDefaultError) - def check_width(d): if d.fieldtype == "Currency" and cint(d.width) < 100: frappe.throw(_("Max width for type Currency is 100px in row {0}").format(d.idx)) - def check_in_list_view(d): if d.in_list_view and (d.fieldtype in not_allowed_in_list_view): frappe.throw(_("'In List View' not allowed for type {0} in row {1}").format(d.fieldtype, d.idx)) - def check_in_global_search(d): if d.in_global_search and d.fieldtype in no_value_fields: frappe.throw(_("'In Global Search' not allowed for type {0} in row {1}") .format(d.fieldtype, d.idx)) - def check_dynamic_link_options(d): if d.fieldtype=="Dynamic Link": doctype_pointer = list(filter(lambda df: df.fieldname==d.options, fields)) @@ -762,7 +723,6 @@ def validate_fields(meta): or (doctype_pointer[0].fieldtype=="Link" and doctype_pointer[0].options!="DocType"): frappe.throw(_("Options 'Dynamic Link' type of field must point to another Link Field with options as 'DocType'")) - def check_illegal_default(d): if d.fieldtype == "Check" and not d.default: d.default = '0' @@ -771,12 +731,10 @@ def validate_fields(meta): if d.fieldtype == "Select" and d.default and (d.default not in d.options.split("\n")): frappe.throw(_("Default for {0} must be an option").format(d.fieldname)) - def check_precision(d): if d.fieldtype in ("Currency", "Float", "Percent") and d.precision is not None and not (1 <= cint(d.precision) <= 6): frappe.throw(_("Precision should be between 1 and 6")) - def check_unique_and_text(docname, d): if meta.issingle: d.unique = 0 @@ -798,7 +756,6 @@ def validate_fields(meta): if d.search_index and d.fieldtype in ("Text", "Long Text", "Small Text", "Code", "Text Editor"): frappe.throw(_("{0}:Fieldtype {1} for {2} cannot be indexed").format(docname, d.fieldtype, d.label), CannotIndexedError) - def check_fold(fields): fold_exists = False for i, f in enumerate(fields): @@ -813,7 +770,6 @@ def validate_fields(meta): else: frappe.throw(_("Fold can not be at the end of the form")) - def check_search_fields(meta, fields): """Throw exception if `search_fields` don't contain valid fields.""" if not meta.search_fields: @@ -830,7 +786,6 @@ def validate_fields(meta): (fieldname not in fieldname_list): frappe.throw(_("Search field {0} is not valid").format(fieldname)) - def check_title_field(meta): """Throw exception if `title_field` isn't a valid fieldname.""" if not meta.get("title_field"): @@ -857,7 +812,6 @@ def validate_fields(meta): _validate_title_field_pattern(df.options) _validate_title_field_pattern(df.default) - def check_image_field(meta): '''check image_field exists and is of type "Attach Image"''' if not meta.image_field: @@ -869,7 +823,6 @@ def validate_fields(meta): if df[0].fieldtype != 'Attach Image': frappe.throw(_("Image field must be of type Attach Image"), InvalidFieldNameError) - def check_is_published_field(meta): if not meta.is_published_field: return @@ -877,7 +830,6 @@ def validate_fields(meta): if meta.is_published_field not in fieldname_list: frappe.throw(_("Is Published Field must be a valid fieldname"), InvalidFieldNameError) - def check_timeline_field(meta): if not meta.timeline_field: return @@ -889,7 +841,6 @@ def validate_fields(meta): if df.fieldtype not in ("Link", "Dynamic Link"): frappe.throw(_("Timeline field must be a Link or Dynamic Link"), InvalidFieldNameError) - def check_sort_field(meta): '''Validate that sort_field(s) is a valid field''' if meta.sort_field: @@ -902,7 +853,6 @@ def validate_fields(meta): frappe.throw(_("Sort field {0} must be a valid fieldname").format(fieldname), InvalidFieldNameError) - def check_illegal_depends_on_conditions(docfield): ''' assignment operation should not be allowed in the depends on condition.''' depends_on_fields = ["depends_on", "collapsible_depends_on", "mandatory_depends_on", "read_only_depends_on"] @@ -912,7 +862,6 @@ def validate_fields(meta): re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", depends_on): frappe.throw(_("Invalid {0} condition").format(frappe.unscrub(field)), frappe.ValidationError) - def check_table_multiselect_option(docfield): '''check if the doctype provided in Option has atleast 1 Link field''' if not docfield.fieldtype == 'Table MultiSelect': return @@ -925,7 +874,6 @@ def validate_fields(meta): frappe.throw(_('DocType {0} provided for the field {1} must have atleast one Link field') .format(doctype, docfield.fieldname), frappe.ValidationError) - def scrub_options_in_select(field): """Strip options for whitespaces""" @@ -937,12 +885,10 @@ def validate_fields(meta): options_list.append(_option) field.options = '\n'.join(options_list) - def scrub_fetch_from(field): if hasattr(field, 'fetch_from') and getattr(field, 'fetch_from'): field.fetch_from = field.fetch_from.strip('\n').strip() - def validate_data_field_type(docfield): if docfield.fieldtype == "Data": if docfield.options and (docfield.options not in data_field_options): @@ -993,7 +939,6 @@ def validate_fields(meta): check_sort_field(meta) check_image_field(meta) - def validate_permissions_for_doctype(doctype, for_remove=False): """Validates if permissions are set correctly.""" doctype = frappe.get_doc("DocType", doctype) @@ -1005,7 +950,6 @@ def validate_permissions_for_doctype(doctype, for_remove=False): clear_permissions_cache(doctype.name) - def clear_permissions_cache(doctype): frappe.clear_cache(doctype=doctype) delete_notification_count_for(doctype) @@ -1020,7 +964,6 @@ def clear_permissions_cache(doctype): """, doctype): frappe.clear_cache(user=user) - def validate_permissions(doctype, for_remove=False): permissions = doctype.get("permissions") if not permissions: @@ -1114,7 +1057,6 @@ def validate_permissions(doctype, for_remove=False): check_level_zero_is_set(d) remove_rights_for_single(d) - def make_module_and_roles(doc, perm_fieldname="permissions"): """Make `Module Def` and `Role` records if already not made. Called while installing.""" try: @@ -1145,7 +1087,6 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): else: raise - def check_if_fieldname_conflicts_with_methods(doctype, fieldname): doc = frappe.get_doc({"doctype": doctype}) method_list = [method for method in dir(doc) if isinstance(method, str) and callable(getattr(doc, method))] @@ -1153,6 +1094,5 @@ def check_if_fieldname_conflicts_with_methods(doctype, fieldname): if fieldname in method_list: frappe.throw(_("Fieldname {0} conflicting with meta object").format(fieldname)) - def clear_linked_doctype_cache(): frappe.cache().delete_value('linked_doctypes_without_ignore_user_permissions_enabled') From 10e939d0126cde262f58b705e8692acd70792f6f Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Mon, 30 Mar 2020 10:46:00 +0530 Subject: [PATCH 25/52] chore: update global_search and add test Signed-off-by: Chinmay D. Pai --- frappe/tests/test_global_search.py | 3 +++ frappe/utils/global_search.py | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_global_search.py b/frappe/tests/test_global_search.py index 01067c85dd..5c3a2df4db 100644 --- a/frappe/tests/test_global_search.py +++ b/frappe/tests/test_global_search.py @@ -191,3 +191,6 @@ class TestGlobalSearch(unittest.TestCase): frappe.db.commit() results = global_search.web_search('unsubscribe') self.assertTrue('Unsubscribe' in results[0].content) + results = global_search.web_search(text='manufacturing', + scope="manufacturing\" UNION ALL SELECT 1,2,3,4,doctype from __global_search") + self.assetTrue(results == []) diff --git a/frappe/utils/global_search.py b/frappe/utils/global_search.py index a5fcca8bb8..3c4b9583f8 100644 --- a/frappe/utils/global_search.py +++ b/frappe/utils/global_search.py @@ -501,7 +501,7 @@ def web_search(text, scope=None, start=0, limit=20): WHERE {conditions} LIMIT %(limit)s OFFSET %(start)s''' - scope_condition = '`route` like "%(scope)s" AND ' if scope else '' + scope_condition = '`route` like %(scope)s AND ' if scope else '' published_condition = '`published` = 1 AND ' mariadb_conditions = postgres_conditions = ' '.join([published_condition, scope_condition]) @@ -514,7 +514,7 @@ def web_search(text, scope=None, start=0, limit=20): "scope": "".join([scope, "%"]) if scope else '', "limit": limit, "start": start, - "text": frappe.db.escape(text) + "text": text } result = frappe.db.multisql({ From 39df9fc8c0a4d07daafde6563362cc7106a80ca5 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Mon, 30 Mar 2020 10:49:20 +0530 Subject: [PATCH 26/52] chore: add test assertion for global_search Signed-off-by: Chinmay D. Pai --- frappe/tests/test_global_search.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/tests/test_global_search.py b/frappe/tests/test_global_search.py index 5c3a2df4db..99e8f0bc58 100644 --- a/frappe/tests/test_global_search.py +++ b/frappe/tests/test_global_search.py @@ -194,3 +194,5 @@ class TestGlobalSearch(unittest.TestCase): results = global_search.web_search(text='manufacturing', scope="manufacturing\" UNION ALL SELECT 1,2,3,4,doctype from __global_search") self.assetTrue(results == []) + results = global_search.web_search('manufacturing') + self.assertTrue('Manufacturing' in results[0].content) From 5cd5d03038c30a5d88b25466b40675d4c334883b Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Mon, 30 Mar 2020 11:11:20 +0530 Subject: [PATCH 27/52] chore: fix errors in global_search test Signed-off-by: Chinmay D. Pai --- frappe/tests/test_global_search.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/tests/test_global_search.py b/frappe/tests/test_global_search.py index 99e8f0bc58..de1ae26845 100644 --- a/frappe/tests/test_global_search.py +++ b/frappe/tests/test_global_search.py @@ -191,8 +191,8 @@ class TestGlobalSearch(unittest.TestCase): frappe.db.commit() results = global_search.web_search('unsubscribe') self.assertTrue('Unsubscribe' in results[0].content) - results = global_search.web_search(text='manufacturing', + results = global_search.web_search(text='open source', scope="manufacturing\" UNION ALL SELECT 1,2,3,4,doctype from __global_search") - self.assetTrue(results == []) - results = global_search.web_search('manufacturing') - self.assertTrue('Manufacturing' in results[0].content) + self.assertTrue(results == []) + results = global_search.web_search('open source') + self.assertTrue('Open Source' in results[0].content) From 6c350f6c7bcef064d1364a0846766472c0c7cf20 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 Mar 2020 11:19:15 +0530 Subject: [PATCH 28/52] chore: update msgprint title fix: circular import --- frappe/core/doctype/doctype/doctype.py | 2 +- frappe/model/base_document.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index de340d65b0..aa0dffffca 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -896,7 +896,7 @@ def validate_fields(meta): text_str = _("{0} is an invalid Data field.").format(df_str) + "
" * 2 + _("Only Options allowed for Data field are:") + "
" df_options_str = "
  • " + "
  • ".join([_(x) for x in data_field_options]) + "
" - frappe.msgprint(text_str + df_options_str, raise_exception=True) + frappe.msgprint(text_str + df_options_str, title="Invalid Data Field", raise_exception=True) fields = meta.get("fields") diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 0880c56a22..4af502f844 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -7,7 +7,6 @@ from six import iteritems, string_types import frappe import datetime from frappe import _ -from frappe.core.doctype.user.user import STANDARD_USERS from frappe.model import default_fields, table_fields from frappe.model.naming import set_new_name from frappe.model.utils.link_count import notify_link_count @@ -546,6 +545,8 @@ class BaseDocument(object): value, comma_options)) def _validate_data_fields(self): + from frappe.core.doctype.user.user import STANDARD_USERS + # data_field options defined in frappe.model.data_field_options for data_field in self.meta.get_data_fields(): data = self.get(data_field.fieldname) From 5f5ab02260b39ad6ef724e94cd8f2e5ab0fa7daf Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 31 Mar 2020 13:35:11 +0530 Subject: [PATCH 29/52] feat: Add ljust_list utility method --- frappe/core/utils.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/frappe/core/utils.py b/frappe/core/utils.py index 55767ffe34..abab9e6160 100644 --- a/frappe/core/utils.py +++ b/frappe/core/utils.py @@ -67,3 +67,18 @@ def find_all(list_of_dict, match_function): if match_function(entry): found.append(entry) return found + +def ljust_list(_list, length, fill_word=None): + '''Similar to ljust but for list + + Usage: + $ ljust_list([1, 2, 3], 5) + > [1, 2, 3, None, None] + ''' + # make a copy to avoid mutation of passed list + _list = list(_list) + fill_length = length - len(_list) + if fill_length > 0: + _list.extend([fill_word] * fill_length) + + return _list \ No newline at end of file From 25d260d5c476c7875fb625eda41862124a24d47b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 Mar 2020 18:16:04 +0530 Subject: [PATCH 30/52] test(DocType): added test_data_field_options --- frappe/core/doctype/doctype/test_doctype.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 969a71ab7d..85381f40d0 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -113,6 +113,22 @@ class TestDocType(unittest.TestCase): if condition: self.assertFalse(re.match(pattern, condition)) + def test_data_field_options(self): + for field in frappe.model.data_field_options: + test_doctype = frappe.get_doc({ + "doctype": "DocType", + "name": "Test Data Fields", + "module": "Core", + "custom": 1, + "fields": [{ + "fieldname": "{0}_field".format(field), + "fieldtype": "Data", + "options": field + ".invalid" + }] + }) + # assert that only data options in frappe.model.data_field_options are valid + self.assertRaises(frappe.ValidationError, test_doctype.insert) + def test_sync_field_order(self): from frappe.modules.import_file import get_file_path import os From 5600d84d3c6e5fdd901dc4e41c37156b82c9d5d1 Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 31 Mar 2020 16:01:57 +0530 Subject: [PATCH 31/52] fix: don't show dashboard chart if not permitted --- frappe/core/page/dashboard/dashboard.js | 17 +++++++++++------ frappe/desk/desktop.py | 5 +++-- frappe/desk/doctype/dashboard/dashboard.py | 9 +++++++++ .../dashboard_chart/dashboard_chart.json | 11 +---------- .../doctype/dashboard_chart/dashboard_chart.py | 12 +++++------- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/frappe/core/page/dashboard/dashboard.js b/frappe/core/page/dashboard/dashboard.js index 511aac7010..92e7e48a8c 100644 --- a/frappe/core/page/dashboard/dashboard.js +++ b/frappe/core/page/dashboard/dashboard.js @@ -59,7 +59,7 @@ class Dashboard { } show_dashboard(current_dashboard_name) { - if(this.dashboard_name !== current_dashboard_name) { + if (this.dashboard_name !== current_dashboard_name) { this.dashboard_name = current_dashboard_name; let title = this.dashboard_name; if (!this.dashboard_name.toLowerCase().includes(__('dashboard'))) { @@ -76,9 +76,8 @@ class Dashboard { } refresh() { - this.get_dashboard_doc().then((doc) => { - this.dashboard_doc = doc; - this.charts = this.dashboard_doc.charts + this.get_permitted_dashboard_charts().then(charts => { + this.charts = charts .map(chart => { return { chart_name: chart.chart, @@ -98,8 +97,14 @@ class Dashboard { }); } - get_dashboard_doc() { - return frappe.model.with_doc('Dashboard', this.dashboard_name); + get_permitted_dashboard_charts() { + return frappe.xcall( + 'frappe.desk.doctype.dashboard.dashboard.get_permitted_charts', + { + dashboard_name: this.dashboard_name + }).then(charts => { + return charts; + }); } set_dropdown() { diff --git a/frappe/desk/desktop.py b/frappe/desk/desktop.py index ef84114745..1cb03355c6 100644 --- a/frappe/desk/desktop.py +++ b/frappe/desk/desktop.py @@ -146,8 +146,9 @@ class Workspace: charts = charts + self.extended_charts for chart in charts: - chart.label = chart.label if chart.label else chart.chart_name - all_charts.append(chart) + if frappe.has_permission('Dashboard Chart', doc=chart.chart_name): + chart.label = chart.label if chart.label else chart.chart_name + all_charts.append(chart) return all_charts diff --git a/frappe/desk/doctype/dashboard/dashboard.py b/frappe/desk/doctype/dashboard/dashboard.py index c8f22d29b9..5c344956bf 100644 --- a/frappe/desk/doctype/dashboard/dashboard.py +++ b/frappe/desk/doctype/dashboard/dashboard.py @@ -12,3 +12,12 @@ class Dashboard(Document): # make all other dashboards non-default frappe.db.sql('''update tabDashboard set is_default = 0 where name != %s''', self.name) + +@frappe.whitelist() +def get_permitted_charts(dashboard_name): + permitted_charts = [] + dashboard = frappe.get_doc('Dashboard', dashboard_name) + for chart in dashboard.charts: + if frappe.has_permission('Dashboard Chart', doc=chart.chart): + permitted_charts.append(chart) + return permitted_charts diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.json b/frappe/desk/doctype/dashboard_chart/dashboard_chart.json index 8c14ea130d..9652ae3945 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.json +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.json @@ -215,7 +215,7 @@ } ], "links": [], - "modified": "2020-03-26 13:41:11.126000", + "modified": "2020-03-31 16:00:01.987059", "modified_by": "Administrator", "module": "Desk", "name": "Dashboard Chart", @@ -245,15 +245,6 @@ "share": 1, "write": 1 }, - { - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "Dashboard User", - "share": 1 - }, { "email": 1, "export": 1, diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index 634b33cd97..b2a6f0a0ff 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -23,15 +23,14 @@ def get_permission_query_conditions(user): return roles = frappe.get_roles(user) - if "System Manager" in roles or "Dashboard Manager" in roles or "Dashboard User" in roles: + if "System Manager" in roles: return None allowed_doctypes = tuple(frappe.permissions.get_doctypes_with_read()) allowed_reports = tuple([key.encode('UTF8') for key in get_allowed_reports()]) return ''' - `tabDashboard Chart`.`chart_type` = 'Custom' - or `tabDashboard Chart`.`document_type` in {allowed_doctypes} + `tabDashboard Chart`.`document_type` in {allowed_doctypes} or `tabDashboard Chart`.`report_name` in {allowed_reports} '''.format( allowed_doctypes=allowed_doctypes, @@ -41,12 +40,11 @@ def get_permission_query_conditions(user): def has_permission(doc, ptype, user): roles = frappe.get_roles(user) - if "System Manager" in roles or "Dashboard Manager" in roles or "Dashboard User" in roles: + if "System Manager" in roles: return True - if doc.chart_type == 'Custom': - return True - elif doc.chart_type == 'Report': + + if doc.chart_type == 'Report': allowed_reports = tuple([key.encode('UTF8') for key in get_allowed_reports()]) if doc.report_name in allowed_reports: return True From d9f1a85d002bdc1759f0e53e8f0c6f770fa298fc Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 31 Mar 2020 16:23:12 +0530 Subject: [PATCH 32/52] fix: only encode if type is not str --- frappe/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index 9835724ce0..0d766aec8d 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -307,7 +307,7 @@ def has_controller_permissions(doc, ptype, user=None): return None def get_doctypes_with_read(): - return list(set([p.parent.encode('UTF8') for p in get_valid_perms()])) + return list(set([p.parent if type(p.parent) == str else p.parent.encode('UTF8') for p in get_valid_perms()])) def get_valid_perms(doctype=None, user=None): '''Get valid permissions for the current user from DocPerm and Custom DocPerm''' From d0aa01367b947b7dea3fad18ec2bc80b6b3cdea2 Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 31 Mar 2020 16:28:08 +0530 Subject: [PATCH 33/52] fix: show message if there are no permitted charts on a dashboard --- frappe/core/page/dashboard/dashboard.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/core/page/dashboard/dashboard.js b/frappe/core/page/dashboard/dashboard.js index 92e7e48a8c..8705804014 100644 --- a/frappe/core/page/dashboard/dashboard.js +++ b/frappe/core/page/dashboard/dashboard.js @@ -77,6 +77,9 @@ class Dashboard { refresh() { this.get_permitted_dashboard_charts().then(charts => { + if (!charts.length) { + frappe.msgprint(__('No Permitted Charts on this Dashboard'), __('No Permitted Charts')) + } this.charts = charts .map(chart => { return { From 256428572ffb81146c056b6fcda1f08be3fceca3 Mon Sep 17 00:00:00 2001 From: marty Date: Tue, 31 Mar 2020 12:54:30 +0000 Subject: [PATCH 34/52] resolve error redis_wrapper --- frappe/utils/redis_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index b0c0990e85..385bac8e4a 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -43,7 +43,7 @@ class RedisWrapper(redis.Redis): try: if expires_in_sec: - self.setex(key, expires_in_sec, pickle.dumps(val)) + self.setex(name=key, time=expires_in_sec, value=pickle.dumps(val)) else: self.set(key, pickle.dumps(val)) From 9393c05b13990ae427084a5ddcee00cace685716 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 1 Apr 2020 10:12:35 +0530 Subject: [PATCH 35/52] tests: updated tests to cover more data field options --- frappe/core/doctype/doctype/test_doctype.py | 37 +++++++++++++-------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 85381f40d0..17a56b5336 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -114,20 +114,29 @@ class TestDocType(unittest.TestCase): self.assertFalse(re.match(pattern, condition)) def test_data_field_options(self): - for field in frappe.model.data_field_options: - test_doctype = frappe.get_doc({ - "doctype": "DocType", - "name": "Test Data Fields", - "module": "Core", - "custom": 1, - "fields": [{ - "fieldname": "{0}_field".format(field), - "fieldtype": "Data", - "options": field + ".invalid" - }] - }) - # assert that only data options in frappe.model.data_field_options are valid - self.assertRaises(frappe.ValidationError, test_doctype.insert) + valid_data_field_options = frappe.model.data_field_options + ("",) + invalid_data_field_options = ("Invalid Option 1", "Invalid Option 2", frappe.utils.random_string(5)) + + for options_set in [valid_data_field_options, invalid_data_field_options]: + for field in options_set: + test_doctype = frappe.get_doc({ + "doctype": "DocType", + "name": "Test Data Fields", + "module": "Core", + "custom": 1, + "fields": [{ + "fieldname": "{0}_field".format(field), + "fieldtype": "Data", + "options": field + }] + }) + if options_set == invalid_data_field_options: + # assert that only data options in frappe.model.data_field_options are valid + self.assertRaises(frappe.ValidationError, test_doctype.insert) + else: + test_doctype.insert() + self.assertEqual(test_doctype.name, doctype_name) + test_doctype.delete() def test_sync_field_order(self): from frappe.modules.import_file import get_file_path From ea52e5176671a8b698a34b91682f4c2d1b01186a Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 1 Apr 2020 11:55:42 +0530 Subject: [PATCH 36/52] fix: Change 'Errored' to 'Failed' Transactions in Bulk Workflow Status prompt --- frappe/model/workflow.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index b134f2f8dc..c63b4337f0 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -185,7 +185,7 @@ def bulk_workflow_approval(docnames, doctype, action): from collections import defaultdict # dictionaries for logging - errored_transactions = defaultdict(list) + failed_transactions = defaultdict(list) successful_transactions = defaultdict(list) # WARN: message log is cleared @@ -206,7 +206,7 @@ def bulk_workflow_approval(docnames, doctype, action): if e.args: message += " : {0}".format(e.args[0]) message_dict = {"docname": docname, "message": message} - errored_transactions[docname].append(message_dict) + failed_transactions[docname].append(message_dict) frappe.db.rollback() frappe.log_error(frappe.get_traceback(), "Workflow {0} threw an error for {1} {2}".format(action, doctype, docname)) @@ -219,20 +219,20 @@ def bulk_workflow_approval(docnames, doctype, action): message_dict = {"docname": docname, "message": message.get("message")} if message.get("raise_exception", False): - errored_transactions[docname].append(message_dict) + failed_transactions[docname].append(message_dict) else: successful_transactions[docname].append(message_dict) else: successful_transactions[docname].append({"docname": docname, "message": None}) - if errored_transactions and successful_transactions: + if failed_transactions and successful_transactions: indicator = "orange" - elif errored_transactions: + elif failed_transactions: indicator = "red" else: indicator = "green" - print_workflow_log(errored_transactions, _("Errored Transactions"), doctype, indicator) + print_workflow_log(failed_transactions, _("Failed Transactions"), doctype, indicator) print_workflow_log(successful_transactions, _("Successful Transactions"), doctype, indicator) def print_workflow_log(messages, title, doctype, indicator): From b5a9250574f808db067faebf81372834dcf8bd47 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 1 Apr 2020 10:59:11 +0530 Subject: [PATCH 37/52] tests: simplifying test_data_field_options Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/core/doctype/doctype/test_doctype.py | 45 +++++++++++---------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 17a56b5336..c766295ee2 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -114,29 +114,30 @@ class TestDocType(unittest.TestCase): self.assertFalse(re.match(pattern, condition)) def test_data_field_options(self): + doctype_name = "Test Data Fields" valid_data_field_options = frappe.model.data_field_options + ("",) - invalid_data_field_options = ("Invalid Option 1", "Invalid Option 2", frappe.utils.random_string(5)) + invalid_data_field_options = ("Invalid Option 1", frappe.utils.random_string(5)) - for options_set in [valid_data_field_options, invalid_data_field_options]: - for field in options_set: - test_doctype = frappe.get_doc({ - "doctype": "DocType", - "name": "Test Data Fields", - "module": "Core", - "custom": 1, - "fields": [{ - "fieldname": "{0}_field".format(field), - "fieldtype": "Data", - "options": field - }] - }) - if options_set == invalid_data_field_options: - # assert that only data options in frappe.model.data_field_options are valid - self.assertRaises(frappe.ValidationError, test_doctype.insert) - else: - test_doctype.insert() - self.assertEqual(test_doctype.name, doctype_name) - test_doctype.delete() + for field_option in (valid_data_field_options + invalid_data_field_options): + test_doctype = frappe.get_doc({ + "doctype": "DocType", + "name": doctype_name, + "module": "Core", + "custom": 1, + "fields": [{ + "fieldname": "{0}_field".format(field_option), + "fieldtype": "Data", + "options": field_option + }] + }) + + if field_option in invalid_data_field_options: + # assert that only data options in frappe.model.data_field_options are valid + self.assertRaises(frappe.ValidationError, test_doctype.insert) + else: + test_doctype.insert() + self.assertEqual(test_doctype.name, doctype_name) + test_doctype.delete() def test_sync_field_order(self): from frappe.modules.import_file import get_file_path @@ -374,4 +375,4 @@ class TestDocType(unittest.TestCase): # delete doctype link_doc.delete() doc.delete() - frappe.db.commit() \ No newline at end of file + frappe.db.commit() From fe54fb67c975d8c88645d8b34dc63263184e940a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 1 Apr 2020 12:34:18 +0530 Subject: [PATCH 38/52] refactor: generate_report_result method - to fix codacy complexity issue --- frappe/core/doctype/report/report.py | 14 +++++++++- frappe/core/utils.py | 7 +++-- frappe/desk/query_report.py | 42 +++++++++------------------- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 2d49915f59..967b28b8b2 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -6,7 +6,7 @@ import frappe import json, datetime from frappe import _, scrub import frappe.desk.query_report -from frappe.utils import cint +from frappe.utils import cint, cstr from frappe.model.document import Document from frappe.modules.export_file import export_to_files from frappe.modules import make_boilerplate @@ -92,6 +92,18 @@ class Report(Document): make_boilerplate("controller.py", self, {"name": self.name}) make_boilerplate("controller.js", self, {"name": self.name}) + def execute_query_report(self, filters): + if not self.query: + frappe.throw(_("Must specify a Query to run"), title=_('Report Document Error')) + + 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()] + + return [columns, result] + def execute_script_report(self, filters): # save the timestamp to automatically set to prepared threshold = 30 diff --git a/frappe/core/utils.py b/frappe/core/utils.py index abab9e6160..55cfbc34d7 100644 --- a/frappe/core/utils.py +++ b/frappe/core/utils.py @@ -69,16 +69,17 @@ def find_all(list_of_dict, match_function): return found def ljust_list(_list, length, fill_word=None): - '''Similar to ljust but for list + """ + Similar to ljust but for list. Usage: $ ljust_list([1, 2, 3], 5) > [1, 2, 3, None, None] - ''' + """ # make a copy to avoid mutation of passed list _list = list(_list) fill_length = length - len(_list) if fill_length > 0: _list.extend([fill_word] * fill_length) - return _list \ No newline at end of file + return _list diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 994f816c8a..aaf859e7fd 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -8,7 +8,7 @@ import os, json from frappe import _ from frappe.modules import scrub, get_module_path -from frappe.utils import flt, cint, get_html_format, cstr, get_url_to_form +from frappe.utils import flt, cint, get_html_format, get_url_to_form from frappe.model.utils import render_include from frappe.translate import send_translations import frappe.desk.reportview @@ -16,6 +16,7 @@ from frappe.permissions import get_role_permissions from six import string_types, iteritems from datetime import timedelta from frappe.utils import gzip_decompress +from frappe.core.utils import ljust_list def get_report_doc(report_name): doc = frappe.get_doc("Report", report_name) @@ -43,44 +44,27 @@ def get_report_doc(report_name): def generate_report_result(report, filters=None, user=None, custom_columns=None): - status = None - if not user: - user = frappe.session.user - if not filters: - filters = [] + user = user or frappe.session.user + filters = filters or [] if filters and isinstance(filters, string_types): filters = json.loads(filters) - columns, result, message, chart, report_summary, skip_total_row = [], [], None, None, None, 0 + + res = [] + if report.report_type == "Query Report": - if not report.query: - status = "error" - frappe.msgprint(_("Must specify a Query to run"), raise_exception=True) - - if not report.query.lower().startswith("select"): - status = "error" - frappe.msgprint(_("Query must be a SELECT"), raise_exception=True) - - result = [list(t) for t in frappe.db.sql(report.query, filters)] - columns = [cstr(c[0]) for c in frappe.db.get_description()] + res = report.execute_query_report(filters) elif report.report_type == 'Script Report': res = report.execute_script_report(filters) - columns, result = res[0], res[1] - if len(res) > 2: - message = res[2] - if len(res) > 3: - chart = res[3] - if len(res) > 4: - report_summary = res[4] - if len(res) > 5: - skip_total_row = cint(res[5]) + columns, result, message, chart, report_summary, skip_total_row = \ + ljust_list(res, 6) if report.custom_columns: columns = json.loads(report.custom_columns) result = add_data_to_custom_columns(columns, result) - elif custom_columns: + if custom_columns: result = add_data_to_custom_columns(custom_columns, result) for custom_column in custom_columns: @@ -98,8 +82,8 @@ def generate_report_result(report, filters=None, user=None, custom_columns=None) "message": message, "chart": chart, "report_summary": report_summary, - "skip_total_row": skip_total_row, - "status": status, + "skip_total_row": skip_total_row or 0, + "status": None, "execution_time": frappe.cache().hget('report_execution_time', report.name) or 0 } From a8af9e91bc62c488fc4413c4c119548264763b95 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 1 Apr 2020 11:05:40 +0530 Subject: [PATCH 39/52] style: Fix indentation --- frappe/core/doctype/doctype/test_doctype.py | 16 ++++++++-------- frappe/public/js/frappe/form/controls/data.js | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index c766295ee2..fe9f88b9b3 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -120,14 +120,14 @@ class TestDocType(unittest.TestCase): for field_option in (valid_data_field_options + invalid_data_field_options): test_doctype = frappe.get_doc({ - "doctype": "DocType", - "name": doctype_name, - "module": "Core", - "custom": 1, - "fields": [{ - "fieldname": "{0}_field".format(field_option), - "fieldtype": "Data", - "options": field_option + "doctype": "DocType", + "name": doctype_name, + "module": "Core", + "custom": 1, + "fields": [{ + "fieldname": "{0}_field".format(field_option), + "fieldtype": "Data", + "options": field_option }] }) diff --git a/frappe/public/js/frappe/form/controls/data.js b/frappe/public/js/frappe/form/controls/data.js index 512c6fbcb0..a7f0050d65 100644 --- a/frappe/public/js/frappe/form/controls/data.js +++ b/frappe/public/js/frappe/form/controls/data.js @@ -94,7 +94,7 @@ frappe.ui.form.ControlData = frappe.ui.form.ControlInput.extend({ return v; } if(this.df.options == 'Phone') { - this.df.invalid = !validate_phone(v) + this.df.invalid = !validate_phone(v); return v; } else if(this.df.options == 'Email') { var email_list = frappe.utils.split_emails(v); From 27074e95da97c61d38beca35e445a2df972b0f5d Mon Sep 17 00:00:00 2001 From: Kenneth Sequeira Date: Wed, 1 Apr 2020 18:56:30 +0530 Subject: [PATCH 40/52] improve error message for logged out user --- frappe/www/desk.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/www/desk.py b/frappe/www/desk.py index 689bf725f0..78a7512b7e 100644 --- a/frappe/www/desk.py +++ b/frappe/www/desk.py @@ -12,8 +12,10 @@ from frappe import _ import frappe.sessions def get_context(context): - if (frappe.session.user == "Guest" or - frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User"): + if (frappe.session.user == "Guest"): + frappe.throw(_("Log in to access this page."), frappe.PermissionError) + + elif (frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User")): frappe.throw(_("You are not permitted to access this page."), frappe.PermissionError) hooks = frappe.get_hooks() From abbd92032089d9d14bac83ae731e8de9ca1d15d6 Mon Sep 17 00:00:00 2001 From: Kenneth Sequeira Date: Wed, 1 Apr 2020 20:37:00 +0530 Subject: [PATCH 41/52] add comments for separation logic --- frappe/www/desk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/www/desk.py b/frappe/www/desk.py index 78a7512b7e..3bd7b68032 100644 --- a/frappe/www/desk.py +++ b/frappe/www/desk.py @@ -12,10 +12,10 @@ from frappe import _ import frappe.sessions def get_context(context): - if (frappe.session.user == "Guest"): + if (frappe.session.user == "Guest"): #show message if user is logged out frappe.throw(_("Log in to access this page."), frappe.PermissionError) - elif (frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User")): + elif (frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User")): #show message if user is website user frappe.throw(_("You are not permitted to access this page."), frappe.PermissionError) hooks = frappe.get_hooks() From 14135054227e406706772a49b95938657e0fa083 Mon Sep 17 00:00:00 2001 From: Kenneth Sequeira Date: Wed, 1 Apr 2020 20:57:24 +0530 Subject: [PATCH 42/52] fix syntax --- frappe/www/desk.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/www/desk.py b/frappe/www/desk.py index 3bd7b68032..b89b688f7a 100644 --- a/frappe/www/desk.py +++ b/frappe/www/desk.py @@ -15,7 +15,7 @@ def get_context(context): if (frappe.session.user == "Guest"): #show message if user is logged out frappe.throw(_("Log in to access this page."), frappe.PermissionError) - elif (frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User")): #show message if user is website user + elif (frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User"): #show message if user is website user frappe.throw(_("You are not permitted to access this page."), frappe.PermissionError) hooks = frappe.get_hooks() From d73f8d85dbb714eddb3ba04b94af734d1c32d2bb Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 1 Apr 2020 22:18:23 +0530 Subject: [PATCH 43/52] style: Fix formatting & remove unnecessary comment --- frappe/www/desk.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/www/desk.py b/frappe/www/desk.py index b89b688f7a..6cb7c8a077 100644 --- a/frappe/www/desk.py +++ b/frappe/www/desk.py @@ -12,10 +12,9 @@ from frappe import _ import frappe.sessions def get_context(context): - if (frappe.session.user == "Guest"): #show message if user is logged out + if frappe.session.user == "Guest": frappe.throw(_("Log in to access this page."), frappe.PermissionError) - - elif (frappe.db.get_value("User", frappe.session.user, "user_type")=="Website User"): #show message if user is website user + elif frappe.db.get_value("User", frappe.session.user, "user_type") == "Website User": frappe.throw(_("You are not permitted to access this page."), frappe.PermissionError) hooks = frappe.get_hooks() From f79a43d5f30bb2ec0f5894465dce4def5952168e Mon Sep 17 00:00:00 2001 From: prssanna Date: Wed, 1 Apr 2020 00:09:40 +0530 Subject: [PATCH 44/52] feat: save dashboard chart config for user --- frappe/core/page/dashboard/dashboard.js | 36 +++++++------ .../doctype/dashboard_settings/__init__.py | 0 .../dashboard_settings/dashboard_settings.js | 8 +++ .../dashboard_settings.json | 51 +++++++++++++++++++ .../dashboard_settings/dashboard_settings.py | 39 ++++++++++++++ frappe/hooks.py | 1 + .../public/js/frappe/utils/dashboard_utils.js | 21 ++++++++ .../public/js/frappe/views/desktop/desktop.js | 40 ++++++++++----- .../public/js/frappe/widgets/chart_widget.js | 49 ++++++++++++------ 9 files changed, 202 insertions(+), 43 deletions(-) create mode 100644 frappe/desk/doctype/dashboard_settings/__init__.py create mode 100644 frappe/desk/doctype/dashboard_settings/dashboard_settings.js create mode 100644 frappe/desk/doctype/dashboard_settings/dashboard_settings.json create mode 100644 frappe/desk/doctype/dashboard_settings/dashboard_settings.py diff --git a/frappe/core/page/dashboard/dashboard.js b/frappe/core/page/dashboard/dashboard.js index 8705804014..88bfba9e84 100644 --- a/frappe/core/page/dashboard/dashboard.js +++ b/frappe/core/page/dashboard/dashboard.js @@ -80,23 +80,27 @@ class Dashboard { if (!charts.length) { frappe.msgprint(__('No Permitted Charts on this Dashboard'), __('No Permitted Charts')) } - this.charts = charts - .map(chart => { - return { - chart_name: chart.chart, - label: chart.chart, - ...chart - } - }); - this.chart_group = new frappe.widget.WidgetGroup({ - title: null, - container: this.container, - type: "chart", - columns: 2, - allow_sorting: false, - widgets: this.charts, - }); + frappe.dashboard_utils.get_dashboard_settings().then((settings) => { + let chart_config = settings.chart_config? JSON.parse(settings.chart_config): {}; + this.charts = + charts.map(chart => { + return { + chart_name: chart.chart, + label: chart.chart, + chart_settings: chart_config[chart.chart] || {}, + ...chart + } + }); + this.chart_group = new frappe.widget.WidgetGroup({ + title: null, + container: this.container, + type: "chart", + columns: 2, + allow_sorting: false, + widgets: this.charts, + }); + }) }); } diff --git a/frappe/desk/doctype/dashboard_settings/__init__.py b/frappe/desk/doctype/dashboard_settings/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/desk/doctype/dashboard_settings/dashboard_settings.js b/frappe/desk/doctype/dashboard_settings/dashboard_settings.js new file mode 100644 index 0000000000..8e7966366d --- /dev/null +++ b/frappe/desk/doctype/dashboard_settings/dashboard_settings.js @@ -0,0 +1,8 @@ +// Copyright (c) 2020, Frappe Technologies and contributors +// For license information, please see license.txt + +frappe.ui.form.on('Dashboard Settings', { + // refresh: function(frm) { + + // } +}); diff --git a/frappe/desk/doctype/dashboard_settings/dashboard_settings.json b/frappe/desk/doctype/dashboard_settings/dashboard_settings.json new file mode 100644 index 0000000000..e1eb75db47 --- /dev/null +++ b/frappe/desk/doctype/dashboard_settings/dashboard_settings.json @@ -0,0 +1,51 @@ +{ + "actions": [], + "autoname": "Prompt", + "creation": "2020-03-31 19:41:45.785014", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "user", + "chart_config" + ], + "fields": [ + { + "fieldname": "user", + "fieldtype": "Link", + "label": "User", + "options": "User", + "read_only": 1 + }, + { + "fieldname": "chart_config", + "fieldtype": "Code", + "label": "Chart Configuration", + "options": "JSON", + "read_only": 1 + } + ], + "in_create": 1, + "links": [], + "modified": "2020-04-01 00:07:26.489561", + "modified_by": "Administrator", + "module": "Desk", + "name": "Dashboard Settings", + "owner": "Administrator", + "permissions": [ + { + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "All", + "share": 1, + "write": 1 + } + ], + "read_only": 1, + "sort_field": "modified", + "sort_order": "DESC", + "track_changes": 1 +} \ No newline at end of file diff --git a/frappe/desk/doctype/dashboard_settings/dashboard_settings.py b/frappe/desk/doctype/dashboard_settings/dashboard_settings.py new file mode 100644 index 0000000000..ce7f2979c7 --- /dev/null +++ b/frappe/desk/doctype/dashboard_settings/dashboard_settings.py @@ -0,0 +1,39 @@ +# -*- 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 +import frappe +import json + +class DashboardSettings(Document): + pass + + +@frappe.whitelist() +def create_dashboard_settings(user): + if not frappe.db.exists("Dashboard Settings", user): + doc = frappe.new_doc('Dashboard Settings') + doc.name = user + doc.insert(ignore_permissions=True) + frappe.db.commit() + return doc + +def get_permission_query_conditions(user): + if not user: user = frappe.session.user + + return '''(`tabDashboard Settings`.name = '{user}')'''.format(user=user) + +@frappe.whitelist() +def save_chart_config(config, chart_name): + config = frappe.parse_json(config) + doc = frappe.get_doc('Dashboard Settings', frappe.session.user) + chart_config = frappe.parse_json(doc.chart_config) or {} + + if not chart_name in chart_config: + chart_config[chart_name] = {} + + chart_config[chart_name].update(config) + frappe.db.set_value('Dashboard Settings', frappe.session.user, 'chart_config', json.dumps(chart_config)) \ No newline at end of file diff --git a/frappe/hooks.py b/frappe/hooks.py index 733cec7a08..4f65303be9 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -86,6 +86,7 @@ permission_query_conditions = { "Event": "frappe.desk.doctype.event.event.get_permission_query_conditions", "ToDo": "frappe.desk.doctype.todo.todo.get_permission_query_conditions", "User": "frappe.core.doctype.user.user.get_permission_query_conditions", + "Dashboard Settings": "frappe.desk.doctype.dashboard_settings.dashboard_settings.get_permission_query_conditions", "Notification Log": "frappe.desk.doctype.notification_log.notification_log.get_permission_query_conditions", "Dashboard Chart": "frappe.desk.doctype.dashboard_chart.dashboard_chart.get_permission_query_conditions", "Notification Settings": "frappe.desk.doctype.notification_settings.notification_settings.get_permission_query_conditions", diff --git a/frappe/public/js/frappe/utils/dashboard_utils.js b/frappe/public/js/frappe/utils/dashboard_utils.js index 7e64f5c143..66f3d5e21f 100644 --- a/frappe/public/js/frappe/utils/dashboard_utils.js +++ b/frappe/public/js/frappe/utils/dashboard_utils.js @@ -54,5 +54,26 @@ frappe.dashboard_utils = { } else { return Promise.resolve(); } + }, + + get_dashboard_settings() { + return frappe.model.with_doc('Dashboard Settings', frappe.session.user).then(settings => { + if (!settings) { + return this.create_dashboard_settings().then(settings => { + return settings; + }); + } else { + return settings; + } + }); + }, + + create_dashboard_settings() { + return frappe.xcall( + 'frappe.desk.doctype.dashboard_settings.dashboard_settings.create_dashboard_settings', + {user: frappe.session.user} + ).then(settings => { + return settings; + }); } }; \ No newline at end of file diff --git a/frappe/public/js/frappe/views/desktop/desktop.js b/frappe/public/js/frappe/views/desktop/desktop.js index 54a25c3771..3a95ec1d90 100644 --- a/frappe/public/js/frappe/views/desktop/desktop.js +++ b/frappe/public/js/frappe/views/desktop/desktop.js @@ -165,11 +165,18 @@ class DesktopPage { this.allow_customization = this.data.allow_customization || false; - !this.sections["onboarding"] && - this.data.charts.items.length && - this.make_charts(); - this.data.shortcuts.items.length && this.make_shortcuts(); - this.data.cards.items.length && this.make_cards(); + let create_shortcuts_and_cards = () => { + this.data.shortcuts.items.length && this.make_shortcuts(); + this.data.cards.items.length && this.make_cards(); + } + + if (!this.sections["onboarding"] && this.data.charts.items.length) { + this.make_charts().then(() => { + create_shortcuts_and_cards(); + }); + } else { + create_shortcuts_and_cards(); + } }); } @@ -224,13 +231,22 @@ class DesktopPage { } make_charts() { - this.sections["charts"] = new frappe.widget.WidgetGroup({ - title: this.data.charts.label || `${this.page_name} Dashboard`, - container: this.page, - type: "chart", - columns: 1, - allow_sorting: false, - widgets: this.data.charts.items + return frappe.dashboard_utils.get_dashboard_settings().then(settings => { + let chart_config = settings.chart_config? JSON.parse(settings.chart_config): {}; + if (this.data.charts.items) { + this.data.charts.items.map(chart => { + chart.chart_settings = chart_config[chart.chart_name] || {} + }); + } + + this.sections["charts"] = new frappe.widget.WidgetGroup({ + title: this.data.charts.label || `${this.page_name} Dashboard`, + container: this.page, + type: "chart", + columns: 1, + allow_sorting: false, + widgets: this.data.charts.items + }); }); } diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index 3388890776..4fc00ef9c9 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -62,6 +62,9 @@ export default class ChartWidget extends Widget { make_chart() { this.get_settings().then(() => { + if (!this.chart_settings) { + this.chart_settings = {}; + } this.setup_container(); this.prepare_chart_object(); this.action_area.empty(); @@ -89,7 +92,7 @@ export default class ChartWidget extends Widget { render_time_series_filters() { let filters = [ { - label: this.chart_doc.timespan, + label: this.chart_settings.timespan || this.chart_doc.timespan, options: [ "Select Date Range", "Last Year", @@ -116,13 +119,16 @@ export default class ChartWidget extends Widget { this.fetch_and_update_chart(); } + this.save_chart_config_for_user({'timespan': this.selected_timespan}) } }, { - label: this.chart_doc.time_interval, + label: this.chart_settings.time_interval || this.chart_doc.time_interval, options: ["Yearly", "Quarterly", "Monthly", "Weekly", "Daily"], action: selected_item => { this.selected_time_interval = selected_item; + + this.save_chart_config_for_user({'time_interval': this.selected_time_interval}) this.fetch_and_update_chart(); } } @@ -138,10 +144,10 @@ export default class ChartWidget extends Widget { fetch_and_update_chart() { this.args = { - timespan: this.selected_timespan, - time_interval: this.selected_time_interval, - from_date: this.selected_from_date, - to_date: this.selected_to_date + timespan: this.selected_timespan || this.chart_settings.timespan, + time_interval: this.selected_time_interval || this.chart_settings.time_interval, + from_date: this.selected_from_date || this.chart_settings.from_date, + to_date: this.selected_to_date || this.chart_settings.to_date }; this.fetch(this.filters, true, this.args).then(data => { @@ -176,16 +182,18 @@ export default class ChartWidget extends Widget { fieldname: "from_date", placeholder: "Date Range", input_class: "input-xs", + default: [this.chart_settings.from_date, this.chart_settings.to_date], reqd: 1, change: () => { let selected_date_range = this.date_range_field.get_value(); this.selected_from_date = selected_date_range[0]; this.selected_to_date = selected_date_range[1]; - if ( - selected_date_range && - selected_date_range.length == 2 - ) { + if (selected_date_range && selected_date_range.length == 2) { + this.save_chart_config_for_user({ + 'from_date': this.selected_from_date, + 'to_date': this.selected_to_date, + }); this.fetch_and_update_chart(); } } @@ -334,6 +342,7 @@ export default class ChartWidget extends Widget { } else { me.filters = values; } + me.save_chart_config_for_user({'filters': me.filters}); me.fetch_and_update_chart(); } }, @@ -350,6 +359,15 @@ export default class ChartWidget extends Widget { dialog.set_values(this.filters); } + save_chart_config_for_user(config) { + Object.assign(this.chart_settings, config); + frappe.xcall('frappe.desk.doctype.dashboard_settings.dashboard_settings.save_chart_config', + { + 'config': this.chart_settings, + 'chart_name': this.chart_doc.chart_name + }); + } + create_filter_group_and_add_filters(parent) { this.filter_group = new frappe.ui.FilterGroup({ parent: parent, @@ -406,10 +424,10 @@ export default class ChartWidget extends Widget { filters: filters, refresh: refresh ? 1 : 0, time_interval: - args && args.time_interval ? args.time_interval : null, - timespan: args && args.timespan ? args.timespan : null, - from_date: args && args.from_date ? args.from_date : null, - to_date: args && args.to_date ? args.to_date : null + args && args.time_interval? args.time_interval: null, + timespan: args && args.timespan? args.timespan: null, + from_date: args && args.from_date? args.from_date: null, + to_date: args && args.to_date? args.to_date: null }; } return frappe.xcall(method, args); @@ -481,8 +499,9 @@ export default class ChartWidget extends Widget { } prepare_chart_object() { + let saved_filters = this.chart_settings.filters || null; this.filters = - this.filters || JSON.parse(this.chart_doc.filters_json || "[]"); + saved_filters || this.filters || JSON.parse(this.chart_doc.filters_json || "[]"); } get_settings() { From a50492eae410a540d47de19c8c1ef52fcd3f8fdf Mon Sep 17 00:00:00 2001 From: prssanna Date: Thu, 2 Apr 2020 11:35:55 +0530 Subject: [PATCH 45/52] feat: add button to reset default chart config --- .../dashboard_settings/dashboard_settings.py | 12 +++++++---- .../public/js/frappe/widgets/chart_widget.js | 20 +++++++++++++++++-- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/frappe/desk/doctype/dashboard_settings/dashboard_settings.py b/frappe/desk/doctype/dashboard_settings/dashboard_settings.py index ce7f2979c7..4697d897fc 100644 --- a/frappe/desk/doctype/dashboard_settings/dashboard_settings.py +++ b/frappe/desk/doctype/dashboard_settings/dashboard_settings.py @@ -27,13 +27,17 @@ def get_permission_query_conditions(user): return '''(`tabDashboard Settings`.name = '{user}')'''.format(user=user) @frappe.whitelist() -def save_chart_config(config, chart_name): - config = frappe.parse_json(config) +def save_chart_config(reset, config, chart_name): + reset = frappe.parse_json(reset) doc = frappe.get_doc('Dashboard Settings', frappe.session.user) chart_config = frappe.parse_json(doc.chart_config) or {} - if not chart_name in chart_config: + if reset: chart_config[chart_name] = {} + else: + config = frappe.parse_json(config) + if not chart_name in chart_config: + chart_config[chart_name] = {} + chart_config[chart_name].update(config) - chart_config[chart_name].update(config) frappe.db.set_value('Dashboard Settings', frappe.session.user, 'chart_config', json.dumps(chart_config)) \ No newline at end of file diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index 4fc00ef9c9..0799b3a072 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -243,7 +243,7 @@ export default class ChartWidget extends Widget { } }, { - label: __("Edit..."), + label: __("Edit"), action: "action-edit", handler: () => { frappe.set_route( @@ -252,6 +252,15 @@ export default class ChartWidget extends Widget { this.chart_doc.name ); } + }, + { + label: __("Reset Chart"), + action: "action-list", + handler: () => { + this.reset_chart(); + delete this.dashboard_chart; + this.make_chart(); + } } ]; @@ -359,10 +368,17 @@ export default class ChartWidget extends Widget { dialog.set_values(this.filters); } - save_chart_config_for_user(config) { + reset_chart() { + this.save_chart_config_for_user(null, 1); + this.chart_settings = {}; + this.filters = null; + } + + save_chart_config_for_user(config, reset=0) { Object.assign(this.chart_settings, config); frappe.xcall('frappe.desk.doctype.dashboard_settings.dashboard_settings.save_chart_config', { + 'reset': reset, 'config': this.chart_settings, 'chart_name': this.chart_doc.chart_name }); From 40d8947ddf038f3d3cdac8b21783b55cdd9dbb7b Mon Sep 17 00:00:00 2001 From: prssanna Date: Thu, 2 Apr 2020 12:04:37 +0530 Subject: [PATCH 46/52] fix: set from date and do date as null when timespan is select date range --- frappe/public/js/frappe/widgets/chart_widget.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index 0799b3a072..686c7d1e3e 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -117,9 +117,14 @@ export default class ChartWidget extends Widget { this.head.css('flex-direction', "row"); } + this.save_chart_config_for_user({ + 'timespan': this.selected_timespan, + 'from_date': null, + 'to_date': null + + }); this.fetch_and_update_chart(); } - this.save_chart_config_for_user({'timespan': this.selected_timespan}) } }, { @@ -191,6 +196,7 @@ export default class ChartWidget extends Widget { if (selected_date_range && selected_date_range.length == 2) { this.save_chart_config_for_user({ + 'timespan': this.selected_timespan, 'from_date': this.selected_from_date, 'to_date': this.selected_to_date, }); From ae8dd80dc0dd36fb98845c06a78690d73114442f Mon Sep 17 00:00:00 2001 From: prssanna Date: Thu, 2 Apr 2020 12:14:31 +0530 Subject: [PATCH 47/52] fix: code formatting --- frappe/public/js/frappe/views/desktop/desktop.js | 4 ++-- frappe/public/js/frappe/widgets/chart_widget.js | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/views/desktop/desktop.js b/frappe/public/js/frappe/views/desktop/desktop.js index 3a95ec1d90..d0993e2e8c 100644 --- a/frappe/public/js/frappe/views/desktop/desktop.js +++ b/frappe/public/js/frappe/views/desktop/desktop.js @@ -168,7 +168,7 @@ class DesktopPage { let create_shortcuts_and_cards = () => { this.data.shortcuts.items.length && this.make_shortcuts(); this.data.cards.items.length && this.make_cards(); - } + }; if (!this.sections["onboarding"] && this.data.charts.items.length) { this.make_charts().then(() => { @@ -235,7 +235,7 @@ class DesktopPage { let chart_config = settings.chart_config? JSON.parse(settings.chart_config): {}; if (this.data.charts.items) { this.data.charts.items.map(chart => { - chart.chart_settings = chart_config[chart.chart_name] || {} + chart.chart_settings = chart_config[chart.chart_name] || {}; }); } diff --git a/frappe/public/js/frappe/widgets/chart_widget.js b/frappe/public/js/frappe/widgets/chart_widget.js index 686c7d1e3e..174f0ae666 100644 --- a/frappe/public/js/frappe/widgets/chart_widget.js +++ b/frappe/public/js/frappe/widgets/chart_widget.js @@ -132,8 +132,7 @@ export default class ChartWidget extends Widget { options: ["Yearly", "Quarterly", "Monthly", "Weekly", "Daily"], action: selected_item => { this.selected_time_interval = selected_item; - - this.save_chart_config_for_user({'time_interval': this.selected_time_interval}) + this.save_chart_config_for_user({'time_interval': this.selected_time_interval}); this.fetch_and_update_chart(); } } @@ -382,8 +381,7 @@ export default class ChartWidget extends Widget { save_chart_config_for_user(config, reset=0) { Object.assign(this.chart_settings, config); - frappe.xcall('frappe.desk.doctype.dashboard_settings.dashboard_settings.save_chart_config', - { + frappe.xcall('frappe.desk.doctype.dashboard_settings.dashboard_settings.save_chart_config', { 'reset': reset, 'config': this.chart_settings, 'chart_name': this.chart_doc.chart_name From 277de227e5e791cb2b79d1a205ef84f81e55d6ac Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 2 Apr 2020 20:49:13 +0530 Subject: [PATCH 48/52] chore: Add CODEOWNERS file - To automate reviewer assignment --- .github/CODEOWNERS | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000000..d0df8e6210 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,30 @@ +# Each line is a file pattern followed by one or more owners. + +# These owners will be the default owners for everything in +# the repo. Unless a later match takes precedence, +# @global-owner1 and @global-owner2 will be requested for +# review when someone opens a pull request. +* @surajshetty3416, @netchampfaris + +# Website +website/ @scmmishra +templates/ @scmmishra +www/ @scmmishra + +integrations/ @Mangesh-Khairnar + +patches/ @surajshetty3416 @sahil28297 + +dashboard/ @prssanna + +email/ @Thunderbottom + +event_streaming/ @ruchamahabal + +data_import* @netchampfaris + +core/ @surajshetty3416 + +requirements.txt @gavindsouza + +chat/ @god \ No newline at end of file From 12c16ff518dc03bcebf325c1cab52d9babc18ef4 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 3 Apr 2020 00:51:48 +0530 Subject: [PATCH 49/52] fix: Variable undefined issue in timeline --- frappe/public/js/frappe/form/footer/timeline.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/form/footer/timeline.js b/frappe/public/js/frappe/form/footer/timeline.js index d27c65548d..beec168dfd 100644 --- a/frappe/public/js/frappe/form/footer/timeline.js +++ b/frappe/public/js/frappe/form/footer/timeline.js @@ -561,10 +561,9 @@ frappe.ui.form.Timeline = class Timeline { } let updater_reference_link = null; - - if (!$.isEmptyObject(data.updater_reference)) { + let updater_reference = data.updater_reference; + if (!$.isEmptyObject(updater_reference)) { let label = updater_reference.label || __('via {0}', [updater_reference.doctype]); - let updater_reference = data.updater_reference; updater_reference_link = frappe.utils.get_form_link( updater_reference.doctype, updater_reference.docname, From a16159b67fd18951174440f85cbc30ded9332d05 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 3 Apr 2020 08:33:42 +0530 Subject: [PATCH 50/52] chore: Move CODEOWNERS file to root folder --- .github/CODEOWNERS | 30 ------------------------------ CODEOWNERS | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 30 deletions(-) delete mode 100644 .github/CODEOWNERS create mode 100644 CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS deleted file mode 100644 index d0df8e6210..0000000000 --- a/.github/CODEOWNERS +++ /dev/null @@ -1,30 +0,0 @@ -# Each line is a file pattern followed by one or more owners. - -# These owners will be the default owners for everything in -# the repo. Unless a later match takes precedence, -# @global-owner1 and @global-owner2 will be requested for -# review when someone opens a pull request. -* @surajshetty3416, @netchampfaris - -# Website -website/ @scmmishra -templates/ @scmmishra -www/ @scmmishra - -integrations/ @Mangesh-Khairnar - -patches/ @surajshetty3416 @sahil28297 - -dashboard/ @prssanna - -email/ @Thunderbottom - -event_streaming/ @ruchamahabal - -data_import* @netchampfaris - -core/ @surajshetty3416 - -requirements.txt @gavindsouza - -chat/ @god \ No newline at end of file diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000000..2ff8752871 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,17 @@ +# Each line is a file pattern followed by one or more owners. + +# These owners will be the default owners for everything in +# the repo. Unless a later match takes precedence, + +* @surajshetty3416, @netchampfaris +website/ @scmmishra +templates/ @scmmishra +www/ @scmmishra +integrations/ @Mangesh-Khairnar +patches/ @surajshetty3416 @sahil28297 +dashboard/ @prssanna +email/ @Thunderbottom +event_streaming/ @ruchamahabal +data_import* @netchampfaris +core/ @surajshetty3416 +requirements.txt @gavindsouza \ No newline at end of file From 5a4251bce83d508dc6ff7feb9b5d096d92cd4fd6 Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Fri, 3 Apr 2020 09:54:34 +0530 Subject: [PATCH 51/52] chore: add test to check null result Signed-off-by: Chinmay D. Pai --- frappe/tests/test_global_search.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/tests/test_global_search.py b/frappe/tests/test_global_search.py index de1ae26845..5cffbaaf64 100644 --- a/frappe/tests/test_global_search.py +++ b/frappe/tests/test_global_search.py @@ -191,8 +191,6 @@ class TestGlobalSearch(unittest.TestCase): frappe.db.commit() results = global_search.web_search('unsubscribe') self.assertTrue('Unsubscribe' in results[0].content) - results = global_search.web_search(text='open source', + results = global_search.web_search(text='unsubscribe', scope="manufacturing\" UNION ALL SELECT 1,2,3,4,doctype from __global_search") self.assertTrue(results == []) - results = global_search.web_search('open source') - self.assertTrue('Open Source' in results[0].content) From 1f3f8b924606cfaa0db3c2b48ac9bf961c620b1c Mon Sep 17 00:00:00 2001 From: "Chinmay D. Pai" Date: Fri, 3 Apr 2020 09:57:11 +0530 Subject: [PATCH 52/52] chore: fix codacy issues Signed-off-by: Chinmay D. Pai --- frappe/core/doctype/user/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index de4dc0c272..b17548d994 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -131,7 +131,7 @@ frappe.ui.form.on('User', { user: frm.doc.email, password: values.new_password, logout: values.logout_sessions - }).then(() => done()); + }); } }); d.show();