Merge pull request #15521 from gavindsouza/owner-field-change

fix: Remove internal owner field used in existing tables

frappe/automation/doctype/assignment_rule/assignment_rule.py

@@ -109,7 +109,7 @@ class AssignmentRule(Document):
 				user = d.user,
 				count = frappe.db.count('ToDo', dict(
 					reference_type = self.document_type,
-					owner = d.user,
+					allocated_to = d.user,
 					status = "Open"))
 			))
 

frappe/automation/doctype/assignment_rule/test_assignment_rule.py

@@ -30,7 +30,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), 'test@example.com')
+		), 'allocated_to'), 'test@example.com')
 
 		note = make_note(dict(public=1))
 
@@ -39,7 +39,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), 'test1@example.com')
+		), 'allocated_to'), 'test1@example.com')
 
 		clear_assignments()
 
@@ -51,7 +51,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), 'test2@example.com')
+		), 'allocated_to'), 'test2@example.com')
 
 		# check loop back to first user
 		note = make_note(dict(public=1))
@@ -60,7 +60,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), 'test@example.com')
+		), 'allocated_to'), 'test@example.com')
 
 	def test_load_balancing(self):
 		self.assignment_rule.rule = 'Load Balancing'
@@ -71,11 +71,11 @@ class TestAutoAssign(unittest.TestCase):
 
 		# check if each user has 10 assignments (?)
 		for user in ('test@example.com', 'test1@example.com', 'test2@example.com'):
-			self.assertEqual(len(frappe.get_all('ToDo', dict(owner = user, reference_type = 'Note'))), 10)
+			self.assertEqual(len(frappe.get_all('ToDo', dict(allocated_to = user, reference_type = 'Note'))), 10)
 
 		# clear 5 assignments for first user
 		# can't do a limit in "delete" since postgres does not support it
-		for d in frappe.get_all('ToDo', dict(reference_type = 'Note', owner = 'test@example.com'), limit=5):
+		for d in frappe.get_all('ToDo', dict(reference_type = 'Note', allocated_to = 'test@example.com'), limit=5):
 			frappe.db.delete("ToDo", {"name": d.name})
 
 		# add 5 more assignments
@@ -84,7 +84,7 @@ class TestAutoAssign(unittest.TestCase):
 
 		# check if each user still has 10 assignments
 		for user in ('test@example.com', 'test1@example.com', 'test2@example.com'):
-			self.assertEqual(len(frappe.get_all('ToDo', dict(owner = user, reference_type = 'Note'))), 10)
+			self.assertEqual(len(frappe.get_all('ToDo', dict(allocated_to = user, reference_type = 'Note'))), 10)
 
 	def test_based_on_field(self):
 		self.assignment_rule.rule = 'Based on Field'
@@ -119,7 +119,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), None)
+		), 'allocated_to'), None)
 
 	def test_clear_assignment(self):
 		note = make_note(dict(public=1))
@@ -132,7 +132,7 @@ class TestAutoAssign(unittest.TestCase):
 		))[0]
 
 		todo = frappe.get_doc('ToDo', todo['name'])
-		self.assertEqual(todo.owner, 'test@example.com')
+		self.assertEqual(todo.allocated_to, 'test@example.com')
 
 		# test auto unassign
 		note.public = 0
@@ -154,7 +154,7 @@ class TestAutoAssign(unittest.TestCase):
 		))[0]
 
 		todo = frappe.get_doc('ToDo', todo['name'])
-		self.assertEqual(todo.owner, 'test@example.com')
+		self.assertEqual(todo.allocated_to, 'test@example.com')
 
 		note.content="Closed"
 		note.save()
@@ -164,7 +164,7 @@ class TestAutoAssign(unittest.TestCase):
 		# check if todo is closed
 		self.assertEqual(todo.status, 'Closed')
 		# check if closed todo retained assignment
-		self.assertEqual(todo.owner, 'test@example.com')
+		self.assertEqual(todo.allocated_to, 'test@example.com')
 
 	def check_multiple_rules(self):
 		note = make_note(dict(public=1, notify_on_login=1))
@@ -174,7 +174,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), 'test@example.com')
+		), 'allocated_to'), 'test@example.com')
 
 	def check_assignment_rule_scheduling(self):
 		frappe.db.delete("Assignment Rule")
@@ -192,7 +192,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), ['test@example.com', 'test1@example.com', 'test2@example.com'])
+		), 'allocated_to'), ['test@example.com', 'test1@example.com', 'test2@example.com'])
 
 		frappe.flags.assignment_day = "Friday"
 		note = make_note(dict(public=1))
@@ -201,7 +201,7 @@ class TestAutoAssign(unittest.TestCase):
 			reference_type = 'Note',
 			reference_name = note.name,
 			status = 'Open'
-		), 'owner'), ['test3@example.com'])
+		), 'allocated_to'), ['test3@example.com'])
 
 	def test_assignment_rule_condition(self):
 		frappe.db.delete("Assignment Rule")

frappe/core/doctype/file/test_file.py

@@ -18,6 +18,7 @@ test_content2 = 'Hello World'
 def make_test_doc():
 	d = frappe.new_doc('ToDo')
 	d.description = 'Test'
+	d.assigned_by = frappe.session.user
 	d.save()
 	return d.doctype, d.name
 

frappe/core/doctype/user/test_user.py

@@ -70,7 +70,7 @@ class TestUser(unittest.TestCase):
 		delete_contact("_test@example.com")
 		delete_doc("User", "_test@example.com")
 
-		self.assertTrue(not frappe.db.sql("""select * from `tabToDo` where owner=%s""",
+		self.assertTrue(not frappe.db.sql("""select * from `tabToDo` where allocated_to=%s""",
 			("_test@example.com",)))
 
 		from frappe.core.doctype.role.test_role import test_records as role_records

frappe/core/doctype/user/user.json

@@ -660,7 +660,7 @@
   {
    "group": "Activity",
    "link_doctype": "ToDo",
-   "link_fieldname": "owner"
+   "link_fieldname": "allocated_to"
   },
   {
    "group": "Integrations",

frappe/core/doctype/user/user.py

@@ -363,7 +363,7 @@ class User(Document):
 			frappe.local.login_manager.logout(user=self.name)
 
 		# delete todos
-		frappe.db.delete("ToDo", {"owner": self.name})
+		frappe.db.delete("ToDo", {"allocated_to": self.name})
 		todo_table = DocType("ToDo")
 		(
 			frappe.qb.update(todo_table)

frappe/core/notifications.py

@@ -23,7 +23,7 @@ def get_things_todo(as_list=False):
 	data = frappe.get_list("ToDo",
 		fields=["name", "description"] if as_list else "count(*)",
 		filters=[["ToDo", "status", "=", "Open"]],
-		or_filters=[["ToDo", "owner", "=", frappe.session.user],
+		or_filters=[["ToDo", "allocated_to", "=", frappe.session.user],
 			["ToDo", "assigned_by", "=", frappe.session.user]],
 		as_list=True)
 

frappe/desk/doctype/event/test_event.py

@@ -93,7 +93,7 @@ class TestEvent(unittest.TestCase):
 
 		# Remove an assignment
 		todo = frappe.get_doc("ToDo", {"reference_type": ev.doctype, "reference_name": ev.name,
-			"owner": self.test_user})
+			"allocated_to": self.test_user})
 		todo.status = "Cancelled"
 		todo.save()
 

frappe/desk/doctype/todo/todo.json

@@ -13,7 +13,7 @@
   "column_break_2",
   "color",
   "date",
-  "owner",
+  "allocated_to",
   "description_section",
   "description",
   "section_break_6",
@@ -69,15 +69,6 @@
    "oldfieldname": "date",
    "oldfieldtype": "Date"
   },
-  {
-   "fieldname": "owner",
-   "fieldtype": "Link",
-   "ignore_user_permissions": 1,
-   "in_global_search": 1,
-   "in_standard_filter": 1,
-   "label": "Allocated To",
-   "options": "User"
-  },
   {
    "fieldname": "description_section",
    "fieldtype": "Section Break"
@@ -153,12 +144,21 @@
    "label": "Assignment Rule",
    "options": "Assignment Rule",
    "read_only": 1
+  },
+  {
+   "fieldname": "allocated_to",
+   "fieldtype": "Link",
+   "ignore_user_permissions": 1,
+   "in_global_search": 1,
+   "in_standard_filter": 1,
+   "label": "Allocated To",
+   "options": "User"
   }
  ],
  "icon": "fa fa-check",
  "idx": 2,
  "links": [],
- "modified": "2020-01-14 17:04:36.971002",
+ "modified": "2021-09-16 11:36:34.586898",
  "modified_by": "Administrator",
  "module": "Desk",
  "name": "ToDo",

frappe/desk/doctype/todo/todo.py

@@ -16,10 +16,10 @@ class ToDo(Document):
 		self._assignment = None
 		if self.is_new():
 
-			if self.assigned_by == self.owner:
+			if self.assigned_by == self.allocated_to:
 				assignment_message = frappe._("{0} self assigned this task: {1}").format(get_fullname(self.assigned_by), self.description)
 			else:
-				assignment_message = frappe._("{0} assigned {1}: {2}").format(get_fullname(self.assigned_by), get_fullname(self.owner), self.description)
+				assignment_message = frappe._("{0} assigned {1}: {2}").format(get_fullname(self.assigned_by), get_fullname(self.allocated_to), self.description)
 
 			self._assignment = {
 				"text": assignment_message,
@@ -29,12 +29,12 @@ class ToDo(Document):
 		else:
 			# NOTE the previous value is only available in validate method
 			if self.get_db_value("status") != self.status:
-				if self.owner == frappe.session.user:
+				if self.allocated_to == frappe.session.user:
 					removal_message = frappe._("{0} removed their assignment.").format(
 						get_fullname(frappe.session.user))
 				else:
 					removal_message = frappe._("Assignment of {0} removed by {1}").format(
-						get_fullname(self.owner), get_fullname(frappe.session.user))
+						get_fullname(self.allocated_to), get_fullname(frappe.session.user))
 
 				self._assignment = {
 					"text": removal_message,
@@ -75,7 +75,7 @@ class ToDo(Document):
 					"reference_name": self.reference_name,
 					"status": ("!=", "Cancelled")
 				},
-				fields=["owner"], as_list=True)]
+				fields=["allocated_to"], as_list=True)]
 
 			assignments.reverse()
 			frappe.db.set_value(self.reference_type, self.reference_name,
@@ -98,8 +98,8 @@ class ToDo(Document):
 	def get_owners(cls, filters=None):
 		"""Returns list of owners after applying filters on todo's.
 		"""
-		rows = frappe.get_all(cls.DocType, filters=filters or {}, fields=['owner'])
-		return [parse_addr(row.owner)[1] for row in rows if row.owner]
+		rows = frappe.get_all(cls.DocType, filters=filters or {}, fields=['allocated_to'])
+		return [parse_addr(row.allocated_to)[1] for row in rows if row.allocated_to]
 
 # NOTE: todo is viewable if a user is an owner, or set as assigned_to value, or has any role that is allowed to access ToDo doctype.
 def on_doctype_update():
@@ -115,7 +115,7 @@ def get_permission_query_conditions(user):
 	if any(check in todo_roles for check in frappe.get_roles(user)):
 		return None
 	else:
-		return """(`tabToDo`.owner = {user} or `tabToDo`.assigned_by = {user})"""\
+		return """(`tabToDo`.allocated_to = {user} or `tabToDo`.assigned_by = {user})"""\
 			.format(user=frappe.db.escape(user))
 
 def has_permission(doc, ptype="read", user=None):
@@ -127,7 +127,7 @@ def has_permission(doc, ptype="read", user=None):
 	if any(check in todo_roles for check in frappe.get_roles(user)):
 		return True
 	else:
-		return doc.owner==user or doc.assigned_by==user
+		return doc.allocated_to==user or doc.assigned_by==user
 
 @frappe.whitelist()
 def new_todo(description):

frappe/desk/form/assign_to.py

@@ -19,7 +19,7 @@ def get(args=None):
 	if not args:
 		args = frappe.local.form_dict
 
-	return frappe.get_all('ToDo', fields=['owner', 'name'], filters=dict(
+	return frappe.get_all('ToDo', fields=['allocated_to', 'name'], filters=dict(
 		reference_type = args.get('doctype'),
 		reference_name = args.get('name'),
 		status = ('!=', 'Cancelled')
@@ -48,7 +48,7 @@ def add(args=None):
 			"reference_type": args['doctype'],
 			"reference_name": args['name'],
 			"status": "Open",
-			"owner": assign_to
+			"allocated_to": assign_to
 		}
 
 		if frappe.get_all("ToDo", filters=filters):
@@ -61,7 +61,7 @@ def add(args=None):
 
 			d = frappe.get_doc({
 				"doctype": "ToDo",
-				"owner": assign_to,
+				"allocated_to": assign_to,
 				"reference_type": args['doctype'],
 				"reference_name": args['name'],
 				"description": args.get('description'),
@@ -87,7 +87,7 @@ def add(args=None):
 			follow_document(args['doctype'], args['name'], assign_to)
 
 			# notify
-			notify_assignment(d.assigned_by, d.owner, d.reference_type, d.reference_name, action='ASSIGN',
+			notify_assignment(d.assigned_by, d.allocated_to, d.reference_type, d.reference_name, action='ASSIGN',
 				description=args.get("description"))
 
 	if shared_with_users:
@@ -112,13 +112,13 @@ def add_multiple(args=None):
 		add(args)
 
 def close_all_assignments(doctype, name):
-	assignments = frappe.db.get_all('ToDo', fields=['owner'], filters =
+	assignments = frappe.db.get_all('ToDo', fields=['allocated_to'], filters =
 		dict(reference_type = doctype, reference_name = name, status=('!=', 'Cancelled')))
 	if not assignments:
 		return False
 
 	for assign_to in assignments:
-		set_status(doctype, name, assign_to.owner, status="Closed")
+		set_status(doctype, name, assign_to.allocated_to, status="Closed")
 
 	return True
 
@@ -130,13 +130,13 @@ def set_status(doctype, name, assign_to, status="Cancelled"):
 	"""remove from todo"""
 	try:
 		todo = frappe.db.get_value("ToDo", {"reference_type":doctype,
-		"reference_name":name, "owner":assign_to, "status": ('!=', status)})
+		"reference_name":name, "allocated_to":assign_to, "status": ('!=', status)})
 		if todo:
 			todo = frappe.get_doc("ToDo", todo)
 			todo.status = status
 			todo.save(ignore_permissions=True)
 
-			notify_assignment(todo.assigned_by, todo.owner, todo.reference_type, todo.reference_name)
+			notify_assignment(todo.assigned_by, todo.allocated_to, todo.reference_type, todo.reference_name)
 	except frappe.DoesNotExistError:
 		pass
 
@@ -150,25 +150,26 @@ def clear(doctype, name):
 	'''
 	Clears assignments, return False if not assigned.
 	'''
-	assignments = frappe.db.get_all('ToDo', fields=['owner'], filters =
+	assignments = frappe.db.get_all('ToDo', fields=['allocated_to'], filters =
 		dict(reference_type = doctype, reference_name = name))
 	if not assignments:
 		return False
 
 	for assign_to in assignments:
-		set_status(doctype, name, assign_to.owner, "Cancelled")
+		set_status(doctype, name, assign_to.allocated_to, "Cancelled")
 
 	return True
 
-def notify_assignment(assigned_by, owner, doc_type, doc_name, action='CLOSE',
+def notify_assignment(assigned_by, allocated_to, doc_type, doc_name, action='CLOSE',
 	description=None):
 	"""
 		Notify assignee that there is a change in assignment
 	"""
-	if not (assigned_by and owner and doc_type and doc_name): return
+	if not (assigned_by and allocated_to and doc_type and doc_name):
+		return
 
 	# return if self assigned or user disabled
-	if assigned_by == owner or not frappe.db.get_value('User', owner, 'enabled'):
+	if assigned_by == allocated_to or not frappe.db.get_value('User', allocated_to, 'enabled'):
 		return
 
 	# Search for email address in description -- i.e. assignee
@@ -194,7 +195,7 @@ def notify_assignment(assigned_by, owner, doc_type, doc_name, action='CLOSE',
 		'email_content': description_html
 	}
 
-	enqueue_create_notification(owner, notification_doc)
+	enqueue_create_notification(allocated_to, notification_doc)
 
 def format_message_for_assign_to(users):
 	return "<br><br>" + "<br>".join(users)

frappe/desk/listview.py

@@ -29,16 +29,16 @@ def get_group_by_count(doctype, current_filters, field):
 	subquery = frappe.get_all(doctype, filters=current_filters, run=False)
 	if field == 'assigned_to':
 		subquery_condition = ' and `tabToDo`.reference_name in ({subquery})'.format(subquery = subquery)
-		return frappe.db.sql("""select `tabToDo`.owner as name, count(*) as count
+		return frappe.db.sql("""select `tabToDo`.allocated_to as name, count(*) as count
 			from
 				`tabToDo`, `tabUser`
 			where
 				`tabToDo`.status!='Cancelled' and
-				`tabToDo`.owner = `tabUser`.name and
+				`tabToDo`.allocated_to = `tabUser`.name and
 				`tabUser`.user_type = 'System User'
 				{subquery_condition}
 			group by
-				`tabToDo`.owner
+				`tabToDo`.allocated_to
 			order by
 				count desc
 			limit 50""".format(subquery_condition = subquery_condition), as_dict=True)

frappe/email/doctype/email_template/email_template.json

@@ -12,7 +12,6 @@
   "use_html",
   "response_html",
   "response",
-  "owner",
   "section_break_4",
   "email_reply_help"
  ],
@@ -32,14 +31,6 @@
    "label": "Response",
    "mandatory_depends_on": "eval:!doc.use_html"
   },
-  {
-   "default": "user",
-   "fieldname": "owner",
-   "fieldtype": "Link",
-   "hidden": 1,
-   "label": "Owner",
-   "options": "User"
-  },
   {
    "fieldname": "section_break_4",
    "fieldtype": "Section Break"
@@ -66,7 +57,7 @@
  ],
  "icon": "fa fa-comment",
  "links": [],
- "modified": "2020-11-30 14:12:50.321633",
+ "modified": "2022-01-04 14:12:50.321633",
  "modified_by": "Administrator",
  "module": "Email",
  "name": "Email Template",

frappe/email/doctype/notification/notification.py

@@ -435,8 +435,8 @@ def get_context(doc):
 def get_assignees(doc):
 	assignees = []
 	assignees = frappe.get_all('ToDo', filters={'status': 'Open', 'reference_name': doc.name,
-		'reference_type': doc.doctype}, fields=['owner'])
+		'reference_type': doc.doctype}, fields=['allocated_to'])
 
-	recipients = [d.owner for d in assignees]
+	recipients = [d.allocated_to for d in assignees]
 
 	return recipients

frappe/model/document.py

@@ -470,8 +470,8 @@ class Document(BaseDocument):
 		self.modified_by = frappe.session.user
 		if not self.creation:
 			self.creation = self.modified
-		if not self.owner:
-			self.owner = self.modified_by
+		if self.is_new():
+			self.owner = self.flags.owner or self.modified_by
 
 		for d in self.get_all_children():
 			d.modified = self.modified
@@ -501,6 +501,7 @@ class Document(BaseDocument):
 		self._sanitize_content()
 		self._save_passwords()
 		self.validate_workflow()
+		self.validate_owner()
 
 		children = self.get_all_children()
 		for d in children:
@@ -543,6 +544,11 @@ class Document(BaseDocument):
 			if not self._action == 'save':
 				set_workflow_state_on_action(self, workflow, self._action)
 
+	def validate_owner(self):
+		"""Validate if the owner of the Document has changed"""
+		if not self.is_new() and self.has_value_changed('owner'):
+			frappe.throw(_('Document owner cannot be changed'))
+
 	def validate_set_only_once(self):
 		"""Validate that fields are not changed if not in insert"""
 		set_only_once_fields = self.meta.get_set_only_once_fields()
@@ -1341,15 +1347,15 @@ class Document(BaseDocument):
 			), frappe.exceptions.InvalidDates)
 
 	def get_assigned_users(self):
-		assignments = frappe.get_all('ToDo',
-			fields=['owner'],
+		assigned_users = frappe.get_all('ToDo',
+			fields=['allocated_to'],
 			filters={
 				'reference_type': self.doctype,
 				'reference_name': self.name,
 				'status': ('!=', 'Cancelled'),
-			})
+			}, pluck='allocated_to')
 
-		users = set([assignment.owner for assignment in assignments])
+		users = set(assigned_users)
 		return users
 
 	def add_tag(self, tag):

frappe/patches.txt

@@ -190,3 +190,4 @@ frappe.patches.v14_0.update_github_endpoints #08-11-2021
 frappe.patches.v14_0.remove_db_aggregation
 frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021
 frappe.patches.v14_0.update_color_names_in_kanban_board_column
+frappe.patches.v14_0.transform_todo_schema

frappe/patches/v14_0/transform_todo_schema.py

@@ -0,0 +1,12 @@
+import frappe
+from frappe.query_builder.utils import DocType
+
+
+def execute():
+	# Email Template & Help Article have owner field that doesn't have any additional functionality
+	# Only ToDo has to be updated.
+
+	ToDo = DocType("ToDo")
+	frappe.reload_doctype("ToDo", force=True)
+
+	frappe.qb.update(ToDo).set(ToDo.allocated_to, ToDo.owner).run()

frappe/tests/test_assign.py

@@ -13,7 +13,7 @@ class TestAssign(unittest.TestCase):
 
 		added = assign(todo, "test@example.com")
 
-		self.assertTrue("test@example.com" in [d.owner for d in added])
+		self.assertTrue("test@example.com" in [d.allocated_to for d in added])
 
 		removed = frappe.desk.form.assign_to.remove(todo.doctype, todo.name, "test@example.com")
 

frappe/tests/test_document.py

@@ -252,3 +252,22 @@ class TestDocument(unittest.TestCase):
 			'currency': 100000
 		})
 		self.assertEquals(d.get_formatted('currency', currency='INR', format="#,###.##"), '₹ 100,000.00')
+
+	def test_owner_changed(self):
+		frappe.delete_doc_if_exists("User", "hello@example.com")
+		frappe.set_user("Administrator")
+
+		d = frappe.get_doc({
+			"doctype": "User",
+			"email": "hello@example.com",
+			"first_name": "John"
+		})
+		d.insert()
+		self.assertEqual(frappe.db.get_value("User", d.owner), d.owner)
+
+		d.set("owner", "johndoe@gmail.com")
+		self.assertRaises(frappe.ValidationError, d.save)
+
+		d.reload()
+		d.save()
+		self.assertEqual(frappe.db.get_value("User", d.owner), d.owner)

frappe/website/doctype/help_article/help_article.json

@@ -15,8 +15,7 @@
   "section_break_7",
   "content",
   "likes",
-  "route",
-  "owner"
+  "route"
  ],
  "fields": [
   {
@@ -79,13 +78,6 @@
    "fieldtype": "Data",
    "in_global_search": 1,
    "label": "Route"
-  },
-  {
-   "default": "user",
-   "fieldname": "owner",
-   "fieldtype": "Link",
-   "label": "Owner",
-   "options": "User"
   }
  ],
  "has_web_view": 1,
@@ -93,7 +85,7 @@
  "index_web_pages_for_search": 1,
  "is_published_field": "published",
  "links": [],
- "modified": "2020-07-21 16:25:18.577325",
+ "modified": "2022-01-04 16:25:18.577325",
  "modified_by": "Administrator",
  "module": "Website",
  "name": "Help Article",