Skip to content

Commit

Permalink
refactor!: Prefix all custom fieldnames created from Desk (frappe#21355)
Browse files Browse the repository at this point in the history
* fix: actualy remove special characterso

`or "_"` is always truthy, this code didn't do anything

* refactor!: Prefix all custom fieldnames created from Desk

 Why?
 - Custom and standard field clashes are frequent. This will prevent it
   from happening. E.g. recent `incoterm` field addition in ERPNext.
 - Apps/fixtures can still specify exact fieldnames

* test: custom field naming
  • Loading branch information
ankush authored Jun 19, 2023
1 parent 38960f4 commit d0ba31c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 25 deletions.
Empty file added .semgrepignore
Empty file.
3 changes: 2 additions & 1 deletion frappe/custom/doctype/custom_field/custom_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ def set_fieldname(self):

# remove special characters from fieldname
self.fieldname = "".join(
filter(lambda x: x.isdigit() or x.isalpha() or "_", cstr(label).replace(" ", "_"))
[c for c in cstr(label).replace(" ", "_") if c.isdigit() or c.isalpha() or c == "_"]
)
self.fieldname = f"custom_{self.fieldname}"

# fieldnames should be lowercase
self.fieldname = self.fieldname.lower()
Expand Down
45 changes: 21 additions & 24 deletions frappe/custom/doctype/customize_form/test_customize_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@

class TestCustomizeForm(FrappeTestCase):
def insert_custom_field(self):
frappe.delete_doc_if_exists("Custom Field", "Event-test_custom_field")
frappe.get_doc(
frappe.delete_doc_if_exists("Custom Field", "Event-custom_test_field")
self.field = frappe.get_doc(
{
"doctype": "Custom Field",
"fieldname": "custom_test_field",
"dt": "Event",
"label": "Test Custom Field",
"description": "A Custom Field for Testing",
Expand All @@ -36,7 +37,7 @@ def setUp(self):
frappe.clear_cache(doctype="Event")

def tearDown(self):
frappe.delete_doc("Custom Field", "Event-test_custom_field")
frappe.delete_doc("Custom Field", self.field.name)
frappe.db.commit()
frappe.clear_cache(doctype="Event")

Expand All @@ -60,7 +61,7 @@ def test_fetch_to_customize(self):
self.assertEqual(d.doc_type, "Event")

self.assertEqual(len(d.get("fields")), len(frappe.get_doc("DocType", d.doc_type).fields) + 1)
self.assertEqual(d.get("fields")[-1].fieldname, "test_custom_field")
self.assertEqual(d.get("fields")[-1].fieldname, self.field.fieldname)
self.assertEqual(d.get("fields", {"fieldname": "event_type"})[0].in_list_view, 1)

return d
Expand Down Expand Up @@ -129,21 +130,21 @@ def test_save_customization_field_property(self):

def test_save_customization_custom_field_property(self):
d = self.get_customize_form("Event")
self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0)
self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "reqd"), 0)

custom_field = d.get("fields", {"fieldname": "test_custom_field"})[0]
custom_field = d.get("fields", {"fieldname": self.field.fieldname})[0]
custom_field.reqd = 1
custom_field.no_copy = 1
d.run_method("save_customization")
self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 1)
self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "no_copy"), 1)
self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "reqd"), 1)
self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "no_copy"), 1)

custom_field = d.get("fields", {"is_custom_field": True})[0]
custom_field.reqd = 0
custom_field.no_copy = 0
d.run_method("save_customization")
self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0)
self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "no_copy"), 0)
self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "reqd"), 0)
self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "no_copy"), 0)

def test_save_customization_new_field(self):
d = self.get_customize_form("Event")
Expand All @@ -157,28 +158,24 @@ def test_save_customization_new_field(self):
},
)
d.run_method("save_customization")

custom_field_name = "Event-custom_test_add_custom_field_via_customize_form"
self.assertEqual(
frappe.db.get_value(
"Custom Field", "Event-test_add_custom_field_via_customize_form", "fieldtype"
),
frappe.db.get_value("Custom Field", custom_field_name, "fieldtype"),
"Data",
)

self.assertEqual(
frappe.db.get_value(
"Custom Field", "Event-test_add_custom_field_via_customize_form", "insert_after"
),
frappe.db.get_value("Custom Field", custom_field_name, "insert_after"),
last_fieldname,
)

frappe.delete_doc("Custom Field", "Event-test_add_custom_field_via_customize_form")
self.assertEqual(
frappe.db.get_value("Custom Field", "Event-test_add_custom_field_via_customize_form"), None
)
frappe.delete_doc("Custom Field", custom_field_name)
self.assertEqual(frappe.db.get_value("Custom Field", custom_field_name), None)

def test_save_customization_remove_field(self):
d = self.get_customize_form("Event")
custom_field = d.get("fields", {"fieldname": "test_custom_field"})[0]
custom_field = d.get("fields", {"fieldname": self.field.fieldname})[0]
d.get("fields").remove(custom_field)
d.run_method("save_customization")

Expand All @@ -200,7 +197,7 @@ def test_reset_to_defaults(self):
def test_set_allow_on_submit(self):
d = self.get_customize_form("Event")
d.get("fields", {"fieldname": "subject"})[0].allow_on_submit = 1
d.get("fields", {"fieldname": "test_custom_field"})[0].allow_on_submit = 1
d.get("fields", {"fieldname": "custom_test_field"})[0].allow_on_submit = 1
d.run_method("save_customization")

d = self.get_customize_form("Event")
Expand All @@ -209,7 +206,7 @@ def test_set_allow_on_submit(self):
self.assertEqual(d.get("fields", {"fieldname": "subject"})[0].allow_on_submit or 0, 0)

# allow for custom field
self.assertEqual(d.get("fields", {"fieldname": "test_custom_field"})[0].allow_on_submit, 1)
self.assertEqual(d.get("fields", {"fieldname": "custom_test_field"})[0].allow_on_submit, 1)

def test_title_field_pattern(self):
d = self.get_customize_form("Web Form")
Expand Down Expand Up @@ -406,7 +403,7 @@ def test_change_to_autoincrement_autoname(self):

def test_system_generated_fields(self):
doctype = "Event"
custom_field_name = "test_custom_field"
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
Expand Down

0 comments on commit d0ba31c

Please sign in to comment.