From ae6f87e59b6d7077a8045092c7e1b6ceedb406c8 Mon Sep 17 00:00:00 2001 From: Ethan Soergel Date: Mon, 11 Sep 2023 12:05:21 -0400 Subject: [PATCH] Ensure duplicate menu instances are merged This is sort of a recurrance of an issue I introduced in https://github.com/dimagi/commcare-hq/pull/33037 and then fixed in https://github.com/dimagi/commcare-hq/pull/33271/ See that second PR description for details This was then broken in this commit: https://github.com/dimagi/commcare-hq/commit/8db8f4a27621307125a7ec627a7b05a2f89f1d6d The test created in that second PR above demonstrated that instances in the second of two modules with the same ID will be propagated to all forms in both modules, but it neglected to verify that same property of instances required by the first of those two modules. That latest commit above had subsequent menus overwriting instances from earlier ones. --- corehq/apps/app_manager/suite_xml/post_process/instances.py | 2 +- corehq/apps/app_manager/tests/test_suite_instances.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/corehq/apps/app_manager/suite_xml/post_process/instances.py b/corehq/apps/app_manager/suite_xml/post_process/instances.py index cae6dd09c8bf..1bdeaf1234c4 100644 --- a/corehq/apps/app_manager/suite_xml/post_process/instances.py +++ b/corehq/apps/app_manager/suite_xml/post_process/instances.py @@ -172,7 +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: - xpaths_by_menu_id[menu.id] = menu.get_all_xpaths() + xpaths_by_menu_id[menu.id].update(menu.get_all_xpaths()) return defaultdict(set, { command.id: xpaths_by_menu_id[menu.id] diff --git a/corehq/apps/app_manager/tests/test_suite_instances.py b/corehq/apps/app_manager/tests/test_suite_instances.py index c3125bc92c02..3c9c88f97a9c 100644 --- a/corehq/apps/app_manager/tests/test_suite_instances.py +++ b/corehq/apps/app_manager/tests/test_suite_instances.py @@ -271,6 +271,7 @@ def test_module_filter_instances_on_all_forms_merged_modules(self): # and if two modules are display_in_root, then on forms in the other module too factory = AppFactory(build_version='2.20.0') # enable_module_filtering self.m0, self.m0f0 = factory.new_basic_module('m0', 'case1') + self.m0.module_filter = "instance('groups')/groups/" self.m0.put_in_root = True self.m1, self.m1f0 = factory.new_basic_module('m1', 'case1') @@ -278,7 +279,10 @@ def test_module_filter_instances_on_all_forms_merged_modules(self): self.m1.put_in_root = True suite = factory.app.create_suite() - instance_xml = "" + instance_xml = """ + + + """ self.assertXmlPartialEqual(instance_xml, suite, "entry/command[@id='m0-f0']/../instance") self.assertXmlPartialEqual(instance_xml, suite, "entry/command[@id='m1-f0']/../instance")