diff --git a/corehq/apps/app_manager/tests/views/test_utils.py b/corehq/apps/app_manager/tests/views/test_session_endpoint_utils.py similarity index 89% rename from corehq/apps/app_manager/tests/views/test_utils.py rename to corehq/apps/app_manager/tests/views/test_session_endpoint_utils.py index 9ba99f929fa9..0f9124888ea1 100644 --- a/corehq/apps/app_manager/tests/views/test_utils.py +++ b/corehq/apps/app_manager/tests/views/test_session_endpoint_utils.py @@ -1,3 +1,4 @@ +from uuid import uuid4 from django.test import SimpleTestCase from corehq.apps.app_manager.models import ( @@ -8,7 +9,7 @@ ShadowModule ) from corehq.apps.app_manager.views.utils import ( - _is_duplicate_endpoint_id, + _duplicate_endpoint_ids, get_cleaned_session_endpoint_id, get_cleaned_and_deduplicated_session_endpoint_id, set_shadow_module_and_form_session_endpoint @@ -17,7 +18,7 @@ from corehq.apps.app_manager.exceptions import AppMisconfigurationError -class TestUtils(SimpleTestCase): +class TestSessionEndpointUtils(SimpleTestCase): new_endpoint = "abc" normal_module_session_endpoint_id = "nmsei" @@ -54,27 +55,34 @@ def create_fixtures(self): return app, normal_module, shadow_module, form + def _is_duplicate_endpoint_id(self, new_id, old_id, app): + if not new_id or new_id == old_id: + return False + + duplicates = _duplicate_endpoint_ids(new_id, [], None, app) + return len(duplicates) > 0 + def test_is_duplicate_endpoint_id_no_change_false(self): app, _, _, _ = self.create_fixtures() - is_duplicate = _is_duplicate_endpoint_id( + is_duplicate = self._is_duplicate_endpoint_id( self.normal_module_session_endpoint_id, self.normal_module_session_endpoint_id, app) self.assertFalse(is_duplicate) def test_is_duplicate_endpoint_id_same_as_other_module_false(self): app, _, _, _ = self.create_fixtures() - is_duplicate = _is_duplicate_endpoint_id( + is_duplicate = self._is_duplicate_endpoint_id( self.shadow_module_session_endpoint_id, self.normal_module_session_endpoint_id, app) self.assertTrue(is_duplicate) def test_is_duplicate_endpoint_id_same_as_form_false(self): app, _, _, _ = self.create_fixtures() - is_duplicate = _is_duplicate_endpoint_id( + is_duplicate = self._is_duplicate_endpoint_id( self.form_session_endpoint_id, self.normal_module_session_endpoint_id, app) self.assertTrue(is_duplicate) def test_is_duplicate_endpoint_id_same_as_shadow_form_false(self): app, _, _, _ = self.create_fixtures() - is_duplicate = _is_duplicate_endpoint_id( + is_duplicate = self._is_duplicate_endpoint_id( self.shadow_form_session_endpoint_id, self.normal_module_session_endpoint_id, app) self.assertTrue(is_duplicate) @@ -240,3 +248,16 @@ def test_set_shadow_module_and_form_session_endpoint_used_twice_in_forms(self): app ) ) + + def test_no_duplicates_in_other_places(self): + app, module, _, _ = self.create_fixtures() + module.forms.append(Form( + unique_id=str(uuid4()), + # this endpoint ID conflicts with the other form + session_endpoint_id=self.form_session_endpoint_id, + )) + # no meaningful change to `module`, but the duplicates should be caught + duplicates = _duplicate_endpoint_ids( + module.session_endpoint_id, [], module.unique_id, app + ) + self.assertEqual(duplicates, [self.form_session_endpoint_id]) diff --git a/corehq/apps/app_manager/views/utils.py b/corehq/apps/app_manager/views/utils.py index 320b8cc83c81..c892ce6f85df 100644 --- a/corehq/apps/app_manager/views/utils.py +++ b/corehq/apps/app_manager/views/utils.py @@ -604,10 +604,13 @@ def get_cleaned_session_endpoint_id(raw_endpoint_id): def get_cleaned_and_deduplicated_session_endpoint_id(module_or_form, raw_endpoint_id, app): cleaned_id = get_cleaned_session_endpoint_id(raw_endpoint_id) - if _is_duplicate_endpoint_id(cleaned_id, module_or_form.session_endpoint_id, app): - raise AppMisconfigurationError(_( - "Session endpoint IDs must be unique. '{endpoint_id}' is already in-use" - ).format(endpoint_id=cleaned_id)) + if cleaned_id: + duplicate_ids = _duplicate_endpoint_ids(cleaned_id, [], module_or_form.unique_id, app) + if len(duplicate_ids) > 0: + duplicates = ", ".join([f"'{d}'" for d in duplicate_ids]) + raise AppMisconfigurationError(_( + "Session endpoint IDs must be unique. The following id(s) are used multiple times: {duplicates}" + ).format(duplicates=duplicates)) return cleaned_id @@ -633,7 +636,7 @@ def set_shadow_module_and_form_session_endpoint( if len(duplicate_ids) > 0: duplicates = ", ".join([f"'{d}'" for d in duplicate_ids]) raise AppMisconfigurationError(_( - f"Session endpoint IDs must be unique. {duplicates} are used multiple times" + f"Session endpoint IDs must be unique. The following id(s) are used multiple times: {duplicates}" ).format(duplicates=duplicates)) shadow_module.session_endpoint_id = raw_endpoint_id @@ -648,15 +651,7 @@ def set_case_list_session_endpoint(module, raw_endpoint_id, app): module.case_list_session_endpoint_id = cleaned_id -def _is_duplicate_endpoint_id(new_id, old_id, app): - if not new_id or new_id == old_id: - return False - - duplicates = _duplicate_endpoint_ids(new_id, [], None, app) - return len(duplicates) > 0 - - -def _duplicate_endpoint_ids(new_session_endpoint_id, new_form_session_endpoint_ids, module_id, app): +def _duplicate_endpoint_ids(new_session_endpoint_id, new_form_session_endpoint_ids, module_or_form_id, app): all_endpoint_ids = [] def append_endpoint(endpoint_id): @@ -668,14 +663,16 @@ def append_endpoint(endpoint_id): append_endpoint(endpoint) for module in app.modules: - if module.unique_id != module_id: + if module.unique_id != module_or_form_id: append_endpoint(module.session_endpoint_id) - if module.module_type != "shadow": - for form in module.get_suite_forms(): + if module.module_type == "shadow": + for endpoint in module.form_session_endpoints: + if endpoint.form_id != module_or_form_id: + append_endpoint(endpoint.session_endpoint_id) + if module.module_type != "shadow": + for form in module.get_suite_forms(): + if form.unique_id != module_or_form_id: append_endpoint(form.session_endpoint_id) - else: - for m in module.form_session_endpoints: - append_endpoint(m.session_endpoint_id) duplicates = [k for k, v in Counter(all_endpoint_ids).items() if v > 1] diff --git a/corehq/apps/data_interfaces/models.py b/corehq/apps/data_interfaces/models.py index 3408746e98c7..462f45aaa34d 100644 --- a/corehq/apps/data_interfaces/models.py +++ b/corehq/apps/data_interfaces/models.py @@ -38,7 +38,6 @@ reset_deduplicate_rule, ) from corehq.apps.data_interfaces.utils import property_references_parent -from corehq.apps.es.cases import CaseES from corehq.apps.hqcase.utils import bulk_update_cases, update_case, AUTO_UPDATE_XMLNS from corehq.apps.users.util import SYSTEM_USER_ID from corehq.form_processor.models import DEFAULT_PARENT_IDENTIFIER