Merge branch 'develop' into assignment_rule

frappe/core/doctype/role/role.py

@@ -22,16 +22,28 @@ class Role(Document):
 				frappe.db.sql("delete from `tabHas Role` where role = %s", self.name)
 				frappe.clear_cache()
 
+	def on_update(self):
+		'''update system user desk access if this has changed in this update'''
+		if frappe.flags.in_install: return
+		if self.has_value_changed('desk_access'):
+			for user_name in get_users(self.name):
+				user = frappe.get_doc('User', user_name)
+				user_type = user.user_type
+				user.set_system_user()
+				if user_type != user.user_type:
+					user.save()
+
 # Get email addresses of all users that have been assigned this role
 def get_emails_from_role(role):
 	emails = []
 
-	users = frappe.get_list("Has Role", filters={"role": role, "parenttype": "User"},
-		fields=["parent"])
-
-	for user in users:
-		user_email, enabled = frappe.db.get_value("User", user.parent, ["email", "enabled"])
+	for user in get_users(role):
+		user_email, enabled = frappe.db.get_value("User", user, ["email", "enabled"])
 		if enabled and user_email not in ["admin@example.com", "guest@example.com"]:
 			emails.append(user_email)
 
-	return emails
+	return emails
+
+def get_users(role):
+	return [d.parent for d in frappe.get_all("Has Role", filters={"role": role, "parenttype": "User"},
+		fields=["parent"])]

frappe/core/doctype/role/test_role.py

@@ -23,3 +23,28 @@ class TestUser(unittest.TestCase):
 
 		frappe.get_doc("User", "test@example.com").add_roles("_Test Role 3")
 		self.assertTrue("_Test Role 3" in frappe.get_roles("test@example.com"))
+
+	def test_change_desk_access(self):
+		'''if we change desk acecss from role, remove from user'''
+		frappe.delete_doc_if_exists('User', 'test-user-for-desk-access@example.com')
+		frappe.delete_doc_if_exists('Role', 'desk-access-test')
+		user = frappe.get_doc(dict(
+			doctype='User',
+			email='test-user-for-desk-access@example.com',
+			first_name='test')).insert()
+		role = frappe.get_doc(dict(
+			doctype = 'Role',
+			role_name = 'desk-access-test',
+			desk_access = 0
+		)).insert()
+		user.add_roles(role.name)
+		user.save()
+		self.assertTrue(user.user_type=='Website User')
+		role.desk_access = 1
+		role.save()
+		user.reload()
+		self.assertTrue(user.user_type=='System User')
+		role.desk_access = 0
+		role.save()
+		user.reload()
+		self.assertTrue(user.user_type=='Website User')

frappe/model/document.py

@@ -396,6 +396,11 @@ class Document(BaseDocument):
 	def get_doc_before_save(self):
 		return getattr(self, '_doc_before_save', None)
 
+	def has_value_changed(self, fieldname):
+		'''Returns true if value is changed before and after saving'''
+		previous = self.get_doc_before_save()
+		return previous.get(fieldname)!=self.get(fieldname) if previous else True
+
 	def set_new_name(self, force=False, set_name=None, set_child_names=True):
 		"""Calls `frappe.naming.set_new_name` for parent and child docs."""
 		if self.flags.name_set and not force:

frappe/tests/test_document.py

@@ -66,6 +66,13 @@ class TestDocument(unittest.TestCase):
 
 		self.assertEqual(frappe.db.get_value(d.doctype, d.name, "subject"), "subject changed")
 
+	def test_value_changed(self):
+		d = self.test_insert()
+		d.subject = "subject changed again"
+		d.save()
+		self.assertTrue(d.has_value_changed('subject'))
+		self.assertFalse(d.has_value_changed('event_type'))
+
 	def test_mandatory(self):
 		# TODO: recheck if it is OK to force delete
 		frappe.delete_doc_if_exists("User", "test_mandatory@example.com", 1)