Merge pull request #33941 from dimagi/ze/fix-no-case-prop-validation-…

…on-dd-import

Fix No Case Property/Type Name Validation on Data Dictionary Imports

corehq/apps/data_dictionary/bulk.py

@@ -21,6 +21,7 @@
     save_case_property,
     get_column_headings,
     map_row_values_to_column_names,
+    is_case_type_or_prop_name_valid,
 )
 
 FHIR_RESOURCE_TYPE_MAPPING_SHEET = "fhir_mapping"
@@ -137,7 +138,14 @@ def _process_sheets(domain, workbook, allowed_value_info):
                     domain, worksheet)
                 errors.extend(_errors)
             continue
+
         case_type = worksheet.title
+        if not is_case_type_or_prop_name_valid(case_type):
+            errors.append(
+                _('Invalid sheet name "{}". It should start with a letter, and only contain '
+                  'letters, numbers, "-", and "_"').format(case_type)
+            )
+            continue
 
         column_headings = []
         for (i, row) in enumerate(itertools.islice(worksheet.iter_rows(), 0, None), start=1):

corehq/apps/data_dictionary/tests/test_import_export.py

@@ -82,8 +82,9 @@ def test_broken_import(self):
             'Case property \"mistake\" referenced in \"caseType1-vl\" sheet that does not exist in '
             '\"caseType1\" sheet. Row(s) affected: 5, 6',
             "Error in \"caseType1\" sheet, row 5: "
-            "{'data_type': [\"Value 'Nonsense' is not a valid choice.\"]}"
-
+            "{'data_type': [\"Value 'Nonsense' is not a valid choice.\"]}",
+            'Error in "caseType1" sheet, row 6: Invalid case property name. It should start with a '
+            'letter, and only contain letters, numbers, "-", and "_"',
         }
         message_str = str(messages[0])
         soup = BeautifulSoup(message_str, features="lxml")

corehq/apps/data_dictionary/tests/test_util.py

@@ -14,7 +14,8 @@
     get_column_headings,
     map_row_values_to_column_names,
     is_case_type_deprecated,
-    get_data_dict_deprecated_case_types
+    get_data_dict_deprecated_case_types,
+    is_case_type_or_prop_name_valid,
 )
 
 
@@ -191,3 +192,20 @@ def test_get_data_dict_deprecated_case_types(self):
         deprecated_case_types = get_data_dict_deprecated_case_types(self.domain)
         self.assertEqual(len(deprecated_case_types), 1)
         self.assertEqual(deprecated_case_types, {self.deprecated_case_type_name})
+
+    def test_is_case_type_or_prop_name_valid(self):
+        valid_names = [
+            'foobar',
+            'f00bar32',
+            'foo-bar_32',
+        ]
+        invalid_names = [
+            'foo bar',
+            '32foobar',
+            'foob@r',
+            '_foobar',
+        ]
+        for valid_name in valid_names:
+            self.assertTrue(is_case_type_or_prop_name_valid(valid_name))
+        for invalid_name in invalid_names:
+            self.assertFalse(is_case_type_or_prop_name_valid(invalid_name))

corehq/apps/data_dictionary/util.py

@@ -1,6 +1,7 @@
 from collections import defaultdict
 from itertools import groupby
 from operator import attrgetter
+import re
 
 from django.core.exceptions import ValidationError
 from django.utils.translation import gettext
@@ -206,6 +207,9 @@ def save_case_property(name, case_type, domain=None, data_type=None,
     """
     if not name:
         return gettext('Case property must have a name')
+    if not is_case_type_or_prop_name_valid(name):
+        return gettext('Invalid case property name. It should start with a letter, and only contain letters, '
+                       'numbers, "-", and "_"')
 
     try:
         prop = CaseProperty.get_or_create(
@@ -360,3 +364,9 @@ def is_case_type_deprecated(domain, case_type):
         return case_type_obj.is_deprecated
     except CaseType.DoesNotExist:
         return False
+
+
+def is_case_type_or_prop_name_valid(case_prop_name):
+    pattern = '^[a-zA-Z][a-zA-Z0-9-_]*$'
+    match_obj = re.match(pattern, case_prop_name)
+    return match_obj is not None