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