fix(meta): ensure that insert_after is always considered when sorting fields (#18682)
* fix(meta): ensure that `insert_after` is always considered when sorting fields * test: add nosemgrep to comment Co-authored-by: Dany Roberts <danyrt@wahni.com>
This commit is contained in:
Sagar Vora
GitHub
parent
623633f35e
commit
cb7d25a293
2 changed files with 106 additions and 41 deletions
|
|
@ -2,6 +2,7 @@
|
|||
# License: MIT. See LICENSE
|
||||
|
||||
import frappe
|
||||
from frappe.custom.doctype.custom_field.custom_field import create_custom_fields
|
||||
from frappe.tests.utils import FrappeTestCase
|
||||
|
||||
test_records = frappe.get_test_records("Custom Field")
|
||||
|
|
@ -9,8 +10,6 @@ test_records = frappe.get_test_records("Custom Field")
|
|||
|
||||
class TestCustomField(FrappeTestCase):
|
||||
def test_create_custom_fields(self):
|
||||
from .custom_field import create_custom_fields
|
||||
|
||||
create_custom_fields(
|
||||
{
|
||||
"Address": [
|
||||
|
|
@ -37,3 +36,48 @@ class TestCustomField(FrappeTestCase):
|
|||
self.assertTrue(frappe.db.exists("Custom Field", "Address-_test_custom_field_1"))
|
||||
self.assertTrue(frappe.db.exists("Custom Field", "Address-_test_custom_field_2"))
|
||||
self.assertTrue(frappe.db.exists("Custom Field", "Contact-_test_custom_field_2"))
|
||||
|
||||
def test_custom_field_sorting(self):
|
||||
try:
|
||||
custom_fields = {
|
||||
"ToDo": [
|
||||
{"fieldname": "a_test_field", "insert_after": "b_test_field"},
|
||||
{"fieldname": "b_test_field", "insert_after": "status"},
|
||||
{"fieldname": "c_test_field", "insert_after": "unknown_custom_field"},
|
||||
{"fieldname": "d_test_field", "insert_after": "status"},
|
||||
]
|
||||
}
|
||||
|
||||
create_custom_fields(custom_fields, ignore_validate=True)
|
||||
|
||||
fields = frappe.get_meta("ToDo", cached=False).fields
|
||||
|
||||
for i, field in enumerate(fields):
|
||||
if field.fieldname == "b_test_field":
|
||||
self.assertEqual(fields[i - 1].fieldname, "status")
|
||||
|
||||
if field.fieldname == "d_test_field":
|
||||
self.assertEqual(fields[i - 1].fieldname, "a_test_field")
|
||||
|
||||
self.assertEqual(fields[-1].fieldname, "c_test_field")
|
||||
|
||||
finally:
|
||||
frappe.db.delete(
|
||||
"Custom Field",
|
||||
{
|
||||
"dt": "ToDo",
|
||||
"fieldname": (
|
||||
"in",
|
||||
(
|
||||
"a_test_field",
|
||||
"b_test_field",
|
||||
"c_test_field",
|
||||
"d_test_field",
|
||||
),
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
# undo changes commited by DDL
|
||||
# nosemgrep
|
||||
frappe.db.commit()
|
||||
|
|
|
|||
|
|
@ -140,10 +140,13 @@ class Meta(Document):
|
|||
self.init_field_map()
|
||||
return
|
||||
|
||||
self.add_custom_fields()
|
||||
has_custom_fields = self.add_custom_fields()
|
||||
self.apply_property_setters()
|
||||
self.init_field_map()
|
||||
self.sort_fields()
|
||||
|
||||
if has_custom_fields:
|
||||
self.sort_fields()
|
||||
|
||||
self.get_valid_columns()
|
||||
self.set_custom_permissions()
|
||||
self.add_custom_links_and_actions()
|
||||
|
|
@ -319,7 +322,7 @@ class Meta(Document):
|
|||
return list_fields
|
||||
|
||||
def get_custom_fields(self):
|
||||
return [d for d in self.fields if d.get("is_custom_field")]
|
||||
return [d for d in self.fields if getattr(d, "is_custom_field", False)]
|
||||
|
||||
def get_title_field(self):
|
||||
"""Return the title field of this doctype,
|
||||
|
|
@ -358,17 +361,20 @@ class Meta(Document):
|
|||
if not frappe.db.table_exists("Custom Field"):
|
||||
return
|
||||
|
||||
custom_fields = frappe.db.sql(
|
||||
"""
|
||||
SELECT * FROM `tabCustom Field`
|
||||
WHERE dt = %s AND docstatus < 2
|
||||
""",
|
||||
(self.name,),
|
||||
as_dict=1,
|
||||
custom_fields = frappe.db.get_values(
|
||||
"Custom Field",
|
||||
filters={"dt": self.name},
|
||||
fieldname="*",
|
||||
as_dict=True,
|
||||
order_by="idx",
|
||||
update={"is_custom_field": 1},
|
||||
)
|
||||
|
||||
if not custom_fields:
|
||||
return
|
||||
|
||||
self.extend("fields", custom_fields)
|
||||
return True
|
||||
|
||||
def apply_property_setters(self):
|
||||
"""
|
||||
|
|
@ -452,43 +458,33 @@ class Meta(Document):
|
|||
self._fields = {field.fieldname: field for field in self.fields}
|
||||
|
||||
def sort_fields(self):
|
||||
"""sort on basis of insert_after"""
|
||||
custom_fields = sorted(self.get_custom_fields(), key=lambda df: df.idx)
|
||||
"""Sort custom fields on the basis of insert_after"""
|
||||
|
||||
if custom_fields:
|
||||
newlist = []
|
||||
field_order = []
|
||||
insert_after_map = {}
|
||||
|
||||
# if custom field is at top
|
||||
# insert_after is false
|
||||
for c in list(custom_fields):
|
||||
if not c.insert_after:
|
||||
newlist.append(c)
|
||||
custom_fields.pop(custom_fields.index(c))
|
||||
for field in self.fields:
|
||||
if not getattr(field, "is_custom_field", False):
|
||||
field_order.append(field.fieldname)
|
||||
|
||||
# standard fields
|
||||
newlist += [df for df in self.get("fields") if not df.get("is_custom_field")]
|
||||
elif insert_after := getattr(field, "insert_after", None):
|
||||
insert_after_map.setdefault(insert_after, []).append(field.fieldname)
|
||||
|
||||
newlist_fieldnames = [df.fieldname for df in newlist]
|
||||
for i in range(2):
|
||||
for df in list(custom_fields):
|
||||
if df.insert_after in newlist_fieldnames:
|
||||
cf = custom_fields.pop(custom_fields.index(df))
|
||||
idx = newlist_fieldnames.index(df.insert_after)
|
||||
newlist.insert(idx + 1, cf)
|
||||
newlist_fieldnames.insert(idx + 1, cf.fieldname)
|
||||
else:
|
||||
# if custom field is at the top, insert after is None
|
||||
field_order.insert(0, field.fieldname)
|
||||
|
||||
if not custom_fields:
|
||||
break
|
||||
if insert_after_map:
|
||||
_update_field_order_based_on_insert_after(field_order, insert_after_map)
|
||||
|
||||
# worst case, add remaining custom fields to last
|
||||
if custom_fields:
|
||||
newlist += custom_fields
|
||||
sorted_fields = []
|
||||
|
||||
# renum idx
|
||||
for i, f in enumerate(newlist):
|
||||
f.idx = i + 1
|
||||
for idx, fieldname in enumerate(field_order, 1):
|
||||
field = self._fields[fieldname]
|
||||
field.idx = idx
|
||||
sorted_fields.append(field)
|
||||
|
||||
self.fields = newlist
|
||||
self.fields = sorted_fields
|
||||
|
||||
def set_custom_permissions(self):
|
||||
"""Reset `permissions` with Custom DocPerm if exists"""
|
||||
|
|
@ -809,3 +805,28 @@ def trim_table(doctype, dry_run=True):
|
|||
frappe.db.sql_ddl(f"ALTER TABLE `tab{doctype}` {columns_to_remove}")
|
||||
|
||||
return DROPPED_COLUMNS
|
||||
|
||||
|
||||
def _update_field_order_based_on_insert_after(field_order, insert_after_map):
|
||||
"""Update the field order based on insert_after_map"""
|
||||
|
||||
retry_field_insertion = True
|
||||
|
||||
while retry_field_insertion:
|
||||
retry_field_insertion = False
|
||||
|
||||
for fieldname in list(insert_after_map):
|
||||
if fieldname not in field_order:
|
||||
continue
|
||||
|
||||
custom_field_index = field_order.index(fieldname)
|
||||
for custom_field_name in insert_after_map.pop(fieldname):
|
||||
custom_field_index += 1
|
||||
field_order.insert(custom_field_index, custom_field_name)
|
||||
|
||||
retry_field_insertion = True
|
||||
|
||||
if insert_after_map:
|
||||
# insert_after is an invalid fieldname, add these fields to the end
|
||||
for fields in insert_after_map.values():
|
||||
field_order.extend(fields)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue