From 4dd5de226fefd5c2fdf040543157357f53ea932e Mon Sep 17 00:00:00 2001
From: Maurice Lam <yukl@google.com>
Date: Mon, 27 Mar 2017 16:24:08 -0700
Subject: [PATCH] Notify the correct position when removing the last item

Change ItemGroup#getChildPosition to return the position even for
empty childrem, so that when removing the last item in a nested
ItemGroup, the notification propagation will still be correct.

Test: ./gradlew connectedAndroidTest test
Bug: 36634677
Change-Id: I612e0c624dabee1bfaa6133fe976527e85523634
---
 .../setupwizardlib/items/ItemGroup.java       | 12 ++++-
 .../setupwizardlib/items/ItemGroupTest.java   | 46 +++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/library/main/src/com/android/setupwizardlib/items/ItemGroup.java b/library/main/src/com/android/setupwizardlib/items/ItemGroup.java
index d645350..ac643e9 100644
--- a/library/main/src/com/android/setupwizardlib/items/ItemGroup.java
+++ b/library/main/src/com/android/setupwizardlib/items/ItemGroup.java
@@ -193,7 +193,8 @@ public class ItemGroup extends AbstractItemHierarchy implements ItemInflater.Ite
     }
 
     /**
-     * @return The "Item Position" of the given child, or -1 if the child is empty or not found.
+     * @return The "Item Position" of the given child, or -1 if the child is not found. If the given
+     *         child is empty, position of the next visible item is returned.
      */
     private int getChildPosition(ItemHierarchy child) {
         // Check the identity of the child rather than using .equals(), because here we want
@@ -204,7 +205,14 @@ public class ItemGroup extends AbstractItemHierarchy implements ItemInflater.Ite
     private int getChildPosition(int childIndex) {
         updateDataIfNeeded();
         if (childIndex != -1) {
-            return mHierarchyStart.get(childIndex, -1);
+            int childPos = -1;
+            int childCount = mChildren.size();
+            for (int i = childIndex; childPos < 0 && i < childCount; i++) {
+                // Find the position of the first visible child after childIndex. This is required
+                // when removing the last item from a nested ItemGroup.
+                childPos = mHierarchyStart.get(i, -1);
+            }
+            return childPos;
         }
         return -1;
     }
diff --git a/library/test/robotest/src/com/android/setupwizardlib/items/ItemGroupTest.java b/library/test/robotest/src/com/android/setupwizardlib/items/ItemGroupTest.java
index 73597c7..e0e7e8d 100644
--- a/library/test/robotest/src/com/android/setupwizardlib/items/ItemGroupTest.java
+++ b/library/test/robotest/src/com/android/setupwizardlib/items/ItemGroupTest.java
@@ -123,6 +123,52 @@ public class ItemGroupTest {
         verifyNoMoreInteractions(mObserver);
     }
 
+    @Test
+    public void testNestedGroupClearNotification() {
+        ItemGroup parentGroup = new ItemGroup();
+        ItemGroup childGroup = new ItemGroup();
+        parentGroup.registerObserver(mObserver);
+
+        parentGroup.addChild(CHILD_1);
+        childGroup.addChild(CHILD_2);
+        childGroup.addChild(CHILD_3);
+        parentGroup.addChild(childGroup);
+        parentGroup.addChild(CHILD_4);
+
+        childGroup.clear();
+
+        final InOrder inOrder = inOrder(mObserver);
+        inOrder.verify(mObserver).onItemRangeInserted(eq(parentGroup), eq(0), eq(1));
+        inOrder.verify(mObserver).onItemRangeInserted(eq(parentGroup), eq(1), eq(2));
+        inOrder.verify(mObserver).onItemRangeInserted(eq(parentGroup), eq(3), eq(1));
+        verify(mObserver).onItemRangeRemoved(eq(parentGroup), eq(1), eq(2));
+        verifyNoMoreInteractions(mObserver);
+    }
+
+    @Test
+    public void testNestedGroupRemoveNotification() {
+        ItemGroup parentGroup = new ItemGroup();
+        ItemGroup childGroup = new ItemGroup();
+        parentGroup.registerObserver(mObserver);
+
+        parentGroup.addChild(CHILD_1);
+        childGroup.addChild(CHILD_2);
+        childGroup.addChild(CHILD_3);
+        parentGroup.addChild(childGroup);
+        parentGroup.addChild(CHILD_4);
+
+        childGroup.removeChild(CHILD_3);
+        childGroup.removeChild(CHILD_2);
+
+        final InOrder inOrder = inOrder(mObserver);
+        inOrder.verify(mObserver).onItemRangeInserted(eq(parentGroup), eq(0), eq(1));
+        inOrder.verify(mObserver).onItemRangeInserted(eq(parentGroup), eq(1), eq(2));
+        inOrder.verify(mObserver).onItemRangeInserted(eq(parentGroup), eq(3), eq(1));
+        inOrder.verify(mObserver).onItemRangeRemoved(eq(parentGroup), eq(2), eq(1));
+        inOrder.verify(mObserver).onItemRangeRemoved(eq(parentGroup), eq(1), eq(1));
+        verifyNoMoreInteractions(mObserver);
+    }
+
     @Test
     public void testNotifyChange() {
         mItemGroup.addChild(CHILD_1);