Fix instance accumulation for menus with same id

dimagi · Oct 28, 2024 · e5dbcf9 · e5dbcf9
1 parent 67bcb5a
commit e5dbcf9
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 4 deletions.
diff --git a/src/main/java/org/commcare/session/CommCareSession.java b/src/main/java/org/commcare/session/CommCareSession.java
@@ -633,7 +633,6 @@ public EvaluationContext getEvaluationContext(InstanceInitializationFactory iif,
         Vector<Entry> entries = getEntriesForCommand(command);
         Vector<Menu> menus = getMenusForCommand(command);
 
-        Menu menu = null;
         Entry entry = null;
         Hashtable<String, DataInstance> instancesInScope = new Hashtable<>();
         Hashtable<String, DataInstance> menuInstances = null;
@@ -649,8 +648,7 @@ public EvaluationContext getEvaluationContext(InstanceInitializationFactory iif,
             }
         }
 
-        if (!menus.isEmpty()) {
-            menu = menus.elementAt(0);
+        for (Menu menu : menus) {
             if (menu != null) {
                 menuInstances = menu.getInstances(instancesToInclude);
             }

diff --git a/src/test/java/org/commcare/backend/session/test/MenuTests.java b/src/test/java/org/commcare/backend/session/test/MenuTests.java
@@ -2,7 +2,10 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
+import org.commcare.modern.session.SessionWrapper;
+import org.commcare.session.CommCareSession;
 import org.commcare.suite.model.AssertionSet;
 import org.commcare.suite.model.Menu;
 import org.commcare.suite.model.Suite;
@@ -12,6 +15,8 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.List;
+
 /**
  * Tests for assertions in menus
  */
@@ -36,4 +41,18 @@ public void testAssertionsEvaluated() throws Exception {
         assertNotNull(assertions);
         assertEquals("custom_assertion.m0.0", assertionFailures.getArgument());
     }
+
+    /**
+     * When there are multiple menu blocks with same ids, we should accumulate the required instances from all
+     * of the menus and their contained entries
+     */
+    @Test
+    public void testMenuInstances_WhenMenuHaveSameIds() {
+        SessionWrapper currentSession = appWithMenuAssertions.getSession();
+        EvaluationContext ec = currentSession.getEvaluationContext(currentSession.getIIF(), "m3", null);
+        List<String> instanceIds = ec.getInstanceIds();
+        assertEquals(2, instanceIds.size());
+        assertTrue(instanceIds.contains("my_instance"));
+        assertTrue(instanceIds.contains("casedb"));
+    }
 }
diff --git a/src/test/resources/app_structure/suite.xml b/src/test/resources/app_structure/suite.xml
@@ -244,9 +244,10 @@
     <text>Menu</text>
     <command id="m0-f0"/>
   </menu>
-  <menu id="m3">
+  <menu id="m3" relevant="count(instance('my_instance'))>0">
     <text>Menu</text>
     <command id="m0-f1"/>
+    <instance id="my_instance" src="my_instance_source"/>
   </menu>
   <endpoint id="case_list" respect-relevancy="false">
     <argument id="selected_cases" instance-id="selected_cases" instance-src="jr://instance/selected-entities"/>