Merge pull request #1639 from dimagi/persistentMenuAutoSelectFix

Ensure Unique Menu Entries in Persistent Menu
dimagi · Nov 6, 2024 · bb7cdbb · bb7cdbb
2 parents 5d405bf + b3af85f
commit bb7cdbb
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 22 deletions.
diff --git a/src/main/java/org/commcare/formplayer/session/PersistentMenuHelper.kt b/src/main/java/org/commcare/formplayer/session/PersistentMenuHelper.kt
@@ -70,10 +70,17 @@ class PersistentMenuHelper(val isPersistentMenuEnabled: Boolean) {
     private fun addPersistentCommand(command: PersistentCommand) {
         // currentMenu!=null implies that we must have added items to persistent menu
         check(currentMenu == null || persistentMenu.size > 0)
-        if (currentMenu == null) {
-            persistentMenu.add(command)
-        } else {
-            currentMenu!!.addCommand(command)
+        if (isCommandNotPresent(command)) {
+            if (currentMenu == null) {
+                persistentMenu.add(command)
+            } else {
+                currentMenu!!.addCommand(command)
+            }
         }
     }
+
+    private fun isCommandNotPresent(command: PersistentCommand): Boolean {
+        val currentCommands = if (currentMenu == null) persistentMenu else currentMenu!!.commands
+        return currentCommands.firstOrNull { persistentCommand -> persistentCommand.index == command.index } == null
+    }
 }
diff --git a/src/test/java/org/commcare/formplayer/mocks/FormPlayerPropertyManagerMock.java b/src/test/java/org/commcare/formplayer/mocks/FormPlayerPropertyManagerMock.java
@@ -57,11 +57,11 @@ public boolean isPersistentMenuEnabled() {
     }
 
     // convenience method to set auto advance menu as true
-    public static void mockAutoAdvanceMenu(FormplayerStorageFactory storageFactoryMock) {
+    public static void mockAutoAdvanceMenu(FormplayerStorageFactory storageFactoryMock, boolean enable) {
         SQLiteDB db = storageFactoryMock.getSQLiteDB();
         FormPlayerPropertyManagerMock propertyManagerMock = new FormPlayerPropertyManagerMock(
                 new SqlStorage(db, Property.class, PropertyManager.STORAGE_KEY));
-        propertyManagerMock.enableAutoAdvanceMenu(true);
+        propertyManagerMock.enableAutoAdvanceMenu(enable);
         when(storageFactoryMock.getPropertyManager()).thenReturn(propertyManagerMock);
     }
 }
diff --git a/src/test/java/org/commcare/formplayer/tests/AutoAdvanceMenuInNestedMultiSelectList.kt b/src/test/java/org/commcare/formplayer/tests/AutoAdvanceMenuInNestedMultiSelectList.kt
@@ -41,7 +41,7 @@ class AutoAdvanceMenuInNestedMultiSelectList : BaseTestClass() {
 
     @Test
     fun testAutoAdvanceMenuInNestedMultiSelectList() {
-        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock)
+        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock, true)
         mockRequest.mockQuery(
             "query_responses/case_search_multi_select_response.xml", 2
         ).use {

diff --git a/src/test/java/org/commcare/formplayer/tests/EndpointLaunchTest.java b/src/test/java/org/commcare/formplayer/tests/EndpointLaunchTest.java
@@ -43,7 +43,7 @@ public void setUp() throws Exception {
         super.setUp();
         configureRestoreFactory("endpointdomain", "endpointusername");
         storageFactoryMock.configure("endpointusername", "endpointdomain", "app_id", "asUser");
-        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock);
+        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock, true);
         mockRequest = new MockRequestUtils(webClientMock, restoreFactoryMock);
     }
 

diff --git a/src/test/java/org/commcare/formplayer/tests/MultiSelectCaseClaimTest.java b/src/test/java/org/commcare/formplayer/tests/MultiSelectCaseClaimTest.java
@@ -9,6 +9,7 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
 
 import org.apache.commons.lang3.ArrayUtils;
@@ -207,17 +208,22 @@ public void testAutoSelection() throws Exception {
             ArrayList<PersistentCommand> subMenu = reponse.getPersistentMenu().get(2).getCommands();
             assertEquals(1, subMenu.size());
             assertEquals("Close", subMenu.get(0).getDisplayText()); // directly contains the form instead of entity selection
-            assertEquals(2, reponse.getBreadcrumbs().length);
+            assertEquals(ImmutableList.of("Case Claim", "Follow Up"),
+                    Arrays.stream(reponse.getBreadcrumbs()).toList());
         }
 
-        ArrayList<String> updatedSelections = new ArrayList<>();
-        updatedSelections.addAll(Arrays.asList(reponse.getSelections()));
+        ArrayList<String> updatedSelections = new ArrayList<>(Arrays.asList(reponse.getSelections()));
         updatedSelections.add("0");
 
-        sessionNavigateWithQuery(updatedSelections.toArray(new String[0]),
+        NewFormResponse formResponse = sessionNavigateWithQuery(updatedSelections.toArray(new String[0]),
                 APP_NAME,
                 null,
-                FormEntryResponseBean.class);
+                NewFormResponse.class);
+        ArrayList<PersistentCommand> subMenu = formResponse.getPersistentMenu().get(2).getCommands();
+        assertEquals(1, subMenu.size());
+        assertEquals("Close", subMenu.get(0).getDisplayText());
+        assertEquals(ImmutableList.of("Case Claim", "Follow Up", "Close"),
+                Arrays.stream(formResponse.getBreadcrumbs()).toList());
     }
 
     @Test
@@ -241,7 +247,7 @@ public void testAutoSelection_WithNoCases() throws Exception {
 
     @Test
     public void testAutoAdvanceMenuWithCaseSearch() throws Exception {
-        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock);
+        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock, true);
         try (MockRequestUtils.VerifiedMock ignore = mockRequest.mockQuery(
                 "query_responses/case_search_multi_select_response.xml")) {
             EntityListResponse entityResp = sessionNavigateWithQuery(new String[]{"1"},

diff --git a/src/test/java/org/commcare/formplayer/tests/MultiSelectCaseListTest.java b/src/test/java/org/commcare/formplayer/tests/MultiSelectCaseListTest.java
@@ -47,7 +47,7 @@ public void setUp() throws Exception {
         super.setUp();
         configureRestoreFactory("caseclaimdomain", "caseclaimusername");
         storageFactoryMock.configure("user", "domain", "app_id", "asUser");
-        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock);
+        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock, true);
     }
 
     @Override
@@ -275,20 +275,34 @@ public void testPersistentMenuWithAutoSelect() throws Exception {
         expectedMenu.add(new PersistentCommand("1", "Case List", null, NavIconState.NEXT));
         expectedMenu.add(new PersistentCommand("2", "Menu with Auto Submit Form", null, NavIconState.NEXT));
         expectedMenu.add(new PersistentCommand("3", "Single Form Auto Select", null, NavIconState.NEXT));
+        PersistentCommand firstMenu = expectedMenu.get(0);
+        firstMenu.addCommand(new PersistentCommand("0","Registration Form", null, NavIconState.JUMP));
+        firstMenu.addCommand(new PersistentCommand("1","Followup Form", null, NavIconState.JUMP));
+        firstMenu.addCommand(new PersistentCommand("2","Followup Form with AutoSelect Datum", "jr://file/commcare/image/m0f2customicon_en.png", NavIconState.NEXT));
+        firstMenu.addCommand(new PersistentCommand("3","Followup Form with AutoSelect Datum", null, NavIconState.NEXT));
+        String[] selections = new String[]{"0", "2"};
+        NewFormResponse formResponse = sessionNavigate(selections, APP, NewFormResponse.class);
+        assertEquals(expectedMenu, formResponse.getPersistentMenu());
+    }
+
+    @Test
+    public void testPersistentMenuWithAutoAdvance() throws Exception {
+        ArrayList<PersistentCommand> expectedMenu = new ArrayList<>();
+        expectedMenu.add(new PersistentCommand("0", "Case List", "jr://file/commcare/image/m0customicon_en.png", NavIconState.NEXT));
+        expectedMenu.add(new PersistentCommand("1", "Case List", null, NavIconState.NEXT));
+        expectedMenu.add(new PersistentCommand("2", "Menu with Auto Submit Form", null, NavIconState.NEXT));
+        expectedMenu.add(new PersistentCommand("3", "Single Form Auto Select", null, NavIconState.NEXT));
 
         // Auto-Advance in a Auto Select Case List
         String[] selections = new String[]{"3"};
         NewFormResponse formResponse = sessionNavigate(selections, APP, NewFormResponse.class);
         assertEquals(expectedMenu, formResponse.getPersistentMenu());
 
-
-        PersistentCommand firstMenu = expectedMenu.get(0);
-        firstMenu.addCommand(new PersistentCommand("0","Registration Form", null, NavIconState.JUMP));
-        firstMenu.addCommand(new PersistentCommand("1","Followup Form", null, NavIconState.JUMP));
-        firstMenu.addCommand(new PersistentCommand("2","Followup Form with AutoSelect Datum", "jr://file/commcare/image/m0f2customicon_en.png", NavIconState.NEXT));
-        firstMenu.addCommand(new PersistentCommand("3","Followup Form with AutoSelect Datum", null, NavIconState.NEXT));
-        selections = new String[]{"0", "2"};
+        FormPlayerPropertyManagerMock.mockAutoAdvanceMenu(storageFactoryMock, false);
+        selections = new String[]{"3", "0"};
         formResponse = sessionNavigate(selections, APP, NewFormResponse.class);
+        expectedMenu.get(3).addCommand(new PersistentCommand("0", "Followup Form with AutoSelect Datum",
+                "jr://file/commcare/image/m0f2customicon_en.png", NavIconState.JUMP));
         assertEquals(expectedMenu, formResponse.getPersistentMenu());
     }
 }