From 8381e0048a1bb5d61860af072f8a1ff61fc7ee13 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Mon, 13 May 2019 20:39:49 +0530 Subject: [PATCH 01/25] feat: Communication refactor initial bringup --- .../doctype/communication/communication.js | 82 +++++++++++++++++- .../doctype/communication/communication.json | 39 ++++----- .../doctype/communication/communication.py | 83 +++++++++++++++++-- frappe/core/doctype/communication/email.py | 45 ++++++++++ frappe/desk/form/load.py | 59 ++++++++----- .../doctype/email_account/email_account.py | 9 +- 6 files changed, 260 insertions(+), 57 deletions(-) diff --git a/frappe/core/doctype/communication/communication.js b/frappe/core/doctype/communication/communication.js index 8e35c388a5..ecf2b4670d 100644 --- a/frappe/core/doctype/communication/communication.js +++ b/frappe/core/doctype/communication/communication.js @@ -54,7 +54,15 @@ frappe.ui.form.on("Communication", { frm.trigger('show_relink_dialog'); }); - if(frm.doc.communication_type=="Communication" + frm.add_custom_button(__("Add link"), function() { + frm.trigger('show_add_link_dialog'); + }); + + frm.add_custom_button(__("Remove link"), function() { + frm.trigger('show_remove_link_dialog'); + }); + + if(frm.doc.communication_type=="Communication" && frm.doc.communication_medium == "Email" && frm.doc.sent_or_received == "Received") { @@ -90,7 +98,7 @@ frappe.ui.form.on("Communication", { } } - if(frm.doc.communication_type=="Communication" + if(frm.doc.communication_type=="Communication" && frm.doc.communication_medium == "Phone" && frm.doc.sent_or_received == "Received"){ @@ -148,6 +156,74 @@ frappe.ui.form.on("Communication", { d.show(); }, + show_add_link_dialog: function(frm){ + var d = new frappe.ui.Dialog ({ + title: __("Add new link to Communication"), + fields: [{ + "fieldtype": "Link", + "options": "DocType", + "label": __("Document Type"), + "fieldname": "link_doctype", + "reqd": 1 + }, + { + "fieldtype": "Dynamic Link", + "options": "link_doctype", + "label": __("Document Name"), + "fieldname": "link_name", + "reqd": 1 + }], + primary_action: ({ link_doctype, link_name }) => { + d.hide(); + frm.call('add_link', { + link_doctype, + link_name, + autosave: true + }).then(() => frm.refresh()); + }, + primary_action_label: __('Add Link') + }); + d.fields_dict.link_doctype.get_query = function() { + return { + "filters": { + "name": ["!=", "Communication"], + } + }; + }; + d.show(); + }, + + show_remove_link_dialog: function(frm){ + let options = ''; + + for(var link in frm.doc.dynamic_links){ + let dynamic_link = frm.doc.dynamic_links[link]; + options += '\n' + dynamic_link.link_doctype + ': ' + dynamic_link.link_name; + } + + var d = new frappe.ui.Dialog ({ + title: __("Remove link from Communication"), + fields: [{ + "fieldtype": "Select", + "options": options, + "label": __("Link"), + "fieldname": "link", + "reqd": 1 + }], + primary_action: ({ link }) => { + d.hide(); + frm.call('remove_link', { + link_doctype: link.split(":")[0].trim(), + link_name: link.split(":")[1].trim(), + autosave: true, + ignore_permissions: false + }).then(() => frm.refresh()); + }, + primary_action_label: __('Remove Link') + }); + d.show(); + }, + mark_as_read_unread: function(frm) { var action = frm.doc.seen? "Unread": "Read"; var flag = "(\\SEEN)"; @@ -185,7 +261,7 @@ frappe.ui.form.on("Communication", { forward_mail: function(frm) { var args = frm.events.get_mail_args(frm) - $.extend(args, { + $.extend(args, { forward: true, subject: __("Fw: {0}", [frm.doc.subject]), }) diff --git a/frappe/core/doctype/communication/communication.json b/frappe/core/doctype/communication/communication.json index 3b845964f4..b315a4ce98 100644 --- a/frappe/core/doctype/communication/communication.json +++ b/frappe/core/doctype/communication/communication.json @@ -1,7 +1,7 @@ { "allow_import": 1, "creation": "2013-01-29 10:47:14", - "description": "Keep a track of all communications", + "description": "Keeps track of all communications", "doctype": "DocType", "document_type": "Setup", "engine": "InnoDB", @@ -43,12 +43,11 @@ "email_template", "link_doctype", "link_name", - "timeline_doctype", - "timeline_name", - "timeline_label", "unread_notification_sent", "seen", "_user_tags", + "timeline_links_sections", + "dynamic_links", "email_inbox", "message_id", "uid", @@ -298,25 +297,6 @@ "options": "link_doctype", "read_only": 1 }, - { - "fieldname": "timeline_doctype", - "fieldtype": "Link", - "label": "Timeline DocType", - "options": "DocType", - "read_only": 1 - }, - { - "fieldname": "timeline_name", - "fieldtype": "Dynamic Link", - "label": "Timeline Name", - "options": "timeline_doctype", - "read_only": 1 - }, - { - "fieldname": "timeline_label", - "fieldtype": "Data", - "label": "Timeline field Name" - }, { "default": "0", "fieldname": "unread_notification_sent", @@ -398,11 +378,22 @@ "label": "Email Template", "options": "Email Template", "read_only": 1 + }, + { + "fieldname": "timeline_links_sections", + "fieldtype": "Section Break", + "label": "Timeline Links" + }, + { + "fieldname": "dynamic_links", + "fieldtype": "Table", + "label": "Dynamic Links", + "options": "Dynamic Link" } ], "icon": "fa fa-comment", "idx": 1, - "modified": "2019-05-04 15:36:35.818714", + "modified": "2019-05-13 19:55:35.757242", "modified_by": "Administrator", "module": "Core", "name": "Communication", diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 77ccefba71..5952ae1fc4 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -8,7 +8,7 @@ from frappe.model.document import Document from frappe.utils import validate_email_address, get_fullname, strip_html, cstr 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 +from frappe.core.utils import get_parent_doc from frappe.utils.bot import BotReply from frappe.utils import parse_addr from frappe.core.doctype.comment.comment import update_comment_in_doc @@ -58,7 +58,6 @@ class Communication(Document): self.set_sender_full_name() validate_email(self) - set_timeline_doc(self) def validate_reference(self): if self.reference_doctype and self.reference_name: @@ -231,26 +230,86 @@ class Communication(Document): if commit: frappe.db.commit() + # Timeline Links + def deduplicate_dynamic_links(self): + if self.dynamic_links: + links, duplicate = [], False + + for l in self.dynamic_links: + t = (l.link_doctype, l.link_name) + if not t in links: + links.append(t) + else: + duplicate = True + + if duplicate: + del self.dynamic_links[:] # make it python 2 compatible as list.clear() is python 3 only + for l in links: + self.add_link(link_doctype=l[0], link_name=l[1]) + + def validate_circular_links(self): + for dynamic_link in self.dynamic_links: + # Prevent circular linking of Communication DocTypes + if dynamic_link.link_doctype == "Communication": + circular_linking = False + circular_level_1 = get_timeline_parent_doc(dynamic_link.link_doctype, dynamic_link.link_name) + + # Level 1 + if circular_level_1: + for link in circular_level_1.dynamic_links: + if link.link_doctype == "Communication": + circular_level_2 = get_timeline_parent_doc(link.link_doctype, link.link_name) + + # Level 2 + if circular_level_2: + for ref_link in circular_level_2.dynamic_links: + if ref_link.link_doctype == "Communication": + circular_level_3 = get_timeline_parent_doc(ref_link.link_doctype, ref_link.link_name) + + # Level 3 + if circular_level_3: + if circular_level_3.name == self.name: + circular_linking = True + break + if circular_linking: + frappe.throw(_("Please make sure the Timeline Communication Docs are not circularly linked."), frappe.CircularLinkingError) + + def add_link(self, link_doctype, link_name, autosave=False): + self.append("dynamic_links", + { + "link_doctype": link_doctype, + "link_name": link_name + } + ) + + if autosave: + self.save(ignore_permissions=True) + + def get_links(self): + return self.dynamic_links + + def remove_link(self, link_doctype, link_name, autosave=False, ignore_permissions=True): + for l in self.dynamic_links: + if l.link_doctype == link_doctype and l.link_name == link_name: + self.dynamic_links.remove(l) + + if autosave: + self.save(ignore_permissions=ignore_permissions) def on_doctype_update(): """Add indexes in `tabCommunication`""" frappe.db.add_index("Communication", ["reference_doctype", "reference_name"]) - frappe.db.add_index("Communication", ["timeline_doctype", "timeline_name"]) frappe.db.add_index("Communication", ["link_doctype", "link_name"]) frappe.db.add_index("Communication", ["status", "communication_type"]) def has_permission(doc, ptype, user): if ptype=="read": - if (doc.reference_doctype == "Communication" and doc.reference_name == doc.name) \ - or (doc.timeline_doctype == "Communication" and doc.timeline_name == doc.name): - return + if doc.reference_doctype == "Communication" and doc.reference_name == doc.name: + return if doc.reference_doctype and doc.reference_name: if frappe.has_permission(doc.reference_doctype, ptype="read", doc=doc.reference_name): return True - if doc.timeline_doctype and doc.timeline_name: - if frappe.has_permission(doc.timeline_doctype, ptype="read", doc=doc.timeline_name): - return True def get_permission_query_conditions_for_communication(user): if not user: user = frappe.session.user @@ -270,3 +329,9 @@ def get_permission_query_conditions_for_communication(user): email_accounts = [ '"%s"'%account.get("email_account") for account in accounts ] return """tabCommunication.email_account in ({email_accounts})"""\ .format(email_accounts=','.join(email_accounts)) + +def get_timeline_parent_doc(link_doctype, link_name): + """Returns document of `link_doctype`, `link_name`""" + if link_doctype and link_name: + parent_doc = frappe.get_doc(link_doctype, link_name) + return parent_doc if parent_doc else None diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 0c68f8b118..bc66223787 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -17,6 +17,7 @@ import frappe.email.smtp import time from frappe import _ from frappe.utils.background_jobs import enqueue +from email.utils import parseaddr @frappe.whitelist() def make(doctype=None, name=None, content=None, subject=None, sent_or_received = "Sent", @@ -78,6 +79,15 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = # if no reference given, then send it against the communication comm.db_set(dict(reference_doctype='Communication', reference_name=comm.name)) + # contacts = get_contacts([sender, recipients, cc, bcc]) + # for contact_name in contacts: + # comm.add_link('Contact', contact_name) + + # #link contact's dynamic links to communication + # add_contact_links_to_communication(comm, contact_name) + + # comm.save(ignore_permissions=True) + if isinstance(attachments, string_types): attachments = json.loads(attachments) @@ -559,3 +569,38 @@ def mark_email_as_seen(name=None): frappe.response["filename"] = "imaginary_pixel.png" frappe.response["filecontent"] = buffered_obj.getvalue() +def get_contacts(email_strings): + email_addrs = [] + + for email_string in email_strings: + if email_string: + for email in email_string.split(","): + parsed_email = parseaddr(email)[1] + if parsed_email: + email_addrs.append(parsed_email) + + contacts = [] + for email in email_addrs: + contact_name = frappe.db.get_value('Contact', {'email_id': email}) + + if not contact_name: + contact = frappe.get_doc({ + "doctype": "Contact", + "first_name": frappe.unscrub(email.split("@")[0]), + "email_id": email + }).insert(ignore_permissions=True) + contact_name = contact.name + + contacts.append(contact_name) + + return contacts + +def add_contact_links_to_communication(communication, contact_name): + contact_links = frappe.get_list("Dynamic Link", filters={ + "parenttype": "Contact", + "parent": contact_name + }, fields=["link_doctype", "link_name"]) + + if contact_links: + for contact_link in contact_links: + communication.add_link(contact_link.link_doctype, contact_link.link_name) \ No newline at end of file diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 14ebbaf7fb..dd6a559dc1 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -161,36 +161,55 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= group_by=None, as_dict=True): '''Returns list of communications for a given document''' if not fields: - fields = '''`name`, `communication_type`,`communication_medium`, `comment_type`, - `communication_date`, `content`, `sender`, `sender_full_name`, `cc`, `bcc`, - `creation`, `subject`, `delivery_status`, `_liked_by`, - `timeline_doctype`, `timeline_name`, `reference_doctype`, `reference_name`, - `link_doctype`, `link_name`, `read_by_recipient`, `rating`, 'Communication' AS `doctype`''' + fields = ''' + `tabCommunication`.name, `tabCommunication`.communication_type, `tabCommunication`.communication_medium, + `tabCommunication`.comment_type, `tabCommunication`.communication_date, `tabCommunication`.content, + `tabCommunication`.sender, `tabCommunication`.sender_full_name, `tabCommunication`.cc, `tabCommunication`.bcc, + `tabCommunication`.creation, `tabCommunication`.subject, `tabCommunication`.delivery_status, + `tabCommunication`._liked_by, `tabCommunication`.reference_doctype, `tabCommunication`.reference_name, + `tabCommunication`.link_doctype, `tabCommunication`.link_name, `tabCommunication`.read_by_recipient, + `tabCommunication`.rating, `tabDynamic Link`.link_doctype, `tabDynamic Link`.link_name + ''' - conditions = '''communication_type in ('Communication', 'Feedback') - and ( - (reference_doctype=%(doctype)s and reference_name=%(name)s) + conditions = ''' + `tabCommunication`.communication_type in ('Communication', 'Feedback') + and ( + (`tabCommunication`.reference_doctype=%(doctype)s and `tabCommunication`.reference_name=%(name)s) or ( - (timeline_doctype=%(doctype)s and timeline_name=%(name)s) - and (communication_type='Communication') + (`tabDynamic Link`.link_doctype=%(doctype)s and `tabDynamic Link`.link_name=%(name)s) + and (`tabCommunication`.communication_type='Communication') ) - )''' - + ) + ''' if after: # find after a particular date - conditions+= ' and creation > {0}'.format(after) + conditions += ''' + and `tabCommunication`.creation > {0} + '''.format(after) if doctype=='User': - conditions+= " and not (reference_doctype='User' and communication_type='Communication')" + conditions += ''' + and not (`tabCommunication`.reference_doctype='User' and `tabCommunication`.communication_type='Communication') + ''' - communications = frappe.db.sql("""select {fields} + communications = frappe.db.sql(''' + select {fields} from `tabCommunication` + left join `tabDynamic Link` + on `tabCommunication`.name=`tabDynamic Link`.parent where {conditions} {group_by} - order by creation desc LIMIT %(limit)s OFFSET %(start)s""".format( - fields = fields, conditions=conditions, group_by=group_by or ""), - { "doctype": doctype, "name": name, "start": frappe.utils.cint(start), "limit": limit }, - as_dict=as_dict) + order by `tabCommunication`.creation desc + limit %(limit)s offset %(start)s'''.format( + fields = fields, + conditions=conditions, + group_by=group_by or "" + ),{ + "doctype": doctype, + "name": name, + "start": frappe.utils.cint(start), + "limit": limit + }, as_dict=as_dict, debug=True) return communications @@ -245,4 +264,4 @@ def get_view_logs(doctype, docname): if view_logs: logs = view_logs - return logs + return logs \ No newline at end of file diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 2d23089f5a..1dd8aa284c 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -20,7 +20,7 @@ from datetime import datetime, timedelta from frappe.desk.form import assign_to from frappe.utils.user import get_system_managers from frappe.utils.background_jobs import enqueue, get_jobs -from frappe.core.doctype.communication.email import set_incoming_outgoing_accounts +from frappe.core.doctype.communication.email import set_incoming_outgoing_accounts, get_contacts, add_contact_links_to_communication from frappe.utils.scheduler import log from frappe.utils.html_utils import clean_email_html @@ -386,6 +386,13 @@ class EmailAccount(Document): users = list(set([ user.get("parent") for user in users ])) communication._seen = json.dumps(users) + # contacts = get_contacts([sender, recipients, cc, bcc]) + # for contact_name in contacts: + # comm.add_link('Contact', contact_name) + + # #link contact's dynamic links to communication + # add_contact_links_to_communication(comm, contact_name) + communication.flags.in_receive = True communication.insert(ignore_permissions = 1) From 5d09fb26a9085ca436a267cb88c4a8da396bc8bb Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Tue, 14 May 2019 12:01:14 +0530 Subject: [PATCH 02/25] fix: timeline docs to dynamic links --- .../core/doctype/activity_log/activity_log.py | 2 +- .../doctype/communication/communication.py | 57 ++++++++----------- frappe/core/doctype/communication/email.py | 18 +++--- .../communication/test_communication.py | 12 ++-- frappe/desk/doctype/event/event.py | 33 ++++++++--- frappe/desk/form/load.py | 7 ++- .../doctype/email_account/email_account.py | 12 ++-- .../email_account/test_email_account.py | 27 ++++++--- frappe/model/delete_doc.py | 11 +++- 9 files changed, 106 insertions(+), 73 deletions(-) diff --git a/frappe/core/doctype/activity_log/activity_log.py b/frappe/core/doctype/activity_log/activity_log.py index 7badf737e4..8b7941c086 100644 --- a/frappe/core/doctype/activity_log/activity_log.py +++ b/frappe/core/doctype/activity_log/activity_log.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals from frappe import _ from frappe.utils import get_fullname, now from frappe.model.document import Document -from frappe.core.utils import get_parent_doc, set_timeline_doc +from frappe.core.utils import set_timeline_doc import frappe class ActivityLog(Document): diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 5952ae1fc4..f25264ceb3 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -8,7 +8,6 @@ from frappe.model.document import Document from frappe.utils import validate_email_address, get_fullname, strip_html, cstr 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 from frappe.utils.bot import BotReply from frappe.utils import parse_addr from frappe.core.doctype.comment.comment import update_comment_in_doc @@ -57,6 +56,8 @@ class Communication(Document): self.set_status() self.set_sender_full_name() + self.validate_dynamic_links() + self.deduplicate_dynamic_links() validate_email(self) def validate_reference(self): @@ -72,12 +73,14 @@ class Communication(Document): # Prevent circular linking of Communication DocTypes if self.reference_doctype == "Communication": circular_linking = False - doc = get_parent_doc(self) - while doc.reference_doctype == "Communication": - if get_parent_doc(doc).name==self.name: - circular_linking = True - break - doc = get_parent_doc(doc) + doc = get_parent_doc(self.reference_doctype, self.reference_name) + if doc: + while doc.reference_doctype == "Communication": + if doc: + if doc.reference_name == self.name: + circular_linking = True + break + doc = get_parent_doc(doc.reference_doctype, doc.reference_name) if circular_linking: frappe.throw(_("Please make sure the Reference Communication Docs are not circularly linked."), frappe.CircularLinkingError) @@ -247,32 +250,22 @@ class Communication(Document): for l in links: self.add_link(link_doctype=l[0], link_name=l[1]) - def validate_circular_links(self): + def validate_dynamic_links(self): + circular_linking = False for dynamic_link in self.dynamic_links: - # Prevent circular linking of Communication DocTypes + + # Prevent circular linking of Timeline DocTypes if dynamic_link.link_doctype == "Communication": - circular_linking = False - circular_level_1 = get_timeline_parent_doc(dynamic_link.link_doctype, dynamic_link.link_name) + doc = get_parent_doc(dynamic_link.link_doctype, dynamic_link.link_name) + if doc: + while doc.reference_doctype == "Communication": + if doc: + if doc.reference_name == self.name: + circular_linking = True + break - # Level 1 - if circular_level_1: - for link in circular_level_1.dynamic_links: - if link.link_doctype == "Communication": - circular_level_2 = get_timeline_parent_doc(link.link_doctype, link.link_name) - - # Level 2 - if circular_level_2: - for ref_link in circular_level_2.dynamic_links: - if ref_link.link_doctype == "Communication": - circular_level_3 = get_timeline_parent_doc(ref_link.link_doctype, ref_link.link_name) - - # Level 3 - if circular_level_3: - if circular_level_3.name == self.name: - circular_linking = True - break - if circular_linking: - frappe.throw(_("Please make sure the Timeline Communication Docs are not circularly linked."), frappe.CircularLinkingError) + if circular_linking: + frappe.throw(_("Please make sure the Timeline Communication Docs are not circularly linked."), frappe.CircularLinkingError) def add_link(self, link_doctype, link_name, autosave=False): self.append("dynamic_links", @@ -330,8 +323,8 @@ def get_permission_query_conditions_for_communication(user): return """tabCommunication.email_account in ({email_accounts})"""\ .format(email_accounts=','.join(email_accounts)) -def get_timeline_parent_doc(link_doctype, link_name): +def get_parent_doc(link_doctype, link_name): """Returns document of `link_doctype`, `link_name`""" if link_doctype and link_name: parent_doc = frappe.get_doc(link_doctype, link_name) - return parent_doc if parent_doc else None + return parent_doc if parent_doc else None \ No newline at end of file diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index bc66223787..e554e5f7cf 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -72,21 +72,21 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = "message_id":get_message_id().strip(" <>"), "read_receipt":read_receipt, "has_attachment": 1 if attachments else 0 - }) - comm.insert(ignore_permissions=True) + }).insert(ignore_permissions=True) if not doctype: # if no reference given, then send it against the communication - comm.db_set(dict(reference_doctype='Communication', reference_name=comm.name)) + comm.reference_doctype = 'Communication' + comm.reference_name = comm.name - # contacts = get_contacts([sender, recipients, cc, bcc]) - # for contact_name in contacts: - # comm.add_link('Contact', contact_name) + contacts = get_contacts([sender, recipients, cc, bcc]) + for contact_name in contacts: + comm.add_link('Contact', contact_name) - # #link contact's dynamic links to communication - # add_contact_links_to_communication(comm, contact_name) + #link contact's dynamic links to communication + add_contact_links_to_communication(comm, contact_name) - # comm.save(ignore_permissions=True) + comm.save(ignore_permissions=True) if isinstance(attachments, string_types): attachments = json.loads(attachments) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 1941ff31cc..53b0981af7 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -44,28 +44,30 @@ class TestCommunication(unittest.TestCase): self.assertFalse(frappe.utils.parse_addr(x)[0]) def test_circular_linking(self): - content = "This was created to test circular linking" a = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", - "content": content, + "content": "This was created to test circular linking: Communication A", }).insert() + b = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", - "content": content, + "content": "This was created to test circular linking: Communication B", "reference_doctype": "Communication", "reference_name": a.name }).insert() + c = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", - "content": content, + "content": "This was created to test circular linking: Communication C", "reference_doctype": "Communication", "reference_name": b.name }).insert() + a = frappe.get_doc("Communication", a.name) a.reference_doctype = "Communication" a.reference_name = c.name - self.assertRaises(frappe.CircularLinkingError, a.save) + self.assertRaises(frappe.CircularLinkingError, a.save) diff --git a/frappe/desk/doctype/event/event.py b/frappe/desk/doctype/event/event.py index 04f7455e2d..db601b418c 100644 --- a/frappe/desk/doctype/event/event.py +++ b/frappe/desk/doctype/event/event.py @@ -43,10 +43,17 @@ class Event(Document): def sync_communication(self): if self.event_participants: for participant in self.event_participants: - communication_name = frappe.db.get_value("Communication", dict(reference_doctype=self.doctype, reference_name=self.name, timeline_doctype=participant.reference_doctype, timeline_name=participant.reference_docname), "name") - if communication_name: - communication = frappe.get_doc("Communication", communication_name) - self.update_communication(participant, communication) + comms = frappe.get_list("Communication", filters=[ + ["Communication", "reference_doctype", "=", self.doctype], + ["Communication", "reference_name", "=", self.name], + ["Dynamic Link", "link_doctype", "=", participant.reference_doctype] + ["Dynamic Link", "link_name", "=", participant.reference_docname] + ], fields=["name"]) + + if comms: + for comm in comms: + communication = frappe.get_doc("Communication", comms.name) + self.update_communication(participant, communication) else: meta = frappe.get_meta(participant.reference_doctype) if hasattr(meta, "allow_events_in_timeline") and meta.allow_events_in_timeline==1: @@ -62,12 +69,11 @@ class Event(Document): communication.subject = self.subject communication.content = self.description if self.description else self.subject communication.communication_date = self.starts_on - communication.timeline_doctype = participant.reference_doctype - communication.timeline_name = participant.reference_docname communication.reference_doctype = self.doctype communication.reference_name = self.name communication.communication_medium = communication_mapping[self.event_category] if self.event_category else "" communication.status = "Linked" + communication.add_link(participant.reference_doctype, participant.reference_docname) communication.save(ignore_permissions=True) @frappe.whitelist() @@ -76,9 +82,18 @@ def delete_communication(event, reference_doctype, reference_docname): if isinstance(event, string_types): event = json.loads(event) - communication_name = frappe.db.get_value("Communication", dict(reference_doctype=event["doctype"], reference_name=event["name"], timeline_doctype=deleted_participant.reference_doctype, timeline_name=deleted_participant.reference_docname), "name") - if communication_name: - deletion = frappe.get_doc("Communication", communication_name).delete() + comms = frappe.get_list("Communication", filters=[ + ["Communication", "reference_doctype", "=", event.get("doctype")], + ["Communication", "reference_name", "=", event.get("name")], + ["Dynamic Link", "link_doctype", "=", deleted_participant.reference_doctype], + ["Dynamic Link", "link_name", "=", deleted_participant.reference_docname] + ], fields=["name"]) + + if comms: + deletion = [] + for comm in comms: + delete = frappe.get_doc("Communication", comm.name).delete() + deletion.append(delete) return deletion diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index dd6a559dc1..0d986e5bc0 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -182,6 +182,11 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= ) ''' + if not group_by: + group_by = ''' + group by `tabCommunication`.name + ''' + if after: # find after a particular date conditions += ''' @@ -210,7 +215,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= "start": frappe.utils.cint(start), "limit": limit }, as_dict=as_dict, debug=True) - + print(communications) return communications def get_assignments(dt, dn): diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 1dd8aa284c..fa8b409b1f 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -386,15 +386,15 @@ class EmailAccount(Document): users = list(set([ user.get("parent") for user in users ])) communication._seen = json.dumps(users) - # contacts = get_contacts([sender, recipients, cc, bcc]) - # for contact_name in contacts: - # comm.add_link('Contact', contact_name) + contacts = get_contacts([email.from_email, email.mail.get("To"), email.mail.get("CC"), email.from_email]) + for contact_name in contacts: + communication.add_link('Contact', contact_name) - # #link contact's dynamic links to communication - # add_contact_links_to_communication(comm, contact_name) + #link contact's dynamic links to communication + add_contact_links_to_communication(communication, contact_name) communication.flags.in_receive = True - communication.insert(ignore_permissions = 1) + communication.insert(ignore_permissions=True) # save attachments communication._attachments = email.save_attachments_in_doc(communication) diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index f098a8b205..ac16f12477 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -26,7 +26,7 @@ class TestEmailAccount(unittest.TestCase): email_account.db_set("enable_incoming", 0) def test_incoming(self): - frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") + cleanup("test_sender@example.com") with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-1.raw"), "r") as f: test_mails = [f.read()] @@ -52,7 +52,8 @@ class TestEmailAccount(unittest.TestCase): "reference_name": comm.reference_name, "status":"Not Sent"})) def test_incoming_with_attach(self): - frappe.db.sql("DELETE FROM `tabCommunication` WHERE sender='test_sender@example.com'") + cleanup("test_sender@example.com") + existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) @@ -75,7 +76,7 @@ class TestEmailAccount(unittest.TestCase): def test_incoming_attached_email_from_outlook_plain_text_only(self): - frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") + cleanup("test_sender@example.com") with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-3.raw"), "r") as f: test_mails = [f.read()] @@ -88,7 +89,7 @@ class TestEmailAccount(unittest.TestCase): self.assertTrue("This is an e-mail message sent automatically by Microsoft Outlook while" in comm.content) def test_incoming_attached_email_from_outlook_layers(self): - frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") + cleanup("test_sender@example.com") with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-4.raw"), "r") as f: test_mails = [f.read()] @@ -123,8 +124,7 @@ class TestEmailAccount(unittest.TestCase): self.assertTrue("test-mail-002" in sent_mail.get("Subject")) def test_threading(self): - frappe.db.sql("""delete from tabCommunication - where sender in ('test_sender@example.com', 'test@example.com')""") + cleanup(["in", ['test_sender@example.com', 'test@example.com']]) # send sent_name = make(subject = "Test", content="test content", @@ -149,8 +149,7 @@ class TestEmailAccount(unittest.TestCase): self.assertEqual(comm.reference_name, sent.reference_name) def test_threading_by_subject(self): - frappe.db.sql("""delete from tabCommunication - where sender in ('test_sender@example.com', 'test@example.com')""") + cleanup(["in", ['test_sender@example.com', 'test@example.com']]) with open(os.path.join(os.path.dirname(__file__), "test_mails", "reply-2.raw"), "r") as f: test_mails = [f.read()] @@ -170,7 +169,7 @@ class TestEmailAccount(unittest.TestCase): self.assertEqual(comm_list[0].reference_name, comm_list[1].reference_name) def test_threading_by_message_id(self): - frappe.db.sql("""delete from tabCommunication""") + cleanup() frappe.db.sql("""delete from `tabEmail Queue`""") # reference document for testing @@ -196,3 +195,13 @@ class TestEmailAccount(unittest.TestCase): # check if threaded correctly self.assertEqual(comm_list[0].reference_doctype, event.doctype) self.assertEqual(comm_list[0].reference_name, event.name) + +def cleanup(sender=None): + filters = {} + if sender: + filters.update({"sender": sender}) + + names = frappe.get_list("Communication", filters=filters, fields=["name"]) + for name in names: + frappe.delete_doc_if_exists("Communication", name.name) + frappe.delete_doc_if_exists("Dynamic Link", {"parent": name.name}) \ No newline at end of file diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index ab15dab74e..85d7a3c72b 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -279,7 +279,7 @@ def delete_dynamic_links(doctype, name): # unlink communications clear_references('Communication', doctype, name) clear_references('Communication', doctype, name, 'link_doctype', 'link_name') - clear_references('Communication', doctype, name, 'timeline_doctype', 'timeline_name') + clear_timeline_references(doctype, name) clear_references('Activity Log', doctype, name) clear_references('Activity Log', doctype, name, 'timeline_doctype', 'timeline_name') @@ -300,6 +300,15 @@ def clear_references(doctype, reference_doctype, reference_name, {1}=%s and {2}=%s'''.format(doctype, reference_doctype_field, reference_name_field), # nosec (reference_doctype, reference_name)) +def clear_timeline_references(link_doctype, link_name): + comms = frappe.get_list("Communication", filters=[ + ["Dynamic Link", "link_doctype", "=", link_doctype], + ["Dynamic Link", "link_name", "=", link_name] + ], fields=["name"]) + + for comm in comms: + doc = frappe.get_doc("Communication", comm.name) + doc.remove_link(link_doctype=link_doctype, link_name=link_name, autosave=True) def insert_feed(doc): from frappe.utils import get_fullname From ee27bd80c8c452460d12bd5aebd58bf37ce091b7 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Tue, 14 May 2019 12:58:16 +0530 Subject: [PATCH 03/25] fix: typo --- frappe/desk/doctype/event/event.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/event/event.py b/frappe/desk/doctype/event/event.py index db601b418c..d99cc64436 100644 --- a/frappe/desk/doctype/event/event.py +++ b/frappe/desk/doctype/event/event.py @@ -46,13 +46,13 @@ class Event(Document): comms = frappe.get_list("Communication", filters=[ ["Communication", "reference_doctype", "=", self.doctype], ["Communication", "reference_name", "=", self.name], - ["Dynamic Link", "link_doctype", "=", participant.reference_doctype] + ["Dynamic Link", "link_doctype", "=", participant.reference_doctype], ["Dynamic Link", "link_name", "=", participant.reference_docname] ], fields=["name"]) if comms: for comm in comms: - communication = frappe.get_doc("Communication", comms.name) + communication = frappe.get_doc("Communication", comm.name) self.update_communication(participant, communication) else: meta = frappe.get_meta(participant.reference_doctype) From 8828fcef24d8cb5608bc22802c2f5a54ec474e66 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Tue, 14 May 2019 17:22:24 +0530 Subject: [PATCH 04/25] fix: use inner join instead of left join --- frappe/desk/form/load.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 9c05e5c05a..b6277a5c31 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -167,7 +167,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= `tabCommunication`.creation, `tabCommunication`.subject, `tabCommunication`.delivery_status, `tabCommunication`._liked_by, `tabCommunication`.reference_doctype, `tabCommunication`.reference_name, `tabCommunication`.link_doctype, `tabCommunication`.link_name, `tabCommunication`.read_by_recipient, - `tabCommunication`.rating, `tabDynamic Link`.link_doctype, `tabDynamic Link`.link_name + `tabCommunication`.rating, `tabDynamic Link`.link_doctype, `tabDynamic Link`.link_name, `tabDynamic Link`.parent ''' conditions = ''' @@ -183,7 +183,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= if not group_by: group_by = ''' - group by `tabCommunication`.name + group by `tabCommunication`.name, `tabDynamic Link`.parent ''' if after: @@ -200,7 +200,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= communications = frappe.db.sql(''' select {fields} from `tabCommunication` - left join `tabDynamic Link` + inner join `tabDynamic Link` on `tabCommunication`.name=`tabDynamic Link`.parent where {conditions} {group_by} order by `tabCommunication`.creation desc From d230bf4cea1cc5af7c7e60839b5aa9844b1be7b8 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Tue, 14 May 2019 17:39:55 +0530 Subject: [PATCH 05/25] patch: migarte timeline links to dynamic links --- .../doctype/dynamic_link/dynamic_link.json | 163 +++++------------- frappe/patches.txt | 3 + .../move_timeline_links_to_dynamic_links.py | 25 +++ 3 files changed, 73 insertions(+), 118 deletions(-) create mode 100644 frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py diff --git a/frappe/core/doctype/dynamic_link/dynamic_link.json b/frappe/core/doctype/dynamic_link/dynamic_link.json index 3689be6a3d..a8b871855c 100644 --- a/frappe/core/doctype/dynamic_link/dynamic_link.json +++ b/frappe/core/doctype/dynamic_link/dynamic_link.json @@ -1,125 +1,52 @@ { - "allow_copy": 0, - "allow_import": 0, - "allow_rename": 0, - "beta": 0, - "creation": "2017-01-13 04:55:18.835023", - "custom": 0, - "docstatus": 0, - "doctype": "DocType", - "document_type": "", - "editable_grid": 1, - "engine": "InnoDB", + "creation": "2017-01-13 04:55:18.835023", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "link_doctype", + "link_title", + "cb_00", + "link_name" + ], "fields": [ { - "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_list_view": 1, - "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": 1, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "fieldname": "link_doctype", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Link DocType", + "options": "DocType", + "reqd": 1 + }, { - "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_list_view": 1, - "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": 1, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "fieldname": "link_name", + "fieldtype": "Dynamic Link", + "in_list_view": 1, + "label": "Link Name", + "options": "link_doctype", + "reqd": 1 + }, { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "link_title", - "fieldtype": "Read Only", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Link Title", - "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, - "unique": 0 + "fieldname": "link_title", + "fieldtype": "Read Only", + "in_list_view": 1, + "label": "Link Title", + "read_only": 1 + }, + { + "fieldname": "cb_00", + "fieldtype": "Column Break" } - ], - "hide_heading": 0, - "hide_toolbar": 0, - "idx": 0, - "image_view": 0, - "in_create": 0, - - "is_submittable": 0, - "issingle": 0, - "istable": 1, - "max_attachments": 0, - "modified": "2017-01-17 14:25:49.140730", - "modified_by": "Administrator", - "module": "Core", - "name": "Dynamic Link", - "name_case": "", - "owner": "Administrator", - "permissions": [], - "quick_entry": 1, - "read_only": 0, - "read_only_onload": 0, - "sort_field": "modified", - "sort_order": "DESC", - "track_changes": 1, - "track_seen": 0 + ], + "istable": 1, + "modified": "2019-05-14 17:36:38.391805", + "modified_by": "Administrator", + "module": "Core", + "name": "Dynamic Link", + "owner": "Administrator", + "permissions": [], + "quick_entry": 1, + "sort_field": "modified", + "sort_order": "DESC", + "track_changes": 1 } \ No newline at end of file diff --git a/frappe/patches.txt b/frappe/patches.txt index c0b2a8238f..15f9ad8c73 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -241,3 +241,6 @@ frappe.patches.v12_0.reset_home_settings frappe.patches.v12_0.update_print_format_type frappe.patches.v11_0.remove_doctype_user_permissions_for_page_and_report #2019-05-01 frappe.patches.v12_0.remove_feedback_rating +execute:frappe.reload_doc('core', 'doctype', 'communication') +execute:frappe.reload_doc('core', 'doctype', 'dynamic_link') +frappe.patches.v12_0.move_timeline_links_to_dynamic_links \ No newline at end of file diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py new file mode 100644 index 0000000000..0bb266269e --- /dev/null +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -0,0 +1,25 @@ +from __future__ import unicode_literals + +import frappe + +def execute(): + comm_lists = [] + for communication in frappe.get_list("Communication", filters={"communication_medium": "Email"}, + fields=[ + "name", "creation", "modified", "modified_by", + "timeline_doctype", "timeline_name", + ]): + if communication.timeline_doctype and communication.timeline_name: + comm_lists.append(( + "1", frappe.generate_hash(length=10), "dynamic_links", "Communication", + communication.name, communication.timeline_doctype, communication.timeline_name, + communication.creation, communication.modified, communication.modified_by + )) + + for comm_list in comm_lists: + frappe.db.sql(""" + insert into table `tabDynamic Link` (idx, name, parentfield, parenttype, parent, link_doctype, link_name, creation, modified, modified_by) + values %(values)s""", + { + "values": comm_list + }) \ No newline at end of file From e29f07898b2436db96e5325e447b3101ad7b022e Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Wed, 15 May 2019 11:29:22 +0530 Subject: [PATCH 06/25] fix: query --- frappe/desk/form/load.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index b6277a5c31..6d834b3b12 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -167,7 +167,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= `tabCommunication`.creation, `tabCommunication`.subject, `tabCommunication`.delivery_status, `tabCommunication`._liked_by, `tabCommunication`.reference_doctype, `tabCommunication`.reference_name, `tabCommunication`.link_doctype, `tabCommunication`.link_name, `tabCommunication`.read_by_recipient, - `tabCommunication`.rating, `tabDynamic Link`.link_doctype, `tabDynamic Link`.link_name, `tabDynamic Link`.parent + `tabCommunication`.rating ''' conditions = ''' @@ -181,11 +181,6 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= ) ''' - if not group_by: - group_by = ''' - group by `tabCommunication`.name, `tabDynamic Link`.parent - ''' - if after: # find after a particular date conditions += ''' @@ -198,17 +193,13 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= ''' communications = frappe.db.sql(''' - select {fields} + select distinct {fields} from `tabCommunication` inner join `tabDynamic Link` on `tabCommunication`.name=`tabDynamic Link`.parent where {conditions} {group_by} order by `tabCommunication`.creation desc - limit %(limit)s offset %(start)s'''.format( - fields = fields, - conditions=conditions, - group_by=group_by or "" - ),{ + limit %(limit)s offset %(start)s'''.format(fields = fields, conditions=conditions, group_by=group_by or ""),{ "doctype": doctype, "name": name, "start": frappe.utils.cint(start), From 77d009747d30a4df3ff8ad3036121dfc4a1af36d Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Wed, 15 May 2019 14:00:25 +0530 Subject: [PATCH 07/25] fix: remove debug parameter --- frappe/desk/form/load.py | 4 ++-- frappe/model/delete_doc.py | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 6d834b3b12..102972f331 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -204,8 +204,8 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= "name": name, "start": frappe.utils.cint(start), "limit": limit - }, as_dict=as_dict, debug=True) - print(communications) + }, as_dict=as_dict) + return communications def get_assignments(dt, dn): diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 351cd5ca30..685f295b10 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -307,9 +307,10 @@ def clear_timeline_references(link_doctype, link_name): ["Dynamic Link", "link_name", "=", link_name] ], fields=["name"]) - for comm in comms: - doc = frappe.get_doc("Communication", comm.name) - doc.remove_link(link_doctype=link_doctype, link_name=link_name, autosave=True) + if comms: + for comm in comms: + doc = frappe.get_doc("Communication", comm.name) + doc.remove_link(link_doctype=link_doctype, link_name=link_name, autosave=True, ignore_permissions=True) def insert_feed(doc): from frappe.utils import get_fullname From b31070d427ea3995f67b523ff8c2203ee948155c Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Wed, 15 May 2019 18:00:16 +0530 Subject: [PATCH 08/25] fix: delete_doc for timeline references --- frappe/core/doctype/communication/email.py | 8 ++++---- frappe/model/delete_doc.py | 20 +++++++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index e554e5f7cf..aea2e148b8 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -74,10 +74,10 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = "has_attachment": 1 if attachments else 0 }).insert(ignore_permissions=True) - if not doctype: - # if no reference given, then send it against the communication - comm.reference_doctype = 'Communication' - comm.reference_name = comm.name + # if not doctype: + # # if no reference given, then send it against the communication + # comm.reference_doctype = 'Communication' + # comm.reference_name = comm.name contacts = get_contacts([sender, recipients, cc, bcc]) for contact_name in contacts: diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 685f295b10..ecc7956b73 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -28,6 +28,7 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa doctype = frappe.form_dict.get('dt') name = frappe.form_dict.get('dn') + print(doctype, name) names = name if isinstance(name, string_types) or isinstance(name, integer_types): names = [name] @@ -302,15 +303,24 @@ def clear_references(doctype, reference_doctype, reference_name, (reference_doctype, reference_name)) def clear_timeline_references(link_doctype, link_name): - comms = frappe.get_list("Communication", filters=[ + links = frappe.get_list("Communication", filters=[ ["Dynamic Link", "link_doctype", "=", link_doctype], ["Dynamic Link", "link_name", "=", link_name] ], fields=["name"]) - if comms: - for comm in comms: - doc = frappe.get_doc("Communication", comm.name) - doc.remove_link(link_doctype=link_doctype, link_name=link_name, autosave=True, ignore_permissions=True) + if links: + for link in links: + frappe.db.sql(""" + delete + from `tabDynamic Link` + where `tabDynamic Link`.parent='%(parent)s', + and `tabDynamic Link`.link_doctype='%(doctype)s', + and `tabDynamic Link`.link_name='%(name)s' + """,{ + "parent": link.name, + "doctype": link_doctype, + "name": link_name + }) def insert_feed(doc): from frappe.utils import get_fullname From d075c3b748d86f12fd368bec23ce2e56e18af28b Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Thu, 16 May 2019 13:26:35 +0530 Subject: [PATCH 09/25] fix: link error while deleting linked doc --- frappe/model/delete_doc.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index ecc7956b73..86f72e7416 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -83,6 +83,7 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa doc.flags.in_delete = True doc.run_method('on_change') + clear_timeline_references(doc.doctype, doc.name) frappe.enqueue('frappe.model.delete_doc.delete_dynamic_links', doctype=doc.doctype, name=doc.name, is_async=False if frappe.flags.in_test else True) @@ -310,17 +311,8 @@ def clear_timeline_references(link_doctype, link_name): if links: for link in links: - frappe.db.sql(""" - delete - from `tabDynamic Link` - where `tabDynamic Link`.parent='%(parent)s', - and `tabDynamic Link`.link_doctype='%(doctype)s', - and `tabDynamic Link`.link_name='%(name)s' - """,{ - "parent": link.name, - "doctype": link_doctype, - "name": link_name - }) + doc = frappe.get_doc("Communication", link.name) + doc.remove_link(link_doctype=link_doctype, link_name=link_name, autosave=True) def insert_feed(doc): from frappe.utils import get_fullname From 005c1a8de178b20f8efc0187049640430a2123fa Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Thu, 16 May 2019 17:40:58 +0530 Subject: [PATCH 10/25] fix: use frappe sql for query --- frappe/model/delete_doc.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 86f72e7416..88c5101617 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -83,7 +83,6 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa doc.flags.in_delete = True doc.run_method('on_change') - clear_timeline_references(doc.doctype, doc.name) frappe.enqueue('frappe.model.delete_doc.delete_dynamic_links', doctype=doc.doctype, name=doc.name, is_async=False if frappe.flags.in_test else True) @@ -304,15 +303,8 @@ def clear_references(doctype, reference_doctype, reference_name, (reference_doctype, reference_name)) def clear_timeline_references(link_doctype, link_name): - links = frappe.get_list("Communication", filters=[ - ["Dynamic Link", "link_doctype", "=", link_doctype], - ["Dynamic Link", "link_name", "=", link_name] - ], fields=["name"]) - - if links: - for link in links: - doc = frappe.get_doc("Communication", link.name) - doc.remove_link(link_doctype=link_doctype, link_name=link_name, autosave=True) + frappe.db.sql('''delete from `tabDynamic Link` + where `tabDynamic Link`.link_doctype={0} and `tabDynamic Link`.link_name={1}'''.format(link_doctype, link_name)) # nosec def insert_feed(doc): from frappe.utils import get_fullname From 43bc6ca8d14ae4e99a8b688648cab5be41938ec7 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Thu, 16 May 2019 18:36:10 +0530 Subject: [PATCH 11/25] fix: missing quotes in where clause --- frappe/model/delete_doc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 88c5101617..75365ce9ba 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -303,8 +303,8 @@ def clear_references(doctype, reference_doctype, reference_name, (reference_doctype, reference_name)) def clear_timeline_references(link_doctype, link_name): - frappe.db.sql('''delete from `tabDynamic Link` - where `tabDynamic Link`.link_doctype={0} and `tabDynamic Link`.link_name={1}'''.format(link_doctype, link_name)) # nosec + frappe.db.sql("""delete from `tabDynamic Link` + where `tabDynamic Link`.link_doctype='{0}' and `tabDynamic Link`.link_name='{1}'""".format(link_doctype, link_name)) # nosec def insert_feed(doc): from frappe.utils import get_fullname From aa98dd28a6413d6d49a966d9b96d58967e3325b6 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Thu, 16 May 2019 20:04:49 +0530 Subject: [PATCH 12/25] test: communication tests --- .../core/doctype/communication/test_communication.py | 7 +++++++ frappe/core/doctype/dynamic_link/dynamic_link.json | 11 +++-------- frappe/model/delete_doc.py | 3 +-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 53b0981af7..3b08aaeddb 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -58,6 +58,8 @@ class TestCommunication(unittest.TestCase): "reference_name": a.name }).insert() + b.add_link(link_doctype="Communication", link_name=a.name, autosave=True) + c = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", @@ -66,8 +68,13 @@ class TestCommunication(unittest.TestCase): "reference_name": b.name }).insert() + c.add_link(link_doctype="Communication", link_name=b.name, autosave=True) + a = frappe.get_doc("Communication", a.name) a.reference_doctype = "Communication" a.reference_name = c.name self.assertRaises(frappe.CircularLinkingError, a.save) + + a.add_link(link_doctype="Communication", link_name=c.name) + self.assertRaises(frappe.CircularLinkingError, c.save) diff --git a/frappe/core/doctype/dynamic_link/dynamic_link.json b/frappe/core/doctype/dynamic_link/dynamic_link.json index a8b871855c..abc47df100 100644 --- a/frappe/core/doctype/dynamic_link/dynamic_link.json +++ b/frappe/core/doctype/dynamic_link/dynamic_link.json @@ -5,9 +5,8 @@ "engine": "InnoDB", "field_order": [ "link_doctype", - "link_title", - "cb_00", - "link_name" + "link_name", + "link_title" ], "fields": [ { @@ -32,14 +31,10 @@ "in_list_view": 1, "label": "Link Title", "read_only": 1 - }, - { - "fieldname": "cb_00", - "fieldtype": "Column Break" } ], "istable": 1, - "modified": "2019-05-14 17:36:38.391805", + "modified": "2019-05-16 19:54:31.400026", "modified_by": "Administrator", "module": "Core", "name": "Dynamic Link", diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 75365ce9ba..73c6719d2b 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -28,7 +28,6 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa doctype = frappe.form_dict.get('dt') name = frappe.form_dict.get('dn') - print(doctype, name) names = name if isinstance(name, string_types) or isinstance(name, integer_types): names = [name] @@ -279,9 +278,9 @@ def delete_dynamic_links(doctype, name): delete_references('Document Follow', doctype, name, 'ref_doctype', 'ref_docname') # unlink communications + clear_timeline_references(doctype, name) clear_references('Communication', doctype, name) clear_references('Communication', doctype, name, 'link_doctype', 'link_name') - clear_timeline_references(doctype, name) clear_references('Activity Log', doctype, name) clear_references('Activity Log', doctype, name, 'timeline_doctype', 'timeline_name') From 0d9b7048d94c97c577cf0f7c0f7508cb16bb15a0 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Fri, 17 May 2019 00:06:38 +0530 Subject: [PATCH 13/25] fix: tests for communication --- .../doctype/communication/test_communication.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 3b08aaeddb..715afe2cb3 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -48,7 +48,7 @@ class TestCommunication(unittest.TestCase): "doctype": "Communication", "communication_type": "Communication", "content": "This was created to test circular linking: Communication A", - }).insert() + }).insert(ignore_permissions=True) b = frappe.get_doc({ "doctype": "Communication", @@ -56,9 +56,7 @@ class TestCommunication(unittest.TestCase): "content": "This was created to test circular linking: Communication B", "reference_doctype": "Communication", "reference_name": a.name - }).insert() - - b.add_link(link_doctype="Communication", link_name=a.name, autosave=True) + }).insert(ignore_permissions=True) c = frappe.get_doc({ "doctype": "Communication", @@ -66,15 +64,10 @@ class TestCommunication(unittest.TestCase): "content": "This was created to test circular linking: Communication C", "reference_doctype": "Communication", "reference_name": b.name - }).insert() - - c.add_link(link_doctype="Communication", link_name=b.name, autosave=True) + }).insert(ignore_permissions=True) a = frappe.get_doc("Communication", a.name) a.reference_doctype = "Communication" a.reference_name = c.name self.assertRaises(frappe.CircularLinkingError, a.save) - - a.add_link(link_doctype="Communication", link_name=c.name) - self.assertRaises(frappe.CircularLinkingError, c.save) From 08fb05233467e665e1e25e98db36a3ad41e7ab27 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Fri, 17 May 2019 14:21:37 +0530 Subject: [PATCH 14/25] fix: rename dynamic links to timeline links --- .../doctype/communication/communication.js | 12 +- .../doctype/communication/communication.json | 12 +- .../doctype/communication/communication.py | 106 +++++++++++------- frappe/core/doctype/communication/email.py | 46 +------- .../doctype/email_account/email_account.py | 9 +- 5 files changed, 80 insertions(+), 105 deletions(-) diff --git a/frappe/core/doctype/communication/communication.js b/frappe/core/doctype/communication/communication.js index 5d17a98315..4734f6a11a 100644 --- a/frappe/core/doctype/communication/communication.js +++ b/frappe/core/doctype/communication/communication.js @@ -43,17 +43,17 @@ frappe.ui.form.on("Communication", { }); } - frm.add_custom_button(__("Relink"), function() { + frm.add_custom_button(__("Add Primary link"), function() { frm.trigger('show_relink_dialog'); - }); + }, "Links"); - frm.add_custom_button(__("Add link"), function() { + frm.add_custom_button(__("Add Timeline Link"), function() { frm.trigger('show_add_link_dialog'); - }); + }, "Links"); - frm.add_custom_button(__("Remove link"), function() { + frm.add_custom_button(__("Remove Timeline Link"), function() { frm.trigger('show_remove_link_dialog'); - }); + }, "Links"); if(frm.doc.communication_type=="Communication" && frm.doc.communication_medium == "Email" diff --git a/frappe/core/doctype/communication/communication.json b/frappe/core/doctype/communication/communication.json index b315a4ce98..6642fd60f4 100644 --- a/frappe/core/doctype/communication/communication.json +++ b/frappe/core/doctype/communication/communication.json @@ -47,7 +47,7 @@ "seen", "_user_tags", "timeline_links_sections", - "dynamic_links", + "timeline_links", "email_inbox", "message_id", "uid", @@ -203,6 +203,7 @@ "label": "Date" }, { + "default": "0", "fieldname": "read_receipt", "fieldtype": "Check", "label": "Sent Read Receipt", @@ -219,6 +220,7 @@ "read_only": 1 }, { + "default": "0", "fieldname": "read_by_recipient", "fieldtype": "Check", "label": "Read by Recipient", @@ -305,6 +307,7 @@ "read_only": 1 }, { + "default": "0", "fieldname": "seen", "fieldtype": "Check", "label": "Seen", @@ -348,6 +351,7 @@ "options": "Open\nSpam\nTrash" }, { + "default": "0", "fieldname": "has_attachment", "fieldtype": "Check", "hidden": 1, @@ -385,15 +389,15 @@ "label": "Timeline Links" }, { - "fieldname": "dynamic_links", + "fieldname": "timeline_links", "fieldtype": "Table", - "label": "Dynamic Links", + "label": "Timeline Links", "options": "Dynamic Link" } ], "icon": "fa fa-comment", "idx": 1, - "modified": "2019-05-13 19:55:35.757242", + "modified": "2019-05-17 13:52:44.471256", "modified_by": "Administrator", "module": "Core", "name": "Communication", diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index f25264ceb3..72885d8288 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -8,10 +8,11 @@ from frappe.model.document import Document from frappe.utils import validate_email_address, get_fullname, strip_html, cstr 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 from frappe.utils.bot import BotReply from frappe.utils import parse_addr from frappe.core.doctype.comment.comment import update_comment_in_doc - +from email.utils import parseaddr from collections import Counter exclude_from_linked_with = True @@ -56,10 +57,12 @@ class Communication(Document): self.set_status() self.set_sender_full_name() - self.validate_dynamic_links() - self.deduplicate_dynamic_links() validate_email(self) + if self.communication_medium == "Email": + self.set_timeline_links() + self.deduplicate_timeline_links() + def validate_reference(self): if self.reference_doctype and self.reference_name: if not self.reference_owner: @@ -73,14 +76,13 @@ class Communication(Document): # Prevent circular linking of Communication DocTypes if self.reference_doctype == "Communication": circular_linking = False - doc = get_parent_doc(self.reference_doctype, self.reference_name) - if doc: - while doc.reference_doctype == "Communication": - if doc: - if doc.reference_name == self.name: - circular_linking = True - break - doc = get_parent_doc(doc.reference_doctype, doc.reference_name) + doc = get_parent_doc(self) + while doc.reference_doctype == "Communication": + if get_parent_doc(doc).name==self.name: + circular_linking = True + break + doc = get_parent_doc(doc) + if circular_linking: frappe.throw(_("Please make sure the Reference Communication Docs are not circularly linked."), frappe.CircularLinkingError) @@ -234,11 +236,19 @@ class Communication(Document): frappe.db.commit() # Timeline Links - def deduplicate_dynamic_links(self): - if self.dynamic_links: + def set_timeline_links(self): + contacts = get_contacts([self.sender, self.recipients, self.cc, self.bcc]) + for contact_name in contacts: + self.add_link('Contact', contact_name) + + #link contact's dynamic links to communication + add_contact_links_to_communication(self, contact_name) + + def deduplicate_timeline_links(self): + if self.timeline_links: links, duplicate = [], False - for l in self.dynamic_links: + for l in self.timeline_links: t = (l.link_doctype, l.link_name) if not t in links: links.append(t) @@ -246,29 +256,12 @@ class Communication(Document): duplicate = True if duplicate: - del self.dynamic_links[:] # make it python 2 compatible as list.clear() is python 3 only + del self.timeline_links[:] # make it python 2 compatible as list.clear() is python 3 only for l in links: self.add_link(link_doctype=l[0], link_name=l[1]) - def validate_dynamic_links(self): - circular_linking = False - for dynamic_link in self.dynamic_links: - - # Prevent circular linking of Timeline DocTypes - if dynamic_link.link_doctype == "Communication": - doc = get_parent_doc(dynamic_link.link_doctype, dynamic_link.link_name) - if doc: - while doc.reference_doctype == "Communication": - if doc: - if doc.reference_name == self.name: - circular_linking = True - break - - if circular_linking: - frappe.throw(_("Please make sure the Timeline Communication Docs are not circularly linked."), frappe.CircularLinkingError) - def add_link(self, link_doctype, link_name, autosave=False): - self.append("dynamic_links", + self.append("timeline_links", { "link_doctype": link_doctype, "link_name": link_name @@ -279,12 +272,12 @@ class Communication(Document): self.save(ignore_permissions=True) def get_links(self): - return self.dynamic_links + return self.timeline_links def remove_link(self, link_doctype, link_name, autosave=False, ignore_permissions=True): - for l in self.dynamic_links: + for l in self.timeline_links: if l.link_doctype == link_doctype and l.link_name == link_name: - self.dynamic_links.remove(l) + self.timeline_links.remove(l) if autosave: self.save(ignore_permissions=ignore_permissions) @@ -292,7 +285,6 @@ class Communication(Document): def on_doctype_update(): """Add indexes in `tabCommunication`""" frappe.db.add_index("Communication", ["reference_doctype", "reference_name"]) - frappe.db.add_index("Communication", ["link_doctype", "link_name"]) frappe.db.add_index("Communication", ["status", "communication_type"]) def has_permission(doc, ptype, user): @@ -323,8 +315,38 @@ def get_permission_query_conditions_for_communication(user): return """tabCommunication.email_account in ({email_accounts})"""\ .format(email_accounts=','.join(email_accounts)) -def get_parent_doc(link_doctype, link_name): - """Returns document of `link_doctype`, `link_name`""" - if link_doctype and link_name: - parent_doc = frappe.get_doc(link_doctype, link_name) - return parent_doc if parent_doc else None \ No newline at end of file +def get_contacts(email_strings): + email_addrs = [] + + for email_string in email_strings: + if email_string: + for email in email_string.split(","): + parsed_email = parseaddr(email)[1] + if parsed_email: + email_addrs.append(parsed_email) + + contacts = [] + for email in email_addrs: + contact_name = frappe.db.get_value('Contact', {'email_id': email}) + + if not contact_name: + contact = frappe.get_doc({ + "doctype": "Contact", + "first_name": frappe.unscrub(email.split("@")[0]), + "email_id": email + }).insert(ignore_permissions=True) + contact_name = contact.name + + contacts.append(contact_name) + + return contacts + +def add_contact_links_to_communication(communication, contact_name): + contact_links = frappe.get_list("Dynamic Link", filters={ + "parenttype": "Contact", + "parent": contact_name + }, fields=["link_doctype", "link_name"]) + + if contact_links: + for contact_link in contact_links: + communication.add_link(contact_link.link_doctype, contact_link.link_name) \ No newline at end of file diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index aea2e148b8..aab0bc9bd6 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -17,7 +17,6 @@ import frappe.email.smtp import time from frappe import _ from frappe.utils.background_jobs import enqueue -from email.utils import parseaddr @frappe.whitelist() def make(doctype=None, name=None, content=None, subject=None, sent_or_received = "Sent", @@ -79,13 +78,6 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = # comm.reference_doctype = 'Communication' # comm.reference_name = comm.name - contacts = get_contacts([sender, recipients, cc, bcc]) - for contact_name in contacts: - comm.add_link('Contact', contact_name) - - #link contact's dynamic links to communication - add_contact_links_to_communication(comm, contact_name) - comm.save(ignore_permissions=True) if isinstance(attachments, string_types): @@ -567,40 +559,4 @@ def mark_email_as_seen(name=None): frappe.response["type"] = 'binary' frappe.response["filename"] = "imaginary_pixel.png" - frappe.response["filecontent"] = buffered_obj.getvalue() - -def get_contacts(email_strings): - email_addrs = [] - - for email_string in email_strings: - if email_string: - for email in email_string.split(","): - parsed_email = parseaddr(email)[1] - if parsed_email: - email_addrs.append(parsed_email) - - contacts = [] - for email in email_addrs: - contact_name = frappe.db.get_value('Contact', {'email_id': email}) - - if not contact_name: - contact = frappe.get_doc({ - "doctype": "Contact", - "first_name": frappe.unscrub(email.split("@")[0]), - "email_id": email - }).insert(ignore_permissions=True) - contact_name = contact.name - - contacts.append(contact_name) - - return contacts - -def add_contact_links_to_communication(communication, contact_name): - contact_links = frappe.get_list("Dynamic Link", filters={ - "parenttype": "Contact", - "parent": contact_name - }, fields=["link_doctype", "link_name"]) - - if contact_links: - for contact_link in contact_links: - communication.add_link(contact_link.link_doctype, contact_link.link_name) \ No newline at end of file + frappe.response["filecontent"] = buffered_obj.getvalue() \ No newline at end of file diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index fa8b409b1f..6ef94883f7 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -20,7 +20,7 @@ from datetime import datetime, timedelta from frappe.desk.form import assign_to from frappe.utils.user import get_system_managers from frappe.utils.background_jobs import enqueue, get_jobs -from frappe.core.doctype.communication.email import set_incoming_outgoing_accounts, get_contacts, add_contact_links_to_communication +from frappe.core.doctype.communication.email import set_incoming_outgoing_accounts from frappe.utils.scheduler import log from frappe.utils.html_utils import clean_email_html @@ -386,13 +386,6 @@ class EmailAccount(Document): users = list(set([ user.get("parent") for user in users ])) communication._seen = json.dumps(users) - contacts = get_contacts([email.from_email, email.mail.get("To"), email.mail.get("CC"), email.from_email]) - for contact_name in contacts: - communication.add_link('Contact', contact_name) - - #link contact's dynamic links to communication - add_contact_links_to_communication(communication, contact_name) - communication.flags.in_receive = True communication.insert(ignore_permissions=True) From dbd0a79ffc7e3672e42de6fd635ac4951e6470dd Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Fri, 17 May 2019 14:26:53 +0530 Subject: [PATCH 15/25] patch: move timeline and link to dynamic links --- .../v12_0/move_timeline_links_to_dynamic_links.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index 0bb266269e..9a22f2d536 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -8,13 +8,23 @@ def execute(): fields=[ "name", "creation", "modified", "modified_by", "timeline_doctype", "timeline_name", + "link_doctype", "link_name" ]): + counter = 1 if communication.timeline_doctype and communication.timeline_name: comm_lists.append(( - "1", frappe.generate_hash(length=10), "dynamic_links", "Communication", + counter, frappe.generate_hash(length=10), "timeline_links", "Communication", communication.name, communication.timeline_doctype, communication.timeline_name, communication.creation, communication.modified, communication.modified_by )) + counter += 1 + + if communication.link_doctype and communication.link_name: + comm_lists.append(( + counter, frappe.generate_hash(length=10), "timeline_links", "Communication", + communication.name, communication.link_doctype, communication.link_name, + communication.creation, communication.modified, communication.modified_by + )) for comm_list in comm_lists: frappe.db.sql(""" From 9f02f4284d156a2b9f4f6e197fd73295c513d642 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Sat, 18 May 2019 00:35:17 +0530 Subject: [PATCH 16/25] patch: optimisations --- .../doctype/communication/communication.json | 19 +---- .../move_timeline_links_to_dynamic_links.py | 73 ++++++++++++++----- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/frappe/core/doctype/communication/communication.json b/frappe/core/doctype/communication/communication.json index 6642fd60f4..9492bd205b 100644 --- a/frappe/core/doctype/communication/communication.json +++ b/frappe/core/doctype/communication/communication.json @@ -41,8 +41,6 @@ "user", "column_break_27", "email_template", - "link_doctype", - "link_name", "unread_notification_sent", "seen", "_user_tags", @@ -285,20 +283,6 @@ "fieldname": "column_break_27", "fieldtype": "Column Break" }, - { - "fieldname": "link_doctype", - "fieldtype": "Link", - "label": "Link DocType", - "options": "DocType", - "read_only": 1 - }, - { - "fieldname": "link_name", - "fieldtype": "Dynamic Link", - "label": "Link Name", - "options": "link_doctype", - "read_only": 1 - }, { "default": "0", "fieldname": "unread_notification_sent", @@ -397,7 +381,7 @@ ], "icon": "fa fa-comment", "idx": 1, - "modified": "2019-05-17 13:52:44.471256", + "modified": "2019-05-18 00:34:57.100559", "modified_by": "Administrator", "module": "Core", "name": "Communication", @@ -432,6 +416,7 @@ } ], "search_fields": "subject", + "sort_field": "modified", "sort_order": "DESC", "title_field": "subject", "track_changes": 1, diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index 9a22f2d536..d8d249ae73 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -4,32 +4,65 @@ import frappe def execute(): comm_lists = [] - for communication in frappe.get_list("Communication", filters={"communication_medium": "Email"}, - fields=[ - "name", "creation", "modified", "modified_by", - "timeline_doctype", "timeline_name", - "link_doctype", "link_name" - ]): + for communication in frappe.db.sql(""" + Select + `tabCommunication`.name, `tabCommunication`.creation, `tabCommunication`.modified, + `tabCommunication`.modified_by,`tabCommunication`.timeline_doctype, `tabCommunication`.timeline_name, + `tabCommunication`.link_doctype, `tabCommunication`.link_name + from `tabCommunication` + where `tabCommunication`.communication_medium='Email' + """, as_dict=True): counter = 1 if communication.timeline_doctype and communication.timeline_name: - comm_lists.append(( - counter, frappe.generate_hash(length=10), "timeline_links", "Communication", - communication.name, communication.timeline_doctype, communication.timeline_name, - communication.creation, communication.modified, communication.modified_by - )) + comm_lists.append( + { + "idx": counter, + "name": frappe.generate_hash(length=10), + "parentfield": "timeline_links", + "parenttype": "Communication", + "parent": communication.name, + "link_doctype": communication.timeline_doctype, + "link_name": communication.timeline_name, + "creation": communication.creation, + "modified": communication.modified, + "modified_by": communication.modified_by + } + ) counter += 1 if communication.link_doctype and communication.link_name: - comm_lists.append(( - counter, frappe.generate_hash(length=10), "timeline_links", "Communication", - communication.name, communication.link_doctype, communication.link_name, - communication.creation, communication.modified, communication.modified_by - )) + comm_lists.append( + { + "idx": counter, + "name": frappe.generate_hash(length=10), + "parentfield": "timeline_links", + "parenttype": "Communication", + "parent": communication.name, + "link_doctype": communication.link_doctype, + "link_name": communication.link_name, + "creation": communication.creation, + "modified": communication.modified, + "modified_by": communication.modified_by + } + ) for comm_list in comm_lists: frappe.db.sql(""" - insert into table `tabDynamic Link` (idx, name, parentfield, parenttype, parent, link_doctype, link_name, creation, modified, modified_by) - values %(values)s""", - { - "values": comm_list + INSERT INTO `tabDynamic Link` + (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, + `modified`, `modified_by`) + VALUES + (%(idx)s, %(name)s, %(parentfield)s, %(parenttype)s, %(parent)s, %(link_doctype)s, %(link_name)s,%(creation)s, + %(modified)s, %(modified_by)s) + """,{ + "idx": comm_list.get("idx"), + "name": comm_list.get("name"), + "parentfield": comm_list.get("parentfield"), + "parenttype": comm_list.get("parenttype"), + "parent": comm_list.get("parent"), + "link_doctype": comm_list.get("link_doctype"), + "link_name": comm_list.get("link_name"), + "creation": comm_list.get("creation"), + "modified": comm_list.get("modified"), + "modified_by": comm_list.get("modified_by") }) \ No newline at end of file From 0050fb58ffe13f9afb7131768e237c16512c671b Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Sat, 18 May 2019 10:40:55 +0530 Subject: [PATCH 17/25] fix: remove clear reference links from delete doc --- frappe/model/delete_doc.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 73c6719d2b..8a557f3b3f 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -280,7 +280,6 @@ def delete_dynamic_links(doctype, name): # unlink communications clear_timeline_references(doctype, name) clear_references('Communication', doctype, name) - clear_references('Communication', doctype, name, 'link_doctype', 'link_name') clear_references('Activity Log', doctype, name) clear_references('Activity Log', doctype, name, 'timeline_doctype', 'timeline_name') From defe1b2eef0da56261673d0c2887c391dce97037 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Sat, 18 May 2019 13:37:00 +0530 Subject: [PATCH 18/25] fix: miscellaneous changes --- .../doctype/communication/communication.js | 2 +- frappe/desk/doctype/todo/todo.py | 7 +- frappe/desk/form/load.py | 3 +- frappe/patches.txt | 2 - .../move_timeline_links_to_dynamic_links.py | 85 ++++++------------- 5 files changed, 35 insertions(+), 64 deletions(-) diff --git a/frappe/core/doctype/communication/communication.js b/frappe/core/doctype/communication/communication.js index 4734f6a11a..180ba8d25c 100644 --- a/frappe/core/doctype/communication/communication.js +++ b/frappe/core/doctype/communication/communication.js @@ -43,7 +43,7 @@ frappe.ui.form.on("Communication", { }); } - frm.add_custom_button(__("Add Primary link"), function() { + frm.add_custom_button(__("Change Reference Link"), function() { frm.trigger('show_relink_dialog'); }, "Links"); diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index 45c3874806..051d26d7be 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -43,8 +43,11 @@ class ToDo(Document): def on_trash(self): # unlink todo from linked comments - frappe.db.sql("""update `tabCommunication` set link_doctype=null, link_name=null - where link_doctype=%(doctype)s and link_name=%(name)s""", {"doctype": self.doctype, "name": self.name}) + frappe.db.sql(""" + delete from `tabDynamic Link` + where link_doctype=%(doctype)s and link_name=%(name)s""", { + "doctype": self.doctype, "name": self.name + }) self.update_in_reference() diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 102972f331..2985a8858e 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -166,8 +166,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= `tabCommunication`.sender, `tabCommunication`.sender_full_name, `tabCommunication`.cc, `tabCommunication`.bcc, `tabCommunication`.creation, `tabCommunication`.subject, `tabCommunication`.delivery_status, `tabCommunication`._liked_by, `tabCommunication`.reference_doctype, `tabCommunication`.reference_name, - `tabCommunication`.link_doctype, `tabCommunication`.link_name, `tabCommunication`.read_by_recipient, - `tabCommunication`.rating + `tabCommunication`.read_by_recipient, `tabCommunication`.rating ''' conditions = ''' diff --git a/frappe/patches.txt b/frappe/patches.txt index 15f9ad8c73..911e793687 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -241,6 +241,4 @@ frappe.patches.v12_0.reset_home_settings frappe.patches.v12_0.update_print_format_type frappe.patches.v11_0.remove_doctype_user_permissions_for_page_and_report #2019-05-01 frappe.patches.v12_0.remove_feedback_rating -execute:frappe.reload_doc('core', 'doctype', 'communication') -execute:frappe.reload_doc('core', 'doctype', 'dynamic_link') frappe.patches.v12_0.move_timeline_links_to_dynamic_links \ No newline at end of file diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index d8d249ae73..9d6cfffb84 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -3,66 +3,37 @@ from __future__ import unicode_literals import frappe def execute(): - comm_lists = [] - for communication in frappe.db.sql(""" - Select - `tabCommunication`.name, `tabCommunication`.creation, `tabCommunication`.modified, - `tabCommunication`.modified_by,`tabCommunication`.timeline_doctype, `tabCommunication`.timeline_name, - `tabCommunication`.link_doctype, `tabCommunication`.link_name - from `tabCommunication` - where `tabCommunication`.communication_medium='Email' - """, as_dict=True): + frappe.reload_doc('core', 'doctype', 'communication') + + sql_query = """INSERT INTO `tabDynamic Link` + (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, + `modified`, `modified_by`) + VALUES """ + + communications = frappe.db.sql(""" + Select + `tabCommunication`.name, `tabCommunication`.creation, `tabCommunication`.modified, + `tabCommunication`.modified_by,`tabCommunication`.timeline_doctype, `tabCommunication`.timeline_name, + `tabCommunication`.link_doctype, `tabCommunication`.link_name + from `tabCommunication` + where `tabCommunication`.communication_medium='Email' + """, as_dict=True) + + for communication in communications: counter = 1 if communication.timeline_doctype and communication.timeline_name: - comm_lists.append( - { - "idx": counter, - "name": frappe.generate_hash(length=10), - "parentfield": "timeline_links", - "parenttype": "Communication", - "parent": communication.name, - "link_doctype": communication.timeline_doctype, - "link_name": communication.timeline_name, - "creation": communication.creation, - "modified": communication.modified, - "modified_by": communication.modified_by - } - ) + sql_query += str(( + counter, frappe.generate_hash(length=10), "timeline_links", "Communication", communication.name, + communication.timeline_doctype, communication.timeline_name, communication.creation, + communication.modified, communication.modified_by + )) + """, """ counter += 1 if communication.link_doctype and communication.link_name: - comm_lists.append( - { - "idx": counter, - "name": frappe.generate_hash(length=10), - "parentfield": "timeline_links", - "parenttype": "Communication", - "parent": communication.name, - "link_doctype": communication.link_doctype, - "link_name": communication.link_name, - "creation": communication.creation, - "modified": communication.modified, - "modified_by": communication.modified_by - } - ) + sql_query += str(( + counter, frappe.generate_hash(length=10), "timeline_links", "Communication", communication.name, + communication.link_doctype, communication.link_name, communication.creation, + communication.modified, communication.modified_by + )) + """, """ - for comm_list in comm_lists: - frappe.db.sql(""" - INSERT INTO `tabDynamic Link` - (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, - `modified`, `modified_by`) - VALUES - (%(idx)s, %(name)s, %(parentfield)s, %(parenttype)s, %(parent)s, %(link_doctype)s, %(link_name)s,%(creation)s, - %(modified)s, %(modified_by)s) - """,{ - "idx": comm_list.get("idx"), - "name": comm_list.get("name"), - "parentfield": comm_list.get("parentfield"), - "parenttype": comm_list.get("parenttype"), - "parent": comm_list.get("parent"), - "link_doctype": comm_list.get("link_doctype"), - "link_name": comm_list.get("link_name"), - "creation": comm_list.get("creation"), - "modified": comm_list.get("modified"), - "modified_by": comm_list.get("modified_by") - }) \ No newline at end of file + frappe.db.sql(sql_query) From 6403cf45ae2baf2110cb5cb2d03a324affcc87bb Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Sat, 18 May 2019 18:54:47 +0530 Subject: [PATCH 19/25] patch: optimisations --- .../move_timeline_links_to_dynamic_links.py | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index 9d6cfffb84..507ed05e18 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -5,11 +5,6 @@ import frappe def execute(): frappe.reload_doc('core', 'doctype', 'communication') - sql_query = """INSERT INTO `tabDynamic Link` - (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, - `modified`, `modified_by`) - VALUES """ - communications = frappe.db.sql(""" Select `tabCommunication`.name, `tabCommunication`.creation, `tabCommunication`.modified, @@ -19,21 +14,32 @@ def execute(): where `tabCommunication`.communication_medium='Email' """, as_dict=True) - for communication in communications: + values = [] + for count, communication in enumerate(communications): counter = 1 if communication.timeline_doctype and communication.timeline_name: - sql_query += str(( - counter, frappe.generate_hash(length=10), "timeline_links", "Communication", communication.name, - communication.timeline_doctype, communication.timeline_name, communication.creation, - communication.modified, communication.modified_by - )) + """, """ + values.append("""({0}, '{1}', 'timeline_links', 'Communication', '{2}', '{3}', '{4}', '{5}', '{6}', '{7}')""".format( + counter, frappe.generate_hash(length=10), communication.name, communication.timeline_doctype, + communication.timeline_name, communication.creation, communication.modified, communication.modified_by + )) counter += 1 - if communication.link_doctype and communication.link_name: - sql_query += str(( - counter, frappe.generate_hash(length=10), "timeline_links", "Communication", communication.name, - communication.link_doctype, communication.link_name, communication.creation, - communication.modified, communication.modified_by - )) + """, """ - - frappe.db.sql(sql_query) + values.append("""({0}, '{1}', 'timeline_links', 'Communication', '{2}', '{3}', '{4}', '{5}', '{6}', '{7}')""".format( + counter, frappe.generate_hash(length=10), communication.name, communication.link_doctype, + communication.link_name, communication.creation, communication.modified, communication.modified_by + )) + if count % 200 == 0 or count == len(communications) - 1: + print(""" + INSERT INTO `tabDynamic Link` + (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, + `modified`, `modified_by`) + VALUES {0} + """.format(", ".join([d for d in values]))) + frappe.db.sql(""" + INSERT INTO `tabDynamic Link` + (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, + `modified`, `modified_by`) + VALUES {0} + """.format(", ".join([d for d in values]))) + # frappe.throw("YA") + values = [] \ No newline at end of file From c52c1611cb462a7f7bf12f7735634db3e4e7edee Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Sat, 18 May 2019 19:17:00 +0530 Subject: [PATCH 20/25] patch: optimisations --- .../v12_0/move_timeline_links_to_dynamic_links.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index 507ed05e18..cbc3846604 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -15,6 +15,7 @@ def execute(): """, as_dict=True) values = [] + for count, communication in enumerate(communications): counter = 1 if communication.timeline_doctype and communication.timeline_name: @@ -28,18 +29,11 @@ def execute(): counter, frappe.generate_hash(length=10), communication.name, communication.link_doctype, communication.link_name, communication.creation, communication.modified, communication.modified_by )) - if count % 200 == 0 or count == len(communications) - 1: - print(""" - INSERT INTO `tabDynamic Link` - (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, - `modified`, `modified_by`) - VALUES {0} - """.format(", ".join([d for d in values]))) + if count % 1000 == 0 or count == len(communications) - 1: frappe.db.sql(""" INSERT INTO `tabDynamic Link` (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, `modified`, `modified_by`) VALUES {0} """.format(", ".join([d for d in values]))) - # frappe.throw("YA") values = [] \ No newline at end of file From 05b3050b630501ef11c413bb280492eb4d32c063 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Sun, 19 May 2019 00:39:11 +0530 Subject: [PATCH 21/25] patch: batch 10000 query for patch --- frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index cbc3846604..ff61208bf4 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -29,7 +29,7 @@ def execute(): counter, frappe.generate_hash(length=10), communication.name, communication.link_doctype, communication.link_name, communication.creation, communication.modified, communication.modified_by )) - if count % 1000 == 0 or count == len(communications) - 1: + if count % 10000 == 0 or count == len(communications) - 1: frappe.db.sql(""" INSERT INTO `tabDynamic Link` (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, From 127cf29a2037c7f33afdcc151526d84265dd1dc5 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Mon, 20 May 2019 13:04:16 +0530 Subject: [PATCH 22/25] tests: added new test cases --- .../doctype/communication/communication.json | 18 ++- .../doctype/communication/communication.py | 6 +- frappe/core/doctype/communication/email.py | 5 - .../communication/test_communication.py | 107 ++++++++++++++++++ .../move_timeline_links_to_dynamic_links.py | 57 ++++++---- 5 files changed, 157 insertions(+), 36 deletions(-) diff --git a/frappe/core/doctype/communication/communication.json b/frappe/core/doctype/communication/communication.json index 9492bd205b..3eeb63ceb9 100644 --- a/frappe/core/doctype/communication/communication.json +++ b/frappe/core/doctype/communication/communication.json @@ -368,6 +368,7 @@ "read_only": 1 }, { + "collapsible": 1, "fieldname": "timeline_links_sections", "fieldtype": "Section Break", "label": "Timeline Links" @@ -376,13 +377,14 @@ "fieldname": "timeline_links", "fieldtype": "Table", "label": "Timeline Links", - "options": "Dynamic Link" + "options": "Dynamic Link", + "permlevel": 2 } ], "icon": "fa fa-comment", "idx": 1, - "modified": "2019-05-18 00:34:57.100559", - "modified_by": "Administrator", + "modified": "2019-05-20 12:34:21.021632", + "modified_by": "faris@erpnext.com", "module": "Core", "name": "Communication", "owner": "Administrator", @@ -407,6 +409,16 @@ "role": "System Manager", "share": 1 }, + { + "email": 1, + "export": 1, + "permlevel": 2, + "print": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1 + }, { "delete": 1, "email": 1, diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 72885d8288..c164f7f8e0 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -321,9 +321,9 @@ def get_contacts(email_strings): for email_string in email_strings: if email_string: for email in email_string.split(","): - parsed_email = parseaddr(email)[1] - if parsed_email: - email_addrs.append(parsed_email) + parsed_email = parseaddr(email)[1] + if parsed_email: + email_addrs.append(parsed_email) contacts = [] for email in email_addrs: diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index aab0bc9bd6..36377a90f7 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -73,11 +73,6 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = "has_attachment": 1 if attachments else 0 }).insert(ignore_permissions=True) - # if not doctype: - # # if no reference given, then send it against the communication - # comm.reference_doctype = 'Communication' - # comm.reference_name = comm.name - comm.save(ignore_permissions=True) if isinstance(attachments, string_types): diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 715afe2cb3..5b0ad3f3b0 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -71,3 +71,110 @@ class TestCommunication(unittest.TestCase): a.reference_name = c.name self.assertRaises(frappe.CircularLinkingError, a.save) + + def test_deduplication_timeline_links(self): + todo = frappe.get_doc({ + "doctype": "ToDo", + "description": "Test ToDo" + }).insert(ignore_permissions=True) + + comm = frappe.get_doc({ + "doctype": "Communication", + "communication_type": "Communication", + "content": "Deduplication of Links", + "communication_medium": "Email", + "timeline_links": [ + { + "link_doctype": "ToDo", + "link_name": todo.name + }, + { + "link_doctype": "ToDo", + "link_name": todo.name + } + ] + }).insert(ignore_permissions=True) + + comm = frappe.get_doc("Communication", comm.name) + + self.assertNotEqual(2, len(comm.timeline_links)) + + def test_contacts_attached(self): + contact_sender = frappe.get_doc({ + "doctype": "Contact", + "first_name": frappe.generate_hash(length=10), + "email_id": "comm_sender@example.com" + }).insert(ignore_permissions=True) + + contact_recipient = frappe.get_doc({ + "doctype": "Contact", + "first_name": frappe.generate_hash(length=10), + "email_id": "comm_recipient@example.com" + }).insert(ignore_permissions=True) + + contact_cc = frappe.get_doc({ + "doctype": "Contact", + "first_name": frappe.generate_hash(length=10), + "email_id": "comm_cc@example.com" + }).insert(ignore_permissions=True) + + comm = frappe.get_doc({ + "doctype": "Communication", + "communication_medium": "Email", + "subject": "Contacts Attached Test", + "sender": "comm_sender@example.com", + "recipients": "comm_recipient@example.com" + }).insert(ignore_permissions=True) + + comm = frappe.get_doc("Communication", comm.name) + + contact_links = [] + for timeline_link in comm.timeline_links: + contact_links.append(timeline_link.link_name) + + self.assertIn(contact_sender.name, contact_links) + self.assertIn(contact_recipient.name, contact_links) + self.assertIn(contact_cc.name, contact_links) + + def test_get_communication_data(self): + from frappe.desk.form.load import get_communication_data + + todo = frappe.get_doc({ + "doctype": "ToDo", + "description": "Test ToDo" + }).insert(ignore_permissions=True) + + comm_todo_1 = frappe.get_doc({ + "doctype": "Communication", + "communication_type": "Communication", + "content": "Test Get Communication Data 1", + "communication_medium": "Email", + "timeline_links": [ + { + "link_doctype": "ToDo", + "link_name": todo.name + } + ] + }).insert(ignore_permissions=True) + + comm_todo_2 = frappe.get_doc({ + "doctype": "Communication", + "communication_type": "Communication", + "content": "Test Get Communication Data 2", + "communication_medium": "Email", + "timeline_links": [ + { + "link_doctype": "ToDo", + "link_name": todo.name + } + ] + }).insert(ignore_permissions=True) + + comms = get_communication_data("ToDo", todo.name, as_dict=True) + + data = [] + for comm in comms: + data.append(comm.name) + + self.assertIn(comm_todo_1.name, data) + self.assertIn(comm_todo_2.name, data) \ No newline at end of file diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index ff61208bf4..cf1d22f27f 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -6,34 +6,41 @@ def execute(): frappe.reload_doc('core', 'doctype', 'communication') communications = frappe.db.sql(""" - Select - `tabCommunication`.name, `tabCommunication`.creation, `tabCommunication`.modified, - `tabCommunication`.modified_by,`tabCommunication`.timeline_doctype, `tabCommunication`.timeline_name, - `tabCommunication`.link_doctype, `tabCommunication`.link_name - from `tabCommunication` - where `tabCommunication`.communication_medium='Email' - """, as_dict=True) - - values = [] + SELECT + `tabCommunication`.name, `tabCommunication`.creation, `tabCommunication`.modified, + `tabCommunication`.modified_by,`tabCommunication`.timeline_doctype, `tabCommunication`.timeline_name, + `tabCommunication`.link_doctype, `tabCommunication`.link_name + FROM `tabCommunication` + WHERE `tabCommunication`.communication_medium='Email' + """, as_dict=True) for count, communication in enumerate(communications): counter = 1 if communication.timeline_doctype and communication.timeline_name: - values.append("""({0}, '{1}', 'timeline_links', 'Communication', '{2}', '{3}', '{4}', '{5}', '{6}', '{7}')""".format( - counter, frappe.generate_hash(length=10), communication.name, communication.timeline_doctype, - communication.timeline_name, communication.creation, communication.modified, communication.modified_by - )) + values = [ + counter, frappe.generate_hash(length=10), 'timeline_links', 'Communication', communication.name, + communication.timeline_doctype, communication.timeline_name, communication.creation, communication.modified, + communication.modified_by + ] + execute_query(values) counter += 1 + if communication.link_doctype and communication.link_name: - values.append("""({0}, '{1}', 'timeline_links', 'Communication', '{2}', '{3}', '{4}', '{5}', '{6}', '{7}')""".format( - counter, frappe.generate_hash(length=10), communication.name, communication.link_doctype, - communication.link_name, communication.creation, communication.modified, communication.modified_by - )) - if count % 10000 == 0 or count == len(communications) - 1: - frappe.db.sql(""" - INSERT INTO `tabDynamic Link` - (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, - `modified`, `modified_by`) - VALUES {0} - """.format(", ".join([d for d in values]))) - values = [] \ No newline at end of file + values = [ + counter, frappe.generate_hash(length=10), 'timeline_links', 'Communication', communication.name, + communication.link_doctype, communication.link_name, communication.creation, communication.modified, + communication.modified_by + ] + execute_query(values) + +def execute_query(values): + try: + frappe.db.sql(""" + INSERT INTO `tabDynamic Link` + (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, + `modified`, `modified_by`) + VALUES ({0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}) + """.format(values[0], values[1], values[2], values[3], values[4], values[5], values[6], values[7], values[8], values[9]) + except IntegrityError as e: + values[1] = frappe.generate_hash(length=10) + execute_query(values) From c86ba036327f89c174dd7d4f9d9df630b10fe3c8 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Mon, 20 May 2019 13:38:01 +0530 Subject: [PATCH 23/25] tests: fix test cases --- frappe/core/doctype/communication/test_communication.py | 3 ++- frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 5b0ad3f3b0..77eb066b1d 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -123,7 +123,8 @@ class TestCommunication(unittest.TestCase): "communication_medium": "Email", "subject": "Contacts Attached Test", "sender": "comm_sender@example.com", - "recipients": "comm_recipient@example.com" + "recipients": "comm_recipient@example.com", + "cc": "comm_cc@example.com" }).insert(ignore_permissions=True) comm = frappe.get_doc("Communication", comm.name) diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index cf1d22f27f..624be6ae40 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -41,6 +41,6 @@ def execute_query(values): `modified`, `modified_by`) VALUES ({0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}) """.format(values[0], values[1], values[2], values[3], values[4], values[5], values[6], values[7], values[8], values[9]) - except IntegrityError as e: + except Exception as e: values[1] = frappe.generate_hash(length=10) execute_query(values) From b4e561c047057fcd5c918e0b143257bd8b1b8ef7 Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Mon, 20 May 2019 14:15:57 +0530 Subject: [PATCH 24/25] tests: fix test cases --- .../doctype/communication/communication.js | 80 +------------------ .../doctype/communication/communication.json | 6 +- .../doctype/communication/communication.py | 1 + .../communication/test_communication.py | 60 ++++++-------- 4 files changed, 31 insertions(+), 116 deletions(-) diff --git a/frappe/core/doctype/communication/communication.js b/frappe/core/doctype/communication/communication.js index 180ba8d25c..924c29bee2 100644 --- a/frappe/core/doctype/communication/communication.js +++ b/frappe/core/doctype/communication/communication.js @@ -43,17 +43,9 @@ frappe.ui.form.on("Communication", { }); } - frm.add_custom_button(__("Change Reference Link"), function() { + frm.add_custom_button(__("Relink"), function() { frm.trigger('show_relink_dialog'); - }, "Links"); - - frm.add_custom_button(__("Add Timeline Link"), function() { - frm.trigger('show_add_link_dialog'); - }, "Links"); - - frm.add_custom_button(__("Remove Timeline Link"), function() { - frm.trigger('show_remove_link_dialog'); - }, "Links"); + }); if(frm.doc.communication_type=="Communication" && frm.doc.communication_medium == "Email" @@ -149,74 +141,6 @@ frappe.ui.form.on("Communication", { d.show(); }, - show_add_link_dialog: function(frm){ - var d = new frappe.ui.Dialog ({ - title: __("Add new link to Communication"), - fields: [{ - "fieldtype": "Link", - "options": "DocType", - "label": __("Document Type"), - "fieldname": "link_doctype", - "reqd": 1 - }, - { - "fieldtype": "Dynamic Link", - "options": "link_doctype", - "label": __("Document Name"), - "fieldname": "link_name", - "reqd": 1 - }], - primary_action: ({ link_doctype, link_name }) => { - d.hide(); - frm.call('add_link', { - link_doctype, - link_name, - autosave: true - }).then(() => frm.refresh()); - }, - primary_action_label: __('Add Link') - }); - d.fields_dict.link_doctype.get_query = function() { - return { - "filters": { - "name": ["!=", "Communication"], - } - }; - }; - d.show(); - }, - - show_remove_link_dialog: function(frm){ - let options = ''; - - for(var link in frm.doc.dynamic_links){ - let dynamic_link = frm.doc.dynamic_links[link]; - options += '\n' + dynamic_link.link_doctype + ': ' + dynamic_link.link_name; - } - - var d = new frappe.ui.Dialog ({ - title: __("Remove link from Communication"), - fields: [{ - "fieldtype": "Select", - "options": options, - "label": __("Link"), - "fieldname": "link", - "reqd": 1 - }], - primary_action: ({ link }) => { - d.hide(); - frm.call('remove_link', { - link_doctype: link.split(":")[0].trim(), - link_name: link.split(":")[1].trim(), - autosave: true, - ignore_permissions: false - }).then(() => frm.refresh()); - }, - primary_action_label: __('Remove Link') - }); - d.show(); - }, - mark_as_read_unread: function(frm) { var action = frm.doc.seen? "Unread": "Read"; var flag = "(\\SEEN)"; diff --git a/frappe/core/doctype/communication/communication.json b/frappe/core/doctype/communication/communication.json index 3eeb63ceb9..68a0adb634 100644 --- a/frappe/core/doctype/communication/communication.json +++ b/frappe/core/doctype/communication/communication.json @@ -383,8 +383,8 @@ ], "icon": "fa fa-comment", "idx": 1, - "modified": "2019-05-20 12:34:21.021632", - "modified_by": "faris@erpnext.com", + "modified": "2019-05-20 14:14:01.514493", + "modified_by": "Administrator", "module": "Core", "name": "Communication", "owner": "Administrator", @@ -410,10 +410,12 @@ "share": 1 }, { + "delete": 1, "email": 1, "export": 1, "permlevel": 2, "print": 1, + "read": 1, "report": 1, "role": "System Manager", "share": 1, diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index c164f7f8e0..aa70024eca 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -238,6 +238,7 @@ class Communication(Document): # Timeline Links def set_timeline_links(self): contacts = get_contacts([self.sender, self.recipients, self.cc, self.bcc]) + print(contacts) for contact_name in contacts: self.add_link('Contact', contact_name) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 77eb066b1d..a783157dc0 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -73,28 +73,23 @@ class TestCommunication(unittest.TestCase): self.assertRaises(frappe.CircularLinkingError, a.save) def test_deduplication_timeline_links(self): - todo = frappe.get_doc({ - "doctype": "ToDo", - "description": "Test ToDo" + note = frappe.get_doc({ + "doctype": "Note", + "title": "deduplication timeline links", + "content": "deduplication timeline links" }).insert(ignore_permissions=True) comm = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", "content": "Deduplication of Links", - "communication_medium": "Email", - "timeline_links": [ - { - "link_doctype": "ToDo", - "link_name": todo.name - }, - { - "link_doctype": "ToDo", - "link_name": todo.name - } - ] + "communication_medium": "Email" }).insert(ignore_permissions=True) + #adding same link twice + comm.add_link(link_doctype="Note", link_name=note.name, autosave=True) + comm.add_link(link_doctype="Note", link_name=note.name, autosave=True) + comm = frappe.get_doc("Communication", comm.name) self.assertNotEqual(2, len(comm.timeline_links)) @@ -140,42 +135,35 @@ class TestCommunication(unittest.TestCase): def test_get_communication_data(self): from frappe.desk.form.load import get_communication_data - todo = frappe.get_doc({ - "doctype": "ToDo", - "description": "Test ToDo" + note = frappe.get_doc({ + "doctype": "Note", + "title": "get communication data", + "content": "get communication data" }).insert(ignore_permissions=True) - comm_todo_1 = frappe.get_doc({ + comm_note_1 = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", "content": "Test Get Communication Data 1", - "communication_medium": "Email", - "timeline_links": [ - { - "link_doctype": "ToDo", - "link_name": todo.name - } - ] + "communication_medium": "Email" }).insert(ignore_permissions=True) - comm_todo_2 = frappe.get_doc({ + comm_note_1.add_link(link_doctype="Note", link_name=note.name, autosave=True) + + comm_note_2 = frappe.get_doc({ "doctype": "Communication", "communication_type": "Communication", "content": "Test Get Communication Data 2", - "communication_medium": "Email", - "timeline_links": [ - { - "link_doctype": "ToDo", - "link_name": todo.name - } - ] + "communication_medium": "Email" }).insert(ignore_permissions=True) - comms = get_communication_data("ToDo", todo.name, as_dict=True) + comm_note_2.add_link(link_doctype="Note", link_name=note.name, autosave=True) + + comms = get_communication_data("Note", note.name, as_dict=True) data = [] for comm in comms: data.append(comm.name) - self.assertIn(comm_todo_1.name, data) - self.assertIn(comm_todo_2.name, data) \ No newline at end of file + self.assertIn(comm_note_1.name, data) + self.assertIn(comm_note_2.name, data) \ No newline at end of file From 81d314bf102011e6cb0f406f4fd3f4fa1a345cee Mon Sep 17 00:00:00 2001 From: Himanshu Warekar Date: Mon, 20 May 2019 14:40:20 +0530 Subject: [PATCH 25/25] fix: typo fixes --- frappe/core/doctype/communication/communication.py | 1 - frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index aa70024eca..c164f7f8e0 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -238,7 +238,6 @@ class Communication(Document): # Timeline Links def set_timeline_links(self): contacts = get_contacts([self.sender, self.recipients, self.cc, self.bcc]) - print(contacts) for contact_name in contacts: self.add_link('Contact', contact_name) diff --git a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py index 624be6ae40..873988c7f3 100644 --- a/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py +++ b/frappe/patches/v12_0/move_timeline_links_to_dynamic_links.py @@ -40,7 +40,7 @@ def execute_query(values): (`idx`, `name`, `parentfield`, `parenttype`, `parent`, `link_doctype`, `link_name`, `creation`, `modified`, `modified_by`) VALUES ({0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}) - """.format(values[0], values[1], values[2], values[3], values[4], values[5], values[6], values[7], values[8], values[9]) + """.format(values[0], values[1], values[2], values[3], values[4], values[5], values[6], values[7], values[8], values[9])) except Exception as e: values[1] = frappe.generate_hash(length=10) execute_query(values)