revert: "refactor: set amended docname to original docname" (#15867)

* Revert: "refactor: set amended docname to original docname"

80d111baf2.

* test: test if amended doc has different name

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)

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__

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}'

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

frappe/patches/v14_0/rename_cancelled_documents.py

@@ -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()
-

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;

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]);

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")