From 32d3cc7277cbe6a5479f4bee893b41d9634a6470 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 17 Mar 2023 14:49:24 +0530 Subject: [PATCH 1/8] fix: Consider user perimission default in get_user_default function --- frappe/public/js/frappe/defaults.js | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/defaults.js b/frappe/public/js/frappe/defaults.js index 03258f4691..ea1d4b4920 100644 --- a/frappe/public/js/frappe/defaults.js +++ b/frappe/public/js/frappe/defaults.js @@ -3,10 +3,14 @@ frappe.defaults = { get_user_default: function (key) { - var defaults = frappe.boot.user.defaults; - var d = defaults[key]; - if (!d && frappe.defaults.is_a_user_permission_key(key)) + let defaults = frappe.boot.user.defaults; + let d = defaults[key]; + if (!d && frappe.defaults.is_a_user_permission_key(key)) { d = defaults[frappe.model.scrub(key)]; + // Check for default user permission values + user_default = this.get_user_permission_default(key); + if (user_default) d = user_default; + } if ($.isArray(d)) d = d[0]; if (!frappe.defaults.in_user_permission(key, d)) { @@ -15,6 +19,21 @@ frappe.defaults = { return d; }, + + get_user_permission_default: function (key) { + let permissions = this.get_user_permissions(); + let user_default = null; + if (permissions[key]) { + permissions[key].forEach((item) => { + if (item.is_default) { + user_default = item.doc; + } + }); + } + + return user_default; + }, + get_user_defaults: function (key) { var defaults = frappe.boot.user.defaults; var d = defaults[key]; From 0a55ac8f0774baac1fb06fbd60e5529ecf9561e3 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 6 Apr 2023 15:53:32 +0530 Subject: [PATCH 2/8] chore: Update server side method --- frappe/defaults.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/frappe/defaults.py b/frappe/defaults.py index fcfef0b2fc..40813da83e 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -28,6 +28,9 @@ def get_user_default(key, user=None): else: d = user_defaults.get(frappe.scrub(key), None) + if not d: + # If no default value is found, use the User Permission value + d = get_user_permission_default(key) value = isinstance(d, (list, tuple)) and d[0] or d if not_in_user_permission(key, value, user): @@ -36,6 +39,18 @@ def get_user_default(key, user=None): return value +def get_user_permission_default(key): + permissions = get_user_permissions() + user_default = "" + if permissions.get(key): + for item in permissions.get(key): + if item.get("is_default"): + user_default = item.get("doc") + break + + return user_default + + def get_user_default_as_list(key, user=None): user_defaults = get_defaults(user or frappe.session.user) d = user_defaults.get(key, None) From e805497681382f0b8734fd6cfd56cd5856f64888 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 7 Apr 2023 11:32:49 +0530 Subject: [PATCH 3/8] fix: Consider global default in user perm --- frappe/defaults.py | 12 +++++++++--- frappe/public/js/frappe/defaults.js | 10 ++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/frappe/defaults.py b/frappe/defaults.py index 40813da83e..edbf784200 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -25,12 +25,12 @@ def get_user_default(key, user=None): if d and isinstance(d, (list, tuple)) and len(d) == 1: # Use User Permission value when only when it has a single value d = d[0] - else: d = user_defaults.get(frappe.scrub(key), None) + user_permission_default = get_user_permission_default(key, user_defaults) if not d: # If no default value is found, use the User Permission value - d = get_user_permission_default(key) + d = user_permission_default value = isinstance(d, (list, tuple)) and d[0] or d if not_in_user_permission(key, value, user): @@ -39,10 +39,16 @@ def get_user_default(key, user=None): return value -def get_user_permission_default(key): +def get_user_permission_default(key, defaults): permissions = get_user_permissions() user_default = "" if permissions.get(key): + # global default in user permission + for item in permissions.get(key): + doc = item.get("doc") + if defaults.get(key) == doc: + user_default = doc + for item in permissions.get(key): if item.get("is_default"): user_default = item.get("doc") diff --git a/frappe/public/js/frappe/defaults.js b/frappe/public/js/frappe/defaults.js index ea1d4b4920..a96f6c3167 100644 --- a/frappe/public/js/frappe/defaults.js +++ b/frappe/public/js/frappe/defaults.js @@ -8,7 +8,7 @@ frappe.defaults = { if (!d && frappe.defaults.is_a_user_permission_key(key)) { d = defaults[frappe.model.scrub(key)]; // Check for default user permission values - user_default = this.get_user_permission_default(key); + user_default = this.get_user_permission_default(key, defaults); if (user_default) d = user_default; } if ($.isArray(d)) d = d[0]; @@ -20,10 +20,16 @@ frappe.defaults = { return d; }, - get_user_permission_default: function (key) { + get_user_permission_default: function (key, defaults) { let permissions = this.get_user_permissions(); let user_default = null; if (permissions[key]) { + permissions[key].forEach((item) => { + if (defaults[key] == item.doc) { + user_default = item.doc; + } + }); + permissions[key].forEach((item) => { if (item.is_default) { user_default = item.doc; From 087caff4cdb889c61ed32ac83dc7b9756ef77c5f Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 7 Apr 2023 11:33:22 +0530 Subject: [PATCH 4/8] test: Add test for user perm defaults --- frappe/tests/test_defaults.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/frappe/tests/test_defaults.py b/frappe/tests/test_defaults.py index 3c04f16ec8..b5185a642d 100644 --- a/frappe/tests/test_defaults.py +++ b/frappe/tests/test_defaults.py @@ -71,3 +71,37 @@ class TestDefaults(FrappeTestCase): frappe.delete_doc("User Permission", perm_doc.name) frappe.set_user(old_user) + + def test_user_permission_defaults(self): + # Create user permission + frappe.set_user("user_default_test@example.com") + set_global_default("Language", "") + clear_user_default("Language") + + perm_doc = frappe.get_doc( + dict( + doctype="User Permission", + user=frappe.session.user, + allow="Language", + for_value="en-US", + ) + ).insert(ignore_permissions=True) + + frappe.db.set_value("User Permission", perm_doc.name, "is_default", 1) + set_global_default("Language", "en-GB") + self.assertEqual(get_user_default("Language"), "en-US") + + frappe.db.set_value("User Permission", perm_doc.name, "is_default", 0) + clear_user_default("Language") + self.assertEqual(get_user_default("Language"), None) + + perm_doc = frappe.get_doc( + dict( + doctype="User Permission", + user=frappe.session.user, + allow="Language", + for_value="en-GB", + ) + ).insert(ignore_permissions=True) + + self.assertEqual(get_user_default("Language"), "en-GB") From 5527048592048d9e1dd5c9908bf9a29332451735 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 7 Apr 2023 11:49:25 +0530 Subject: [PATCH 5/8] test: Create user --- frappe/tests/test_defaults.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/tests/test_defaults.py b/frappe/tests/test_defaults.py index b5185a642d..9e8368a0d5 100644 --- a/frappe/tests/test_defaults.py +++ b/frappe/tests/test_defaults.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import frappe +from frappe.core.doctype.user_permission.test_user_permission import create_user from frappe.defaults import * from frappe.tests.utils import FrappeTestCase @@ -74,6 +75,7 @@ class TestDefaults(FrappeTestCase): def test_user_permission_defaults(self): # Create user permission + create_user("user_default_test@example.com", "Blogger") frappe.set_user("user_default_test@example.com") set_global_default("Language", "") clear_user_default("Language") From 68b1051f69430d934f08fe7725370ed504d0daeb Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sun, 9 Apr 2023 20:43:10 +0530 Subject: [PATCH 6/8] test: Set defualts --- frappe/tests/test_defaults.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/tests/test_defaults.py b/frappe/tests/test_defaults.py index 9e8368a0d5..0c58e59c78 100644 --- a/frappe/tests/test_defaults.py +++ b/frappe/tests/test_defaults.py @@ -49,6 +49,9 @@ class TestDefaults(FrappeTestCase): self.assertEqual(get_user_default("key6"), None) def test_user_permission_on_defaults(self): + add_global_default("language", "en") + set_user_default("language", "en") + self.assertEqual(get_global_default("language"), "en") self.assertEqual(get_user_default("language"), "en") self.assertEqual(get_user_default_as_list("language"), ["en"]) @@ -91,6 +94,7 @@ class TestDefaults(FrappeTestCase): frappe.db.set_value("User Permission", perm_doc.name, "is_default", 1) set_global_default("Language", "en-GB") + clear_user_default("Language") self.assertEqual(get_user_default("Language"), "en-US") frappe.db.set_value("User Permission", perm_doc.name, "is_default", 0) From 5d61ed2d8f268345cf0fc457c9124778befef4d5 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sun, 9 Apr 2023 21:26:35 +0530 Subject: [PATCH 7/8] test: Update test --- frappe/tests/test_defaults.py | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/frappe/tests/test_defaults.py b/frappe/tests/test_defaults.py index 0c58e59c78..17f5f9098e 100644 --- a/frappe/tests/test_defaults.py +++ b/frappe/tests/test_defaults.py @@ -49,9 +49,6 @@ class TestDefaults(FrappeTestCase): self.assertEqual(get_user_default("key6"), None) def test_user_permission_on_defaults(self): - add_global_default("language", "en") - set_user_default("language", "en") - self.assertEqual(get_global_default("language"), "en") self.assertEqual(get_user_default("language"), "en") self.assertEqual(get_user_default_as_list("language"), ["en"]) @@ -80,34 +77,33 @@ class TestDefaults(FrappeTestCase): # Create user permission create_user("user_default_test@example.com", "Blogger") frappe.set_user("user_default_test@example.com") - set_global_default("Language", "") - clear_user_default("Language") + set_global_default("Country", "") + clear_user_default("Country") perm_doc = frappe.get_doc( dict( doctype="User Permission", user=frappe.session.user, - allow="Language", - for_value="en-US", + allow="Country", + for_value="India", ) ).insert(ignore_permissions=True) frappe.db.set_value("User Permission", perm_doc.name, "is_default", 1) - set_global_default("Language", "en-GB") - clear_user_default("Language") - self.assertEqual(get_user_default("Language"), "en-US") + set_global_default("Country", "United States") + self.assertEqual(get_user_default("Country"), "India") frappe.db.set_value("User Permission", perm_doc.name, "is_default", 0) - clear_user_default("Language") - self.assertEqual(get_user_default("Language"), None) + clear_user_default("Country") + self.assertEqual(get_user_default("Country"), None) perm_doc = frappe.get_doc( dict( doctype="User Permission", user=frappe.session.user, - allow="Language", - for_value="en-GB", + allow="Country", + for_value="United States", ) ).insert(ignore_permissions=True) - self.assertEqual(get_user_default("Language"), "en-GB") + self.assertEqual(get_user_default("Country"), "United States") From c0ae00168dfac16f8b12cd2ea7985fc4281d65fb Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 10 Apr 2023 13:58:06 +0530 Subject: [PATCH 8/8] chore: Only run on mariadb --- frappe/tests/test_defaults.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/tests/test_defaults.py b/frappe/tests/test_defaults.py index 17f5f9098e..a8c8ed2697 100644 --- a/frappe/tests/test_defaults.py +++ b/frappe/tests/test_defaults.py @@ -3,6 +3,8 @@ import frappe from frappe.core.doctype.user_permission.test_user_permission import create_user from frappe.defaults import * +from frappe.query_builder.utils import db_type_is +from frappe.tests.test_query_builder import run_only_if from frappe.tests.utils import FrappeTestCase @@ -73,6 +75,7 @@ class TestDefaults(FrappeTestCase): frappe.delete_doc("User Permission", perm_doc.name) frappe.set_user(old_user) + @run_only_if(db_type_is.MARIADB) def test_user_permission_defaults(self): # Create user permission create_user("user_default_test@example.com", "Blogger")