From ebbcd4593e3cb7fac06f2b3ad9c05fc3f4cf88dc Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Thu, 10 Mar 2016 14:44:02 +0530 Subject: [PATCH] [refactor] [optimize] dynamic links on deletion --- .../doctype/custom_field/custom_field.py | 1 - frappe/model/delete_doc.py | 72 +++++++++++------- frappe/model/dynamic_links.py | 41 +++++++++++ frappe/model/meta.py | 8 +- frappe/model/rename_doc.py | 40 +++++----- frappe/tests/test_dynamic_links.py | 73 +++++++++++++++++++ frappe/utils/testutils.py | 18 +++++ 7 files changed, 199 insertions(+), 54 deletions(-) create mode 100644 frappe/model/dynamic_links.py create mode 100644 frappe/tests/test_dynamic_links.py create mode 100644 frappe/utils/testutils.py diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index b91d625db3..85c38e813b 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -5,7 +5,6 @@ from __future__ import unicode_literals import frappe from frappe.utils import cstr from frappe import _ -import json from frappe.model.document import Document class CustomField(Document): diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 883c51d542..7c842f9763 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -5,10 +5,10 @@ from __future__ import unicode_literals import frappe import frappe.model.meta +from frappe.model.dynamic_links import get_dynamic_link_map import frappe.defaults from frappe.utils.file_manager import remove_all from frappe import _ -from rename_doc import dynamic_link_queries from frappe.model.naming import revert_series_if_last def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reload=False, @@ -71,9 +71,19 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa if not ignore_on_trash: doc.run_method("on_trash") - delete_linked_todos(doc) - delete_linked_communications(doc) - delete_shared(doc) + dynamic_linked_doctypes = [df.parent for df in get_dynamic_link_map().get(doc.doctype, [])] + if "ToDo" in dynamic_linked_doctypes: + delete_linked_todos(doc) + + if "Communication" in dynamic_linked_doctypes: + delete_linked_communications(doc) + + if "DocShare" in dynamic_linked_doctypes: + delete_shared(doc) + + if "Email Unsubscribe": + delete_email_subscribe(doc) + # check if links exist if not force: check_if_doc_is_linked(doc) @@ -159,39 +169,47 @@ def check_if_doc_is_linked(doc, method="Delete"): frappe.LinkExistsError) def check_if_doc_is_dynamically_linked(doc, method="Delete"): - for query in dynamic_link_queries: - for df in frappe.db.sql(query, as_dict=True): - if frappe.get_meta(df.parent).issingle: + '''Raise `frappe.LinkExistsError` if the document is dynamically linked''' + for df in get_dynamic_link_map().get(doc.doctype, []): + if df.parent in ("Communication", "ToDo", "DocShare", "Email Unsubscribe"): + # don't check for communication and todo! + continue - # dynamic link in single doc - refdoc = frappe.db.get_singles_dict(df.parent) - if (refdoc.get(df.options)==doc.doctype - and refdoc.get(df.fieldname)==doc.name - and ((method=="Delete" and refdoc.docstatus < 2) - or (method=="Cancel" and refdoc.docstatus==1)) - ): + meta = frappe.get_meta(df.parent) + if meta.issingle: + # dynamic link in single doc + refdoc = frappe.db.get_singles_dict(df.parent) + if (refdoc.get(df.options)==doc.doctype + and refdoc.get(df.fieldname)==doc.name + and ((method=="Delete" and refdoc.docstatus < 2) + or (method=="Cancel" and refdoc.docstatus==1)) + ): + # raise exception only if + # linked to an non-cancelled doc when deleting + # or linked to a submitted doc when cancelling + frappe.throw(_("Cannot delete or cancel because {0} {1} is linked with {2} {3}").format(doc.doctype, + doc.name, df.parent, ""), frappe.LinkExistsError) + else: + # dynamic link in table + for refdoc in frappe.db.sql("""select name, docstatus from `tab{parent}` where + {options}=%s and {fieldname}=%s""".format(**df), (doc.doctype, doc.name), as_dict=True): + + if ((method=="Delete" and refdoc.docstatus < 2) or (method=="Cancel" and refdoc.docstatus==1)): # raise exception only if # linked to an non-cancelled doc when deleting # or linked to a submitted doc when cancelling - frappe.throw(_("Cannot delete or cancel because {0} {1} is linked with {2} {3}").format(doc.doctype, - doc.name, df.parent, ""), frappe.LinkExistsError) - else: - # dynamic link in table - for refdoc in frappe.db.sql("""select name, docstatus from `tab{parent}` where - {options}=%s and {fieldname}=%s""".format(**df), (doc.doctype, doc.name), as_dict=True): - - if ((method=="Delete" and refdoc.docstatus < 2) or (method=="Cancel" and refdoc.docstatus==1)): - # raise exception only if - # linked to an non-cancelled doc when deleting - # or linked to a submitted doc when cancelling - frappe.throw(_("Cannot delete or cancel because {0} {1} is linked with {2} {3}")\ - .format(doc.doctype, doc.name, df.parent, refdoc.name), frappe.LinkExistsError) + frappe.throw(_("Cannot delete or cancel because {0} {1} is linked with {2} {3}")\ + .format(doc.doctype, doc.name, df.parent, refdoc.name), frappe.LinkExistsError) def delete_linked_todos(doc): delete_doc("ToDo", frappe.db.sql_list("""select name from `tabToDo` where reference_type=%s and reference_name=%s""", (doc.doctype, doc.name)), ignore_permissions=True) +def delete_email_subscribe(doc): + frappe.db.sql('''delete from `tabEmail Unsubscribe` + where reference_doctype=%s and reference_name=%s''', (doc.doctype, doc.name)) + def delete_linked_communications(doc): # delete comments frappe.db.sql("""delete from `tabCommunication` diff --git a/frappe/model/dynamic_links.py b/frappe/model/dynamic_links.py new file mode 100644 index 0000000000..27949f1346 --- /dev/null +++ b/frappe/model/dynamic_links.py @@ -0,0 +1,41 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# MIT License. See license.txt + +import frappe + +dynamic_link_queries = [ + """select parent, fieldname, options from tabDocField where fieldtype='Dynamic Link'""", + """select dt as parent, fieldname, options from `tabCustom Field` where fieldtype='Dynamic Link'""", +] + +def get_dynamic_link_map(for_delete=False): + '''Build a map of all dynamically linked tables. For example, + if Note is dynamically linked to ToDo, the function will return + `{"Note": ["ToDo"], "Sales Invoice": ["Journal Entry Detail"]}` + + Note: Will not map single doctypes + ''' + if not hasattr(frappe.local, 'dynamic_link_map'): + # Build from scratch + dynamic_link_map = {} + for df in get_dynamic_links(): + meta = frappe.get_meta(df.parent) + if meta.issingle: + # always check in Single DocTypes + dynamic_link_map.setdefault(meta.name, []).append(df) + else: + links = frappe.db.sql_list("""select distinct {options} from `tab{parent}`""".format(**df)) + for doctype in links: + dynamic_link_map.setdefault(doctype, []).append(df) + + frappe.local.dynamic_link_map = dynamic_link_map + + return frappe.local.dynamic_link_map + +def get_dynamic_links(): + '''Return list of dynamic link fields as DocField. + Uses cache if possible''' + df = [] + for query in dynamic_link_queries: + df += frappe.db.sql(query, as_dict=True) + return df diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 52201e630e..924186703c 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -61,7 +61,9 @@ class Meta(Document): return self.get("fields", {"fieldtype": "Link", "options":["!=", "[Select]"]}) def get_dynamic_link_fields(self): - return self.get("fields", {"fieldtype": "Dynamic Link"}) + if not hasattr(self, '_dynamic_link_fields'): + self._dynamic_link_fields = self.get("fields", {"fieldtype": "Dynamic Link"}) + return self._dynamic_link_fields def get_select_fields(self): return self.get("fields", {"fieldtype": "Select", "options":["not in", @@ -358,8 +360,8 @@ def trim_tables(): def clear_cache(doctype=None): cache = frappe.cache() - cache.delete_value("is_table") - cache.delete_value("doctype_modules") + for key in ('is_table', 'doctype_modules'): + cache.delete_value(key) groups = ["meta", "form_meta", "table_columns", "last_modified", "linked_doctypes"] diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 36b7bb7091..605cb55097 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -6,6 +6,7 @@ import frappe from frappe import _ from frappe.utils import cint from frappe.model.naming import validate_name +from frappe.model.dynamic_links import get_dynamic_link_map @frappe.whitelist() def rename_doc(doctype, old, new, force=False, merge=False, ignore_permissions=False): @@ -297,33 +298,26 @@ def update_parenttype_values(old, new): where parenttype=%s""" % (doctype, '%s', '%s'), (new, old)) -dynamic_link_queries = [ - """select parent, fieldname, options from tabDocField where fieldtype='Dynamic Link'""", - """select dt as parent, fieldname, options from `tabCustom Field` where fieldtype='Dynamic Link'""", -] - def rename_dynamic_links(doctype, old, new): - for query in dynamic_link_queries: - for df in frappe.db.sql(query, as_dict=True): + for df in get_dynamic_link_map().get(doctype, []): + # dynamic link in single, just one value to check + if frappe.get_meta(df.parent).issingle: + refdoc = frappe.db.get_singles_dict(df.parent) + if refdoc.get(df.options)==doctype and refdoc.get(df.fieldname)==old: - # dynamic link in single, just one value to check - if frappe.get_meta(df.parent).issingle: - refdoc = frappe.db.get_singles_dict(df.parent) - if refdoc.get(df.options)==doctype and refdoc.get(df.fieldname)==old: + frappe.db.sql("""update tabSingles set value=%s where + field=%s and value=%s and doctype=%s""", (new, df.fieldname, old, df.parent)) + else: + # because the table hasn't been renamed yet! + parent = df.parent if df.parent != new else old - frappe.db.sql("""update tabSingles set value=%s where - field=%s and value=%s and doctype=%s""", (new, df.fieldname, old, df.parent)) - else: - # because the table hasn't been renamed yet! - parent = df.parent if df.parent != new else old + # replace for each value where renamed + for to_change in frappe.db.sql_list("""select name from `tab{parent}` where + {options}=%s and {fieldname}=%s""".format(parent=parent, options=df.options, + fieldname=df.fieldname), (doctype, old)): - # replace for each value where renamed - for to_change in frappe.db.sql_list("""select name from `tab{parent}` where - {options}=%s and {fieldname}=%s""".format(parent=parent, options=df.options, - fieldname=df.fieldname), (doctype, old)): - - frappe.db.sql("""update `tab{parent}` set {fieldname}=%s - where name=%s""".format(**df), (new, to_change)) + frappe.db.sql("""update `tab{parent}` set {fieldname}=%s + where name=%s""".format(**df), (new, to_change)) def bulk_rename(doctype, rows=None, via_console = False): """Bulk rename documents diff --git a/frappe/tests/test_dynamic_links.py b/frappe/tests/test_dynamic_links.py new file mode 100644 index 0000000000..41f16dc3be --- /dev/null +++ b/frappe/tests/test_dynamic_links.py @@ -0,0 +1,73 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# MIT License. See license.txt +from __future__ import unicode_literals + +import frappe, unittest + +class TestDynamicLinks(unittest.TestCase): + def setUp(self): + frappe.db.sql('delete from `tabEmail Unsubscribe`') + + def test_delete_normal(self): + event = frappe.get_doc({ + 'doctype': 'Event', + 'subject':'test-for-delete', + 'starts_on': '2014-01-01', + 'event_type': 'Public' + }).insert() + + unsub = frappe.get_doc({ + 'doctype': 'Email Unsubscribe', + 'email': 'test@example.com', + 'reference_doctype': event.doctype, + 'reference_name': event.name + }).insert() + + event.delete() + + self.assertFalse(frappe.db.exists('Email Unsubscribe', unsub.name)) + + def test_delete_with_comment(self): + event = frappe.get_doc({ + 'doctype': 'Event', + 'subject':'test-for-delete-1', + 'starts_on': '2014-01-01', + 'event_type': 'Public' + }).insert() + event.add_comment('Comment', 'test') + + self.assertTrue(frappe.get_all('Communication', + filters={'reference_doctype':'Event', 'reference_name':event.name})) + event.delete() + self.assertFalse(frappe.get_all('Communication', + filters={'reference_doctype':'Event', 'reference_name':event.name})) + + def test_custom_fields(self): + from frappe.utils.testutils import add_custom_field, clear_custom_fields + add_custom_field('Event', 'test_ref_doc', 'Link', 'DocType') + add_custom_field('Event', 'test_ref_name', 'Dynamic Link', 'test_ref_doc') + + unsub = frappe.get_doc({ + 'doctype': 'Email Unsubscribe', + 'email': 'test@example.com', + 'global_unsubscribe': 1 + }).insert() + + event = frappe.get_doc({ + 'doctype': 'Event', + 'subject':'test-for-delete-2', + 'starts_on': '2014-01-01', + 'event_type': 'Public', + 'test_ref_doc': unsub.doctype, + 'test_ref_name': unsub.name + }).insert() + + self.assertRaises(frappe.LinkExistsError, unsub.delete) + + event.test_ref_doc = None + event.test_ref_name = None + event.save() + + unsub.delete() + + clear_custom_fields('Event') diff --git a/frappe/utils/testutils.py b/frappe/utils/testutils.py new file mode 100644 index 0000000000..4af3ad647c --- /dev/null +++ b/frappe/utils/testutils.py @@ -0,0 +1,18 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# MIT License. See license.txt +from __future__ import unicode_literals + +import frappe + +def add_custom_field(doctype, fieldname, fieldtype='Data', options=None): + frappe.get_doc({ + "doctype": "Custom Field", + "dt": doctype, + "fieldname": fieldname, + "fieldtype": fieldtype, + "options": options + }).insert() + +def clear_custom_fields(doctype): + frappe.db.sql('delete from `tabCustom Field` where dt=%s', doctype) + frappe.clear_cache(doctype=doctype)