BREAKING CHANGE: Comments are now handled separately from Communication and Version

frappe/app.py

@@ -22,7 +22,7 @@ import frappe.website.render
 from frappe.utils import get_site_name
 from frappe.middlewares import StaticDataMiddleware
 from frappe.utils.error import make_error_snapshot
-from frappe.core.doctype.communication.comment import update_comments_in_parent_after_request
+from frappe.core.doctype.comment.comment import update_comments_in_parent_after_request
 from frappe import _
 
 local_manager = LocalManager([frappe.local])

frappe/core/doctype/activity_log/feed.py

@@ -56,10 +56,13 @@ def logout_feed(user, reason):
 		subject = _("{0} logged out: {1}").format(get_fullname(user), frappe.bold(reason))
 		add_authentication_log(subject, user, operation="Logout")
 
-def get_feed_match_conditions(user=None, force=True):
+def get_feed_match_conditions(user=None, doctype='Comment'):
 	if not user: user = frappe.session.user
 
-	conditions = ['`tabCommunication`.owner={user} or `tabCommunication`.reference_owner={user}'.format(user=frappe.db.escape(user))]
+	conditions = ['`tab{doctype}`.owner={user} or `tab{doctype}`.reference_owner={user}'.format(
+			user = frappe.db.escape(user),
+			doctype = doctype
+		)]
 
 	user_permissions = frappe.permissions.get_user_permissions(user)
 	can_read = frappe.get_user().get_can_read()
@@ -68,9 +71,13 @@ def get_feed_match_conditions(user=None, force=True):
 		list(set(can_read) - set(list(user_permissions)))]
 
 	if can_read_doctypes:
-		conditions += ["""(`tabCommunication`.reference_doctype is null
-			or `tabCommunication`.reference_doctype = ''
-			or `tabCommunication`.reference_doctype in ({}))""".format(", ".join(can_read_doctypes))]
+		conditions += ["""(`tab{doctype}`.reference_doctype is null
+			or `tab{doctype}`.reference_doctype = ''
+			or `tab{doctype}`.reference_doctype
+			in ({values}))""".format(
+				doctype = doctype,
+				values =", ".join(can_read_doctypes)
+			)]
 
 		if user_permissions:
 			can_read_docs = []
@@ -79,7 +86,8 @@ def get_feed_match_conditions(user=None, force=True):
 					can_read_docs.append('{}|{}'.format(doctype, frappe.db.escape(n.get('doc', ''))))
 
 			if can_read_docs:
-				conditions.append("concat_ws('|', `tabCommunication`.reference_doctype, `tabCommunication`.reference_name) in ({})".format(
-					", ".join(can_read_docs)))
+				conditions.append("concat_ws('|', `tab{doctype}`.reference_doctype, `tab{doctype}`.reference_name) in ({values})".format(
+					doctype = doctype,
+					values = ", ".join(can_read_docs)))
 
-	return "(" + " or ".join(conditions) + ")"
+	return  "(" + " or ".join(conditions) + ")"

frappe/core/doctype/comment/comment.js

@@ -0,0 +1,8 @@
+// Copyright (c) 2019, Frappe Technologies and contributors
+// For license information, please see license.txt
+
+frappe.ui.form.on('Comment', {
+	refresh: function(frm) {
+
+	}
+});

frappe/core/doctype/comment/comment.json

@@ -0,0 +1,535 @@
+{
+ "allow_copy": 0,
+ "allow_events_in_timeline": 0,
+ "allow_guest_to_view": 0,
+ "allow_import": 0,
+ "allow_rename": 0,
+ "beta": 0,
+ "creation": "2019-02-07 10:10:46.845678",
+ "custom": 0,
+ "docstatus": 0,
+ "doctype": "DocType",
+ "document_type": "",
+ "editable_grid": 1,
+ "engine": "InnoDB",
+ "fields": [
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "default": "Comment",
+   "fieldname": "comment_type",
+   "fieldtype": "Data",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Comment Type",
+   "length": 0,
+   "no_copy": 0,
+   "options": "Comment\nLike\nInfo\nLabel\nWorkflow\nCreated\nSubmitted\nCancelled\nUpdated\nDeleted\nAssigned\nAssignment Completed\nAttachment\nAttachment Removed\nShared\nUnshared\nBot\nRelinked",
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 1,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "comment_email",
+   "fieldtype": "Data",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 1,
+   "in_standard_filter": 0,
+   "label": "Comment Email",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "subject",
+   "fieldtype": "Data",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Subject",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "comment_by",
+   "fieldtype": "Data",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 1,
+   "in_standard_filter": 0,
+   "label": "Comment By",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "published",
+   "fieldtype": "Check",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Published",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "seen",
+   "fieldtype": "Check",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Seen",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "column_break_5",
+   "fieldtype": "Column Break",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "reference_doctype",
+   "fieldtype": "Link",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 1,
+   "in_standard_filter": 0,
+   "label": "Reference Document Type",
+   "length": 0,
+   "no_copy": 0,
+   "options": "DocType",
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "reference_name",
+   "fieldtype": "Dynamic Link",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Reference Name",
+   "length": 0,
+   "no_copy": 0,
+   "options": "reference_doctype",
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "link_doctype",
+   "fieldtype": "Link",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Link DocType",
+   "length": 0,
+   "no_copy": 0,
+   "options": "DocType",
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "link_name",
+   "fieldtype": "Dynamic Link",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Link Name",
+   "length": 0,
+   "no_copy": 0,
+   "options": "link_doctype",
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "reference_owner",
+   "fieldtype": "Data",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "label": "Reference Owner",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 1,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "section_break_10",
+   "fieldtype": "Section Break",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 0,
+   "in_standard_filter": 0,
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 0,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  },
+  {
+   "allow_bulk_edit": 0,
+   "allow_in_quick_entry": 0,
+   "allow_on_submit": 0,
+   "bold": 0,
+   "collapsible": 0,
+   "columns": 0,
+   "fieldname": "content",
+   "fieldtype": "HTML Editor",
+   "hidden": 0,
+   "ignore_user_permissions": 0,
+   "ignore_xss_filter": 0,
+   "in_filter": 0,
+   "in_global_search": 0,
+   "in_list_view": 1,
+   "in_standard_filter": 0,
+   "label": "Content",
+   "length": 0,
+   "no_copy": 0,
+   "permlevel": 0,
+   "precision": "",
+   "print_hide": 0,
+   "print_hide_if_no_value": 0,
+   "read_only": 0,
+   "remember_last_selected_value": 0,
+   "report_hide": 0,
+   "reqd": 1,
+   "search_index": 0,
+   "set_only_once": 0,
+   "translatable": 0,
+   "unique": 0
+  }
+ ],
+ "has_web_view": 0,
+ "hide_heading": 0,
+ "hide_toolbar": 0,
+ "idx": 0,
+ "image_view": 0,
+ "in_create": 0,
+ "is_submittable": 0,
+ "issingle": 0,
+ "istable": 0,
+ "max_attachments": 0,
+ "modified": "2019-02-07 15:26:21.867083",
+ "modified_by": "Administrator",
+ "module": "Core",
+ "name": "Comment",
+ "name_case": "",
+ "owner": "Administrator",
+ "permissions": [
+  {
+   "amend": 0,
+   "cancel": 0,
+   "create": 1,
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "if_owner": 0,
+   "import": 0,
+   "permlevel": 0,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "System Manager",
+   "set_user_permissions": 0,
+   "share": 1,
+   "submit": 0,
+   "write": 1
+  },
+  {
+   "amend": 0,
+   "cancel": 0,
+   "create": 1,
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "if_owner": 0,
+   "import": 0,
+   "permlevel": 0,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "Website Manager",
+   "set_user_permissions": 0,
+   "share": 1,
+   "submit": 0,
+   "write": 1
+  }
+ ],
+ "quick_entry": 1,
+ "read_only": 0,
+ "read_only_onload": 0,
+ "show_name_in_global_search": 0,
+ "sort_field": "modified",
+ "sort_order": "DESC",
+ "title_field": "comment_type",
+ "track_changes": 1,
+ "track_seen": 0,
+ "track_views": 0
+}

frappe/core/doctype/comment/comment.py

@@ -0,0 +1,171 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2019, Frappe Technologies and contributors
+# For license information, please see license.txt
+
+from __future__ import unicode_literals, absolute_import
+import frappe
+from frappe import _
+import json
+from frappe.model.document import Document
+from frappe.core.doctype.user.user import extract_mentions
+from frappe.utils import get_fullname, get_link_to_form
+from frappe.website.render import clear_cache
+from frappe.database.schema import add_column
+from frappe.exceptions import ImplicitCommitError
+
+class Comment(Document):
+	def after_insert(self):
+		self.notify_mentions()
+
+		frappe.publish_realtime('new_communication', self.as_dict(),
+			doctype=self.reference_doctype, docname=self.reference_name,
+			after_commit=True)
+
+	def validate(self):
+		if not self.comment_email:
+			self.comment_email = frappe.session.user
+
+	def on_update(self):
+		self.update_comment_in_doc()
+
+	def on_trash(self):
+		self.remove_comment_from_cache()
+		frappe.publish_realtime('delete_communication', self.as_dict(),
+			doctype= self.reference_doctype, docname = self.reference_name,
+			after_commit=True)
+
+	def remove_comment_from_cache(self):
+		_comments = self.get_comments_from_parent()
+		for c in _comments:
+			if c.get("name")==self.name:
+				_comments.remove(c)
+
+		update_comments_in_parent(self.reference_doctype, self.reference_name, _comments)
+
+	def update_comment_in_doc(self):
+		"""Updates `_comments` (JSON) property in parent Document.
+		Creates a column `_comments` if property does not exist.
+
+		Only user created comments Communication or Comment of type Comment are saved.
+
+		`_comments` format
+
+			{
+				"comment": [String],
+				"by": [user],
+				"name": [Comment Document name]
+			}"""
+
+		def get_truncated(content):
+			return (content[:97] + '...') if len(content) > 100 else content
+
+		if self.reference_doctype and self.reference_name and self.content:
+			_comments = self.get_comments_from_parent()
+
+			updated = False
+			for c in _comments:
+				if c.get("name")==self.name:
+					c["comment"] = get_truncated(self.content)
+					updated = True
+
+			if not updated:
+				_comments.append({
+					"comment": get_truncated(self.content),
+					"by": self.comment_email or self.owner,
+					"name": self.name
+				})
+
+			update_comments_in_parent(self.reference_doctype, self.reference_name, _comments)
+
+	def notify_mentions(self):
+		if self.reference_doctype and self.reference_name and self.content:
+			mentions = extract_mentions(self.content)
+
+			if not mentions:
+				return
+
+			sender_fullname = get_fullname(frappe.session.user)
+			title_field = frappe.get_meta(self.reference_doctype).get_title_field()
+			title = self.reference_name if title_field == "name" else \
+				frappe.db.get_value(self.reference_doctype, self.reference_name, title_field)
+
+			if title != self.reference_name:
+				parent_doc_label = "{0}: {1} (#{2})".format(_(self.reference_doctype),
+					title, self.reference_name)
+			else:
+				parent_doc_label = "{0}: {1}".format(_(self.reference_doctype),
+					self.reference_name)
+
+			subject = _("{0} mentioned you in a comment in {1}").format(sender_fullname, parent_doc_label)
+
+			recipients = [frappe.db.get_value("User", {"enabled": 1, "name": name, "user_type": "System User", "allowed_in_mentions": 1}, "email")
+				for name in mentions]
+			link = get_link_to_form(self.reference_doctype, self.reference_name, label=parent_doc_label)
+
+			frappe.sendmail(
+				recipients = recipients,
+				sender = frappe.session.user,
+				subject = subject,
+				template = "mentioned_in_comment",
+				args = {
+					"body_content": _("{0} mentioned you in a comment in {1}").format(sender_fullname, link),
+					"comment": doc,
+					"link": link
+				},
+				header = [_('New Mention'), 'orange']
+			)
+
+	def get_comments_from_parent(self):
+		'''
+		get the list of comments cached in the document record in the column
+		`_comments`
+		'''
+		try:
+			_comments = frappe.db.get_value(self.reference_doctype, self.reference_name, "_comments") or "[]"
+
+		except Exception as e:
+			if frappe.db.is_missing_table_or_column(e):
+				_comments = "[]"
+
+			else:
+				raise
+
+		try:
+			return json.loads(_comments)
+		except ValueError:
+			return []
+
+def update_comments_in_parent(reference_doctype, reference_name, _comments):
+	"""Updates `_comments` property in parent Document with given dict.
+
+	:param _comments: Dict of comments."""
+	if not reference_doctype or frappe.db.get_value("DocType", reference_doctype, "issingle"):
+		return
+
+	try:
+		# use sql, so that we do not mess with the timestamp
+		frappe.db.sql("""update `tab%s` set `_comments`=%s where name=%s""" % (reference_doctype,
+			"%s", "%s"), (json.dumps(_comments), reference_name))
+
+	except Exception as e:
+		if frappe.db.is_column_missing(e) and getattr(frappe.local, 'request', None):
+			# missing column and in request, add column and update after commit
+			frappe.local._comments = (getattr(frappe.local, "_comments", [])
+				+ [(reference_doctype, reference_name, _comments)])
+		else:
+			raise ImplicitCommitError
+
+	else:
+		if not frappe.flags.in_patch:
+			reference_doc = frappe.get_doc(reference_doctype, reference_name)
+			if getattr(reference_doc, "route", None):
+				clear_cache(reference_doc.route)
+
+def update_comments_in_parent_after_request():
+	"""update _comments in parent if _comments column is missing"""
+	if hasattr(frappe.local, "_comments"):
+		for (reference_doctype, reference_name, _comments) in frappe.local._comments:
+			add_column(reference_doctype, "_comments", "Text")
+			update_comments_in_parent(reference_doctype, reference_name, _comments)
+
+		frappe.db.commit()

frappe/core/doctype/comment/test_comment.py

@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2019, Frappe Technologies and Contributors
+# See license.txt
+from __future__ import unicode_literals
+
+import frappe
+import unittest
+
+class TestComment(unittest.TestCase):
+	pass

frappe/core/doctype/communication/comment.py

@@ -1,172 +0,0 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
-# MIT License. See license.txt
-
-from __future__ import unicode_literals, absolute_import
-import frappe
-from frappe import _
-import json
-from frappe.core.doctype.user.user import extract_mentions
-from frappe.utils import get_fullname, get_link_to_form
-from frappe.website.render import clear_cache
-from frappe.database.schema import add_column
-from frappe.exceptions import ImplicitCommitError
-
-def on_trash(doc):
-	if doc.communication_type != "Comment":
-		return
-
-	if doc.reference_doctype == "Message":
-		return
-
-	if (doc.comment_type or "Comment") != "Comment":
-		frappe.only_for("System Manager")
-
-	_comments = get_comments_from_parent(doc)
-	for c in _comments:
-		if c.get("name")==doc.name:
-			_comments.remove(c)
-
-	update_comments_in_parent(doc.reference_doctype, doc.reference_name, _comments)
-
-def update_comment_in_doc(doc):
-	"""Updates `_comments` (JSON) property in parent Document.
-	Creates a column `_comments` if property does not exist.
-
-	Only user created comments Communication or Comment of type Comment are saved.
-
-	`_comments` format
-
-		{
-			"comment": [String],
-			"by": [user],
-			"name": [Comment Document name]
-		}"""
-
-	if doc.communication_type not in ("Comment", "Communication"):
-		return
-
-	if doc.communication_type == 'Comment' and doc.comment_type != 'Comment':
-		# other updates
-		return
-
-	def get_content(doc):
-		return (doc.content[:97] + '...') if len(doc.content) > 100 else doc.content
-
-	if doc.reference_doctype and doc.reference_name and doc.content:
-		_comments = get_comments_from_parent(doc)
-
-		updated = False
-		for c in _comments:
-			if c.get("name")==doc.name:
-				c["comment"] = get_content(doc)
-				updated = True
-
-		if not updated:
-			_comments.append({
-				"comment": get_content(doc),
-				"by": doc.sender or doc.owner,
-				"name": doc.name
-			})
-
-		update_comments_in_parent(doc.reference_doctype, doc.reference_name, _comments)
-
-def notify_mentions(doc):
-	if doc.communication_type != "Comment":
-		return
-
-	if doc.reference_doctype and doc.reference_name and doc.content and doc.comment_type=="Comment":
-		mentions = extract_mentions(doc.content)
-
-		if not mentions:
-			return
-
-		sender_fullname = get_fullname(frappe.session.user)
-		title_field = frappe.get_meta(doc.reference_doctype).get_title_field()
-		title = doc.reference_name if title_field == "name" else \
-			frappe.db.get_value(doc.reference_doctype, doc.reference_name, title_field)
-
-		if title != doc.reference_name:
-			parent_doc_label = "{0}: {1} (#{2})".format(_(doc.reference_doctype),
-				title, doc.reference_name)
-		else:
-			parent_doc_label = "{0}: {1}".format(_(doc.reference_doctype),
-				doc.reference_name)
-
-		subject = _("{0} mentioned you in a comment in {1}").format(sender_fullname, parent_doc_label)
-
-		recipients = [frappe.db.get_value("User", {"enabled": 1, "name": name, "user_type": "System User", "allowed_in_mentions": 1}, "email")
-			for name in mentions]
-		link = get_link_to_form(doc.reference_doctype, doc.reference_name, label=parent_doc_label)
-
-		frappe.sendmail(
-			recipients=recipients,
-			sender=frappe.session.user,
-			subject=subject,
-			template="mentioned_in_comment",
-			args={
-				"body_content": _("{0} mentioned you in a comment in {1}").format(sender_fullname, link),
-				"comment": doc,
-				"link": link
-			},
-			header=[_('New Mention'), 'orange']
-		)
-
-def get_comments_from_parent(doc):
-	try:
-		_comments = frappe.db.get_value(doc.reference_doctype, doc.reference_name, "_comments") or "[]"
-
-	except Exception as e:
-		if frappe.db.is_missing_table_or_column(e):
-			_comments = "[]"
-
-		else:
-			raise
-
-	try:
-		return json.loads(_comments)
-	except ValueError:
-		return []
-
-def update_comments_in_parent(reference_doctype, reference_name, _comments):
-	"""Updates `_comments` property in parent Document with given dict.
-
-	:param _comments: Dict of comments."""
-	if not reference_doctype or frappe.db.get_value("DocType", reference_doctype, "issingle"):
-		return
-
-	try:
-		# use sql, so that we do not mess with the timestamp
-		frappe.db.sql("""update `tab%s` set `_comments`=%s where name=%s""" % (reference_doctype,
-			"%s", "%s"), (json.dumps(_comments), reference_name))
-
-	except Exception as e:
-		if frappe.db.is_column_missing(e) and getattr(frappe.local, 'request', None):
-			# missing column and in request, add column and update after commit
-			frappe.local._comments = (getattr(frappe.local, "_comments", [])
-				+ [(reference_doctype, reference_name, _comments)])
-		else:
-			raise ImplicitCommitError
-
-	else:
-		if not frappe.flags.in_patch:
-			reference_doc = frappe.get_doc(reference_doctype, reference_name)
-			if getattr(reference_doc, "route", None):
-				clear_cache(reference_doc.route)
-
-def add_info_comment(**kwargs):
-	kwargs.update({
-		"doctype": "Communication",
-		"communication_type": "Comment",
-		"comment_type": "Info",
-		"status": "Closed"
-	})
-	return frappe.get_doc(kwargs).insert(ignore_permissions=True)
-
-def update_comments_in_parent_after_request():
-	"""update _comments in parent if _comments column is missing"""
-	if hasattr(frappe.local, "_comments"):
-		for (reference_doctype, reference_name, _comments) in frappe.local._comments:
-			add_column(reference_doctype, "_comments", "Text")
-			update_comments_in_parent(reference_doctype, reference_name, _comments)
-
-		frappe.db.commit()

frappe/core/doctype/communication/communication.py

@@ -6,8 +6,6 @@ import frappe
 from frappe import _
 from frappe.model.document import Document
 from frappe.utils import validate_email_add, get_fullname, strip_html, cstr
-from frappe.core.doctype.communication.comment import (notify_mentions,
-	update_comment_in_doc, on_trash)
 from frappe.core.doctype.communication.email import (validate_email,
 	notify, _notify, update_parent_mins_to_first_response)
 from frappe.core.utils import get_parent_doc, set_timeline_doc
@@ -87,19 +85,15 @@ class Communication(Document):
 		if not (self.reference_doctype and self.reference_name):
 			return
 
-		if self.reference_doctype == "Communication" and self.sent_or_received == "Sent" and \
-			self.communication_type != 'Comment':
+		if self.reference_doctype == "Communication" and self.sent_or_received == "Sent":
 			frappe.db.set_value("Communication", self.reference_name, "status", "Replied")
 
-		if self.communication_type in ("Communication", "Comment"):
+		if self.communication_type == "Communication":
 			# send new comment to listening clients
 			frappe.publish_realtime('new_communication', self.as_dict(),
 				doctype=self.reference_doctype, docname=self.reference_name,
 				after_commit=True)
 
-			if self.communication_type == "Comment":
-				notify_mentions(self)
-
 		elif self.communication_type in ("Chat", "Notification", "Bot"):
 			if self.reference_name == frappe.session.user:
 				message = self.as_dict()
@@ -114,23 +108,14 @@ class Communication(Document):
 		"""Update parent status as `Open` or `Replied`."""
 		if self.comment_type != 'Updated':
 			update_parent_mins_to_first_response(self)
-			update_comment_in_doc(self)
 			self.bot_reply()
 
 	def on_trash(self):
-		if (not self.flags.ignore_permissions
-			and self.communication_type=="Comment" and self.comment_type != "Comment"):
-
-			# prevent deletion of auto-created comments if not ignore_permissions
-			frappe.throw(_("Sorry! You cannot delete auto-generated comments"))
-
-		if self.communication_type in ("Communication", "Comment"):
+		if self.communication_type == "Communication":
 			# send delete comment to listening clients
 			frappe.publish_realtime('delete_communication', self.as_dict(),
 				doctype= self.reference_doctype, docname = self.reference_name,
 				after_commit=True)
-			# delete the comments from _comment
-			on_trash(self)
 
 	def set_status(self):
 		if not self.is_new():

frappe/core/notifications.py

@@ -44,14 +44,14 @@ def get_todays_events(as_list=False):
 
 def get_unseen_likes():
 	"""Returns count of unseen likes"""
-	return frappe.db.sql("""select count(*) from `tabCommunication`
+	return frappe.db.sql("""select count(*) from `tabComment`
 		where
-			communication_type='Comment'
+			comment_type='Like'
 			and modified >= (NOW() - INTERVAL '1' YEAR)
-			and comment_type='Like'
 			and owner is not null and owner!=%(user)s
 			and reference_owner=%(user)s
-			and seen=0""", {"user": frappe.session.user})[0][0]
+			and seen=0
+			""", {"user": frappe.session.user})[0][0]
 
 def get_unread_emails():
 	"returns unread emails for a user"

frappe/desk/form/load.py

@@ -94,6 +94,7 @@ def get_docinfo(doc=None, doctype=None, name=None):
 	frappe.response["docinfo"] = {
 		"attachments": get_attachments(doc.doctype, doc.name),
 		"communications": _get_communications(doc.doctype, doc.name),
+		'comments': get_comments(doc.doctype, doc.name),
 		'total_comments': len(json.loads(doc.get('_comments') or '[]')),
 		'versions': get_versions(doc),
 		"assignments": get_assignments(doc.doctype, doc.name),
@@ -120,6 +121,19 @@ def get_communications(doctype, name, start=0, limit=20):
 	return _get_communications(doctype, name, start, limit)
 
 
+def get_comments(doctype, name):
+	comments = frappe.get_all('Comment', fields = ['*'], filters = dict(
+		reference_doctype = doctype,
+		reference_name = name
+	))
+
+	# convert to markdown (legacy ?)
+	for c in comments:
+		if c.comment_type == 'Comment':
+			c.content = frappe.utils.markdown(c.content)
+
+	return comments
+
 def _get_communications(doctype, name, start=0, limit=20):
 	communications = get_communication_data(doctype, name, start, limit)
 	for c in communications:
@@ -130,8 +144,6 @@ def _get_communications(doctype, name, start=0, limit=20):
 					"attached_to_name": c.name}
 				))
 
-		elif c.communication_type=="Comment" and c.comment_type=="Comment":
-			c.content = frappe.utils.markdown(c.content)
 	return communications
 
 def get_communication_data(doctype, name, start=0, limit=20, after=None, fields=None,
@@ -144,17 +156,13 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields=
 			`timeline_doctype`, `timeline_name`, `reference_doctype`, `reference_name`,
 			`link_doctype`, `link_name`, `read_by_recipient`, `rating`, 'Communication' AS `doctype`'''
 
-	conditions = '''communication_type in ('Communication', 'Comment', 'Feedback')
+	conditions = '''communication_type in ('Communication', 'Feedback')
 			and (
 				(reference_doctype=%(doctype)s and reference_name=%(name)s)
 				or (
-				(timeline_doctype=%(doctype)s and timeline_name=%(name)s)
-				and (
-				communication_type='Communication'
-				or (
-					communication_type='Comment'
-					and comment_type in ('Created', 'Updated', 'Submitted', 'Cancelled', 'Deleted')
-				)))
+					(timeline_doctype=%(doctype)s and timeline_name=%(name)s)
+					and (communication_type='Communication')
+				)
 			)'''
 
 

frappe/desk/form/utils.py

@@ -57,23 +57,22 @@ def validate_link():
 		frappe.response['message'] = 'Ok'
 
 @frappe.whitelist()
-def add_comment(doc):
+def add_comment(reference_doctype, reference_name, content, comment_email):
 	"""allow any logged user to post a comment"""
-	doc = frappe.get_doc(json.loads(doc))
-
-	doc.content = clean_email_html(doc.content)
-
-	if not (doc.doctype=="Communication" and doc.communication_type=='Comment'):
-		frappe.throw(_("This method can only be used to create a Comment"), frappe.PermissionError)
-
-	doc.insert(ignore_permissions=True)
+	doc = frappe.get_doc(dict(
+		doctype = 'Comment',
+		reference_doctype = reference_doctype,
+		reference_name = reference_name,
+		content = clean_email_html(content),
+		comment_email = comment_email
+	)).insert(ignore_permissions = True)
 
 	return doc.as_dict()
 
 @frappe.whitelist()
 def update_comment(name, content):
 	"""allow only owner to update comment"""
-	doc = frappe.get_doc('Communication', name)
+	doc = frappe.get_doc('Comment', name)
 
 	if frappe.session.user not in ['Administrator', doc.owner]:
 		frappe.throw(_('Comment can only be edited by the owner'), frappe.PermissionError)

frappe/desk/like.py

@@ -64,13 +64,12 @@ def _toggle_like(doctype, name, add, user=None):
 def remove_like(doctype, name):
 	"""Remove previous Like"""
 	# remove Comment
-	frappe.delete_doc("Communication", [c.name for c in frappe.get_all("Communication",
+	frappe.delete_doc("Comment", [c.name for c in frappe.get_all("Comment",
 		filters={
-			"communication_type": "Comment",
+			"comment_type": "Like",
 			"reference_doctype": doctype,
 			"reference_name": name,
 			"owner": frappe.session.user,
-			"comment_type": "Like"
 		}
 	)], ignore_permissions=True)
 

frappe/desk/page/activity/activity.js

@@ -53,14 +53,7 @@ frappe.pages['activity'].on_page_load = function(wrapper) {
 		}
 
 		frappe.set_route("List", "Activity Log", "Report");
-	}, 'fa fa-th')
-
-	this.page.add_menu_item(__('Show Likes'), function() {
-		frappe.route_options = {
-			show_likes: true
-		};
-		me.page.list.refresh();
-	}, 'octicon octicon-heart');
+	}, 'fa fa-th');
 };
 
 frappe.pages['activity'].on_page_show = function() {
@@ -158,9 +151,7 @@ frappe.activity.render_heatmap = function(page) {
 					data: {}
 				});
 
-				heatmap.update({
-					dataPoints: r.message
-				});
+				heatmap.update(r.message);
 			}
 		}
 	})
@@ -196,8 +187,7 @@ frappe.views.Activity = class Activity extends frappe.views.BaseList {
 	get_args() {
 		return {
 			start: this.start,
-			page_length: this.page_length,
-			show_likes: (frappe.route_options || {}).show_likes || 0
+			page_length: this.page_length
 		};
 	}
 

frappe/desk/page/activity/activity.py

@@ -7,43 +7,45 @@ from frappe.utils import cint
 from frappe.core.doctype.activity_log.feed import get_feed_match_conditions
 
 @frappe.whitelist()
-def get_feed(start, page_length, show_likes=False):
+def get_feed(start, page_length):
 	"""get feed"""
-	match_conditions = get_feed_match_conditions(frappe.session.user)
+	match_conditions_communication = get_feed_match_conditions(frappe.session.user, 'Communication')
+	match_conditions_comment = get_feed_match_conditions(frappe.session.user, 'Comment')
 
 	result = frappe.db.sql("""select X.*
 		from (select name, owner, modified, creation, seen, comment_type,
-			reference_doctype, reference_name, link_doctype, link_name, subject,
-			communication_type, communication_medium, content
-			from `tabCommunication`
+				reference_doctype, reference_name, link_doctype, link_name, subject,
+				communication_type, communication_medium, content
+			from
+				`tabCommunication`
 			where
-			communication_type in ("Communication", "Comment")
-			and communication_medium != "Email"
-			and (comment_type is null or comment_type != "Like"
-				or (comment_type="Like" and (owner=%(user)s or reference_owner=%(user)s)))
-			{match_conditions}
-			{show_likes}
-			union
+				communication_type = "Communication"
+				and communication_medium != "Email"
+				and {match_conditions_communication}
+		UNION
 			select name, owner, modified, creation, '0', 'Updated',
-			reference_doctype, reference_name, link_doctype, link_name, subject,
-			'Comment', '', content
-			from `tabActivity Log`) X
+				reference_doctype, reference_name, link_doctype, link_name, subject,
+				'Comment', '', content
+			from
+				`tabActivity Log`
+		UNION
+			select name, owner, modified, creation, '0', comment_type,
+				reference_doctype, reference_name, link_doctype, link_name, '',
+				'Comment', '', content
+			from
+				`tabComment`
+			where
+				{match_conditions_comment}
+		) X
 		order by X.creation DESC
 		limit %(start)s, %(page_length)s"""
-		.format(match_conditions="and {0}".format(match_conditions) if match_conditions else "",
-			show_likes="and comment_type='Like'" if show_likes else ""),
-		{
+		.format(match_conditions_comment = match_conditions_comment,
+			match_conditions_communication = match_conditions_communication), {
 			"user": frappe.session.user,
 			"start": cint(start),
 			"page_length": cint(page_length)
 		}, as_dict=True)
 
-	if show_likes:
-		# mark likes as seen!
-		frappe.db.sql("""update `tabCommunication` set seen=1
-			where comment_type='Like' and reference_owner=%s""", frappe.session.user)
-		frappe.local.flags.commit = True
-
 	return result
 
 @frappe.whitelist()

frappe/desk/reportview.py

@@ -212,11 +212,16 @@ def delete_items():
 	"""delete selected items"""
 	import json
 
-	il = sorted(json.loads(frappe.form_dict.get('items')), reverse=True)
+	items = sorted(json.loads(frappe.form_dict.get('items')), reverse=True)
 	doctype = frappe.form_dict.get('doctype')
 
-	failed = []
+	if len(items) > 10:
+		frappe.enqueue('frappe.desk.reportview.delete_bulk',
+			doctype=doctype, items=items)
+	else:
+		delete_bulk(doctype, items)
 
+def delete_bulk(doctype, items):
 	for i, d in enumerate(il):
 		try:
 			frappe.delete_doc(doctype, d)
@@ -227,8 +232,6 @@ def delete_items():
 		except Exception:
 			failed.append(d)
 
-	return failed
-
 @frappe.whitelist()
 @frappe.read_only()
 def get_sidebar_stats(stats, doctype, filters=[]):

frappe/email/doctype/email_unsubscribe/email_unsubscribe.py

@@ -35,5 +35,5 @@ class EmailUnsubscribe(Document):
 	def on_update(self):
 		if self.reference_doctype and self.reference_name:
 			doc = frappe.get_doc(self.reference_doctype, self.reference_name)
-			doc.add_comment("Label", _("Left this conversation"), comment_by=self.email)
+			doc.add_comment("Label", _("Left this conversation"), comment_email=self.email)
 

frappe/model/delete_doc.py

@@ -195,7 +195,7 @@ def check_if_doc_is_linked(doc, method="Delete"):
 			for item in frappe.db.get_values(link_dt, {link_field:doc.name},
 				["name", "parent", "parenttype", "docstatus"], as_dict=True):
 				linked_doctype = item.parenttype if item.parent else link_dt
-				if linked_doctype in ("Communication", "ToDo", "DocShare", "Email Unsubscribe", 'File', 'Version', "Activity Log"):
+				if linked_doctype in ("Communication", "ToDo", "DocShare", "Email Unsubscribe", 'File', 'Version', "Activity Log", 'Comment'):
 					# don't check for communication and todo!
 					continue
 
@@ -220,7 +220,7 @@ def check_if_doc_is_linked(doc, method="Delete"):
 def check_if_doc_is_dynamically_linked(doc, method="Delete"):
 	'''Raise `frappe.LinkExistsError` if the document is dynamically linked'''
 	for df in get_dynamic_link_map().get(doc.doctype, []):
-		if df.parent in ("Communication", "ToDo", "DocShare", "Email Unsubscribe", "Activity Log", 'File', 'Version', 'View Log'):
+		if df.parent in ("Communication", "ToDo", "DocShare", "Email Unsubscribe", "Activity Log", 'File', 'Version', 'View Log', 'Comment'):
 			# don't check for communication and todo!
 			continue
 
@@ -266,59 +266,37 @@ def raise_link_exists_exception(doc, reference_doctype, reference_docname, row='
 		.format(doc.doctype, doc_link, reference_doctype, reference_link, row), frappe.LinkExistsError)
 
 def delete_dynamic_links(doctype, name):
-	delete_doc("ToDo", frappe.db.sql_list("""select name from `tabToDo`
-		where reference_type=%s and reference_name=%s""", (doctype, name)),
-		ignore_permissions=True, force=True)
-
-	frappe.db.sql('''delete from `tabEmail Unsubscribe`
-		where reference_doctype=%s and reference_name=%s''', (doctype, name))
-
-	# delete shares
-	frappe.db.sql("""delete from `tabDocShare`
-		where share_doctype=%s and share_name=%s""", (doctype, name))
-
-	# delete versions
-	frappe.db.sql('delete from tabVersion where ref_doctype=%s and docname=%s', (doctype, name))
-
-	# delete comments
-	frappe.db.sql("""delete from `tabCommunication`
-		where
-			communication_type = 'Comment'
-			and reference_doctype=%s and reference_name=%s""", (doctype, name))
-
-	# delete view logs
-	frappe.db.sql("""delete from `tabView Log`
-		where reference_doctype=%s and reference_name=%s""", (doctype, name))
+	delete_references('ToDo', doctype, name)
+	delete_references('Email Unsubscribe', doctype, name)
+	delete_references('DocShare', doctype, name, 'share_doctype', 'share_name')
+	delete_references('Version', doctype, name, 'ref_doctype', 'docname')
+	delete_references('Comment', doctype, name)
+	delete_references('View Log', doctype, name)
 
 	# unlink communications
-	frappe.db.sql("""update `tabCommunication`
-		set reference_doctype=null, reference_name=null
-		where
-			communication_type = 'Communication'
-			and reference_doctype=%s
-			and reference_name=%s""", (doctype, name))
-
-	# unlink secondary references
-	frappe.db.sql("""update `tabCommunication`
-		set link_doctype=null, link_name=null
-		where link_doctype=%s and link_name=%s""", (doctype, name))
-
-	# unlink feed
-	frappe.db.sql("""update `tabCommunication`
-		set timeline_doctype=null, timeline_name=null
-		where timeline_doctype=%s and timeline_name=%s""", (doctype, name))
-
-	# unlink activity_log reference_doctype
-	frappe.db.sql("""update `tabActivity Log`
-		set reference_doctype=null, reference_name=null
+	clear_references('Communication', doctype, name)
+	clear_references('Communication', doctype, name, 'link_doctype', 'link_name')
+	clear_references('Communication', doctype, name, 'timeline_doctype', 'timeline_name')
+
+	clear_references('Activity Log', doctype, name)
+	clear_references('Activity Log', doctype, name, 'timeline_doctype', 'timeline_name')
+
+def delete_references(doctype, reference_doctype, reference_name,
+		reference_doctype_field = 'reference_doctype', reference_name_field = 'reference_name'):
+	frappe.db.sql('''delete from `tab{0}`
+		where {1}=%s and {2}=%s'''.format(doctype, reference_doctype_field, reference_name_field),
+		(reference_doctype, reference_name))
+
+def clear_references(doctype, reference_doctype, reference_name,
+		reference_doctype_field = 'reference_doctype', reference_name_field = 'reference_name'):
+	frappe.db.sql('''update
+			`tab{0}`
+		set
+			{1}=NULL, {2}=NULL
 		where
-			reference_doctype=%s
-			and reference_name=%s""", (doctype, name))
+			{1}=%s and {2}=%s'''.format(doctype, reference_doctype_field, reference_name_field),
+		(reference_doctype, reference_name))
 
-	# unlink activity_log timeline_doctype
-	frappe.db.sql("""update `tabActivity Log`
-		set timeline_doctype=null, timeline_name=null
-		where timeline_doctype=%s and timeline_name=%s""", (doctype, name))
 
 def insert_feed(doc):
 	from frappe.utils import get_fullname
@@ -327,8 +305,7 @@ def insert_feed(doc):
 		return
 
 	frappe.get_doc({
-		"doctype": "Communication",
-		"communication_type": "Comment",
+		"doctype": "Comment",
 		"comment_type": "Deleted",
 		"reference_doctype": doc.doctype,
 		"subject": "{0} {1}".format(_(doc.doctype), doc.name),

frappe/model/document.py

@@ -1116,33 +1116,22 @@ class Document(BaseDocument):
 		"""Returns Desk URL for this document. `/desk#Form/{doctype}/{name}`"""
 		return "/desk#Form/{doctype}/{name}".format(doctype=self.doctype, name=self.name)
 
-	def add_comment(self, comment_type, text=None, comment_by=None, link_doctype=None, link_name=None):
+	def add_comment(self, comment_type='Comment', text=None, comment_email=None, link_doctype=None, link_name=None, comment_by=None):
 		"""Add a comment to this document.
 
 		:param comment_type: e.g. `Comment`. See Communication for more info."""
 
-		if comment_type=='Comment':
-			out = frappe.get_doc({
-				"doctype":"Communication",
-				"communication_type": "Comment",
-				"sender": comment_by or frappe.session.user,
-				"comment_type": comment_type,
-				"reference_doctype": self.doctype,
-				"reference_name": self.name,
-				"content": text or comment_type,
-				"link_doctype": link_doctype,
-				"link_name": link_name
-			}).insert(ignore_permissions=True)
-		else:
-			out = frappe.get_doc(dict(
-				doctype='Version',
-				ref_doctype= self.doctype,
-				docname= self.name,
-				data = frappe.as_json(dict(comment_type=comment_type, comment=text))
-			))
-			if comment_by:
-				out.owner = comment_by
-			out.insert(ignore_permissions=True)
+		out = frappe.get_doc({
+			"doctype":"Comment",
+			'comment_type': comment_type,
+			"comment_email": comment_email or frappe.session.user,
+			"comment_by": comment_by,
+			"reference_doctype": self.doctype,
+			"reference_name": self.name,
+			"content": text or comment_type,
+			"link_doctype": link_doctype,
+			"link_name": link_name
+		}).insert(ignore_permissions=True)
 		return out
 
 	def add_seen(self, user=None):

frappe/patches/v12_0/setup_comments_from_communications.py

@@ -0,0 +1,24 @@
+from __future__ import unicode_literals
+
+import frappe
+
+def execute():
+    for comment in frappe.get_all('Communication', fields = ['*'],
+        filters = dict(communication_type = 'Comment'))
+
+        new_comment = frappe.new_doc('Comment')
+        new_comment.comment_type = comment.comment_type
+        new_comment.comment_email = comment.sender
+        new_comment.comment_by = comment.sender_full_name
+        new_comment.reference_doctype = comment.reference_doctype
+        new_comment.reference_name = comment.reference_name
+        new_comment.link_doctype = comment.link_doctype
+        new_comment.link_name = comment.link_name
+        new_comment.creation = comment.creation
+        new_comment.modified = comment.modified
+        new_comment.owner = comment.owner
+        new_comment.modified_by = comment.modified_by
+        new_comment.db_insert()
+
+    # clean up
+    frappe.db.sql('delete from tabCommunication where communication_type = "Comment"')

frappe/patches/v7_0/add_communication_in_doc.py

@@ -1,7 +1,7 @@
 from __future__ import unicode_literals
 import frappe
 
-from frappe.core.doctype.communication.comment import update_comment_in_doc
+from frappe.core.doctype.comment.comment import update_comment_in_doc
 
 def execute():
 	for d in frappe.db.get_all("Communication",

frappe/public/js/frappe/form/footer/timeline.js

@@ -28,7 +28,7 @@ frappe.ui.form.Timeline = class Timeline {
 			render_input: true,
 			only_input: true,
 			on_submit: (val) => {
-				val && this.insert_comment("Comment", val, this.comment_area.button);
+				val && this.insert_comment(val, this.comment_area.button);
 			}
 		});
 
@@ -149,10 +149,17 @@ frappe.ui.form.Timeline = class Timeline {
 		this.wrapper.toggle(true);
 		this.list.empty();
 		this.comment_area.set_value('');
+
+		// get all communications
 		let communications = this.get_communications(true);
-		var views = this.get_view_logs();
 
+		// append views
+		var views = this.get_view_logs();
 		var timeline = communications.concat(views);
+
+		// append comments
+		timeline = timeline.concat(this.get_comments());
+		// sort
 		timeline
 			.filter(a => a.content)
 			.sort((b, c) => me.compare_dates(b, c))
@@ -312,7 +319,7 @@ frappe.ui.form.Timeline = class Timeline {
 		c["delete"] = "";
 		c["edit"] = "";
 		if(c.communication_type=="Comment" && (c.comment_type || "Comment") === "Comment") {
-			if(frappe.model.can_delete("Communication")) {
+			if(frappe.model.can_delete("Comment")) {
 				c["delete"] = '<a class="close delete-comment" title="Delete"  href="#"><i class="octicon octicon-x"></i></a>';
 			}
 
@@ -498,6 +505,22 @@ frappe.ui.form.Timeline = class Timeline {
 		return out;
 	}
 
+	get_comments() {
+		let docinfo = this.frm.get_docinfo();
+
+		for (let c of docinfo.comments) {
+			this.cast_comment_as_communication(c);
+		}
+
+		return docinfo.comments;
+	}
+
+	cast_comment_as_communication(c) {
+		c.sender = c.comment_email;
+		c.sender_full_name = c.comment_by;
+		c.communication_type = 'Comment';
+	}
+
 	build_version_comments(docinfo, out) {
 		var me = this;
 		docinfo.versions.forEach(function(version) {
@@ -530,8 +553,8 @@ frappe.ui.form.Timeline = class Timeline {
 							if(field_display_status === 'Read' || field_display_status === 'Write') {
 								parts.push(__('{0} from {1} to {2}', [
 									__(df.label),
-									(frappe.ellipsis(p[1], 40) || '""').bold(),
-									(frappe.ellipsis(p[2], 40) || '""').bold()
+									(frappe.ellipsis(frappe.utils.html2text(p[1]), 40) || '""').bold(),
+									(frappe.ellipsis(frappe.utils.html2text(p[2]), 40) || '""').bold()
 								]));
 							}
 						}
@@ -539,7 +562,7 @@ frappe.ui.form.Timeline = class Timeline {
 					return parts.length < 3;
 				});
 				if(parts.length) {
-					out.push(me.get_version_comment(version, __("changed value of {0}", [parts.join(', ')])));
+					out.push(me.get_version_comment(version, __("changed value of {0}", [parts.join(', ').bold()])));
 				}
 			}
 
@@ -616,20 +639,15 @@ frappe.ui.form.Timeline = class Timeline {
 		};
 	}
 
-	insert_comment(comment_type, comment, btn) {
+	insert_comment(comment, btn) {
 		var me = this;
 		return frappe.call({
 			method: "frappe.desk.form.utils.add_comment",
 			args: {
-				doc:{
-					doctype: "Communication",
-					communication_type: "Comment",
-					comment_type: comment_type || "Comment",
-					reference_doctype: this.frm.doctype,
-					reference_name: this.frm.docname,
-					content: comment,
-					sender: frappe.session.user
-				}
+				reference_doctype: this.frm.doctype,
+				reference_name: this.frm.docname,
+				content: comment,
+				comment_email: frappe.session.user
 			},
 			btn: btn,
 			callback: function(r) {
@@ -638,7 +656,7 @@ frappe.ui.form.Timeline = class Timeline {
 					frappe.utils.play_sound("click");
 
 					var comment = r.message;
-					var comments = me.get_communications();
+					var comments = me.get_comments();
 					var comment_exists = false;
 					for (var i=0, l=comments.length; i<l; i++) {
 						if (comments[i].name==comment.name) {
@@ -650,7 +668,7 @@ frappe.ui.form.Timeline = class Timeline {
 						return;
 					}
 
-					me.frm.get_docinfo().communications = comments.concat([r.message]);
+					me.frm.get_docinfo().comments = comments.concat([r.message]);
 					me.refresh(true);
 				}
 			}
@@ -665,7 +683,7 @@ frappe.ui.form.Timeline = class Timeline {
 			return frappe.call({
 				method: "frappe.client.delete",
 				args: {
-					doctype: "Communication",
+					doctype: "Comment",
 					name: name
 				},
 				callback: function(r) {

frappe/public/js/frappe/misc/utils.js

@@ -88,6 +88,13 @@ Object.assign(frappe.utils, {
 	escape_html: function(txt) {
 		return $("<div></div>").text(txt || "").html();
 	},
+
+	html2text: function(html) {
+		let d = document.createElement('div');
+		d.innerHTML = html;
+		return d.textContent;
+	},
+
 	is_url: function(txt) {
 		return txt.toLowerCase().substr(0,7)=='http://'
 			|| txt.toLowerCase().substr(0,8)=='https://'

frappe/public/js/frappe/ui/toolbar/notifications.js

@@ -4,17 +4,7 @@ frappe.ui.notifications = {
 	config: {
 		"ToDo": { label: __("To Do") },
 		"Event": { label: __("Calendar"), route: "List/Event/Calendar" },
-		"Email": { label: __("Email"), route: "List/Communication/Inbox" },
-		"Likes": { label: __("Likes"),
-			click: function() {
-				frappe.route_options = { show_likes: true };
-				if (frappe.get_route()[0]=="activity") {
-					frappe.pages['activity'].page.list.refresh();
-				} else {
-					frappe.set_route("activity");
-				}
-			}
-		},
+		"Email": { label: __("Email"), route: "List/Communication/Inbox" }
 	},
 
 	update_notifications: function() {

frappe/templates/includes/comments/comment.html

@@ -1,11 +1,11 @@
 <div class="comment-row">
 	<div class="inline-block" style="vertical-align: top">
 		<div class="avatar avatar-medium" style="margin-top: 11px;">
-			<img itemprop="thumbnailUrl" src="{{ frappe.get_gravatar(comment.sender) }}" />
+			<img itemprop="thumbnailUrl" src="{{ frappe.get_gravatar(comment.comment_email) }}" />
 		</div>
 	</div>
 	<div class="inline-block" style="width: calc(100% - 50px)">
-		<h6 itemprop="name">{{ comment.sender_full_name }}
+		<h6 itemprop="name">{{ comment.comment_by }}
 			<span class="text-muted pull-right" itemprop="commentTime">
 				{{ comment.creation|global_date_format }}
 			</span>

frappe/templates/includes/comments/comments.html

@@ -31,11 +31,11 @@
 					<div class="row {% if _login_required %}hide{% endif %}"
 						style="margin-bottom: 15px;">
 						<div class="col-sm-6">
-							<input class="form-control" name="comment_by_fullname"
+							<input class="form-control" name="comment_by"
 								placeholder="{{ _("Your Name") }}" type="text">
 						</div>
 						<div class="col-sm-6">
-							<input class="form-control" name="comment_by"
+							<input class="form-control" name="comment_email"
 								placeholder="{{ _("Your Email Address") }}" type="email">
 						</div>
 					</div>
@@ -74,8 +74,8 @@
                 full_name = frappe.get_cookie("full_name");
                 user_id = frappe.get_cookie("user_id");
                 if(user_id != "Guest") {
-        			$("[name='comment_by']").val(user_id);
-        			$("[name='comment_by_fullname']").val(full_name);
+        			$("[name='comment_email']").val(user_id);
+        			$("[name='comment_by']").val(full_name);
                 }
             }
 			$("#comment-form textarea").val("");
@@ -83,8 +83,8 @@
 
 		$("#submit-comment").click(function() {
 			var args = {
-				comment_by_fullname: $("[name='comment_by_fullname']").val(),
 				comment_by: $("[name='comment_by']").val(),
+				comment_email: $("[name='comment_email']").val(),
 				comment: $("[name='comment']").val(),
 				reference_doctype: "{{ reference_doctype or doctype }}",
 				reference_name: "{{ reference_name or name }}",
@@ -92,12 +92,12 @@
 				route: "{{ pathname }}",
 			}
 
-			if(!args.comment_by_fullname || !args.comment_by || !args.comment) {
+			if(!args.comment_by || !args.comment_email || !args.comment) {
 				frappe.msgprint("{{ _("All fields are necessary to submit the comment.") }}");
 				return false;
 			}
 
-			if (args.comment_by!=='Administrator' && !validate_email(args.comment_by)) {
+			if (args.comment_email!=='Administrator' && !validate_email(args.comment_email)) {
 				frappe.msgprint("{{ _("Please enter a valid email address.") }}");
 				return false;
 			}
@@ -112,7 +112,7 @@
 						if(r._server_messages)
 							frappe.msgprint(r._server_messages);
 					} else {
-						$(r.message).appendTo("#comment-list");
+						frappe.msgprint('{{ _("Thank you for your comment. It will be published after approval") }}');
 						$(".no-comment, .add-comment").toggle(false);
 						$("#comment-form").toggle();
 					}

frappe/templates/includes/comments/comments.py

@@ -9,53 +9,32 @@ from frappe.website.render import clear_cache
 from frappe import _
 
 @frappe.whitelist(allow_guest=True)
-def add_comment(args=None):
-	"""
-		args = {
-			'comment': '',
-			'comment_by': '',
-			'comment_by_fullname': '',
-			'reference_doctype': '',
-			'reference_name': '',
-			'route': '',
-		}
-	"""
-
-	if not args:
-		args = frappe.local.form_dict
-
-	route = args.get("route")
-
-	doc = frappe.get_doc(args["reference_doctype"], args["reference_name"])
-	comment = doc.add_comment("Comment", args["comment"], comment_by=args["comment_by"])
-	comment.flags.ignore_permissions = True
-	comment.sender_full_name = args["comment_by_fullname"]
-	comment.save()
+def add_comment(comment, comment_email, comment_by, reference_doctype, reference_name, route):
+	doc = frappe.get_doc(reference_doctype, reference_name)
+
+	comment = doc.add_comment(
+		text = comment,
+		comment_email = comment_email,
+		comment_by = comment_by)
 
 	# since comments are embedded in the page, clear the web cache
 	clear_cache(route)
 
-	# notify commentors
-	commentors = [d[0] for d in frappe.db.sql("""select sender from `tabCommunication`
-		where
-			communication_type = 'Comment' and comment_type = 'Comment'
-			and reference_doctype=%s
-			and reference_name=%s""", (comment.reference_doctype, comment.reference_name))]
-
-	owner = frappe.db.get_value(doc.doctype, doc.name, "owner")
-	recipients = list(set(commentors if owner=="Administrator" else (commentors + [owner])))
-
-	message = _("{0} by {1}").format(frappe.utils.markdown(args.get("comment")), comment.sender_full_name)
-	message += "<p><a href='{0}/{1}' style='font-size: 80%'>{2}</a></p>".format(frappe.utils.get_request_site_address(),
-		route, _("View it in your browser"))
-
-	from frappe.email.queue import send
-
-	send(recipients=recipients,
-		subject = _("New comment on {0} {1}").format(doc.doctype, doc.name),
-		message = message,
-		reference_doctype=doc.doctype, reference_name=doc.name)
-
+	content = (doc.content
+		+ "<p><a href='{0}/desk/#Form/Comment/{1}' style='font-size: 80%'>{2}</a></p>".format(frappe.utils.get_request_site_address(),
+			doc.name,
+			route, _("Open Comment Form")))
+
+	# notify creator
+	frappe.sendmail(
+		recipients = doc.owner,
+		subject = _('Please Approve New Comment on {0}: {1}').format(doc.doctype, doc.name),
+		message = content,
+		reference_doctype=doc.doctype,
+		reference_name=doc.name
+	)
+
+	# revert with template
 	template = frappe.get_template("templates/includes/comments/comment.html")
 
 	return template.render({"comment": comment.as_dict()})

frappe/website/doctype/blog_post/blog_post.py

@@ -8,7 +8,8 @@ from frappe import _
 from frappe.website.website_generator import WebsiteGenerator
 from frappe.website.render import clear_cache
 from frappe.utils import today, cint, global_date_format, get_fullname, strip_html_tags, markdown
-from frappe.website.utils import (find_first_image, get_comment_list, get_html_content_based_on_type)
+from frappe.website.utils import (find_first_image, get_html_content_based_on_type,
+	get_comment_list)
 
 class BlogPost(WebsiteGenerator):
 	website = frappe._dict(
@@ -69,7 +70,17 @@ class BlogPost(WebsiteGenerator):
 		if image:
 			context.metatags["image"] = image
 
+		self.load_comments(context)
+
+		context.category = frappe.db.get_value("Blog Category",
+			context.doc.blog_category, ["title", "route"], as_dict=1)
+		context.parents = [{"name": _("Home"), "route":"/"},
+			{"name": "Blog", "route": "/blog"},
+			{"label": context.category.title, "route":context.category.route}]
+
+	def load_comments(self, context):
 		context.comment_list = get_comment_list(self.doctype, self.name)
+
 		if not context.comment_list:
 			context.comment_text = _('No comments yet')
 		else:
@@ -78,11 +89,6 @@ class BlogPost(WebsiteGenerator):
 			else:
 				context.comment_text = _('{0} comments').format(len(context.comment_list))
 
-		context.category = frappe.db.get_value("Blog Category",
-			context.doc.blog_category, ["title", "route"], as_dict=1)
-		context.parents = [{"name": _("Home"), "route":"/"},
-			{"name": "Blog", "route": "/blog"},
-			{"label": context.category.title, "route":context.category.route}]
 
 def get_list_context(context=None):
 	list_context = frappe._dict(
@@ -156,9 +162,8 @@ def get_blog_list(doctype, txt=None, filters=None, limit_start=0, limit_page_len
 				t1.content as content,
 				ifnull(t1.blog_intro, t1.content) as intro,
 				t2.full_name, t2.avatar, t1.blogger,
-				(select count(name) from `tabCommunication`
+				(select count(name) from `tabComment`
 					where
-						communication_type='Comment'
 						and comment_type='Comment'
 						and reference_doctype='Blog Post'
 						and reference_name=t1.name) as comments

frappe/website/utils.py

@@ -34,16 +34,15 @@ def can_cache(no_cache=False):
 	return not no_cache
 
 def get_comment_list(doctype, name):
-	return frappe.db.sql("""select
-		content, sender_full_name, creation, sender
-		from `tabCommunication`
-		where
-			communication_type='Comment'
-			and reference_doctype=%s
-			and reference_name=%s
-			and (comment_type is null or comment_type in ('Comment', 'Communication'))
-			and modified >= (NOW() - INTERVAL '1' YEAR)
-		order by creation""", (doctype, name), as_dict=1) or []
+	return frappe.get_all('Comment',
+			fields = ['name', 'creation', 'owner', 'comment_email', 'comment_by', 'content'],
+			filters = dict(
+				reference_doctype = doctype,
+				reference_name = name,
+				comment_type = comment,
+				published = 1
+			),
+			order_by = 'creation asc')
 
 def get_home_page():
 	if frappe.local.flags.home_page: