Merge pull request #31923 from dimagi/sk/add-name

allow submitting system forms with a name for display to users

corehq/apps/data_interfaces/models.py

@@ -910,7 +910,7 @@ def when_case_matches(self, case, rule):
             if case_id == case.case_id:
                 continue
             result = update_case(case.domain, case_id, case_properties=properties, close=False,
-                                 xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id)
+                                 xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id, form_name=rule.name)
             rule.log_submission(result[0].form_id)
             num_related_updates += 1
 
@@ -923,7 +923,7 @@ def when_case_matches(self, case, rule):
 
         if close_case or properties:
             result = update_case(case.domain, case.case_id, case_properties=properties, close=close_case,
-                                 xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id)
+                                 xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id, form_name=rule.name)
 
             rule.log_submission(result[0].form_id)
 

corehq/apps/hqcase/templates/hqcase/xml/case_block.xml

@@ -1,5 +1,5 @@
 <?xml version='1.0' ?>
-<system version="1" uiVersion="1" xmlns="{{ xmlns }}" xmlns:orx="http://openrosa.org/jr/xforms">
+<system version="1" uiVersion="1" xmlns="{{ xmlns }}" xmlns:orx="http://openrosa.org/jr/xforms"{% if name %} name="{{ name }}"{% endif %}>
     {% if form_data %}
     {% for name, value in form_data.items %}
     <{{ name }}>{{ value }}</{{ name }}>

corehq/apps/hqcase/utils.py

@@ -35,36 +35,38 @@
 
 def submit_case_blocks(case_blocks, domain, username="system", user_id=None,
                        xmlns=None, attachments=None, form_id=None,
-                       form_extras=None, case_db=None, device_id=None, max_wait=...):
+                       submission_extras=None, case_db=None, device_id=None, form_name=None, max_wait=...):
     """
     Submits casexml in a manner similar to how they would be submitted from a phone.
 
-    :param xmlns: Form XMLNS. Format: IRI or URN. Historically this was
-    used in some places to uniquely identify the subsystem that posted
-    the cases; `device_id` is now recommended for that purpose. Going
-    forward, it is recommended to use the default value along with
-    `device_id`, which indicates that the cases were submitted by an
-    internal system process.
+    :param xmlns: Form XMLNS. Format: IRI or URN. This should be used to
+    identify the subsystem that posted the cases. This should be a constant
+    value without any dynamic components. Ideally the XMLNS should be
+    added to ``SYSTEM_FORM_XMLNS_MAP`` for more user-friendly display.
+    See ``SYSTEM_FORM_XMLNS_MAP`` form examples.
     :param device_id: Identifier for the source of posted cases. Ideally
-    this should uniquely identify the subsystem that is posting cases to
-    make it easier to trace the source. All new code should use this
-    argument. A human recognizable value is recommended outside of test
-    code. Example: "auto-close-rule-<GUID>"
-    :param form_extras: Dict of additional kwargs to pass through to ``SubmissionPost``
+    this should uniquely identify the exact subsystem configuration that
+    is posting cases to make it easier to trace the source. Used in combination with
+    XMLNS this allows pinpointing the exact source. Example: If the cases are being
+    generated by an Automatic Case Rule, then the device_id should be the rule's ID.
+    :param form_name: Human readable version of the device_id. For example the
+    Automatic Case Rule name.
+    :param submission_extras: Dict of additional kwargs to pass through to ``SubmissionPost``
     :param max_wait: Maximum time (in seconds) to allow the process to be delayed if
     the project is over its submission rate limit.
     See the docstring for submit_form_locally for meaning of values.
 
     returns the UID of the resulting form.
     """
-    form_extras = form_extras or {}
+    submission_extras = submission_extras or {}
     attachments = attachments or {}
     now = json_format_datetime(datetime.datetime.utcnow())
     if not isinstance(case_blocks, str):
         case_blocks = ''.join(case_blocks)
     form_id = form_id or uuid.uuid4().hex
     form_xml = render_to_string('hqcase/xml/case_block.xml', {
         'xmlns': xmlns or SYSTEM_FORM_XMLNS,
+        'name': form_name,
         'case_block': case_blocks,
         'time': now,
         'uid': form_id,
@@ -79,7 +81,7 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None,
         attachments=attachments,
         case_db=case_db,
         max_wait=max_wait,
-        **form_extras
+        **submission_extras
     )
     return result.xform, result.cases
 
@@ -133,16 +135,17 @@ def _get_update_or_close_case_block(case_id, case_properties=None, close=False,
 
 
 def update_case(domain, case_id, case_properties=None, close=False,
-                xmlns=None, device_id=None, owner_id=None, max_wait=...):
+                xmlns=None, device_id=None, form_name=None, owner_id=None, max_wait=...):
     """
     Updates or closes a case (or both) by submitting a form.
     domain - the case's domain
     case_id - the case's id
     case_properties - to update the case, pass in a dictionary of {name1: value1, ...}
                       to ignore case updates, leave this argument out
     close - True to close the case, False otherwise
-    xmlns - pass in an xmlns to use it instead of the default
+    xmlns - see submit_case_blocks xmlns docs
     device_id - see submit_case_blocks device_id docs
+    form_name - see submit_case_blocks form_name docs
     max_wait - Maximum time (in seconds) to allow the process to be delayed if
                the project is over its submission rate limit.
                See the docstring for submit_form_locally for meaning of values
@@ -154,6 +157,7 @@ def update_case(domain, case_id, case_properties=None, close=False,
         user_id=SYSTEM_USER_ID,
         xmlns=xmlns,
         device_id=device_id,
+        form_name=form_name,
         max_wait=max_wait
     )
 

corehq/apps/reports/display.py

@@ -26,7 +26,7 @@ def replace(self, *args):
         return StringWithAttributes(string)
 
 
-class FormDisplay():
+class FormDisplay:
 
     def __init__(self, form_doc, report, lang=None):
         self.form = form_doc
@@ -74,54 +74,72 @@ def readable_form_name(self):
             self.report.domain,
             self.form.get("xmlns"),
             app_id=self.form.get("app_id"),
-            lang=self.lang
+            lang=self.lang,
+            form_name=self.form.get("@name"),
         )
 
 
 class _FormType(object):
 
-    def __init__(self, domain, xmlns, app_id):
+    def __init__(self, domain, xmlns, app_id, form_name):
         self.domain = domain
         self.xmlns = xmlns
         self.app_id = app_id
+        self.form_name = form_name
 
     def get_label(self, lang=None, separator=None):
         if separator is None:
             separator = " > "
+
+        return (
+            self.get_label_from_app(lang, separator)
+            or self.get_name_from_xml(separator)
+            or self.append_form_name(self.xmlns, separator)
+        )
+
+    def get_label_from_app(self, lang, separator):
         form = get_form_analytics_metadata(self.domain, self.app_id, self.xmlns)
-        if form and form.get('app'):
-            langs = form['app']['langs']
-            app_name = form['app']['name']
+        app = form and form.get('app')
+        if not app:
+            return
 
-            if form.get('is_user_registration'):
-                form_name = "User Registration"
-                title = separator.join([app_name, form_name])
-            else:
-                def _menu_name(menu, lang):
-                    if lang and menu.get(lang):
-                        return menu.get(lang)
-                    else:
-                        for lang in langs + list(menu):
-                            menu_name = menu.get(lang)
-                            if menu_name is not None:
-                                return menu_name
-                        return "?"
-
-                module_name = _menu_name(form["module"]["name"], lang)
-                form_name = _menu_name(form["form"]["name"], lang)
-                title = separator.join([app_name, module_name, form_name])
-
-            if form.get('app_deleted'):
-                title += ' [Deleted]'
-            if form.get('duplicate'):
-                title += " [Multiple Forms]"
-            name = title
-        elif self.xmlns in SYSTEM_FORM_XMLNS_MAP:
-            name = SYSTEM_FORM_XMLNS_MAP[self.xmlns]
+        langs = form['app']['langs']
+        app_name = form['app']['name']
+
+        if form.get('is_user_registration'):
+            form_name = "User Registration"
+            title = separator.join([app_name, form_name])
         else:
-            name = self.xmlns
+            def _menu_name(menu, lang):
+                if lang and menu.get(lang):
+                    return menu.get(lang)
+                else:
+                    for lang in langs + list(menu):
+                        menu_name = menu.get(lang)
+                        if menu_name is not None:
+                            return menu_name
+                    return "?"
+
+            module_name = _menu_name(form["module"]["name"], lang)
+            form_name = _menu_name(form["form"]["name"], lang)
+            title = separator.join([app_name, module_name, form_name])
+
+        if form.get('app_deleted'):
+            title += ' [Deleted]'
+        if form.get('duplicate'):
+            title += " [Multiple Forms]"
+        return title
+
+    def get_name_from_xml(self, separator):
+        if self.xmlns in SYSTEM_FORM_XMLNS_MAP:
+            readable_xmlns = str(SYSTEM_FORM_XMLNS_MAP[self.xmlns])
+            return self.append_form_name(readable_xmlns, separator)
+
+    def append_form_name(self, name, separator):
+        if self.form_name:
+            name = separator.join([name, self.form_name])
         return name
 
 
-def xmlns_to_name(domain, xmlns, app_id, lang=None, separator=None):
-    return _FormType(domain, xmlns, app_id).get_label(lang, separator)
+def xmlns_to_name(domain, xmlns, app_id, lang=None, separator=None, form_name=None):
+    return _FormType(domain, xmlns, app_id, form_name).get_label(lang, separator)

corehq/apps/reports/standard/cases/case_data.py

@@ -268,6 +268,7 @@ def form_to_json(domain, form, timezone):
         form.xmlns,
         app_id=form.app_id,
         lang=get_language(),
+        form_name=form.form_data.get('@name')
     )
     received_on = ServerTime(form.received_on).user_time(timezone).done().strftime(DATE_FORMAT)
 

corehq/apps/reports/standard/forms/reports.py

@@ -142,6 +142,7 @@ def _fmt_date(somedate):
                     self.domain,
                     xform_dict.get('xmlns'),
                     app_id=xform_dict.get('app_id'),
+                    form_name=xform_dict['form'].get('@name'),
                 )
                 form_username = xform_dict['form']['meta'].get('username', EMPTY_USER)
             else:

corehq/apps/reports/tests/test_display.py

@@ -0,0 +1,27 @@
+from unittest.mock import patch
+
+from django.test import SimpleTestCase
+
+from corehq.apps.hqcase.utils import SYSTEM_FORM_XMLNS, SYSTEM_FORM_XMLNS_MAP
+
+from corehq.apps.reports.display import xmlns_to_name
+from corehq.util.test_utils import generate_cases
+
+UNKNOWN_XMLNS = "http://demo.co/form"
+KNOWN_XMLNS = SYSTEM_FORM_XMLNS
+KNOWN_XMLNS_DISPLAY = SYSTEM_FORM_XMLNS_MAP[KNOWN_XMLNS]
+
+
+@patch("corehq.apps.reports.display.get_form_analytics_metadata", new=lambda *a, **k: None)
+class TestXmlnsToName(SimpleTestCase):
+    @generate_cases([
+        (UNKNOWN_XMLNS, UNKNOWN_XMLNS),
+        (f"{UNKNOWN_XMLNS} > form name", UNKNOWN_XMLNS, "form name"),
+        (f"{UNKNOWN_XMLNS} ] form name", UNKNOWN_XMLNS, "form name", " ] "),
+        (KNOWN_XMLNS_DISPLAY, KNOWN_XMLNS),
+        (f"{KNOWN_XMLNS_DISPLAY} > form name", KNOWN_XMLNS, "form name"),
+        (f"{KNOWN_XMLNS_DISPLAY} ] form name", KNOWN_XMLNS, "form name", " ] "),
+    ])
+    def test_xmlns_to_name(self, expected, xmlns, form_name=None, separator=None):
+        name = xmlns_to_name("domain", xmlns, "123", separator=separator, form_name=form_name)
+        self.assertEqual(name, expected)

corehq/ex-submodules/casexml/apps/case/cleanup.py

@@ -119,17 +119,17 @@ def claim_case(domain, restore_user, host_id, host_type=None, host_name=None, de
             )
         }
     ).as_xml()
-    form_extras = {}
+    submission_extras = {}
     if restore_user.request_user:
-        form_extras["auth_context"] = AuthContext(
+        submission_extras["auth_context"] = AuthContext(
             domain=domain,
             user_id=restore_user.request_user_id,
             authenticated=True
         )
     submit_case_blocks(
         [ElementTree.tostring(claim_case_block, encoding='utf-8').decode('utf-8')],
         domain=domain,
-        form_extras=form_extras,
+        submission_extras=submission_extras,
         username=restore_user.full_username,
         user_id=restore_user.user_id,
         device_id=device_id,

corehq/ex-submodules/casexml/apps/case/mock/mock.py

@@ -70,10 +70,10 @@ class CaseFactory(object):
     creates and saves the case directly, which is faster.
     """
 
-    def __init__(self, domain=None, case_defaults=None, form_extras=None):
+    def __init__(self, domain=None, case_defaults=None, submission_extras=None):
         self.domain = domain or 'test-domain'
         self.case_defaults = case_defaults if case_defaults is not None else {}
-        self.form_extras = form_extras if form_extras is not None else {}
+        self.submission_extras = submission_extras if submission_extras is not None else {}
 
     def get_case_block(self, case_id, **kwargs):
         for k, v in self.case_defaults.items():
@@ -94,17 +94,19 @@ def get_blocks(structure):
                     for block in get_blocks(index.related_structure):
                         yield block
 
-        return [block for structure in case_structures
-                      for block in get_blocks(structure)]
+        return [
+            block for structure in case_structures
+            for block in get_blocks(structure)
+        ]
 
-    def post_case_blocks(self, caseblocks, form_extras=None, user_id=None, device_id=None, xmlns=None):
+    def post_case_blocks(self, caseblocks, submission_extras=None, user_id=None, device_id=None, xmlns=None):
         from corehq.apps.hqcase.utils import submit_case_blocks
-        submit_form_extras = copy.copy(self.form_extras)
-        if form_extras is not None:
-            submit_form_extras.update(form_extras)
+        submit_form_extras = copy.copy(self.submission_extras)
+        if submission_extras is not None:
+            submit_form_extras.update(submission_extras)
         return submit_case_blocks(
             caseblocks,
-            form_extras=submit_form_extras,
+            submission_extras=submit_form_extras,
             domain=self.domain,
             user_id=user_id,
             device_id=device_id,
@@ -144,16 +146,24 @@ def close_case(self, case_id):
         """
         return self.create_or_update_case(CaseStructure(case_id=case_id, attrs={'close': True}))[0]
 
-    def create_or_update_case(self, case_structure, form_extras=None, user_id=None, device_id=None, xmlns=None):
+    def create_or_update_case(
+        self, case_structure, submission_extras=None, user_id=None, device_id=None, xmlns=None
+    ):
         return self.create_or_update_cases(
-            [case_structure], form_extras=form_extras, user_id=user_id, device_id=device_id, xmlns=xmlns
+            [case_structure],
+            submission_extras=submission_extras,
+            user_id=user_id,
+            device_id=device_id,
+            xmlns=xmlns
         )
 
-    def create_or_update_cases(self, case_structures, form_extras=None, user_id=None, device_id=None, xmlns=None):
+    def create_or_update_cases(
+        self, case_structures, submission_extras=None, user_id=None, device_id=None, xmlns=None
+    ):
         from corehq.form_processor.models import CommCareCase
         self.post_case_blocks(
             [b.as_text() for b in self.get_case_blocks(case_structures)],
-            form_extras,
+            submission_extras,
             user_id=user_id,
             device_id=device_id,
             xmlns=xmlns,

corehq/ex-submodules/casexml/apps/case/tests/test_factory.py

@@ -175,7 +175,7 @@ def test_form_extras(self):
         token_id = uuid.uuid4().hex
         factory = CaseFactory(domain=domain)
         [case] = factory.create_or_update_case(CaseStructure(
-            attrs={'create': True}), form_extras={'last_sync_token': token_id})
+            attrs={'create': True}), submission_extras={'last_sync_token': token_id})
         form = XFormInstance.objects.get_form(case.xform_ids[0], domain)
         self.assertEqual(token_id, form.last_sync_token)
 
@@ -184,16 +184,16 @@ def test_form_extras_default(self):
         # have to enable loose sync token validation for the domain or create actual SyncLog documents.
         # this is the easier path.
         token_id = uuid.uuid4().hex
-        factory = CaseFactory(domain=domain, form_extras={'last_sync_token': token_id})
+        factory = CaseFactory(domain=domain, submission_extras={'last_sync_token': token_id})
         case = factory.create_case()
         form = XFormInstance.objects.get_form(case.xform_ids[0], domain)
         self.assertEqual(token_id, form.last_sync_token)
 
     def test_form_extras_override_defaults(self):
         domain = uuid.uuid4().hex
         token_id = uuid.uuid4().hex
-        factory = CaseFactory(domain=domain, form_extras={'last_sync_token': token_id})
+        factory = CaseFactory(domain=domain, submission_extras={'last_sync_token': token_id})
         [case] = factory.create_or_update_case(CaseStructure(
-            attrs={'create': True}), form_extras={'last_sync_token': 'differenttoken'})
+            attrs={'create': True}), submission_extras={'last_sync_token': 'differenttoken'})
         form = XFormInstance.objects.get_form(case.xform_ids[0], domain)
         self.assertEqual('differenttoken', form.last_sync_token)