From bd3ee0d4c02f90a1022473c23d907486ee57f433 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Fri, 20 Jan 2023 22:03:21 +0530 Subject: [PATCH 01/11] fix: Password Strength Progress Bar --- .../js/frappe/form/controls/password.js | 63 ++++++++++++++----- frappe/public/scss/common/controls.scss | 42 +++++++++---- 2 files changed, 78 insertions(+), 27 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/password.js b/frappe/public/js/frappe/form/controls/password.js index f7e5165472..dbc0fe0b81 100644 --- a/frappe/public/js/frappe/form/controls/password.js +++ b/frappe/public/js/frappe/form/controls/password.js @@ -7,22 +7,29 @@ frappe.ui.form.ControlPassword = class ControlPassword extends frappe.ui.form.Co make_input() { var me = this; super.make_input(); - this.$input - .parent() - .append($('')); - this.$wrapper - .find(".control-input-wrapper") - .append($('')); - this.indicator = this.$wrapper.find(".password-strength-indicator"); + this.indicator = $( + `` + ).insertAfter(this.$input); + + this.progress_text = this.indicator.find(".progress-text"); + this.progress_bar = this.indicator.find(".progress-bar"); this.message = this.$wrapper.find(".help-box"); - this.$input.on("keyup", () => { - clearTimeout(this.check_password_timeout); - this.check_password_timeout = setTimeout(() => { + this.$input.on( + "keyup", + frappe.utils.debounce(() => { me.get_password_strength(me.$input.val()); - }, 500); - }); + }, 500) + ); } disable_password_checks() { @@ -33,6 +40,13 @@ frappe.ui.form.ControlPassword = class ControlPassword extends frappe.ui.form.Co if (!this.enable_password_checks) { return; } + + if (!value) { + this.indicator.addClass("hidden"); + this.message.addClass("hidden"); + return; + } + var me = this; frappe.call({ type: "POST", @@ -43,15 +57,34 @@ frappe.ui.form.ControlPassword = class ControlPassword extends frappe.ui.form.Co callback: function (r) { if (r.message) { let score = r.message.score; - var indicators = ["red", "red", "orange", "yellow", "green"]; + var indicators = ["red", "red", "orange", "blue", "green"]; me.set_strength_indicator(indicators[score]); } }, }); } set_strength_indicator(color) { - var message = __("Include symbols, numbers and capital letters in the password"); - this.indicator.removeClass().addClass("password-strength-indicator indicator " + color); + let strength = { + red: [__("Weak"), "danger", 25], + orange: [__("Average"), "warning", 50], + blue: [__("Strong"), "info", 75], + green: [__("Excellent"), "success", 100], + }; + let progress_text = strength[color][0]; + let progress_percent = strength[color][2]; + let progress_color = strength[color][1]; + + this.indicator.removeClass("hidden"); + + this.progress_text.html(progress_text).css("color", `var(--${color}-500)`); + + this.progress_bar + .css("width", progress_percent + "%") + .attr("aria-valuenow", progress_percent) + .removeClass() + .addClass("progress-bar progress-bar-" + progress_color); + + let message = __("Include symbols, numbers and capital letters in the password"); this.message.html(message).toggleClass("hidden", color == "green"); } }; diff --git a/frappe/public/scss/common/controls.scss b/frappe/public/scss/common/controls.scss index 0f42f6af9d..c663d67eac 100644 --- a/frappe/public/scss/common/controls.scss +++ b/frappe/public/scss/common/controls.scss @@ -5,20 +5,34 @@ @import "phone_picker"; // password -.form-control[data-fieldtype="Password"] { - position: inherit; -} +.frappe-control[data-fieldtype="Password"] { + .control-input-wrapper { + position: relative; -.password-strength-indicator { - // TODO: Review - float: right; - padding: 15px; - margin-top: -41px; - margin-right: -7px; -} + .form-control[data-fieldtype="Password"] { + position: inherit; + } -.password-strength-message { - margin-top: -10px; + .password-strength-indicator { + display: flex; + align-items: center; + position: absolute; + gap: 5px; + top: -20px; + right: 0px; + + .progress-text { + font-size: var(--text-xs); + font-weight: 600; + } + + .progress { + background-color: var(--bg-light-gray); + width: 100px; + height: 5px; + } + } + } } // select @@ -232,6 +246,10 @@ a.progress-small { background-color: var(--red-500); } +.progress-bar-info { + background-color: var(--blue-500); +} + .progress-bar-warning { background-color: var(--orange-500); } From 6c1624d41e1bec74b279b172ff783f847ec03848 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 23 Jan 2023 16:23:44 +0530 Subject: [PATCH 02/11] feat: toggle password icon --- .../js/frappe/form/controls/password.js | 22 ++++++++++++++++++- frappe/public/scss/common/controls.scss | 9 ++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/password.js b/frappe/public/js/frappe/form/controls/password.js index dbc0fe0b81..2e575e3e19 100644 --- a/frappe/public/js/frappe/form/controls/password.js +++ b/frappe/public/js/frappe/form/controls/password.js @@ -27,9 +27,29 @@ frappe.ui.form.ControlPassword = class ControlPassword extends frappe.ui.form.Co this.$input.on( "keyup", frappe.utils.debounce(() => { + let hide_icon = me.$input.val() && !me.$input.val().includes("*"); + me.toggle_password.toggleClass("hidden", !hide_icon); me.get_password_strength(me.$input.val()); }, 500) ); + + this.toggle_password = $(` + + `).insertAfter(this.$input); + + this.toggle_password.on("click", () => { + if (this.$input.attr("type") === "password") { + this.$input.attr("type", "text"); + this.toggle_password.html(frappe.utils.icon("hide", "sm")); + } else { + this.$input.attr("type", "password"); + this.toggle_password.html(frappe.utils.icon("unhide", "sm")); + } + }); + + !this.value && this.toggle_password.removeClass("hidden"); } disable_password_checks() { @@ -71,8 +91,8 @@ frappe.ui.form.ControlPassword = class ControlPassword extends frappe.ui.form.Co green: [__("Excellent"), "success", 100], }; let progress_text = strength[color][0]; - let progress_percent = strength[color][2]; let progress_color = strength[color][1]; + let progress_percent = strength[color][2]; this.indicator.removeClass("hidden"); diff --git a/frappe/public/scss/common/controls.scss b/frappe/public/scss/common/controls.scss index c663d67eac..08debbbde9 100644 --- a/frappe/public/scss/common/controls.scss +++ b/frappe/public/scss/common/controls.scss @@ -32,6 +32,15 @@ height: 5px; } } + + .toggle-password { + position: absolute; + top: 4px; + right: 8px; + padding: 3px; + z-index: 3; + cursor: pointer; + } } } From 3d280a2d3f35b2845719d4f7c8ab9be0d4a27f46 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 23 Jan 2023 18:17:21 +0530 Subject: [PATCH 03/11] fix: child table readonly field in dialog is not readonly --- frappe/public/js/frappe/model/perm.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/model/perm.js b/frappe/public/js/frappe/model/perm.js index fdd915ebfc..3884bbfe40 100644 --- a/frappe/public/js/frappe/model/perm.js +++ b/frappe/public/js/frappe/model/perm.js @@ -195,7 +195,9 @@ $.extend(frappe.perm, { } if (!perm) { - return df && (cint(df.hidden) || cint(df.hidden_due_to_dependency)) ? "None" : "Write"; + let is_hidden = df && (cint(df.hidden) || cint(df.hidden_due_to_dependency)); + let is_read_only = df && cint(df.read_only); + return is_hidden ? "None" : is_read_only ? "Read" : "Write"; } if (!df.permlevel) df.permlevel = 0; From 71420eb4e63c3c5e3b6c10e639f56f458e687249 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 23 Jan 2023 19:57:26 +0000 Subject: [PATCH 04/11] refactor: simplified `get_controller` (#19684) * refactor: simplified `get_controller` * chore: more refactor, better error if not subclass * chore: more correct condition * refactor: `class_` > `_class` * chore: use `Meta` instead of DB calls * chore: `_get_controller` => `import_controller` * style: remove else block --- frappe/model/base_document.py | 80 +++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 1d6d87a7bd..8cd074a4c0 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -35,54 +35,60 @@ DOCTYPES_FOR_DOCTYPE = {"DocType", *TABLE_DOCTYPES_FOR_DOCTYPE.values()} def get_controller(doctype): - """Returns the **class** object of the given DocType. + """ + Returns the locally cached **class** object of the given DocType. For `custom` type, returns `frappe.model.document.Document`. - :param doctype: DocType name as string.""" - - def _get_controller(): - from frappe.model.document import Document - from frappe.utils.nestedset import NestedSet - - module_name, custom = frappe.db.get_value( - "DocType", doctype, ("module", "custom"), cache=True - ) or ("Core", False) - - if custom: - is_tree = frappe.db.get_value("DocType", doctype, "is_tree", ignore=True, cache=True) - _class = NestedSet if is_tree else Document - else: - class_overrides = frappe.get_hooks("override_doctype_class") - if class_overrides and class_overrides.get(doctype): - import_path = class_overrides[doctype][-1] - module_path, classname = import_path.rsplit(".", 1) - module = frappe.get_module(module_path) - if not hasattr(module, classname): - raise ImportError(f"{doctype}: {classname} does not exist in module {module_path}") - else: - module = load_doctype_module(doctype, module_name) - classname = doctype.replace(" ", "").replace("-", "") - - if hasattr(module, classname): - _class = getattr(module, classname) - if issubclass(_class, BaseDocument): - _class = getattr(module, classname) - else: - raise ImportError(doctype) - else: - raise ImportError(doctype) - return _class + :param doctype: DocType name as string. + """ if frappe.local.dev_server: - return _get_controller() + return import_controller(doctype) site_controllers = frappe.controllers.setdefault(frappe.local.site, {}) if doctype not in site_controllers: - site_controllers[doctype] = _get_controller() + site_controllers[doctype] = import_controller(doctype) return site_controllers[doctype] +def import_controller(doctype): + from frappe.model.document import Document + from frappe.utils.nestedset import NestedSet + + module_name = "Core" + if doctype not in DOCTYPES_FOR_DOCTYPE: + meta = frappe.get_meta(doctype) + if meta.custom: + return NestedSet if meta.get("is_tree") else Document + + module_name = meta.module + + module_path = None + class_overrides = frappe.get_hooks("override_doctype_class") + if class_overrides and class_overrides.get(doctype): + import_path = class_overrides[doctype][-1] + module_path, classname = import_path.rsplit(".", 1) + module = frappe.get_module(module_path) + + else: + module = load_doctype_module(doctype, module_name) + classname = doctype.replace(" ", "").replace("-", "") + + class_ = getattr(module, classname, None) + if class_ is None: + raise ImportError( + doctype + if module_path is None + else f"{doctype}: {classname} does not exist in module {module_path}" + ) + + if not issubclass(class_, BaseDocument): + raise ImportError(f"{doctype}: {classname} is not a subclass of BaseDocument") + + return class_ + + class BaseDocument: _reserved_keywords = { "doctype", From 229ca404ae1e4345b9724b4f6c6cf4afa62dffcd Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 Jan 2023 11:22:05 +0530 Subject: [PATCH 05/11] fix: LDAP - default user role mandatory logic broken --- frappe/integrations/doctype/ldap_settings/ldap_settings.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.json b/frappe/integrations/doctype/ldap_settings/ldap_settings.json index b8f73cebed..0b3bf06239 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.json +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.json @@ -88,8 +88,7 @@ "fieldtype": "Link", "label": "Default User Role", "mandatory_depends_on": "eval: doc.default_user_type == \"System User\"", - "options": "Role", - "reqd": 1 + "options": "Role" }, { "description": "Must be enclosed in '()' and include '{0}', which is a placeholder for the user/login name. i.e. (&(objectclass=user)(uid={0}))", @@ -302,7 +301,7 @@ "in_create": 1, "issingle": 1, "links": [], - "modified": "2022-12-05 21:52:31.146035", + "modified": "2023-01-24 11:20:06.049708", "modified_by": "Administrator", "module": "Integrations", "name": "LDAP Settings", From 788d512a69639f3eeb826f897babed0ca6d8d537 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 24 Jan 2023 11:33:00 +0530 Subject: [PATCH 06/11] chore(deps): bump cookiejar from 2.1.2 to 2.1.4 (#19737) Bumps [cookiejar](https://github.com/bmeck/node-cookiejar) from 2.1.2 to 2.1.4. - [Release notes](https://github.com/bmeck/node-cookiejar/releases) - [Commits](https://github.com/bmeck/node-cookiejar/commits) --- updated-dependencies: - dependency-name: cookiejar dependency-type: indirect ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 550ccbbded..d2ee8e62a5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -742,9 +742,9 @@ cookie@^0.4.0, cookie@~0.4.1: integrity sha512-aSWTXFzaKWkvHO1Ny/s+ePFpvKsPnjc551iI41v3ny/ow6tBG5Vd+FuqGNhh1LxOmVzOlGUriIlOaokOvhaStA== cookiejar@^2.1.0: - version "2.1.2" - resolved "https://registry.yarnpkg.com/cookiejar/-/cookiejar-2.1.2.tgz#dd8a235530752f988f9a0844f3fc589e3111125c" - integrity sha512-Mw+adcfzPxcPeI+0WlvRrr/3lGVO0bD75SxX6811cxSh1Wbxx7xZBGK1eVtDf6si8rg2lhnUjsVLMFMfbRIuwA== + version "2.1.4" + resolved "https://registry.yarnpkg.com/cookiejar/-/cookiejar-2.1.4.tgz#ee669c1fea2cf42dc31585469d193fef0d65771b" + integrity sha512-LDx6oHrK+PhzLKJU9j5S7/Y3jM/mUHvD/DeI1WQmJn652iPC5Y4TBzC9l+5OMOXlyTTA+SmVUPm0HQUwpD5Jqw== copy-anything@^2.0.1: version "2.0.3" From 1d97023b34cbf20528be0d0e7a64022330647d33 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 Jan 2023 12:33:18 +0530 Subject: [PATCH 07/11] fix(UX): Grid in 3 column layout is broken --- frappe/public/scss/common/grid.scss | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/frappe/public/scss/common/grid.scss b/frappe/public/scss/common/grid.scss index b3781d5501..775bf90704 100644 --- a/frappe/public/scss/common/grid.scss +++ b/frappe/public/scss/common/grid.scss @@ -43,16 +43,18 @@ } } -// hide row index in 6 column child tables -.form-column.col-sm-6 .form-grid { - .row-index { - display: none; - } - - .btn-open-row { - .edit-grid-row { +// hide row index in 6/4 column child tables +.form-column.col-sm-6, .form-column.col-sm-4 { + .form-grid { + .row-index { display: none; } + + .btn-open-row { + .edit-grid-row { + display: none; + } + } } } From 479e0c1439ce7f7165fc9b285fe131b1dd5f2a64 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 24 Jan 2023 12:39:55 +0530 Subject: [PATCH 08/11] fix: clear cache on every update in notifications This feels like duplicate but ensures that it gets cleared in every case. E.g. Some class might have overridden validate or ignore_validate might be set in which case cache doesn't get cleared. --- frappe/email/doctype/notification/notification.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/email/doctype/notification/notification.py b/frappe/email/doctype/notification/notification.py index 41fdfeeda1..ce19fb7b07 100644 --- a/frappe/email/doctype/notification/notification.py +++ b/frappe/email/doctype/notification/notification.py @@ -45,6 +45,7 @@ class Notification(Document): frappe.cache().hdel("notifications", self.document_type) def on_update(self): + frappe.cache().hdel("notifications", self.document_type) path = export_module_json(self, self.is_standard, self.module) if path: # js From be7fd7a58d0ffe8210f09c0bc8c1c849f8ac574d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 24 Jan 2023 12:01:00 +0530 Subject: [PATCH 09/11] fix: point to custom field link instead --- frappe/desk/form/meta.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index 8b1408f1b5..90d20c8fc4 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -11,6 +11,7 @@ from frappe.model.utils import render_include from frappe.modules import get_module_path, load_doctype_module, scrub from frappe.translate import extract_messages_from_code, make_dict_from_messages from frappe.utils import get_html_format +from frappe.utils.data import get_link_to_form ASSET_KEYS = ( "__js", @@ -199,14 +200,19 @@ class FormMeta(Meta): # customizations are removed or some custom app is removed but hasn't cleaned # up after itself. frappe.clear_last_message() - customize_form_link = f'Customize Form' - frappe.throw( - _( - "Field {0} is referring to non-existing doctype {1}, please remove the field from {2} or add the required doctype." - ).format(frappe.bold(df.fieldname), frappe.bold(df.options), customize_form_link), - title=_("Missing DocType"), + + msg = _("Field {0} is referring to non-existing doctype {1}.").format( + frappe.bold(df.fieldname), frappe.bold(df.options) ) + if df.get("is_custom_field"): + custom_field_link = get_link_to_form("Custom Field", df.name) + msg += " " + _("Please delete the field from {2} or add the required doctype.").format( + custom_field_link + ) + + frappe.throw(msg, title=_("Missing DocType")) + def add_linked_document_type(self): for df in self.get("fields", {"fieldtype": "Link"}): if df.options: From 0173d2abfbc0f59c8e74da0a25f22896dcc2d678 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 Jan 2023 17:56:10 +0530 Subject: [PATCH 10/11] test: fixed failing LDAP test --- frappe/integrations/doctype/ldap_settings/test_ldap_settings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py b/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py index 9080e0c82a..fd54d12af2 100644 --- a/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py @@ -182,6 +182,8 @@ class LDAP_TestCase: with contextlib.suppress(MandatoryError, ValidationError): frappe.get_doc(localdoc).save() + if mandatory_field == "default_role" and localdoc["default_user_type"] == "System User": + continue self.fail(f"Document LDAP Settings field [{mandatory_field}] is not mandatory") for non_mandatory_field in self.doc: # Ensure remaining fields have not been made mandatory From b4ff826711ffdf596e8c76b125922904685bf439 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 Jan 2023 18:30:51 +0530 Subject: [PATCH 11/11] fix: default_user_type should be Website User if not set --- frappe/integrations/doctype/ldap_settings/ldap_settings.py | 2 +- .../integrations/doctype/ldap_settings/test_ldap_settings.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py index 094c440672..21e5c5b312 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py @@ -26,7 +26,7 @@ if TYPE_CHECKING: class LDAPSettings(Document): def validate(self): - self.default_user_type = self.default_user_type or "System User" + self.default_user_type = self.default_user_type or "Website User" if not self.enabled: return diff --git a/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py b/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py index fd54d12af2..0417ea30e4 100644 --- a/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py @@ -173,7 +173,6 @@ class LDAP_TestCase: "ldap_username_field", "ldap_first_name_field", "require_trusted_certificate", - "default_role", ] # fields that are required to have ldap functioning need to be mandatory for mandatory_field in mandatory_fields: @@ -182,8 +181,6 @@ class LDAP_TestCase: with contextlib.suppress(MandatoryError, ValidationError): frappe.get_doc(localdoc).save() - if mandatory_field == "default_role" and localdoc["default_user_type"] == "System User": - continue self.fail(f"Document LDAP Settings field [{mandatory_field}] is not mandatory") for non_mandatory_field in self.doc: # Ensure remaining fields have not been made mandatory