From b719e1481de9a629eee42559f5b0599a4c049d6f Mon Sep 17 00:00:00 2001 From: leela Date: Wed, 28 Jul 2021 17:00:12 +0530 Subject: [PATCH] Revert "refactor: set amended docname to original docname" This reverts commit d459847ae39f8e9917686c3db6340ee95103085e. --- frappe/core/doctype/doctype/test_doctype.py | 8 +- frappe/model/document.py | 9 +- frappe/model/naming.py | 83 +++---------------- frappe/patches.txt | 1 - frappe/patches/v13_0/rename_cancelled_docs.py | 27 ------ frappe/public/js/frappe/form/form.js | 24 +++--- frappe/public/js/frappe/router.js | 6 -- frappe/tests/test_naming.py | 34 -------- 8 files changed, 28 insertions(+), 164 deletions(-) delete mode 100644 frappe/patches/v13_0/rename_cancelled_docs.py diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 4b6d0e4794..1e1a01a685 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -348,7 +348,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 = '{}-CAN-0'.format(data_doc.name) data_doc.load_from_db() self.assertEqual(data_link_doc.docstatus, 2) self.assertEqual(data_doc.docstatus, 2) @@ -372,7 +371,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') @@ -387,7 +386,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') @@ -402,7 +401,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') @@ -433,7 +432,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 = '{}-CAN-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 e974ae2a3e..61160e1f01 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, gen_new_name_for_cancelled_doc +from frappe.model.naming import set_new_name from werkzeug.exceptions import NotFound, Forbidden import hashlib, json from frappe.model import optional_fields, table_fields @@ -705,6 +705,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: @@ -915,12 +916,8 @@ 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 6dff0aaff6..fe136adce8 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -28,7 +28,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): @@ -221,15 +221,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 if doc is amended, since cancelled docs still exist - if doc.docstatus != 2 and doc.amended_from: - return - - # for first cancelled doc - if doc.docstatus == 2 and not doc.amended_from: - name, _ = NameParser.parse_docname(doc.name, sep='-CAN-') - if ".#" in key: prefix, hashes = key.rsplit(".", 1) if "#" not in hashes: @@ -312,9 +303,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): """ @@ -325,6 +323,7 @@ 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. @@ -355,61 +354,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 all the 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_name(self): - if not hasattr(self.doc, "amended_from"): - return (self.doc.name, None, None) - - #If document is cancelled document - if hasattr(self.doc, "amended_from") and self.doc.docstatus == 2: - return self.parse_docname(self.doc.name, sep='-CAN-') - return self.parse_docname(self.doc.name) - - def parse_amended_from(self): - if not getattr(self.doc, 'amended_from', None): - return (None, None) - return self.parse_docname(self.doc.amended_from, '-CAN-') - - @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}-CAN-' - - 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}-CAN-{counter+1}' diff --git a/frappe/patches.txt b/frappe/patches.txt index a9c5807df0..7605d8ea2b 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -180,4 +180,3 @@ frappe.patches.v12_0.rename_uploaded_files_with_proper_name 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.rename_cancelled_docs diff --git a/frappe/patches/v13_0/rename_cancelled_docs.py b/frappe/patches/v13_0/rename_cancelled_docs.py deleted file mode 100644 index 2e99a6f3cd..0000000000 --- a/frappe/patches/v13_0/rename_cancelled_docs.py +++ /dev/null @@ -1,27 +0,0 @@ -import frappe -from frappe.model.naming import NameParser -from frappe.model.rename_doc import rename_doc - -def execute(): - """Rename already cancelled documents by adding `CAN-X` postfix instead of `-X`. - """ - for doctype in frappe.db.get_all('DocType'): - doctype = frappe.get_doc('DocType', doctype.name) - if doctype.is_submittable and frappe.db.table_exists(doctype.name): - cancelled_docs = frappe.db.get_all(doctype.name, ['amended_from', 'name'], {'docstatus':2}) - - for doc in cancelled_docs: - if '-CAN-' in doc.name: - continue - - current_name = doc.name - - if getattr(doc, "amended_from", None): - orig_name, counter = NameParser.parse_docname(doc.name) - else: - orig_name, counter = doc.name, 0 - new_name = f'{orig_name}-CAN-{counter or 0}' - - print(f"Renaming {doctype.name} record from {current_name} to {new_name}") - rename_doc(doctype.name, current_name, new_name, ignore_permissions=True, show_alert=False) - frappe.db.commit() diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 3bbc883b0c..faaa3dfbd9 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -770,36 +770,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)); } }; @@ -821,7 +817,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 3f8ce7fee2..8a29dc1040 100644 --- a/frappe/public/js/frappe/router.js +++ b/frappe/public/js/frappe/router.js @@ -234,12 +234,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 78071a4120..557993882f 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -116,37 +116,3 @@ 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, "{}-CAN-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, "{}-CAN-1".format(original_name)) - - submittable_doctype.delete()