From 86f37dc0ea4e02d811e062401428894228ec50ca Mon Sep 17 00:00:00 2001
From: Anurag Mishra <mishranaman123@gmail.com>
Date: Wed, 6 Feb 2019 12:34:10 +0530
Subject: [PATCH] Code refractor

---
 .../user_permission/test_user_permission.py   | 85 ++++++++-----------
 .../user_permission/user_permission_list.js   |  4 +-
 2 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py
index a974aadb19..f002664eea 100644
--- a/frappe/core/doctype/user_permission/test_user_permission.py
+++ b/frappe/core/doctype/user_permission/test_user_permission.py
@@ -10,40 +10,19 @@ import unittest
 class TestUserPermission(unittest.TestCase):
 	def test_apply_to_all(self):
 		user = get_user()
-		created = add_user_permissions({
-			"user": user.name,
-			"doctype":"User",
-			"docname":user.name ,
-			"apply_to_all_doctypes":1})
+		param = get_params(user, apply = 1)
+		created = add_user_permissions(param)
 		self.assertEquals(created, 1)
 
 	def test_for_applicables_on_update_from_apply_to_all(self):
 		user = get_user()
-		create = add_user_permissions({
-			"user": user.name,
-			"doctype":"User",
-			"docname":user.name ,
-			"apply_to_all_doctypes":0,
-			"applicable_doctypes":["Chat Room","Chat Message"]})
+		param = get_params(user, applicable = ["Chat Room", "Chat Message"])
+		create = add_user_permissions(param)
 		frappe.db.commit()
 
-		removed_apply_to_all = frappe.db.exists("User Permission", {
-			"user": user.name,
-			"allow": "User",
-			"for_value": user.name,
-			"apply_to_all_doctypes": 1})
-		created_applicable_first = frappe.db.exists("User Permission", {
-			"user": user.name,
-			"allow": "User",
-			"for_value": user.name,
-			"apply_to_all_doctypes": 0,
-			"applicable_for": "Chat Room"})
-		created_applicable_second = frappe.db.exists("User Permission", {
-			"user": user.name,
-			"allow": "User",
-			"for_value": user.name,
-			"apply_to_all_doctypes": 0,
-			"applicable_for": "Chat Message"})
+		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"))
 
 		self.assertIsNone(removed_apply_to_all)
 		self.assertIsNotNone(created_applicable_first)
@@ -52,27 +31,11 @@ class TestUserPermission(unittest.TestCase):
 
 	def test_for_apply_to_all_on_update_from_applicables(self):
 		user = get_user()
-		created = add_user_permissions({
-			"user": user.name,
-			"doctype":"User",
-			"docname":user.name ,
-			"apply_to_all_doctypes":1})
-		created_apply_to_all = frappe.db.exists("User Permission", {
-			"user": user.name,
-			"allow": "User",
-			"for_value": user.name,
-			"apply_to_all_doctypes": 1})
-		removed_applicable_first = frappe.db.exists("User Permission", {
-			"user": user.name,
-			"allow": "User",
-			"for_value": user.name,
-			"apply_to_all_doctypes": 0,
-			"applicable_for": "Chat Room"})
-		removed_applicable_second = frappe.db.exists("User Permission", {
-			"user": user.name, "allow": "User",
-			"for_value": user.name,
-			"apply_to_all_doctypes": 0,
-			"applicable_for": "Chat Message"})
+		param = get_params(user, apply = 1)
+		created = add_user_permissions(param)
+		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"))
 
 
 		self.assertIsNotNone(created_apply_to_all)
@@ -90,4 +53,28 @@ def get_user():
 		user.add_roles("System Manager")
 		return user
 
+def get_params(user, apply = None , applicable=  None):
+	param = {
+		"user": user.name,
+		"doctype":"User",
+		"docname":user.name
+	}
+	if apply:
+		param.update({"apply_to_all_doctypes": 1})
+		param.update({"applicable_doctypes": []})
+	if applicable:
+		param.update({"apply_to_all_doctypes": 0})
+		param.update({"applicable_doctypes": applicable})
+	return param
 
+def get_exists_param(user, applicable = None):
+	param = {
+		"user": user.name,
+		"allow": "User",
+		"for_value": user.name,
+	}
+	if applicable:
+		param.update({"applicable_for": applicable})
+	else:
+		param.update({"apply_to_all_doctypes": 1})
+	return param
diff --git a/frappe/core/doctype/user_permission/user_permission_list.js b/frappe/core/doctype/user_permission/user_permission_list.js
index d25e9be674..b5b94891a0 100644
--- a/frappe/core/doctype/user_permission/user_permission_list.js
+++ b/frappe/core/doctype/user_permission/user_permission_list.js
@@ -3,7 +3,7 @@ frappe.listview_settings['User Permission'] = {
 	onload: function(list_view) {
 		var me = this;
 		list_view.page.add_inner_button( __("Create/Update User Permissions"), function() {
-			var dialog =new frappe.ui.Dialog({
+			let dialog =new frappe.ui.Dialog({
 				title : __('Add User Permissions'),
 				fields: [
 					{
@@ -208,7 +208,7 @@ frappe.listview_settings['User Permission'] = {
 			dialog.set_primary_action("Update");
 			dialog.set_title("Update User Permissions");
 			dialog.set_df_property("applicable_doctypes", "options", options);
-			if(dialog.fields_dict.applicable_doctypes.get_checked_options().length == options.length){
+			if(dialog.fields_dict.applicable_doctypes.get_checked_options().length == options.length) {
 				dialog.set_df_property("applicable_doctypes", "hidden", 1);
 				dialog.set_value("apply_to_all_doctypes", "checked", 1);
 			} else {