From 4ff86e1d63cd6887d0b90d8fbf6be284ff6ec21b Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Fri, 4 Feb 2022 17:22:18 +0530 Subject: [PATCH] revert: "refactor: set amended docname to original docname" (#15867) * Revert: "refactor: set amended docname to original docname" 80d111baf2c4f26a776d4a73678a05fde90438c1. * test: test if amended doc has different name --- frappe/core/doctype/doctype/test_doctype.py | 8 +- frappe/model/document.py | 11 +- frappe/model/naming.py | 118 +--------- 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 | 14 +- 8 files changed, 31 insertions(+), 364 deletions(-) delete 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 66858f2028..50882f51bd 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -354,7 +354,6 @@ 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) @@ -378,7 +377,7 @@ class TestDocType(unittest.TestCase): for data in link_doc.get('permissions'): data.submit = 1 data.cancel = 1 - link_doc.insert(ignore_if_duplicate=True) + link_doc.insert() #create first parent doctype test_doc_1 = new_doctype('Test Doctype 1') @@ -393,7 +392,7 @@ class TestDocType(unittest.TestCase): for data in test_doc_1.get('permissions'): data.submit = 1 data.cancel = 1 - test_doc_1.insert(ignore_if_duplicate=True) + test_doc_1.insert() #crete second parent doctype doc = new_doctype('Test Doctype 2') @@ -408,7 +407,7 @@ class TestDocType(unittest.TestCase): for data in link_doc.get('permissions'): data.submit = 1 data.cancel = 1 - doc.insert(ignore_if_duplicate=True) + doc.insert() # create doctype data data_link_doc_1 = frappe.new_doc('Test Linked Doctype 1') @@ -439,7 +438,6 @@ 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 3639c20a3d..7b6b212ebc 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -9,7 +9,7 @@ import frappe 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, gen_new_name_for_cancelled_doc +from frappe.model.naming import set_new_name from frappe.model.docstatus import DocStatus from frappe.model import optional_fields, table_fields from frappe.model.workflow import validate_workflow @@ -311,9 +311,6 @@ class Document(BaseDocument): self.check_permission("write", "save") - if self.docstatus.is_cancelled(): - self._rename_doc_on_cancel() - self.set_user_and_timestamp() self.set_docstatus() self.check_if_latest() @@ -724,6 +721,7 @@ 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: @@ -1376,11 +1374,6 @@ class Document(BaseDocument): from frappe.desk.doctype.tag.tag import DocTags return DocTags(self.doctype).get_tags(self.name).split(",")[1:] - def _rename_doc_on_cancel(self): - 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 - def __repr__(self): name = self.name or "unsaved" doctype = self.__class__.__name__ diff --git a/frappe/model/naming.py b/frappe/model/naming.py index f3d68f3715..b2d11a4cfc 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -1,14 +1,3 @@ -"""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 # License: MIT. See LICENSE @@ -40,7 +29,7 @@ def set_new_name(doc): doc.name = None if getattr(doc, "amended_from", None): - doc.name = _get_amended_name(doc) + _set_amended_name(doc) return elif getattr(doc.meta, "issingle", False): @@ -256,18 +245,6 @@ 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: @@ -356,9 +333,16 @@ def append_number_if_name_exists(doctype, value, fieldname="name", separator="-" return value -def _get_amended_name(doc): - name, _ = NameParser(doc).parse_amended_from() - return name +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 _field_autoname(autoname, doc, skip_slicing=None): """ @@ -399,83 +383,3 @@ 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 9880596e27..7c2c6d5dc5 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -185,7 +185,6 @@ frappe.patches.v13_0.queryreport_columns frappe.patches.v13_0.jinja_hook frappe.patches.v13_0.update_notification_channel_if_empty frappe.patches.v13_0.set_first_day_of_the_week -frappe.patches.v14_0.rename_cancelled_documents frappe.patches.v14_0.update_workspace2 # 20.09.2021 frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021 frappe.patches.v14_0.transform_todo_schema diff --git a/frappe/patches/v14_0/rename_cancelled_documents.py b/frappe/patches/v14_0/rename_cancelled_documents.py deleted file mode 100644 index 4b565d4f76..0000000000 --- a/frappe/patches/v14_0/rename_cancelled_documents.py +++ /dev/null @@ -1,213 +0,0 @@ -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 5c0b6b1399..c0c7ce8b4e 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -860,36 +860,32 @@ frappe.ui.form.Form = class FrappeForm { } _cancel(btn, callback, on_error, skip_confirm) { + const me = this; const cancel_doc = () => { frappe.validated = true; - this.script_manager.trigger("before_cancel").then(() => { + me.script_manager.trigger("before_cancel").then(() => { if (!frappe.validated) { - return this.handle_save_fail(btn, on_error); + return me.handle_save_fail(btn, on_error); } - const original_name = this.docname; - const after_cancel = (r) => { + var after_cancel = function(r) { if (r.exc) { - this.handle_save_fail(btn, on_error); + me.handle_save_fail(btn, on_error); } else { frappe.utils.play_sound("cancel"); + me.refresh(); callback && callback(); - 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(), - ]); + me.script_manager.trigger("after_cancel"); } }; - frappe.ui.form.save(this, "cancel", after_cancel, btn); + frappe.ui.form.save(me, "cancel", after_cancel, btn); }); } if (skip_confirm) { cancel_doc(); } else { - frappe.confirm(__("Permanently Cancel {0}?", [this.docname]), cancel_doc, this.handle_save_fail(btn, on_error)); + frappe.confirm(__("Permanently Cancel {0}?", [this.docname]), cancel_doc, me.handle_save_fail(btn, on_error)); } }; @@ -911,7 +907,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 amend it again')); + frappe.throw(__('This document is already amended, you cannot ammend 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 7bea7f0584..1b038b6265 100644 --- a/frappe/public/js/frappe/router.js +++ b/frappe/public/js/frappe/router.js @@ -250,12 +250,6 @@ 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 3031d3e344..2309d8fc2b 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -144,6 +144,7 @@ class TestNaming(unittest.TestCase): current_index = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] self.assertEqual(current_index.get('current'), 2) + frappe.db.delete("Series", {"name": series}) def test_naming_for_cancelled_and_amended_doc(self): @@ -166,25 +167,20 @@ class TestNaming(unittest.TestCase): doc.submit() doc.cancel() cancelled_name = doc.name - self.assertEqual(cancelled_name, "{}-CANC-0".format(original_name)) + self.assertEqual(cancelled_name, 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) + self.assertEqual(amended_doc.name, "{}-1".format(original_name)) amended_doc.submit() amended_doc.cancel() - self.assertEqual(amended_doc.name, "{}-CANC-1".format(original_name)) + self.assertEqual(amended_doc.name, "{}-1".format(original_name)) submittable_doctype.delete() - def test_parse_naming_series_for_consecutive_week_number(self): - week = determine_consecutive_week_number(now_datetime()) - name = parse_naming_series('PREFIX-.WW.-SUFFIX') - expected_name = 'PREFIX-{}-SUFFIX'.format(week) - self.assertEqual(name, expected_name) def test_determine_consecutive_week_number(self): from datetime import datetime @@ -207,4 +203,4 @@ class TestNaming(unittest.TestCase): dt = datetime.fromisoformat("2021-12-31") w = determine_consecutive_week_number(dt) - self.assertEqual(w, "52") + self.assertEqual(w, "52") \ No newline at end of file