Merge pull request #33437 from dimagi/sk/add-instances-custom-icon

check xpath expressions in text for instance references

corehq/apps/app_manager/suite_xml/post_process/instances.py

@@ -95,7 +95,7 @@ def update_suite(self):
         for remote_request in self.suite.remote_requests:
             self.add_entry_instances(remote_request)
         if self.app.supports_menu_instances:
-            for menu in self.suite.menus:
+            for menu in self.suite.localized_menus:
                 self._add_menu_instances(menu)
 
     def add_entry_instances(self, entry):
@@ -145,6 +145,11 @@ def _get_all_xpaths_for_entry(self, entry):
         if not self.app.supports_menu_instances:
             xpaths.update(self._menu_xpaths_by_command[entry.command.id])
 
+        if self.app.enable_localized_menu_media and hasattr(entry, 'localized_command'):
+            xpaths.update(entry.localized_command.get_all_xpaths())
+        else:
+            xpaths.update(entry.command.get_all_xpaths())
+
         if entry.command.id in self._relevancy_xpaths_by_command:
             xpaths.add(self._relevancy_xpaths_by_command[entry.command.id])
 
@@ -167,10 +172,7 @@ def _menu_xpaths_by_command(self):
         # multiple menus can have the same ID - merge them first
         xpaths_by_menu_id = defaultdict(set)
         for menu in self.suite.menus:
-            if menu.relevant:
-                xpaths_by_menu_id[menu.id].add(menu.relevant)
-            for assertion in menu.assertions:
-                xpaths_by_menu_id[menu.id].add(assertion.test)
+            xpaths_by_menu_id[menu.id] = menu.get_all_xpaths()
 
         return defaultdict(set, {
             command.id: xpaths_by_menu_id[menu.id]
@@ -284,15 +286,13 @@ def update_instance_order(entry):
     def _add_menu_instances(self, menu):
         # 2.54 and later only (supports_menu_instances)
         # Prior to that, instances are added to entries
-        xpaths = {assertion.test for assertion in menu.assertions}
-        if menu.relevant:
-            xpaths.add(menu.relevant)
-
+        xpaths = menu.get_all_xpaths()
         known_instances, unknown_instance_ids = get_all_instances_referenced_in_xpaths(self.app, xpaths)
         assert_no_unknown_instances(unknown_instance_ids)
         for instance in known_instances:
             menu.instances.append(instance)
 
+
 _factory_map = {}
 
 
@@ -435,6 +435,7 @@ def assert_no_unknown_instances(instance_ids):
             .format(instance_id, getattr(instance_id, 'xpath', "(XPath Unknown)"))
         )
 
+
 instance_re = re.compile(r"""instance\(\s*['"]([\w\-:]+)['"]\s*\)""", re.UNICODE)
 
 

corehq/apps/app_manager/suite_xml/xml_models.py

@@ -137,6 +137,14 @@ class Text(XmlObject):
     locale = NodeField('locale', Locale)
     locale_id = StringField('locale/@id')
 
+    def get_all_xpaths(self):
+        result = {self.xpath_function}
+        if self.xpath:
+            for variable in self.xpath.variables:
+                if variable.xpath:
+                    result.add(variable.xpath.function)
+        return result - {None}
+
 
 class ConfigurationItem(Text):
     ROOT_NAME = "text"
@@ -271,6 +279,14 @@ def set_display(self, locale_id=None, media_image=None, media_audio=None):
         elif text:
             self.text = text
 
+    def get_all_xpaths(self):
+        result = set()
+        if self.text:
+            result.update(self.text.get_all_xpaths())
+        if self.display:
+            result.update(self.display.text.get_all_xpaths())
+        return result - {None}
+
 
 class LocaleId(XmlObject):
     ROOT_NAME = 'locale'
@@ -284,6 +300,14 @@ class MediaText(XmlObject):
     xpath = NodeField('xpath', TextXPath)
     xpath_function = XPathField('xpath/@function')
 
+    def get_all_xpaths(self):
+        result = {self.xpath_function}
+        if self.xpath:
+            for variable in self.xpath.variables:
+                if variable.xpath:
+                    result.add(variable.xpath.function)
+        return result - {None}
+
 
 class LocalizedMediaDisplay(XmlObject):
     ROOT_NAME = 'display'
@@ -332,6 +356,15 @@ def __init__(self, node=None, context=None, custom_icon_locale_id=None, custom_i
         elif text:
             self.text = text
 
+    def get_all_xpaths(self):
+        result = set()
+        if self.text:
+            result.update(self.text.get_all_xpaths())
+        if self.display:
+            for text in self.display.media_text:
+                result.update(text.get_all_xpaths())
+        return result - {None}
+
 
 class CommandMixin(XmlObject):
     ROOT_NAME = 'command'
@@ -576,9 +609,12 @@ class Entry(OrderedXmlObject, XmlObject):
 
     form = StringField('form')
     post = NodeField('post', RemoteRequestPost)
+
+    # command and localized_command are mutually exclusive based on the app version
     command = NodeField('command', Command)
-    instances = NodeListField('instance', Instance)
+    localized_command = NodeField('command', LocalizedCommand)
 
+    instances = NodeListField('instance', Instance)
     datums = NodeListField('session/datum', SessionDatum)
     queries = NodeListField('session/query', RemoteRequestQuery)
     session_children = NodeListField('session/*', _wrap_session_datums)
@@ -636,14 +672,26 @@ class Menu(MenuMixin, DisplayNode, IdNode):
     """
         For CC < 2.21
     """
-    pass
+
+    def get_all_xpaths(self):
+        result = super().get_all_xpaths()
+        result.update({assertion.test for assertion in self.assertions})
+        if self.relevant:
+            result.add(self.relevant)
+        return result - {None}
 
 
 class LocalizedMenu(MenuMixin, TextOrDisplay, IdNode):
     """
         For CC >= 2.21
     """
-    pass
+
+    def get_all_xpaths(self):
+        result = super().get_all_xpaths()
+        result.update({assertion.test for assertion in self.assertions})
+        if self.relevant:
+            result.add(self.relevant)
+        return result - {None}
 
 
 class AbstractTemplate(XmlObject):
@@ -921,12 +969,8 @@ def _get_graph_config_xpaths(configuration):
                     result.add(series.nodeset)
                     result.update(_get_graph_config_xpaths(series.configuration))
             else:
-                result.add(field.header.text.xpath_function)
-                result.add(field.template.text.xpath_function)
-                if field.template.text.xpath:
-                    for variable in field.template.text.xpath.variables:
-                        if variable.xpath:
-                            result.add(str(variable.xpath.function))
+                result.update(field.header.text.get_all_xpaths())
+                result.update(field.template.text.get_all_xpaths())
 
         for detail in self.details:
             result.update(detail.get_all_xpaths())
@@ -985,7 +1029,11 @@ class Suite(OrderedXmlObject):
 
     details = NodeListField('detail', Detail)
     entries = NodeListField('entry', Entry)
+
+    # menus and localized_menus are mutually exclusive based on the app version
     menus = NodeListField('menu', Menu)
+    localized_menus = NodeListField('menu', LocalizedMenu)
+
     endpoints = NodeListField('endpoint', SessionEndpoint)
     remote_requests = NodeListField('remote-request', RemoteRequest)
 

corehq/apps/app_manager/tests/test_media_suite.py

@@ -309,7 +309,7 @@ def setUp(self):
         self.app = Application.new_app('domain', "my app")
         self.module = self.app.add_module(Module.new_module("Module 1", None))
         self.form = self.app.new_form(0, "Form 1", None)
-        self.min_spec = BuildSpec.from_string('2.21.0/latest')
+        self.min_spec = BuildSpec.from_string('2.54.0/latest')
         self.app.build_spec = self.min_spec
 
     def makeXML(self, menu_locale_id, image_locale_id, audio_locale_id):
@@ -398,7 +398,7 @@ def test_module_suite(self):
     def test_custom_icons_in_modules(self):
         self._test_custom_icon_in_suite(
             self.module, "modules.m0",
-            id_strings.module_custom_icon_locale, "./menu[@id='m0']/display")
+            id_strings.module_custom_icon_locale, "./menu[@id='m0']", "display")
 
     @patch_get_xform_resource_overrides()
     def test_case_list_form_media(self):
@@ -428,7 +428,7 @@ def test_case_list_form_media(self):
     def test_custom_icons_in_forms(self):
         self._test_custom_icon_in_suite(
             self.form, "forms.m0f0",
-            id_strings.form_custom_icon_locale, "./entry/command[@id='m0-f0']/")
+            id_strings.form_custom_icon_locale, "./entry", "command[@id='m0-f0']/")
 
     @patch_get_xform_resource_overrides()
     def test_case_list_menu_media(self):
@@ -595,13 +595,16 @@ def _assert_valid_media_translation(self, app, lang, media_locale_id, media_path
         app_strings = commcare_translations.loads(app.create_app_strings(lang))
         self.assertEqual(app_strings[media_locale_id], media_path)
 
-    def _test_custom_icon_in_suite(self, form_or_module, locale_id, custom_icon_locale_method, xml_node):
+    def _test_custom_icon_in_suite(self, form_or_module, locale_id, custom_icon_locale_method, xpath_base,
+                                   xpath_display_node):
         """
         :param form_or_module: form or module for which to test
         :param locale_id: text locale id in display block for form or module
         :param custom_icon_locale_method: method to find locale id in app strings for custom icon
         :param xml_node: where to find the xml partial for comparison
         """
+
+        xpath_full = f"{xpath_base}/{xpath_display_node}"
         custom_icon = CustomIcon(form="badge", text={'en': 'IconText', 'hin': 'चित्र'})
         form_or_module.custom_icons = [custom_icon]
 
@@ -623,7 +626,7 @@ def _test_custom_icon_in_suite(self, form_or_module, locale_id, custom_icon_loca
         # check for text locale
         custom_icon_block = custom_icon_block_template.format(locale_id=locale_id,
                                                               locale_or_xpath=text_locale_partial)
-        self.assertXmlPartialEqual(custom_icon_block, self.app.create_suite(), xml_node)
+        self.assertXmlPartialEqual(custom_icon_block, self.app.create_suite(), xpath_full)
         self._assert_app_strings_available(self.app, 'en')
 
         # check for translation for text locale
@@ -633,11 +636,18 @@ def _test_custom_icon_in_suite(self, form_or_module, locale_id, custom_icon_loca
         self._assert_valid_media_translation(self.app, 'secret', custom_icon_locale, custom_icon.text['en'])
 
         # check for xpath being set for custom icon
-        custom_icon.xpath = "if(1=1, 'a', 'b')"
+        custom_icon.xpath = "if(1=1, 'a', instance('casedb')/casedb/case[@case_id='b']/case_name)"
         custom_icon.text = {}
         form_or_module.custom_icons = [custom_icon]
         custom_icon_block = custom_icon_block_template.format(
             locale_id=locale_id,
             locale_or_xpath='<xpath function="{xpath}"/>'.format(xpath=custom_icon.xpath)
         )
-        self.assertXmlPartialEqual(custom_icon_block, self.app.create_suite(), xml_node)
+        suite = self.app.create_suite()
+        self.assertXmlPartialEqual(custom_icon_block, suite, xpath_full)
+        expected_instances = """
+        <partial>
+            <instance id="casedb" src="jr://instance/casedb"/>
+        </partial>
+        """
+        self.assertXmlPartialEqual(expected_instances, suite, f"{xpath_base}/instance")