Merge pull request #33938 from dimagi/riese/dynamic_map_pop_up

Dynamically load case details when clicking markers

corehq/apps/app_manager/helpers/validators.py

@@ -565,10 +565,13 @@ def validate_case_list_field_actions(self):
 class ModuleDetailValidatorMixin(object):
 
     __invalid_tile_configuration_type: str = "invalid tile configuration"
-
     __invalid_clickable_icon_configuration: str = "invalid clickable icon configuration"
+    __deprecated_popup_configuration: str = "deprecated popup configuration"
+
+    __address_popup = 'address-popup'
+    __address_popup_display = 'Address Popup'
 
-    def _validate_fields_with_format(
+    def _validate_fields_with_format_duplicate(
         self,
         format_value: str,
         format_display: str,
@@ -585,6 +588,20 @@ def _validate_fields_with_format(
                             .format(format_display, fields_with_address_format_str))
             })
 
+    def _validate_address_popup_in_long(
+        self,
+        errors: list
+    ):
+        fields_with_address_format = \
+            {c.field for c in self.module.case_details.short.columns if c.format == self.__address_popup}
+        if len(fields_with_address_format) > 0:
+            errors.append({
+                'type': self.__deprecated_popup_configuration,
+                'module': self.get_module_info(),
+                'reason': _('Format "{}" should be used in the Case Detail not Case List.'
+                            .format(self.__address_popup_display))
+            })
+
     def _validate_clickable_icons(
         self,
         columns: list,
@@ -622,6 +639,7 @@ def validate_details_for_build(self):
                     'module': self.get_module_info(),
                     'filter': self.module.case_list_filter,
                 })
+
         for detail in [self.module.case_details.short, self.module.case_details.long]:
             if detail.case_tile_template:
                 if not detail.display == "short":
@@ -638,8 +656,7 @@ def validate_details_for_build(self):
                             'module': self.get_module_info(),
                             'reason': _('A case property must be assigned to the "{}" tile field.'.format(field))
                         })
-            self._validate_fields_with_format('address', 'Address', detail.columns, errors)
-            self._validate_fields_with_format('address-popup', 'Address Popup', detail.columns, errors)
+            self._validate_fields_with_format_duplicate('address', 'Address', detail.columns, errors)
             self._validate_clickable_icons(detail.columns, errors)
 
             if detail.has_persistent_tile() and self.module.report_context_tile:
@@ -650,6 +667,16 @@ def validate_details_for_build(self):
                         A menu may not use both a persistent case list tile and a persistent report tile.
                     """),
                 })
+
+        self._validate_fields_with_format_duplicate(
+            self.__address_popup,
+            self.__address_popup_display,
+            self.module.case_details.long.columns,
+            errors)
+
+        # Temporarily comment out until migrate_address_popup has been run
+        # self._validate_address_popup_in_long(errors)
+
         return errors
 
     def get_case_errors(self, needs_case_type, needs_case_detail, needs_referral_detail=False):

corehq/apps/app_manager/management/commands/migrate_address_popup.py

@@ -0,0 +1,55 @@
+from corehq.apps.app_manager.management.commands.helpers import AppMigrationCommandBase
+from corehq.toggles import CASE_LIST_MAP
+
+
+class Command(AppMigrationCommandBase):
+    help = """Migrate case list address popup fields to case detail"""
+
+    include_builds = True
+    include_linked_apps = True
+    DOMAIN_LIST_FILENAME = 'migrate_address_popup_domain.txt'
+    DOMAIN_PROGRESS_NUMBER_FILENAME = 'migrate_address_popup_progress.txt'
+    chunk_size = 1
+
+    def add_arguments(self, parser):
+        super(Command, self).add_arguments(parser)
+
+        parser.add_argument(
+            '--reverse',
+            action='store_true',
+            default=False,
+            help="Perform the migration in reverse",
+        )
+
+    def get_domains(self):
+        return CASE_LIST_MAP.get_enabled_domains()
+
+    @staticmethod
+    def migrate_app_impl(app, reverse):
+        app_was_changed = False
+
+        for module in app['modules']:
+            detail_pair = module.get('case_details')
+            if detail_pair:
+                if reverse:
+                    detail_from, detail_to = detail_pair['long'], detail_pair['short']
+                else:
+                    detail_from, detail_to = detail_pair['short'], detail_pair['long']
+
+                address_popup_columns = \
+                    [c for c in detail_from.get('columns') if c.get('format') == 'address-popup']
+                if len(address_popup_columns) > 0:
+                    app_was_changed = app_was_changed or True
+                    other_columns = [c for c in detail_from.get('columns') if c.get('format') != 'address-popup']
+
+                    detail_from['columns'] = other_columns
+                    detail_to_columns = detail_to.get('columns', [])
+                    detail_to_columns.extend(address_popup_columns)
+                    detail_to['columns'] = detail_to_columns
+
+        if app_was_changed:
+            return app
+
+    def migrate_app(self, app):
+        reverse = self.options['reverse']
+        return Command.migrate_app_impl(app, reverse)

corehq/apps/app_manager/suite_xml/case_tile_templates/one_3X_two_4X_one_2X.json

@@ -3,7 +3,7 @@
     "filename": "one_3X_two_4X_one_2X.xml",
     "has_map": true,
     "fields": ["header", "top", "middle0", "middle1_left_img", "middle1_right", "middle2_left_img", "middle2_right", "middle3_left_img",
-        "middle3_right", "middle4_left_img", "middle4_right", "middle5", "bottom", "map", "map_popup"],
+        "middle3_right", "middle4_left_img", "middle4_right", "middle5", "bottom", "map"],
     "grid": {
         "header": {"x": 0, "y": 0, "width": 12, "height": 1, "horz-align": "left", "vert-align": "center"},
         "top": {"x": 0, "y": 1, "width": 12, "height": 1, "horz-align": "left", "vert-align": "center"},

corehq/apps/app_manager/suite_xml/case_tile_templates/one_3X_two_4X_one_2X.xml

@@ -250,21 +250,6 @@
     </template>
   </field>
 
-  <field>
-    <header>
-      <text>
-        <locale id="{map_popup[locale_id]}"/>
-      </text>
-    </header>
-    <template form="{map_popup[format]}" width="0">
-      <text>
-        <xpath function="{map_popup[xpath_function]}">
-          {map_popup[variables]}
-        </xpath>
-      </text>
-    </template>
-  </field>
-
   <field>
     <style horz-align="left" vert-align="center" font-size="small" show-border="true">
       <grid grid-height="1" grid-width="12" grid-x="0" grid-y="4"/>

corehq/apps/app_manager/suite_xml/case_tile_templates/one_one_two.json

@@ -2,7 +2,7 @@
     "slug": "one_one_two",
     "filename": "one_one_two.xml",
     "has_map": true,
-    "fields": ["title", "top", "bottom_left", "bottom_right", "map", "map_popup"],
+    "fields": ["title", "top", "bottom_left", "bottom_right", "map"],
     "grid": {
         "title": {"x": 0, "y": 0, "width": 5, "height": 1, "horz-align": "left", "vert-align": "center"},
         "top": {"x": 0, "y": 1, "width": 5, "height": 1, "horz-align": "left", "vert-align": "center"},

corehq/apps/app_manager/suite_xml/case_tile_templates/one_one_two.xml

@@ -123,19 +123,5 @@
       </text>
     </template>
   </field>
-  <field>
-    <header>
-      <text>
-        <locale id="{map_popup[locale_id]}"/>
-      </text>
-    </header>
-    <template form="{map_popup[format]}" width="0">
-      <text>
-        <xpath function="{map_popup[xpath_function]}">
-          {map_popup[variables]}
-        </xpath>
-      </text>
-    </template>
-  </field>
 
 </detail>

corehq/apps/app_manager/suite_xml/case_tile_templates/one_two_one.json

@@ -2,7 +2,7 @@
     "slug": "one_two_one",
     "filename": "one_two_one.xml",
     "has_map": true,
-    "fields": ["header", "map", "top_left", "top_right", "bottom", "map_popup"],
+    "fields": ["header", "map", "top_left", "top_right", "bottom"],
     "grid": {
         "header": {"x": 0, "y": 0, "width": 12, "height": 1, "horz-align": "left", "vert-align": "center"},
         "top_left": {"x": 0, "y": 1, "width": 7, "height": 1, "horz-align": "left", "vert-align": "center"},

corehq/apps/app_manager/suite_xml/case_tile_templates/one_two_one.xml

@@ -39,22 +39,6 @@
     </template>
   </field>
 
-  <field>
-    <header>
-      <text>
-        <locale id="{map_popup[locale_id]}"/>
-      </text>
-    </header>
-    <template form="{map_popup[format]}" width="0">
-      <text>
-        <xpath function="{map_popup[xpath_function]}">
-          {map_popup[variables]}
-        </xpath>
-      </text>
-    </template>
-    {map_popup[endpoint_action]}
-  </field>
-
   <field>
     <style horz-align="left" vert-align="center" font-size="medium">
       <grid grid-height="1" grid-width="7" grid-x="0" grid-y="1"/>

corehq/apps/app_manager/suite_xml/case_tile_templates/one_two_one_one.json

@@ -2,7 +2,7 @@
     "slug": "one_two_one_one",
     "filename": "one_two_one_one.xml",
     "has_map": true,
-    "fields": ["header", "map", "map_popup", "top_left", "top_right", "middle", "bottom"],
+    "fields": ["header", "map", "top_left", "top_right", "middle", "bottom"],
     "grid": {
         "header": {"x": 0, "y": 0, "width": 12, "height": 1, "horz-align": "left", "vert-align": "center"},
         "top_left": {"x": 0, "y": 1, "width": 7, "height": 1, "horz-align": "left", "vert-align": "center"},

corehq/apps/app_manager/suite_xml/case_tile_templates/one_two_one_one.xml

@@ -39,21 +39,6 @@
     </template>
   </field>
 
-  <field>
-    <header>
-      <text>
-        <locale id="{map_popup[locale_id]}"/>
-      </text>
-    </header>
-    <template form="{map_popup[format]}" width="0">
-      <text>
-        <xpath function="{map_popup[xpath_function]}">
-          {map_popup[variables]}
-        </xpath>
-      </text>
-    </template>
-  </field>
-
   <field>
     <style horz-align="left" vert-align="center" font-size="small">
       <grid grid-height="1" grid-width="7" grid-x="0" grid-y="1"/>

corehq/apps/app_manager/tests/test_build_errors.py

@@ -147,15 +147,21 @@ def test_case_tile_configuration_errors(self, *args):
             self._clean_unique_id(errors)
             self.assertIn(case_tile_error, errors)
 
+    def create_app_with_module(self):
+        factory = AppFactory(build_version='2.51.0')
+        app = factory.app
+        module = factory.new_basic_module('first', 'case', with_form=False)
+
+        return app, module
+
     def test_clickable_icon_configuration_errors(self, *args):
         case_tile_error = {
             'type': "invalid clickable icon configuration",
             'module': {'id': 0, 'name': {'en': 'first module'}},
             'reason': 'Column/Field "field": Clickable Icons require a form to be configured.'
         }
-        factory = AppFactory(build_version='2.51.0')
-        app = factory.app
-        module = factory.new_basic_module('first', 'case', with_form=False)
+        app, module = self.create_app_with_module()
+
         module.case_details.short.columns.append(DetailColumn(
             format='clickable-icon',
             field='field',
@@ -167,6 +173,45 @@ def test_clickable_icon_configuration_errors(self, *args):
         self._clean_unique_id(errors)
         self.assertIn(case_tile_error, errors)
 
+    # Temporarily comment out until migrate_address_popup has been run and
+    # _validate_address_popup_in_long has been added back in
+    # def test_address_popup_defined_in_case_list(self, *args):
+    #     case_tile_error = {
+    #         'type': "invalid tile configuration",
+    #         'module': {'id': 0, 'name': {'en': 'first module'}},
+    #         'reason': 'Format "Address Popup" should be used in the Case Detail not Case List.'
+    #     }
+    #     app, module = self.create_app_with_module()
+    #     module.case_details.short.columns.append(DetailColumn(
+    #         format='address-popup',
+    #         field='field',
+    #         header={'en': 'Column'},
+    #         model='case',
+    #     ))
+    #
+    #     errors = app.validate_app()
+    #     self._clean_unique_id(errors)
+    #     self.assertIn(case_tile_error, errors)
+
+    def test_address__defined_twice(self, *args):
+        case_tile_error = {
+            'type': "invalid tile configuration",
+            'module': {'id': 0, 'name': {'en': 'first module'}},
+            'reason': 'Format "Address" can only be used once but is used by multiple properties: "f1", "f2"'
+        }
+        app, module = self.create_app_with_module()
+        for field_id in [1, 2]:
+            module.case_details.short.columns.append(DetailColumn(
+                format='address',
+                field=f'f{field_id}',
+                header={'en': 'Column'},
+                model='case',
+            ))
+
+        errors = app.validate_app()
+        self._clean_unique_id(errors)
+        self.assertIn(case_tile_error, errors)
+
     def test_case_list_form_advanced_module_different_case_config(self, *args):
         case_tile_error = {
             'type': "all forms in case list module must load the same cases",