Merge pull request #33558 from dimagi/jls/fix-duplicate-session-id-me…

…ssage

Fix duplicate session id error message

corehq/apps/app_manager/tests/views/test_utils.py → 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])

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]
 

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