Move some methods to MenuReplaceUtilities

4 files changed, 47 insertions, 33 deletions

diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilitiesTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilitiesTests.java
index 85e57fee0..61587dbdc 100644
--- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilitiesTests.java
+++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilitiesTests.java
@@ -51,7 +51,7 @@ import java.util.Arrays;
 import java.util.List;
 
 import static com.smartdevicelink.managers.screen.menu.BaseMenuManager.parentIdNotFound;
-import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.updateIdsOnMenuCells;
+import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.addIdsToMenuCells;
 import static junit.framework.TestCase.assertEquals;
 import static junit.framework.TestCase.assertFalse;
 import static junit.framework.TestCase.assertTrue;
@@ -306,7 +306,7 @@ public class MenuReplaceUtilitiesTests {
         MenuCell menuCell4 = new MenuCell("c4", subMenuLayout, sdlArtwork, new ArrayList<>(Arrays.asList(menuCell4_1, menuCell4_2)));
 
         List<MenuCell> menuCellList = new ArrayList<>(Arrays.asList(menuCell1, menuCell2, menuCell3, menuCell4));
-        updateIdsOnMenuCells(menuCellList, parentIdNotFound);
+        addIdsToMenuCells(menuCellList, parentIdNotFound);
 
         return menuCellList ;
     }
@@ -336,7 +336,7 @@ public class MenuReplaceUtilitiesTests {
         MenuCell menuCell5 = new MenuCell("c5", subMenuLayout, sdlArtwork, new ArrayList<>(Arrays.asList(menuCell5_1, menuCell5_2)));
 
         List<MenuCell> newMenuList = new ArrayList<>(Arrays.asList(menuCell5));
-        updateIdsOnMenuCells(newMenuList, parentIdNotFound);
+        addIdsToMenuCells(newMenuList, parentIdNotFound);
 
         return newMenuList ;
     }
diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java
index 964c999ec..42e4ac439 100644
--- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java
+++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java
@@ -182,6 +182,7 @@ abstract class BaseMenuManager extends BaseSubManager {
             public void onComplete(boolean success, List<MenuCell> currentMenuCells) {
                 BaseMenuManager.this.currentMenuCells = currentMenuCells;
                 updateMenuReplaceOperationsWithNewCurrentMenu();
+                DebugTool.logInfo(TAG, "Finished updating menu");
             }
         });
 
diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceOperation.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceOperation.java
index 51ada9d19..48beae8d1 100644
--- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceOperation.java
+++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceOperation.java
@@ -13,7 +13,9 @@ import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.posi
 import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.removeCellFromList;
 import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.sendRPCs;
 import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.subMenuCommandsForCells;
-import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.updateIdsOnMenuCells;
+import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.addIdsToMenuCells;
+import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.transferCellIDsFromCells;
+import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.transferCellListenersFromCells;
 
 import com.livio.taskmaster.Task;
 import com.smartdevicelink.managers.CompletionListener;
@@ -85,7 +87,7 @@ class MenuReplaceOperation extends Task {
     }
 
     private void updateMenuCells(final CompletionListener listener) {
-        updateIdsOnMenuCells(updatedMenu, parentIdNotFound);
+        addIdsToMenuCells(updatedMenu, parentIdNotFound);
 
         // Strip the "current menu" and the new menu of properties that are not displayed on the head unit
         List<MenuCell> updatedStrippedMenu = cellsWithRemovedPropertiesFromCells(updatedMenu, windowCapability);
@@ -125,12 +127,12 @@ class MenuReplaceOperation extends Task {
         final List<MenuCell> oldKeeps = filterMenuCellsWithStatusList(currentMenu, deleteMenuStatus, MenuCellState.KEEP);
         final List<MenuCell> newKeeps = filterMenuCellsWithStatusList(updatedMenu, addMenuStatus, MenuCellState.KEEP);
 
-        // Since we are creating a new menu but keeping old cells, we must first transfer the old cellIDs to the new menu kept cells.
-        // This is needed for the onCommands to still work
+        // Old kept cells ids need to be moved to the new kept cells so that submenu changes have correct parent ids
         // We will transfer the ids for subCells later
-        transferCellIDsFromOldCells(oldKeeps, newKeeps);
+        transferCellIDsFromCells(oldKeeps, newKeeps);
 
-        transferListenersFromNewCells(newKeeps, oldKeeps);
+        // Transfer new cells' listeners to the old cells, which are stored in the current menu
+        transferCellListenersFromCells(newKeeps, oldKeeps);
 
         // Upload the Artworks, then we will start updating the main menu
         uploadMenuArtworks(new CompletionListener() {
@@ -249,12 +251,12 @@ class MenuReplaceOperation extends Task {
             final List<MenuCell> cellsToDelete = filterMenuCellsWithStatusList(oldKeptCells.get(startIndex).getSubCells(), deleteMenuStatus, MenuCellState.DELETE);
             final List<MenuCell> cellsToAdd = filterMenuCellsWithStatusList(newKeptCells.get(startIndex).getSubCells(), addMenuStatus, MenuCellState.ADD);
 
-            final List<MenuCell> oldKeeps = filterMenuCellsWithStatusList(oldKeptCells.get(startIndex).getSubCells(), deleteMenuStatus, MenuCellState.KEEP);
-            final List<MenuCell> newKeeps = filterMenuCellsWithStatusList(newKeptCells.get(startIndex).getSubCells(), addMenuStatus, MenuCellState.KEEP);
+            final List<MenuCell> oldSubcellKeeps = filterMenuCellsWithStatusList(oldKeptCells.get(startIndex).getSubCells(), deleteMenuStatus, MenuCellState.KEEP);
+            final List<MenuCell> newSubcellKeeps = filterMenuCellsWithStatusList(newKeptCells.get(startIndex).getSubCells(), addMenuStatus, MenuCellState.KEEP);
 
-            transferCellIDsFromOldCells(oldKeeps, newKeeps);
+            transferCellIDsFromCells(oldSubcellKeeps, newSubcellKeeps);
 
-            transferListenersFromNewCells(newKeeps, oldKeeps);
+            transferCellListenersFromCells(newSubcellKeeps, oldSubcellKeeps);
 
             sendDeleteCurrentMenu(cellsToDelete, new CompletionListener() {
                 @Override
@@ -400,24 +402,6 @@ class MenuReplaceOperation extends Task {
         return filteredCells;
     }
 
-    private void transferCellIDsFromOldCells(List<MenuCell> oldCells, List<MenuCell> newCells) {
-        if (oldCells == null || oldCells.isEmpty()) {
-            return;
-        }
-        for (int i = 0; i < newCells.size(); i++) {
-            newCells.get(i).setCellId(oldCells.get(i).getCellId());
-        }
-    }
-
-    private void transferListenersFromNewCells(List<MenuCell> newCells, List<MenuCell> oldCells) {
-        if (oldCells == null || oldCells.isEmpty()) {
-            return;
-        }
-        for (int i = 0; i < newCells.size(); i++) {
-            oldCells.get(i).setMenuSelectionListener(newCells.get(i).getMenuSelectionListener());
-        }
-    }
-
     private String convertErrorsMapToString(Map<RPCRequest, String> errors) {
         if (errors == null) {
             return null;
diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilities.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilities.java
index 1c0546ad0..62ea8dc2e 100644
--- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilities.java
+++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilities.java
@@ -40,18 +40,47 @@ class MenuReplaceUtilities {
      * @param menuCells The array of menu cells to update
      * @param parentId The parent id to assign if needed
      */
-    static void updateIdsOnMenuCells(List<MenuCell> menuCells, int parentId) {
+    static void addIdsToMenuCells(List<MenuCell> menuCells, int parentId) {
         for (MenuCell cell : menuCells) {
             cell.setCellId(getNextMenuId());
             if (parentId != parentIdNotFound) {
                 cell.setParentCellId(parentId);
             }
             if (cell.isSubMenuCell() && !cell.getSubCells().isEmpty()) {
-                updateIdsOnMenuCells(cell.getSubCells(), cell.getCellId());
+                addIdsToMenuCells(cell.getSubCells(), cell.getCellId());
             }
         }
     }
 
+    static void transferCellIDsFromCells(List<MenuCell> fromCells, List<MenuCell> toCells) {
+        if (fromCells == null || toCells == null || fromCells.isEmpty() || fromCells.size() != toCells.size()) {
+            return;
+        }
+        for (int i = 0; i < toCells.size(); i++) {
+            toCells.get(i).setCellId(fromCells.get(i).getCellId());
+        }
+
+        // Update parent ids
+        for (MenuCell cell : toCells) {
+            if (!cell.isSubMenuCell()) {
+                continue;
+            }
+
+            for (MenuCell subCell : cell.getSubCells()) {
+                subCell.setParentCellId(cell.getCellId());
+            }
+        }
+    }
+
+    static void transferCellListenersFromCells(List<MenuCell> fromCells, List<MenuCell> toCells) {
+        if (fromCells == null || toCells == null || fromCells.isEmpty() || fromCells.size() != toCells.size()) {
+            return;
+        }
+        for (int i = 0; i < fromCells.size(); i++) {
+            toCells.get(i).setMenuSelectionListener(fromCells.get(i).getMenuSelectionListener());
+        }
+    }
+
     static List<SdlArtwork> findAllArtworksToBeUploadedFromCells(List<MenuCell> cells, FileManager fileManager, WindowCapability windowCapability) {
         // Make sure we can use images in the menus
         if (!hasImageFieldOfName(windowCapability, ImageFieldName.cmdIcon)) {