From 2a4a658c3c4e5fdc8eecb059c3ac3a05d72ccc95 Mon Sep 17 00:00:00 2001 From: Anurag Mishra <32095923+Anurag810@users.noreply.github.com> Date: Fri, 24 May 2019 08:37:50 +0530 Subject: [PATCH] fix: Bulk user permission unhandled condition(v11) (#7402) * fix: unhandled condition * test: if changed from apply_to_all to apply_to_all * fix: test_cases * Update frappe/core/doctype/user_permission/test_user_permission.py Co-Authored-By: Suraj Shetty * fix: tests * fix: refactor --- .../user_permission/test_user_permission.py | 64 ++++++++++++++----- .../user_permission/user_permission.py | 9 ++- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py index b83d103013..bcf155f0a2 100644 --- a/frappe/core/doctype/user_permission/test_user_permission.py +++ b/frappe/core/doctype/user_permission/test_user_permission.py @@ -8,43 +8,75 @@ import frappe import unittest class TestUserPermission(unittest.TestCase): + def setUp(self): + frappe.db.sql("Delete from `tabUser Permission` where user='test_bulk_creation_update@example.com'") + def test_apply_to_all(self): ''' Create User permission for User having access to all applicable Doctypes''' user = get_user() - param = get_params(user, apply = 1) - created = add_user_permissions(param) - self.assertEquals(created, 1) + param = get_params(user, apply_to_all = 1) + is_created = add_user_permissions(param) + self.assertEquals(is_created, 1) + + def test_for_apply_to_all_on_update_from_apply_all(self): + user = get_user() + param = get_params(user, apply_to_all=1) + + # Initially create User Permission document with apply_to_all checked + is_created = add_user_permissions(param) + + self.assertEquals(is_created, 1) + is_created = add_user_permissions(param) + + # User Permission should not be changed + self.assertEquals(is_created, 0) def test_for_applicable_on_update_from_apply_to_all(self): ''' Update User Permission from all to some applicable Doctypes''' user = get_user() param = get_params(user, applicable = ["Chat Room", "Chat Message"]) - create = add_user_permissions(param) + + # Initially create User Permission document with apply_to_all checked + is_created = add_user_permissions(get_params(user, apply_to_all= 1)) + + self.assertEquals(is_created, 1) + is_created = add_user_permissions(param) frappe.db.commit() removed_apply_to_all = frappe.db.exists("User Permission", get_exists_param(user)) - created_applicable_first = frappe.db.exists("User Permission", get_exists_param(user, applicable = "Chat Room")) - created_applicable_second = frappe.db.exists("User Permission", get_exists_param(user, applicable = "Chat Message")) + is_created_applicable_first = frappe.db.exists("User Permission", get_exists_param(user, applicable = "Chat Room")) + is_created_applicable_second = frappe.db.exists("User Permission", get_exists_param(user, applicable = "Chat Message")) + # Check that apply_to_all is removed self.assertIsNone(removed_apply_to_all) - self.assertIsNotNone(created_applicable_first) - self.assertIsNotNone(created_applicable_second) - self.assertEquals(create, 1) + + # Check that User Permissions for applicable is created + self.assertIsNotNone(is_created_applicable_first) + self.assertIsNotNone(is_created_applicable_second) + self.assertEquals(is_created, 1) def test_for_apply_to_all_on_update_from_applicable(self): ''' Update User Permission from some to all applicable Doctypes''' user = get_user() - param = get_params(user, apply = 1) - created = add_user_permissions(param) - created_apply_to_all = frappe.db.exists("User Permission", get_exists_param(user)) + param = get_params(user, apply_to_all = 1) + + # create User permissions that with applicable + is_created = add_user_permissions(get_params(user, applicable = ["Chat Room", "Chat Message"])) + + self.assertEquals(is_created, 1) + + is_created = add_user_permissions(param) + is_created_apply_to_all = frappe.db.exists("User Permission", get_exists_param(user)) removed_applicable_first = frappe.db.exists("User Permission", get_exists_param(user, applicable = "Chat Room")) removed_applicable_second = frappe.db.exists("User Permission", get_exists_param(user, applicable = "Chat Message")) + # To check that a User permission with apply_to_all exists + self.assertIsNotNone(is_created_apply_to_all) - self.assertIsNotNone(created_apply_to_all) + # Check that all User Permission with applicable is removed self.assertIsNone(removed_applicable_first) self.assertIsNone(removed_applicable_second) - self.assertEquals(created, 1) + self.assertEquals(is_created, 1) def get_user(): if frappe.db.exists('User', 'test_bulk_creation_update@example.com'): @@ -56,14 +88,14 @@ def get_user(): user.add_roles("System Manager") return user -def get_params(user, apply = None , applicable = None): +def get_params(user, apply_to_all = None , applicable = None): ''' Return param to insert ''' param = { "user": user.name, "doctype":"User", "docname":user.name } - if apply: + if apply_to_all: param.update({"apply_to_all_doctypes": 1}) param.update({"applicable_doctypes": []}) if applicable: diff --git a/frappe/core/doctype/user_permission/user_permission.py b/frappe/core/doctype/user_permission/user_permission.py index 4dd152b54e..4f20e93a4e 100644 --- a/frappe/core/doctype/user_permission/user_permission.py +++ b/frappe/core/doctype/user_permission/user_permission.py @@ -158,12 +158,17 @@ def add_user_permissions(data): data = frappe._dict(data) d = check_applicable_doc_perm(data.user, data.doctype, data.docname) - exists = frappe.db.exists("User Permission", {"user": data.user, "allow": data.doctype, "for_value": data.docname, "apply_to_all_doctypes": 1}) + exists = frappe.db.exists("User Permission", { + "user": data.user, + "allow": data.doctype, + "for_value": data.docname, + "apply_to_all_doctypes": 1 + }) if data.apply_to_all_doctypes == 1 and not exists: remove_applicable(d, data.user, data.doctype, data.docname) insert_user_perm(data.user, data.doctype, data.docname, apply_to_all = 1) return 1 - else: + elif len(data.applicable_doctypes) > 0 and data.apply_to_all_doctypes != 1: remove_apply_to_all(data.user, data.doctype, data.docname) update_applicable(d, data.applicable_doctypes, data.user, data.doctype, data.docname) for applicable in data.applicable_doctypes :