diff --git a/bandit.yml b/bandit.yml index fce28629e8..b8560e97c8 100644 --- a/bandit.yml +++ b/bandit.yml @@ -1 +1 @@ -skips: ['B605', 'B404', 'B603', 'B607'] \ No newline at end of file +skips: ['E0203', 'B605', 'B404', 'B603', 'B607'] \ No newline at end of file diff --git a/frappe/.pylintrc b/frappe/.pylintrc new file mode 100644 index 0000000000..4b2ea0a564 --- /dev/null +++ b/frappe/.pylintrc @@ -0,0 +1 @@ +disable=access-member-before-definition \ No newline at end of file diff --git a/frappe/app.py b/frappe/app.py index bc4ecc9b2d..c0aaf87ca6 100644 --- a/frappe/app.py +++ b/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]) diff --git a/frappe/core/doctype/activity_log/feed.py b/frappe/core/doctype/activity_log/feed.py index b4d46eb5c1..e09436196e 100644 --- a/frappe/core/doctype/activity_log/feed.py +++ b/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) + ")" diff --git a/frappe/core/doctype/comment/__init__.py b/frappe/core/doctype/comment/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/comment/comment.js b/frappe/core/doctype/comment/comment.js new file mode 100644 index 0000000000..a793f766cb --- /dev/null +++ b/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) { + + // } +}); diff --git a/frappe/core/doctype/comment/comment.json b/frappe/core/doctype/comment/comment.json new file mode 100644 index 0000000000..344d6399b9 --- /dev/null +++ b/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": "Select", + "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\nEdit", + "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": 0, + "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-08 09:18:33.843171", + "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 +} \ No newline at end of file diff --git a/frappe/core/doctype/comment/comment.py b/frappe/core/doctype/comment/comment.py new file mode 100644 index 0000000000..a9bf171c72 --- /dev/null +++ b/frappe/core/doctype/comment/comment.py @@ -0,0 +1,176 @@ +# -*- 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): + update_comment_in_doc(self) + + 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 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_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] + }""" + + # only comments get updates, not likes, assignments etc. + if doc.comment_type != 'Comment': + return + + def get_truncated(content): + return (content[:97] + '...') if len(content) > 100 else content + + if doc.reference_doctype and doc.reference_name and doc.content: + _comments = doc.get_comments_from_parent() + + updated = False + for c in _comments: + if c.get("name")==doc.name: + c["comment"] = get_truncated(doc.content) + updated = True + + if not updated: + _comments.append({ + "comment": get_truncated(doc.content), + "by": doc.comment_email or doc.owner, + "name": doc.name + }) + + update_comments_in_parent(doc.reference_doctype, doc.reference_name, _comments) + + +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{0}` set `_comments`=%s where name=%s""".format(reference_doctype), # nosec + (json.dumps(_comments[-50:]), 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() diff --git a/frappe/core/doctype/comment/test_comment.py b/frappe/core/doctype/comment/test_comment.py new file mode 100644 index 0000000000..0f46f0b3b5 --- /dev/null +++ b/frappe/core/doctype/comment/test_comment.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2019, Frappe Technologies and Contributors +# See license.txt +from __future__ import unicode_literals + +import frappe, json +import unittest + +class TestComment(unittest.TestCase): + def test_comment_creation(self): + test_doc = frappe.get_doc(dict(doctype = 'ToDo', description = 'test')) + test_doc.insert() + comment = test_doc.add_comment('Comment', 'test comment') + + test_doc.reload() + + # check if updated in _comments cache + comments = json.loads(test_doc.get('_comments')) + self.assertEqual(comments[0].get('name'), comment.name) + self.assertEqual(comments[0].get('comment'), comment.content) + + # check document creation + comment_1 = frappe.get_all('Comment', fields = ['*'], filters = dict( + reference_doctype = test_doc.doctype, + reference_name = test_doc.name + ))[0] + + self.assertEqual(comment_1.content, 'test comment') + + # test via blog + def test_public_comment(self): + from frappe.website.doctype.blog_post.test_blog_post import make_test_blog + test_blog = make_test_blog() + + frappe.db.sql("delete from `tabComment` where reference_doctype = 'Blog Post'") + + from frappe.templates.includes.comments.comments import add_comment + add_comment('hello', 'test@test.com', 'Good Tester', + 'Blog Post', test_blog.name, test_blog.route) + + self.assertEqual(frappe.get_all('Comment', fields = ['*'], filters = dict( + reference_doctype = test_blog.doctype, + reference_name = test_blog.name + ))[0].published, 1) + + frappe.db.sql("delete from `tabComment` where reference_doctype = 'Blog Post'") + + add_comment('pleez vizits my site http://mysite.com', 'test@test.com', 'bad commentor', + 'Blog Post', test_blog.name, test_blog.route) + + self.assertEqual(frappe.get_all('Comment', fields = ['*'], filters = dict( + reference_doctype = test_blog.doctype, + reference_name = test_blog.name + ))[0].published, 0) + + + diff --git a/frappe/core/doctype/communication/comment.py b/frappe/core/doctype/communication/comment.py deleted file mode 100644 index b7a008cd38..0000000000 --- a/frappe/core/doctype/communication/comment.py +++ /dev/null @@ -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() diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index d0a7433e72..3a5d9cf812 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/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(): diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 0799238432..f29f2ad346 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -33,7 +33,7 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = :param sender: Communcation sender (default current user). :param recipients: Communication recipients as list. :param communication_medium: Medium of communication (default **Email**). - :param send_mail: Send via email (default **False**). + :param send_email: Send via email (default **False**). :param print_html: HTML Print format to be sent as attachment. :param print_format: Print Format name of parent document to be sent as attachment. :param attachments: List of attachments as list of files or JSON string. @@ -50,6 +50,9 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = if not sender: sender = get_formatted_email(frappe.session.user) + if isinstance(recipients, list): + recipients = ', '.join(recipients) + comm = frappe.get_doc({ "doctype":"Communication", "subject": subject, @@ -497,9 +500,9 @@ def sendmail(communication_name, print_html=None, print_format=None, attachments communication._notify(print_html=print_html, print_format=print_format, attachments=attachments, recipients=recipients, cc=cc, bcc=bcc) - except frappe.db.InternalError: + except frappe.db.InternalError as e: # deadlock, try again - if frappe.db.is_deadlocked(): + if frappe.db.is_deadlocked(e): frappe.db.rollback() time.sleep(1) continue diff --git a/frappe/core/notifications.py b/frappe/core/notifications.py index 619231e5cd..0f932d9f67 100644 --- a/frappe/core/notifications.py +++ b/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" diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index c64cc6cfab..7a8b059baa 100644 --- a/frappe/desk/form/load.py +++ b/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') + ) )''' diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index 403da16361..af0b2c9cec 100644 --- a/frappe/desk/form/utils.py +++ b/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) diff --git a/frappe/desk/like.py b/frappe/desk/like.py index ec01eace83..e7d1ff298c 100644 --- a/frappe/desk/like.py +++ b/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) diff --git a/frappe/desk/page/activity/activity.js b/frappe/desk/page/activity/activity.js index 4f012512e4..fe81fdf0fa 100644 --- a/frappe/desk/page/activity/activity.js +++ b/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 }; } diff --git a/frappe/desk/page/activity/activity.py b/frappe/desk/page/activity/activity.py index 54d643ade2..e25c94d0e5 100644 --- a/frappe/desk/page/activity/activity.py +++ b/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() diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index cc7688b7f0..3ac6a7e560 100644 --- a/frappe/desk/reportview.py +++ b/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=[]): diff --git a/frappe/email/doctype/email_unsubscribe/email_unsubscribe.py b/frappe/email/doctype/email_unsubscribe/email_unsubscribe.py index 2f879f98ee..e532e2b7eb 100644 --- a/frappe/email/doctype/email_unsubscribe/email_unsubscribe.py +++ b/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) diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 20e0d00199..153065c5ce 100644 --- a/frappe/model/delete_doc.py +++ b/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, 'reference_type') + 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 + 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), # nosec + (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 - communication_type = 'Communication' - and reference_doctype=%s - and reference_name=%s""", (doctype, name)) + {1}=%s and {2}=%s'''.format(doctype, reference_doctype_field, reference_name_field), # nosec + (reference_doctype, reference_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 - where - reference_doctype=%s - and reference_name=%s""", (doctype, 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), diff --git a/frappe/model/document.py b/frappe/model/document.py index d707fa81cf..d6876c7347 100644 --- a/frappe/model/document.py +++ b/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): diff --git a/frappe/patches.txt b/frappe/patches.txt index 1389408862..2d9a67e32e 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -9,6 +9,7 @@ execute:frappe.reload_doc('core', 'doctype', 'doctype', force=True) #2017-09-22 execute:frappe.reload_doc('core', 'doctype', 'docfield', force=True) #2018-02-20 execute:frappe.reload_doc('core', 'doctype', 'custom_docperm') execute:frappe.reload_doc('core', 'doctype', 'docperm') #2018-05-29 +execute:frappe.reload_doc('core', 'doctype', 'comment') frappe.patches.v8_0.drop_is_custom_from_docperm execute:frappe.reload_doc('core', 'doctype', 'module_def') #2017-09-22 execute:frappe.reload_doc('core', 'doctype', 'version') #2017-04-01 diff --git a/frappe/patches/v12_0/setup_comments_from_communications.py b/frappe/patches/v12_0/setup_comments_from_communications.py new file mode 100644 index 0000000000..2ea6a609b6 --- /dev/null +++ b/frappe/patches/v12_0/setup_comments_from_communications.py @@ -0,0 +1,25 @@ +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.subject = comment.subject + 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"') \ No newline at end of file diff --git a/frappe/patches/v7_0/add_communication_in_doc.py b/frappe/patches/v7_0/add_communication_in_doc.py index 92120634ef..4db02c5bab 100644 --- a/frappe/patches/v7_0/add_communication_in_doc.py +++ b/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", diff --git a/frappe/public/js/frappe/desk.js b/frappe/public/js/frappe/desk.js index a04e4eed4b..d0464d46cd 100644 --- a/frappe/public/js/frappe/desk.js +++ b/frappe/public/js/frappe/desk.js @@ -397,6 +397,7 @@ frappe.Application = Class.extend({ } }); dialog.set_primary_action(__('Login'), () => { + dialog.set_message(__('Authenticating...')); frappe.call({ method: 'login', args: { diff --git a/frappe/public/js/frappe/form/footer/timeline.js b/frappe/public/js/frappe/form/footer/timeline.js index 86f85971c5..81359e2ea2 100644 --- a/frappe/public/js/frappe/form/footer/timeline.js +++ b/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(''); - let communications = this.get_communications(true); - var views = this.get_view_logs(); + // get all communications + let communications = this.get_communications(true); + + // 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"] = ''; } @@ -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").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://' diff --git a/frappe/public/js/frappe/ui/toolbar/notifications.js b/frappe/public/js/frappe/ui/toolbar/notifications.js index 3c12c45e7f..657f53bc42 100644 --- a/frappe/public/js/frappe/ui/toolbar/notifications.js +++ b/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() { diff --git a/frappe/templates/includes/comments/comment.html b/frappe/templates/includes/comments/comment.html index e0f923b2e0..b444c382cf 100644 --- a/frappe/templates/includes/comments/comment.html +++ b/frappe/templates/includes/comments/comment.html @@ -1,11 +1,11 @@
- +
-
{{ comment.sender_full_name }} +
{{ comment.comment_by }} {{ comment.creation|global_date_format }} diff --git a/frappe/templates/includes/comments/comments.html b/frappe/templates/includes/comments/comments.html index bde4972c5b..3b18d8c2cb 100644 --- a/frappe/templates/includes/comments/comments.html +++ b/frappe/templates/includes/comments/comments.html @@ -31,11 +31,11 @@ @@ -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,12 @@ if(r._server_messages) frappe.msgprint(r._server_messages); } else { - $(r.message).appendTo("#comment-list"); + if (r.message) { + $(r.message).appendTo("#comment-list"); + } else { + // probably spam + frappe.msgprint('{{ _("Thank you for your comment. It will be published after approval") }}'); + } $(".no-comment, .add-comment").toggle(false); $("#comment-form").toggle(); } diff --git a/frappe/templates/includes/comments/comments.py b/frappe/templates/includes/comments/comments.py index 6554215bf1..8dd5039e36 100644 --- a/frappe/templates/includes/comments/comments.py +++ b/frappe/templates/includes/comments/comments.py @@ -9,53 +9,43 @@ 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': '', - } - """ +def add_comment(comment, comment_email, comment_by, reference_doctype, reference_name, route): + doc = frappe.get_doc(reference_doctype, reference_name) - if not args: - args = frappe.local.form_dict + comment = doc.add_comment( + text = comment, + comment_email = comment_email, + comment_by = comment_by) - route = args.get("route") + blacklist = ['http://', 'https://', '@gmail.com'] - 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() + if not any([b in comment.content for b in blacklist]): + # probably not spam! + comment.db_set('published', 1) # since comments are embedded in the page, clear the web cache - clear_cache(route) + if route: + 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))] + content = (doc.content + + "

{2}

".format(frappe.utils.get_request_site_address(), + doc.name, + _("View Comment"))) - owner = frappe.db.get_value(doc.doctype, doc.name, "owner") - recipients = list(set(commentors if owner=="Administrator" else (commentors + [owner]))) + # notify creator + frappe.sendmail( + recipients = frappe.db.get_value('User', doc.owner, 'email') or doc.owner, + subject = _('New Comment on {0}: {1}').format(doc.doctype, doc.name), + message = content, + reference_doctype=doc.doctype, + reference_name=doc.name + ) - message = _("{0} by {1}").format(frappe.utils.markdown(args.get("comment")), comment.sender_full_name) - message += "

{2}

".format(frappe.utils.get_request_site_address(), - route, _("View it in your browser")) + if comment.published: + # revert with template if all clear (no backlinks) + template = frappe.get_template("templates/includes/comments/comment.html") - from frappe.email.queue import send + return template.render({"comment": comment.as_dict()}) - send(recipients=recipients, - subject = _("New comment on {0} {1}").format(doc.doctype, doc.name), - message = message, - reference_doctype=doc.doctype, reference_name=doc.name) - - template = frappe.get_template("templates/includes/comments/comment.html") - - return template.render({"comment": comment.as_dict()}) + else: + return '' diff --git a/frappe/tests/test_dynamic_links.py b/frappe/tests/test_dynamic_links.py index 41f16dc3be..98da2cbc35 100644 --- a/frappe/tests/test_dynamic_links.py +++ b/frappe/tests/test_dynamic_links.py @@ -36,10 +36,10 @@ class TestDynamicLinks(unittest.TestCase): }).insert() event.add_comment('Comment', 'test') - self.assertTrue(frappe.get_all('Communication', + self.assertTrue(frappe.get_all('Comment', filters={'reference_doctype':'Event', 'reference_name':event.name})) event.delete() - self.assertFalse(frappe.get_all('Communication', + self.assertFalse(frappe.get_all('Comment', filters={'reference_doctype':'Event', 'reference_name':event.name})) def test_custom_fields(self): diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index f1194edaf4..5587704c27 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -105,7 +105,7 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, if (retry < 5 and (isinstance(e, frappe.RetryBackgroundJobError) or - (frappe.db.is_deadlocked() or frappe.db.is_timedout()))): + (frappe.db.is_deadlocked(e) or frappe.db.is_timedout(e)))): # retry the job if # 1213 = deadlock # 1205 = lock wait timeout diff --git a/frappe/website/doctype/blog_post/blog_post.json b/frappe/website/doctype/blog_post/blog_post.json index dbd665bbf1..d7839d8026 100644 --- a/frappe/website/doctype/blog_post/blog_post.json +++ b/frappe/website/doctype/blog_post/blog_post.json @@ -354,7 +354,7 @@ "read_only": 0, "remember_last_selected_value": 0, "report_hide": 0, - "reqd": 1, + "reqd": 0, "search_index": 0, "set_only_once": 0, "translatable": 0, @@ -470,7 +470,7 @@ "issingle": 0, "istable": 0, "max_attachments": 5, - "modified": "2019-02-01 16:34:03.418705", + "modified": "2019-02-09 11:27:05.619819", "modified_by": "Administrator", "module": "Website", "name": "Blog Post", diff --git a/frappe/website/doctype/blog_post/blog_post.py b/frappe/website/doctype/blog_post/blog_post.py index 7943ead3bc..f6fd164e51 100644 --- a/frappe/website/doctype/blog_post/blog_post.py +++ b/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 diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index d38349ba07..3980245c8c 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -6,6 +6,7 @@ import unittest from frappe.tests.test_website import set_request from frappe.website.render import render +from frappe.utils import random_string class TestBlogPost(unittest.TestCase): def test_generator_view(self): @@ -30,3 +31,27 @@ class TestBlogPost(unittest.TestCase): response = render() self.assertTrue(response.status_code, 404) + +def make_test_blog(): + if not frappe.db.exists('Blog Category', 'Test Blog Category'): + frappe.get_doc(dict( + doctype = 'Blog Category', + category_name = 'Test Blog Category', + title='Test Blog Category')).insert() + if not frappe.db.exists('Blogger', 'test-blogger'): + frappe.get_doc(dict( + doctype = 'Blogger', + short_name='test-blogger', + full_name='Test Blogger')).insert() + test_blog = frappe.get_doc(dict( + doctype = 'Blog Post', + blog_category = 'Test Blog Category', + blogger = 'test-blogger', + title = random_string(20), + route = random_string(20), + content = random_string(20), + published = 1 + )).insert() + + return test_blog + diff --git a/frappe/website/utils.py b/frappe/website/utils.py index d3d2c33c3f..be8bb2a4f8 100644 --- a/frappe/website/utils.py +++ b/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: