feat: rearranging standard fields in customize form (backport #19822, #…

…20168) (#22120)

* fix!: create property setters for system generated custom fields

(cherry picked from commit 43b5d95)

* fix!: make system generated fields unsortable

(cherry picked from commit 9a88acf)

* style: use `const` instead of `let`

(cherry picked from commit 1ed4b2d)

* test: use correct fieldname

* feat: rearranging standard fields in customize form (#19822)

* feat: rearranging standing fields

* fix: fixed creation of property setter

* refactor: renamed setup_sortable

* fix: loading field_order property

* refactor: removed redundant db call

* fix: field_order not found

* test: Added tests for field order in customize form

* refactor: better naming

* refactor: simplified logic

* feat: Updating field order on custom field creation

* feat: Added support for custom fiels

* refactor: moving to meta

* refactor: changed property type to json

* fix: new standard field insert order.

* fix: don't modify insert_after of system generated custom fields.

# This is because system generated fields are to be treated as standard fields. If the user restores the form to default, this value will be used to reset the original position.

# The new position of form fields are stored in the field_order Property Setter.

* fix: treat system generated fields as standard fields when sorting.

* revert: check for is_system_generated

* Revert "fix: new standard field insert order."

This reverts commit 6cdbe42.

* fix: prioritize field_order over insert_after.

# Use insert_after as fallback in event the field doesn't exist in field_order

* fix(test): delete existing custom field

* fix: order of standard fields without field_order property.

* Revert "Revert "fix: new standard field insert order.""

This reverts commit c830f1b.

* test: field order of newly migrated standard fields.

* fix(test): clear test_standard_field from previous test run.

* fix: sort with insert_after for system generated fields.

* fix(test): reset standard field creation before re-run and after successful test.

* fix: insert_after position should be + 1

* chore: remove debug statement

* test: system generated customized fields

* chore: remove print

* chore: lint all

* fix: show quick link to Table MultiSelect DocTypes

* refactor: change backend implementation of `CustomizeForm` and `Meta`

* test: simplify tests

* fix: rename `idx` to `index` for clarity

* perf: define `existing_fields` conditionally

---------

Co-authored-by: Aradhya <aradhyatripathi51@gmail.com>
Co-authored-by: Aradhya Tripathi <67282231+Aradhya-Tripathi@users.noreply.github.com>
Co-authored-by: Sagar Vora <sagar@resilient.tech>

---------

Co-authored-by: Sagar Vora <sagar@resilient.tech>
Co-authored-by: Devin Slauenwhite <devin.slauenwhite@gmail.com>
Co-authored-by: Aradhya <aradhyatripathi51@gmail.com>
Co-authored-by: Aradhya Tripathi <67282231+Aradhya-Tripathi@users.noreply.github.com>

frappe/custom/doctype/custom_field/custom_field.js

@@ -25,6 +25,27 @@ frappe.ui.form.on("Custom Field", {
 		frm.toggle_enable("dt", frm.doc.__islocal);
 		frm.trigger("dt");
 		frm.toggle_reqd("label", !frm.doc.fieldname);
+
+		if (frm.doc.is_system_generated) {
+			frm.dashboard.add_comment(
+				__(
+					"<strong>Warning:</strong> This field is system generated and may be overwritten by a future update. Modify it using {0} instead.",
+					[
+						frappe.utils.get_form_link(
+							"Customize Form",
+							"Customize Form",
+							true,
+							__("Customize Form"),
+							{
+								doc_type: frm.doc.dt,
+							}
+						),
+					]
+				),
+				"yellow",
+				true
+			);
+		}
 	},
 	dt: function (frm) {
 		if (!frm.doc.dt) {

frappe/custom/doctype/customize_form/customize_form.js

@@ -49,14 +49,6 @@ frappe.ui.form.on("Customize Form", {
 				grid_row.row.addClass("highlight");
 			}
 		});
-
-		$(frm.wrapper).on("grid-make-sortable", function (e, frm) {
-			frm.trigger("setup_sortable");
-		});
-
-		$(frm.wrapper).on("grid-move-row", function (e, frm) {
-			frm.trigger("setup_sortable");
-		});
 	},
 
 	doc_type: function (frm) {
@@ -71,7 +63,7 @@ frappe.ui.form.on("Customize Form", {
 							frm.set_value("doc_type", "");
 						} else {
 							frm.refresh();
-							frm.trigger("setup_sortable");
+							frm.trigger("add_customize_child_table_button");
 							frm.trigger("setup_default_views");
 						}
 					}
@@ -87,23 +79,16 @@ frappe.ui.form.on("Customize Form", {
 		frm.trigger("setup_default_views");
 	},
 
-	setup_sortable: function (frm) {
+	add_customize_child_table_button: function (frm) {
 		frm.doc.fields.forEach(function (f) {
-			if (!f.is_custom_field) {
-				f._sortable = false;
-			}
+			if (!in_list(["Table", "Table MultiSelect"], f.fieldtype)) return;
 
-			if (f.fieldtype == "Table") {
-				frm.add_custom_button(
-					f.options,
-					function () {
-						frm.set_value("doc_type", f.options);
-					},
-					__("Customize Child Table")
-				);
-			}
+			frm.add_custom_button(
+				f.options,
+				() => frm.set_value("doc_type", f.options),
+				__("Customize Child Table")
+			);
 		});
-		frm.fields_dict.fields.grid.refresh();
 	},
 
 	refresh: function (frm) {
@@ -236,10 +221,23 @@ frappe.ui.form.on("Customize Form", {
 // can't delete standard fields
 frappe.ui.form.on("Customize Form Field", {
 	before_fields_remove: function (frm, doctype, name) {
-		var row = frappe.get_doc(doctype, name);
+		const row = frappe.get_doc(doctype, name);
+
+		if (row.is_system_generated) {
+			frappe.throw(
+				__(
+					"Cannot delete system generated field <strong>{0}</strong>. You can hide it instead.",
+					[__(row.label) || row.fieldname]
+				)
+			);
+		}
+
 		if (!(row.is_custom_field || row.__islocal)) {
-			frappe.msgprint(__("Cannot delete standard field. You can hide it if you want"));
-			throw "cannot delete standard field";
+			frappe.throw(
+				__("Cannot delete standard field <strong>{0}</strong>. You can hide it instead.", [
+					__(row.label) || row.fieldname,
+				])
+			);
 		}
 	},
 	fields_add: function (frm, cdt, cdn) {

frappe/custom/doctype/customize_form/customize_form.py

@@ -195,18 +195,47 @@ def set_property_setters(self):
 		# docfield
 		for df in self.get("fields"):
 			meta_df = meta.get("fields", {"fieldname": df.fieldname})
-			if not meta_df or meta_df[0].get("is_custom_field"):
+			if not meta_df or not is_standard_or_system_generated_field(meta_df[0]):
 				continue
+
 			self.set_property_setters_for_docfield(meta, df, meta_df)
 
 		# action and links
 		self.set_property_setters_for_actions_and_links(meta)
 
+	def set_property_setter_for_field_order(self, meta):
+		new_order = [df.fieldname for df in self.fields]
+		existing_order = getattr(meta, "field_order", None)
+		default_order = [
+			fieldname for fieldname, df in meta._fields.items() if not getattr(df, "is_custom_field", False)
+		]
+
+		if new_order == default_order:
+			if existing_order:
+				delete_property_setter(self.doc_type, "field_order")
+
+			return
+
+		if existing_order and new_order == json.loads(existing_order):
+			return
+
+		frappe.make_property_setter(
+			{
+				"doctype": self.doc_type,
+				"doctype_or_field": "DocType",
+				"property": "field_order",
+				"value": json.dumps(new_order),
+			},
+			is_system_generated=False,
+		)
+
 	def set_property_setters_for_doctype(self, meta):
 		for prop, prop_type in doctype_properties.items():
 			if self.get(prop) != meta.get(prop):
 				self.make_property_setter(prop, self.get(prop), prop_type)
 
+		self.set_property_setter_for_field_order(meta)
+
 	def set_property_setters_for_docfield(self, meta, df, meta_df):
 		for prop, prop_type in docfield_properties.items():
 			if prop != "idx" and (df.get(prop) or "") != (meta_df[0].get(prop) or ""):
@@ -352,12 +381,14 @@ def clear_removed_items(self, doctype, items):
 
 	def update_custom_fields(self):
 		for i, df in enumerate(self.get("fields")):
-			if df.get("is_custom_field"):
-				if not frappe.db.exists("Custom Field", {"dt": self.doc_type, "fieldname": df.fieldname}):
-					self.add_custom_field(df, i)
-					self.flags.update_db = True
-				else:
-					self.update_in_custom_field(df, i)
+			if is_standard_or_system_generated_field(df):
+				continue
+
+			if not frappe.db.exists("Custom Field", {"dt": self.doc_type, "fieldname": df.fieldname}):
+				self.add_custom_field(df, i)
+				self.flags.update_db = True
+			else:
+				self.update_in_custom_field(df, i)
 
 		self.delete_custom_fields()
 
@@ -382,7 +413,7 @@ def add_custom_field(self, df, i):
 	def update_in_custom_field(self, df, i):
 		meta = frappe.get_meta(self.doc_type)
 		meta_df = meta.get("fields", {"fieldname": df.fieldname})
-		if not (meta_df and meta_df[0].get("is_custom_field")):
+		if not meta_df or is_standard_or_system_generated_field(meta_df[0]):
 			# not a custom field
 			return
 
@@ -418,7 +449,7 @@ def delete_custom_fields(self):
 		}
 		for fieldname in fields_to_remove:
 			df = meta.get("fields", {"fieldname": fieldname})[0]
-			if df.get("is_custom_field"):
+			if not is_standard_or_system_generated_field(df):
 				frappe.delete_doc("Custom Field", df.name)
 
 	def make_property_setter(
@@ -563,6 +594,10 @@ def reset_customization(doctype):
 	frappe.clear_cache(doctype=doctype)
 
 
+def is_standard_or_system_generated_field(df):
+	return not df.get("is_custom_field") or df.get("is_system_generated")
+
+
 doctype_properties = {
 	"search_fields": "Data",
 	"title_field": "Data",

frappe/custom/doctype/customize_form/test_customize_form.py

@@ -400,3 +400,37 @@ def test_change_to_autoincrement_autoname(self):
 
 		with self.assertRaises(frappe.ValidationError):
 			d.run_method("save_customization")
+
+	def test_system_generated_fields(self):
+		doctype = "Event"
+		custom_field_name = "custom_test_field"
+
+		custom_field = frappe.get_doc("Custom Field", {"dt": doctype, "fieldname": custom_field_name})
+		custom_field.is_system_generated = 1
+		custom_field.save()
+
+		d = self.get_customize_form(doctype)
+		custom_field = d.getone("fields", {"fieldname": custom_field_name})
+		custom_field.description = "Test Description"
+		d.run_method("save_customization")
+
+		property_setter_filters = {
+			"doc_type": doctype,
+			"field_name": custom_field_name,
+			"property": "description",
+		}
+		self.assertEqual(
+			frappe.db.get_value("Property Setter", property_setter_filters, "value"), "Test Description"
+		)
+
+	def test_custom_field_order(self):
+		# shuffle fields
+		customize_form = self.get_customize_form(doctype="ToDo")
+		customize_form.fields.insert(0, customize_form.fields.pop())
+		customize_form.save_customization()
+
+		field_order_property = json.loads(
+			frappe.db.get_value("Property Setter", {"doc_type": "ToDo", "property": "field_order"}, "value")
+		)
+
+		self.assertEqual(field_order_property, [df.fieldname for df in frappe.get_meta("ToDo").fields])

frappe/model/meta.py

@@ -134,13 +134,10 @@ def process(self):
 			self.init_field_caches()
 			return
 
-		has_custom_fields = self.add_custom_fields()
+		self.add_custom_fields()
 		self.apply_property_setters()
 		self.init_field_caches()
-
-		if has_custom_fields:
-			self.sort_fields()
-
+		self.sort_fields()
 		self.get_valid_columns()
 		self.set_custom_permissions()
 		self.add_custom_links_and_actions()
@@ -362,7 +359,6 @@ def add_custom_fields(self):
 			return
 
 		self.extend("fields", custom_fields)
-		return True
 
 	def apply_property_setters(self):
 		"""
@@ -373,11 +369,11 @@ def apply_property_setters(self):
 		if not frappe.db.table_exists("Property Setter"):
 			return
 
-		property_setters = frappe.db.sql(
-			"""select * from `tabProperty Setter` where
-			doc_type=%s""",
-			(self.name,),
-			as_dict=1,
+		property_setters = frappe.db.get_values(
+			"Property Setter",
+			filters={"doc_type": self.name},
+			fieldname="*",
+			as_dict=True,
 		)
 
 		if not property_setters:
@@ -453,14 +449,56 @@ def init_field_caches(self):
 			self._table_fields = self.get("fields", {"fieldtype": ["in", table_fields]})
 
 	def sort_fields(self):
-		"""Sort custom fields on the basis of insert_after"""
+		"""
+		Sort fields on the basis of following rules (priority descending):
+		- `field_order` property setter
+		- `insert_after` computed based on default order for standard fields
+		- `insert_after` property for custom fields
+		"""
+
+		if field_order := getattr(self, "field_order", []):
+			field_order = [fieldname for fieldname in json.loads(field_order) if fieldname in self._fields]
+
+			# all fields match, best case scenario
+			if len(field_order) == len(self.fields):
+				self._update_fields_based_on_order(field_order)
+				return
+
+			# if the first few standard fields are not in the field order, prepare to prepend them
+			if self.fields[0].fieldname not in field_order:
+				fields_to_prepend = []
+				standard_field_found = False
+
+				for fieldname, field in self._fields.items():
+					if getattr(field, "is_custom_field", False):
+						# all custom fields from here on
+						break
+
+					if fieldname in field_order:
+						standard_field_found = True
+						break
+
+					fields_to_prepend.append(fieldname)
+
+				if standard_field_found:
+					field_order = fields_to_prepend + field_order
+				else:
+					# worst case scenario, invalidate field_order
+					field_order = fields_to_prepend
 
-		field_order = []
+		existing_fields = set(field_order) if field_order else False
 		insert_after_map = {}
 
-		for field in self.fields:
+		for index, field in enumerate(self.fields):
+			if existing_fields and field.fieldname in existing_fields:
+				continue
+
 			if not getattr(field, "is_custom_field", False):
-				field_order.append(field.fieldname)
+				if existing_fields:
+					# compute insert_after from previous field
+					insert_after_map.setdefault(self.fields[index - 1].fieldname, []).append(field.fieldname)
+				else:
+					field_order.append(field.fieldname)
 
 			elif insert_after := getattr(field, "insert_after", None):
 				insert_after_map.setdefault(insert_after, []).append(field.fieldname)
@@ -472,6 +510,9 @@ def sort_fields(self):
 		if insert_after_map:
 			_update_field_order_based_on_insert_after(field_order, insert_after_map)
 
+		self._update_fields_based_on_order(field_order)
+
+	def _update_fields_based_on_order(self, field_order):
 		sorted_fields = []
 
 		for idx, fieldname in enumerate(field_order, 1):