Merge pull request #1351 from anandpdoshi/customize-title-field

[fix] Customize Title Field using Customize Form. Validate title field customization. Fixes frappe/erpnext#4177

frappe/core/doctype/doctype/doctype.py

@@ -3,6 +3,7 @@
 
 from __future__ import unicode_literals
 
+import re
 import frappe
 from frappe import _
 
@@ -12,6 +13,9 @@ from frappe.model.document import Document
 from frappe.custom.doctype.property_setter.property_setter import make_property_setter
 from frappe.desk.notifications import delete_notification_count_for
 from frappe.modules import make_boilerplate
+from frappe.model.db_schema import validate_column_name
+
+class InvalidFieldNameError(frappe.ValidationError): pass
 
 form_grid_templates = {
 	"fields": "templates/form_grid/fields.html"
@@ -36,7 +40,6 @@ class DocType(Document):
 				frappe.throw(_("{0} not allowed in name").format(c))
 		self.validate_series()
 		self.scrub_field_names()
-		self.validate_title_field()
 		self.validate_document_type()
 		validate_fields(self)
 
@@ -77,13 +80,6 @@ class DocType(Document):
 					else:
 						d.fieldname = d.fieldtype.lower().replace(" ","_") + "_" + str(d.idx)
 
-
-	def validate_title_field(self):
-		"""Throw exception if `title_field` is not a valid field."""
-		if self.title_field and \
-			self.title_field not in [d.fieldname for d in self.get("fields")]:
-			frappe.throw(_("Title field must be a valid fieldname"))
-
 	def validate_series(self, autoname=None, name=None):
 		"""Validate if `autoname` property is correctly set."""
 		if not autoname: autoname = self.autoname
@@ -207,7 +203,7 @@ class DocType(Document):
 		return max_idx and max_idx[0][0] or 0
 
 def validate_fields_for_doctype(doctype):
-	validate_fields(frappe.get_meta(doctype))
+	validate_fields(frappe.get_meta(doctype, cached=False))
 
 # this is separate because it is also called via custom field
 def validate_fields(meta):
@@ -223,13 +219,11 @@ def validate_fields(meta):
 	9. Precision is set in numeric fields and is between 1 & 6.
 	10. Fold is not at the end (if set).
 	11. `search_fields` are valid.
+	12. `title_field` and title field pattern are valid.
 
 	:param meta: `frappe.model.meta.Meta` object to check."""
 	def check_illegal_characters(fieldname):
-		for c in ['.', ',', ' ', '-', '&', '%', '=', '"', "'", '*', '$',
-			'(', ')', '[', ']', '/']:
-			if c in fieldname:
-				frappe.throw(_("{0} not allowed in fieldname {1}").format(c, fieldname))
+		validate_column_name(fieldname)
 
 	def check_unique_fieldname(fieldname):
 		duplicates = filter(None, map(lambda df: df.fieldname==fieldname and str(df.idx) or None, fields))
@@ -305,6 +299,7 @@ def validate_fields(meta):
 					frappe.throw(_("Fold can not be at the end of the form"))
 
 	def check_search_fields(meta):
+		"""Throw exception if `search_fields` don't contain valid fields."""
 		if not meta.search_fields:
 			return
 
@@ -314,7 +309,32 @@ def validate_fields(meta):
 			if fieldname not in fieldname_list:
 				frappe.throw(_("Search Fields should contain valid fieldnames"))
 
+	def check_title_field(meta):
+		"""Throw exception if `title_field` isn't a valid fieldname."""
+		if not meta.title_field:
+			return
+
+		fieldname_list = [d.fieldname for d in fields]
+
+		if meta.title_field not in fieldname_list:
+			frappe.throw(_("Title field must be a valid fieldname"), InvalidFieldNameError)
+
+		def _validate_title_field_pattern(pattern):
+			if not pattern:
+				return
+
+			for fieldname in re.findall("{(.*?)}", pattern, re.UNICODE):
+				if fieldname.startswith("{"):
+					# edge case when double curlies are used for escape
+					continue
+
+				if fieldname not in fieldname_list:
+					frappe.throw(_("{{{0}}} is not a valid fieldname pattern. It should be {{field_name}}.").format(fieldname),
+						InvalidFieldNameError)
 
+		df = meta.get_field(meta.title_field)
+		_validate_title_field_pattern(df.options)
+		_validate_title_field_pattern(df.default)
 
 	fields = meta.get("fields")
 	for d in fields:
@@ -334,6 +354,7 @@ def validate_fields(meta):
 
 	check_fold(fields)
 	check_search_fields(meta)
+	check_title_field(meta)
 
 def validate_permissions_for_doctype(doctype, for_remove=False):
 	"""Validates if permissions are set correctly."""

frappe/core/doctype/file/test_file.py

@@ -15,6 +15,12 @@ class TestFile(unittest.TestCase):
 		self.delete_test_data()
 		self.upload_file()
 
+	def tearDown(self):
+		try:
+			frappe.get_doc("File", {"file_name": "file_copy.txt"}).delete()
+		except frappe.DoesNotExistError:
+			pass
+
 	def delete_test_data(self):
 		for f in frappe.db.sql('''select name, file_name from tabFile where
 			is_home_folder = 0 and is_attachments_folder = 0 order by rgt-lft asc'''):

frappe/custom/doctype/customize_form/customize_form.json

@@ -78,14 +78,35 @@
    "allow_on_submit": 0, 
    "bold": 0, 
    "collapsible": 0, 
-   "description": "Fields separated by comma (,) will be included in the \"Search By\" list of Search dialog box", 
-   "fieldname": "search_fields", 
-   "fieldtype": "Data", 
+   "depends_on": "", 
+   "fieldname": "max_attachments", 
+   "fieldtype": "Int", 
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "in_filter": 0, 
-   "in_list_view": 1, 
-   "label": "Search Fields", 
+   "in_list_view": 0, 
+   "label": "Max Attachments", 
+   "no_copy": 0, 
+   "permlevel": 0, 
+   "print_hide": 0, 
+   "read_only": 0, 
+   "report_hide": 0, 
+   "reqd": 0, 
+   "search_index": 0, 
+   "set_only_once": 0, 
+   "unique": 0
+  }, 
+  {
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "fieldname": "allow_copy", 
+   "fieldtype": "Check", 
+   "hidden": 0, 
+   "ignore_user_permissions": 0, 
+   "in_filter": 0, 
+   "in_list_view": 0, 
+   "label": "Hide Copy", 
    "no_copy": 0, 
    "permlevel": 0, 
    "print_hide": 0, 
@@ -121,16 +142,17 @@
    "allow_on_submit": 0, 
    "bold": 0, 
    "collapsible": 0, 
-   "depends_on": "", 
-   "fieldname": "max_attachments", 
-   "fieldtype": "Int", 
+   "description": "Use this fieldname to generate title", 
+   "fieldname": "title_field", 
+   "fieldtype": "Data", 
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "in_filter": 0, 
    "in_list_view": 0, 
-   "label": "Max Attachments", 
+   "label": "Title Field", 
    "no_copy": 0, 
    "permlevel": 0, 
+   "precision": "", 
    "print_hide": 0, 
    "read_only": 0, 
    "report_hide": 0, 
@@ -143,13 +165,14 @@
    "allow_on_submit": 0, 
    "bold": 0, 
    "collapsible": 0, 
-   "fieldname": "allow_copy", 
-   "fieldtype": "Check", 
+   "description": "Fields separated by comma (,) will be included in the \"Search By\" list of Search dialog box", 
+   "fieldname": "search_fields", 
+   "fieldtype": "Data", 
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "in_filter": 0, 
-   "in_list_view": 0, 
-   "label": "Hide Copy", 
+   "in_list_view": 1, 
+   "label": "Search Fields", 
    "no_copy": 0, 
    "permlevel": 0, 
    "print_hide": 0, 
@@ -234,7 +257,7 @@
    "ignore_user_permissions": 0, 
    "in_filter": 0, 
    "in_list_view": 0, 
-   "label": "Sort Order",  
+   "label": "Sort Order", 
    "no_copy": 0, 
    "options": "ASC\nDESC", 
    "permlevel": 0, 
@@ -301,7 +324,7 @@
  "is_submittable": 0, 
  "issingle": 1, 
  "istable": 0, 
- "modified": "2015-10-02 07:17:18.939161", 
+ "modified": "2015-10-21 08:11:19.151364", 
  "modified_by": "Administrator", 
  "module": "Custom", 
  "name": "Customize Form", 

frappe/custom/doctype/customize_form/customize_form.py

@@ -16,6 +16,7 @@ from frappe.core.doctype.doctype.doctype import validate_fields_for_doctype
 class CustomizeForm(Document):
 	doctype_properties = {
 		'search_fields': 'Data',
+		'title_field': 'Data',
 		'sort_field': 'Data',
 		'sort_order': 'Data',
 		'default_print_format': 'Data',

frappe/custom/doctype/customize_form/test_customize_form.py

@@ -4,10 +4,12 @@
 from __future__ import unicode_literals
 import frappe, unittest, json
 from frappe.test_runner import make_test_records_for_doctype
+from frappe.core.doctype.doctype.doctype import InvalidFieldNameError
 
 test_dependencies = ["Custom Field", "Property Setter"]
 class TestCustomizeForm(unittest.TestCase):
 	def insert_custom_field(self):
+		frappe.delete_doc_if_exists("Custom Field", "User-test_custom_field")
 		frappe.get_doc({
 			"doctype": "Custom Field",
 			"dt": "User",
@@ -174,3 +176,29 @@ class TestCustomizeForm(unittest.TestCase):
 
 		# allow for custom field
 		self.assertEquals(d.get("fields", {"fieldname": "test_custom_field"})[0].allow_on_submit, 1)
+
+	def test_title_field_pattern(self):
+		d = self.get_customize_form("Web Form")
+
+		df = d.get("fields", {"fieldname": "title"})[0]
+
+		# invalid fieldname
+		df.options = """{doc_type} - {introduction_test}"""
+		self.assertRaises(InvalidFieldNameError, d.run_method, "save_customization")
+
+		# space in formatter
+		df.options = """{doc_type} - {introduction text}"""
+		self.assertRaises(InvalidFieldNameError, d.run_method, "save_customization")
+
+		# valid fieldname
+		df.options = """{doc_type} - {introduction_text}"""
+		d.run_method("save_customization")
+
+		# valid fieldname with escaped curlies
+		df.options = """{{ {doc_type} }} - {introduction_text}"""
+		d.run_method("save_customization")
+
+		# undo
+		df.options = None
+		d.run_method("save_customization")
+

frappe/email/doctype/email_account/test_email_account.py

@@ -58,6 +58,11 @@ class TestEmailAccount(unittest.TestCase):
 		attachments = get_attachments(comm.doctype, comm.name)
 		self.assertTrue("erpnext-conf-14.png" in [f.file_name for f in attachments])
 
+		# cleanup
+		existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'})
+		frappe.delete_doc("File", existing_file.name)
+		delete_file_from_filesystem(existing_file)
+
 	def test_outgoing(self):
 		frappe.flags.sent_mail = None
 		make(subject = "test-mail-000", content="test mail 000", recipients="test_receiver@example.com",

frappe/utils/file_manager.py

@@ -164,9 +164,10 @@ def save_file(fname, content, dt, dn, folder=None, decode=False):
 	f = frappe.get_doc(file_data)
 	f.flags.ignore_permissions = True
 	try:
-		f.insert();
+		f.insert()
 	except frappe.DuplicateEntryError:
 		return frappe.get_doc("File", f.duplicate_entry)
+
 	return f
 
 def get_file_data_from_hash(content_hash):