From 80d111baf2c4f26a776d4a73678a05fde90438c1 Mon Sep 17 00:00:00 2001 From: leela Date: Tue, 13 Jul 2021 20:25:32 +0530 Subject: [PATCH] refactor: set amended docname to original docname Currently, whenever a document is amended it's name is set to name-X(X is a counter) when amended again and so on. In this PR, we have postfixed all cancelled document names with '-CAN' and new cancelled documents gets a name as original_name-CANC-X. so that amended docs can use the original name instead of name-X. --- frappe/core/doctype/doctype/test_doctype.py | 8 +- frappe/model/document.py | 9 +- frappe/model/naming.py | 119 +++++++++- frappe/patches.txt | 1 + .../v14_0/rename_cancelled_documents.py | 213 ++++++++++++++++++ frappe/public/js/frappe/form/form.js | 24 +- frappe/public/js/frappe/router.js | 6 + frappe/tests/test_naming.py | 34 +++ 8 files changed, 386 insertions(+), 28 deletions(-) create mode 100644 frappe/patches/v14_0/rename_cancelled_documents.py diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 1e1a01a685..9aaaf5a1ac 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -348,6 +348,7 @@ class TestDocType(unittest.TestCase): dump_docs = json.dumps(docs.get('docs')) cancel_all_linked_docs(dump_docs) data_link_doc.cancel() + data_doc.name = '{}-CANC-0'.format(data_doc.name) data_doc.load_from_db() self.assertEqual(data_link_doc.docstatus, 2) self.assertEqual(data_doc.docstatus, 2) @@ -371,7 +372,7 @@ class TestDocType(unittest.TestCase): for data in link_doc.get('permissions'): data.submit = 1 data.cancel = 1 - link_doc.insert() + link_doc.insert(ignore_if_duplicate=True) #create first parent doctype test_doc_1 = new_doctype('Test Doctype 1') @@ -386,7 +387,7 @@ class TestDocType(unittest.TestCase): for data in test_doc_1.get('permissions'): data.submit = 1 data.cancel = 1 - test_doc_1.insert() + test_doc_1.insert(ignore_if_duplicate=True) #crete second parent doctype doc = new_doctype('Test Doctype 2') @@ -401,7 +402,7 @@ class TestDocType(unittest.TestCase): for data in link_doc.get('permissions'): data.submit = 1 data.cancel = 1 - doc.insert() + doc.insert(ignore_if_duplicate=True) # create doctype data data_link_doc_1 = frappe.new_doc('Test Linked Doctype 1') @@ -432,6 +433,7 @@ class TestDocType(unittest.TestCase): # checking that doc for Test Doctype 2 is not canceled self.assertRaises(frappe.LinkExistsError, data_link_doc_1.cancel) + data_doc_2.name = '{}-CANC-0'.format(data_doc_2.name) data_doc.load_from_db() data_doc_2.load_from_db() self.assertEqual(data_link_doc_1.docstatus, 2) diff --git a/frappe/model/document.py b/frappe/model/document.py index b44d95716e..f3633a0009 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -5,7 +5,7 @@ import time from frappe import _, msgprint, is_whitelisted from frappe.utils import flt, cstr, now, get_datetime_str, file_lock, date_diff from frappe.model.base_document import BaseDocument, get_controller -from frappe.model.naming import set_new_name +from frappe.model.naming import set_new_name, gen_new_name_for_cancelled_doc from werkzeug.exceptions import NotFound, Forbidden import hashlib, json from frappe.model import optional_fields, table_fields @@ -708,7 +708,6 @@ class Document(BaseDocument): else: tmp = frappe.db.sql("""select modified, docstatus from `tab{0}` where name = %s for update""".format(self.doctype), self.name, as_dict=True) - if not tmp: frappe.throw(_("Record does not exist")) else: @@ -919,8 +918,12 @@ class Document(BaseDocument): @whitelist.__func__ def _cancel(self): - """Cancel the document. Sets `docstatus` = 2, then saves.""" + """Cancel the document. Sets `docstatus` = 2, then saves. + """ self.docstatus = 2 + new_name = gen_new_name_for_cancelled_doc(self) + frappe.rename_doc(self.doctype, self.name, new_name, force=True, show_alert=False) + self.name = new_name self.save() @whitelist.__func__ diff --git a/frappe/model/naming.py b/frappe/model/naming.py index fe136adce8..7705002706 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -1,3 +1,14 @@ +"""utilities to generate a document name based on various rules defined. + +NOTE: +Till version 13, whenever a submittable document is amended it's name is set to orig_name-X, +where X is a counter and it increments when amended again and so on. + +From Version 14, The naming pattern is changed in a way that amended documents will +have the original name `orig_name` instead of `orig_name-X`. To make this happen +the cancelled document naming pattern is changed to 'orig_name-CANC-X'. +""" + # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt @@ -28,7 +39,7 @@ def set_new_name(doc): doc.name = None if getattr(doc, "amended_from", None): - _set_amended_name(doc) + doc.name = _get_amended_name(doc) return elif getattr(doc.meta, "issingle", False): @@ -221,6 +232,18 @@ def revert_series_if_last(key, name, doc=None): * prefix = #### and hashes = 2021 (hash doesn't exist) * will search hash in key then accordingly get prefix = "" """ + if hasattr(doc, 'amended_from'): + # Do not revert the series if the document is amended. + if doc.amended_from: + return + + # Get document name by parsing incase of fist cancelled document + if doc.docstatus == 2 and not doc.amended_from: + if doc.name.endswith('-CANC'): + name, _ = NameParser.parse_docname(doc.name, sep='-CANC') + else: + name, _ = NameParser.parse_docname(doc.name, sep='-CANC-') + if ".#" in key: prefix, hashes = key.rsplit(".", 1) if "#" not in hashes: @@ -303,16 +326,9 @@ def append_number_if_name_exists(doctype, value, fieldname="name", separator="-" return value -def _set_amended_name(doc): - am_id = 1 - am_prefix = doc.amended_from - if frappe.db.get_value(doc.doctype, doc.amended_from, "amended_from"): - am_id = cint(doc.amended_from.split("-")[-1]) + 1 - am_prefix = "-".join(doc.amended_from.split("-")[:-1]) # except the last hyphen - - doc.name = am_prefix + "-" + str(am_id) - return doc.name - +def _get_amended_name(doc): + name, _ = NameParser(doc).parse_amended_from() + return name def _field_autoname(autoname, doc, skip_slicing=None): """ @@ -323,7 +339,6 @@ def _field_autoname(autoname, doc, skip_slicing=None): name = (cstr(doc.get(fieldname)) or "").strip() return name - def _prompt_autoname(autoname, doc): """ Generate a name using Prompt option. This simply means the user will have to set the name manually. @@ -354,3 +369,83 @@ def _format_autoname(autoname, doc): name = re.sub(r"(\{[\w | #]+\})", get_param_value_for_match, autoname_value) return name + +class NameParser: + """Parse document name and return parts of it. + + NOTE: It handles cancellend and amended doc parsing for now. It can be expanded. + """ + def __init__(self, doc): + self.doc = doc + + def parse_amended_from(self): + """ + Cancelled document naming will be in one of these formats + + * original_name-X-CANC - This is introduced to migrate old style naming to new style + * original_name-CANC - This is introduced to migrate old style naming to new style + * original_name-CANC-X - This is the new style naming + + New style naming: In new style naming amended documents will have original name. That says, + when a document gets cancelled we need rename the document by adding `-CANC-X` to the end + so that amended documents can use the original name. + + Old style naming: cancelled documents stay with original name and when amended, amended one + gets a new name as `original_name-X`. To bring new style naming we had to change the existing + cancelled document names and that is done by adding `-CANC` to cancelled documents through patch. + """ + if not getattr(self.doc, 'amended_from', None): + return (None, None) + + # Handle old style cancelled documents (original_name-X-CANC, original_name-CANC) + if self.doc.amended_from.endswith('-CANC'): + name, _ = self.parse_docname(self.doc.amended_from, '-CANC') + amended_from_doc = frappe.get_all( + self.doc.doctype, + filters = {'name': self.doc.amended_from}, + fields = ['amended_from'], + limit=1) + + # Handle format original_name-X-CANC. + if amended_from_doc and amended_from_doc[0].amended_from: + return self.parse_docname(name, '-') + return name, None + + # Handle new style cancelled documents + return self.parse_docname(self.doc.amended_from, '-CANC-') + + @classmethod + def parse_docname(cls, name, sep='-'): + split_list = name.rsplit(sep, 1) + + if len(split_list) == 1: + return (name, None) + return (split_list[0], split_list[1]) + +def get_cancelled_doc_latest_counter(tname, docname): + """Get the latest counter used for cancelled docs of given docname. + """ + name_prefix = f'{docname}-CANC-' + + rows = frappe.db.sql(""" + select + name + from `tab{tname}` + where + name like %(name_prefix)s and docstatus=2 + """.format(tname=tname), {'name_prefix': name_prefix+'%'}, as_dict=1) + + if not rows: + return -1 + return max([int(row.name.replace(name_prefix, '') or -1) for row in rows]) + +def gen_new_name_for_cancelled_doc(doc): + """Generate a new name for cancelled document. + """ + if getattr(doc, "amended_from", None): + name, _ = NameParser(doc).parse_amended_from() + else: + name = doc.name + + counter = get_cancelled_doc_latest_counter(doc.doctype, name) + return f'{name}-CANC-{counter+1}' diff --git a/frappe/patches.txt b/frappe/patches.txt index 493c4dc9f6..989b13e049 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -181,3 +181,4 @@ frappe.patches.v13_0.queryreport_columns frappe.patches.v13_0.jinja_hook frappe.patches.v13_0.update_notification_channel_if_empty frappe.patches.v14_0.drop_data_import_legacy +frappe.patches.v14_0.rename_cancelled_documents diff --git a/frappe/patches/v14_0/rename_cancelled_documents.py b/frappe/patches/v14_0/rename_cancelled_documents.py new file mode 100644 index 0000000000..fbe49c2351 --- /dev/null +++ b/frappe/patches/v14_0/rename_cancelled_documents.py @@ -0,0 +1,213 @@ +import functools +import traceback + +import frappe + +def execute(): + """Rename cancelled documents by adding a postfix. + """ + rename_cancelled_docs() + +def get_submittable_doctypes(): + """Returns list of submittable doctypes in the system. + """ + return frappe.db.get_all('DocType', filters={'is_submittable': 1}, pluck='name') + +def get_cancelled_doc_names(doctype): + """Return names of cancelled document names those are in old format. + """ + docs = frappe.db.get_all(doctype, filters={'docstatus': 2}, pluck='name') + return [each for each in docs if not (each.endswith('-CANC') or ('-CANC-' in each))] + +@functools.lru_cache() +def get_linked_doctypes(): + """Returns list of doctypes those are linked with given doctype using 'Link' fieldtype. + """ + filters=[['fieldtype','=', 'Link']] + links = frappe.get_all("DocField", + fields=["parent", "fieldname", "options as linked_to"], + filters=filters, + as_list=1) + + links+= frappe.get_all("Custom Field", + fields=["dt as parent", "fieldname", "options as linked_to"], + filters=filters, + as_list=1) + + links_by_doctype = {} + for doctype, fieldname, linked_to in links: + links_by_doctype.setdefault(linked_to, []).append((doctype, fieldname)) + return links_by_doctype + +@functools.lru_cache() +def get_single_doctypes(): + return frappe.get_all("DocType", filters={'issingle': 1}, pluck='name') + +@functools.lru_cache() +def get_dynamic_linked_doctypes(): + filters=[['fieldtype','=', 'Dynamic Link']] + + # find dynamic links of parents + links = frappe.get_all("DocField", + fields=["parent as doctype", "fieldname", "options as doctype_fieldname"], + filters=filters, + as_list=1) + links+= frappe.get_all("Custom Field", + fields=["dt as doctype", "fieldname", "options as doctype_fieldname"], + filters=filters, + as_list=1) + return links + +@functools.lru_cache() +def get_child_tables(): + """ + """ + filters =[['fieldtype', 'in', ('Table', 'Table MultiSelect')]] + links = frappe.get_all("DocField", + fields=["parent as doctype", "options as child_table"], + filters=filters, + as_list=1) + + links+= frappe.get_all("Custom Field", + fields=["dt as doctype", "options as child_table"], + filters=filters, + as_list=1) + + map = {} + for doctype, child_table in links: + map.setdefault(doctype, []).append(child_table) + return map + +def update_cancelled_document_names(doctype, cancelled_doc_names): + return frappe.db.sql(""" + update + `tab{doctype}` + set + name=CONCAT(name, '-CANC') + where + docstatus=2 + and + name in %(cancelled_doc_names)s; + """.format(doctype=doctype), {'cancelled_doc_names': cancelled_doc_names}) + +def update_amended_field(doctype, cancelled_doc_names): + return frappe.db.sql(""" + update + `tab{doctype}` + set + amended_from=CONCAT(amended_from, '-CANC') + where + amended_from in %(cancelled_doc_names)s; + """.format(doctype=doctype), {'cancelled_doc_names': cancelled_doc_names}) + +def update_attachments(doctype, cancelled_doc_names): + frappe.db.sql(""" + update + `tabFile` + set + attached_to_name=CONCAT(attached_to_name, '-CANC') + where + attached_to_doctype=%(dt)s and attached_to_name in %(cancelled_doc_names)s + """, {'cancelled_doc_names': cancelled_doc_names, 'dt': doctype}) + +def update_versions(doctype, cancelled_doc_names): + frappe.db.sql(""" + UPDATE + `tabVersion` + SET + docname=CONCAT(docname, '-CANC') + WHERE + ref_doctype=%(dt)s AND docname in %(cancelled_doc_names)s + """, {'cancelled_doc_names': cancelled_doc_names, 'dt': doctype}) + +def update_linked_doctypes(doctype, cancelled_doc_names): + single_doctypes = get_single_doctypes() + + for linked_dt, field in get_linked_doctypes().get(doctype, []): + if linked_dt not in single_doctypes: + frappe.db.sql(""" + update + `tab{linked_dt}` + set + {column}=CONCAT({column}, '-CANC') + where + {column} in %(cancelled_doc_names)s; + """.format(linked_dt=linked_dt, column=field), + {'cancelled_doc_names': cancelled_doc_names}) + else: + doc = frappe.get_single(linked_dt) + if getattr(doc, field) in cancelled_doc_names: + setattr(doc, field, getattr(doc, field)+'-CANC') + doc.flags.ignore_mandatory=True + doc.flags.ignore_validate=True + doc.save(ignore_permissions=True) + +def update_dynamic_linked_doctypes(doctype, cancelled_doc_names): + single_doctypes = get_single_doctypes() + + for linked_dt, fieldname, doctype_fieldname in get_dynamic_linked_doctypes(): + if linked_dt not in single_doctypes: + frappe.db.sql(""" + update + `tab{linked_dt}` + set + {column}=CONCAT({column}, '-CANC') + where + {column} in %(cancelled_doc_names)s and {doctype_fieldname}=%(dt)s; + """.format(linked_dt=linked_dt, column=fieldname, doctype_fieldname=doctype_fieldname), + {'cancelled_doc_names': cancelled_doc_names, 'dt': doctype}) + else: + doc = frappe.get_single(linked_dt) + if getattr(doc, doctype_fieldname) == doctype and getattr(doc, fieldname) in cancelled_doc_names: + setattr(doc, fieldname, getattr(doc, fieldname)+'-CANC') + doc.flags.ignore_mandatory=True + doc.flags.ignore_validate=True + doc.save(ignore_permissions=True) + +def update_child_tables(doctype, cancelled_doc_names): + child_tables = get_child_tables().get(doctype, []) + single_doctypes = get_single_doctypes() + + for table in child_tables: + if table not in single_doctypes: + frappe.db.sql(""" + update + `tab{table}` + set + parent=CONCAT(parent, '-CANC') + where + parenttype=%(dt)s and parent in %(cancelled_doc_names)s; + """.format(table=table), {'cancelled_doc_names': cancelled_doc_names, 'dt': doctype}) + else: + doc = frappe.get_single(table) + if getattr(doc, 'parenttype')==doctype and getattr(doc, 'parent') in cancelled_doc_names: + setattr(doc, 'parent', getattr(doc, 'parent')+'-CANC') + doc.flags.ignore_mandatory=True + doc.flags.ignore_validate=True + doc.save(ignore_permissions=True) + +def rename_cancelled_docs(): + submittable_doctypes = get_submittable_doctypes() + + for dt in submittable_doctypes: + for retry in range(2): + try: + cancelled_doc_names = tuple(get_cancelled_doc_names(dt)) + if not cancelled_doc_names: + break + update_cancelled_document_names(dt, cancelled_doc_names) + update_amended_field(dt, cancelled_doc_names) + update_child_tables(dt, cancelled_doc_names) + update_linked_doctypes(dt, cancelled_doc_names) + update_dynamic_linked_doctypes(dt, cancelled_doc_names) + update_attachments(dt, cancelled_doc_names) + update_versions(dt, cancelled_doc_names) + print(f"Renaming cancelled records of {dt} doctype") + frappe.db.commit() + break + except Exception: + if retry == 1: + print(f"Failed to rename the cancelled records of {dt} doctype, moving on!") + traceback.print_exc() + frappe.db.rollback() + diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index faaa3dfbd9..3bbc883b0c 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -770,32 +770,36 @@ frappe.ui.form.Form = class FrappeForm { } _cancel(btn, callback, on_error, skip_confirm) { - const me = this; const cancel_doc = () => { frappe.validated = true; - me.script_manager.trigger("before_cancel").then(() => { + this.script_manager.trigger("before_cancel").then(() => { if (!frappe.validated) { - return me.handle_save_fail(btn, on_error); + return this.handle_save_fail(btn, on_error); } - var after_cancel = function(r) { + const original_name = this.docname; + const after_cancel = (r) => { if (r.exc) { - me.handle_save_fail(btn, on_error); + this.handle_save_fail(btn, on_error); } else { frappe.utils.play_sound("cancel"); - me.refresh(); callback && callback(); - me.script_manager.trigger("after_cancel"); + this.script_manager.trigger("after_cancel"); + frappe.run_serially([ + () => this.rename_notify(this.doctype, original_name, r.docs[0].name), + () => frappe.router.clear_re_route(this.doctype, original_name), + () => this.refresh(), + ]); } }; - frappe.ui.form.save(me, "cancel", after_cancel, btn); + frappe.ui.form.save(this, "cancel", after_cancel, btn); }); } if (skip_confirm) { cancel_doc(); } else { - frappe.confirm(__("Permanently Cancel {0}?", [this.docname]), cancel_doc, me.handle_save_fail(btn, on_error)); + frappe.confirm(__("Permanently Cancel {0}?", [this.docname]), cancel_doc, this.handle_save_fail(btn, on_error)); } }; @@ -817,7 +821,7 @@ frappe.ui.form.Form = class FrappeForm { 'docname': this.doc.name }).then(is_amended => { if (is_amended) { - frappe.throw(__('This document is already amended, you cannot ammend it again')); + frappe.throw(__('This document is already amended, you cannot amend it again')); } this.validate_form_action("Amend"); var me = this; diff --git a/frappe/public/js/frappe/router.js b/frappe/public/js/frappe/router.js index e99cf75cd4..d0f93882fb 100644 --- a/frappe/public/js/frappe/router.js +++ b/frappe/public/js/frappe/router.js @@ -232,6 +232,12 @@ frappe.router = { } }, + clear_re_route(doctype, docname) { + delete frappe.re_route[ + `${encodeURIComponent(frappe.router.slug(doctype))}/${encodeURIComponent(docname)}` + ]; + }, + set_title(sub_path) { if (frappe.route_titles[sub_path]) { frappe.utils.set_title(frappe.route_titles[sub_path]); diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index 557993882f..830c39442e 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -116,3 +116,37 @@ class TestNaming(unittest.TestCase): self.assertEqual(current_index.get('current'), 2) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) + + def test_naming_for_cancelled_and_amended_doc(self): + submittable_doctype = frappe.get_doc({ + "doctype": "DocType", + "module": "Core", + "custom": 1, + "is_submittable": 1, + "permissions": [{ + "role": "System Manager", + "read": 1 + }], + "name": 'Submittable Doctype' + }).insert(ignore_if_duplicate=True) + + doc = frappe.new_doc('Submittable Doctype') + doc.save() + original_name = doc.name + + doc.submit() + doc.cancel() + cancelled_name = doc.name + self.assertEqual(cancelled_name, "{}-CANC-0".format(original_name)) + + amended_doc = frappe.copy_doc(doc) + amended_doc.docstatus = 0 + amended_doc.amended_from = doc.name + amended_doc.save() + self.assertEqual(amended_doc.name, original_name) + + amended_doc.submit() + amended_doc.cancel() + self.assertEqual(amended_doc.name, "{}-CANC-1".format(original_name)) + + submittable_doctype.delete()