Fix PeopleHub UI population once visually full

Bug: 149925807
Test: manual, atest
Change-Id: I21d043c7493698838f41fba9039e85783b961c95

packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHub.kt

@@ -19,7 +19,13 @@ package com.android.systemui.statusbar.notification.people
 import android.app.PendingIntent
 import android.graphics.drawable.Drawable
 
-/** `ViewModel` for PeopleHub view. */
+/**
+ * `ViewModel` for PeopleHub view.
+ *
+ * @param people ViewModels for individual people in PeopleHub, in order that they should be
+ *  displayed
+ * @param isVisible Whether or not the whole PeopleHub UI is visible
+ **/
 data class PeopleHubViewModel(val people: Sequence<PersonViewModel>, val isVisible: Boolean)
 
 /** `ViewModel` for a single "Person' in PeopleHub. */
@@ -29,7 +35,11 @@ data class PersonViewModel(
     val onClick: () -> Unit
 )
 
-/** `Model` for PeopleHub. */
+/**
+ * `Model` for PeopleHub.
+ *
+ * @param people Models for individual people in PeopleHub, in order that they should be displayed
+ **/
 data class PeopleHubModel(val people: Collection<PersonModel>)
 
 /** `Model` for a single "Person" in PeopleHub. */

packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt

@@ -195,7 +195,10 @@ private fun NotificationLockscreenUserManager.registerListener(
 
 class PeopleHubManager {
 
+    // People currently visible in the notification shade, and so are not in the hub
     private val activePeople = mutableMapOf<PersonKey, PersonModel>()
+
+    // People that were once "active" and have been dismissed, and so can be displayed in the hub
     private val inactivePeople = ArrayDeque<PersonModel>(MAX_STORED_INACTIVE_PEOPLE)
 
     fun removeActivePerson(key: PersonKey): Boolean {
@@ -203,7 +206,7 @@ class PeopleHubManager {
             if (inactivePeople.size >= MAX_STORED_INACTIVE_PEOPLE) {
                 inactivePeople.removeLast()
             }
-            inactivePeople.add(data)
+            inactivePeople.addFirst(data)
             return true
         }
         return false

packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubViewController.kt

@@ -38,7 +38,14 @@ interface PeopleHubViewBoundary {
     /** View used for animating the activity launch caused by clicking a person in the hub. */
     val associatedViewForClickAnimation: View
 
-    /** [DataListener]s for individual people in the hub. */
+    /**
+     * [DataListener]s for individual people in the hub.
+     *
+     * These listeners should be ordered such that the first element will be bound to the most
+     * recent person to be added to the hub, and then continuing in descending order. If there are
+     * not enough people to satisfy each listener, `null` will be passed instead, indicating that
+     * the `View` should render a placeholder.
+     */
     val personViewAdapters: Sequence<DataListener<PersonViewModel?>>
 
     /** Sets the visibility of the Hub in the notification shade. */
@@ -80,8 +87,8 @@ private class PeopleHubDataListenerImpl(
         )
         viewBoundary.setVisible(viewModel.isVisible)
         val padded = viewModel.people + repeated(null)
-        for ((personAdapter, personModel) in viewBoundary.personViewAdapters.zip(padded)) {
-            personAdapter.onDataChanged(personModel)
+        for ((adapter, model) in viewBoundary.personViewAdapters.zip(padded)) {
+            adapter.onDataChanged(model)
         }
     }
 }

packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/PeopleHubView.kt

@@ -38,13 +38,13 @@ class PeopleHubView(context: Context, attrs: AttributeSet) :
     override fun onFinishInflate() {
         contents = requireViewById(R.id.people_list)
         personViewAdapters = (0 until contents.childCount)
-                .reversed()
-                .asSequence()
+                .asSequence() // so we can map
                 .mapNotNull { idx ->
+                    // get all our people slots
                     (contents.getChildAt(idx) as? ImageView)?.let(::PersonDataListenerImpl)
                 }
-                .toList()
-                .asSequence()
+                .toList() // cache it
+                .asSequence() // but don't reveal it's a list
         super.onFinishInflate()
         setVisible(true /* nowVisible */, false /* animate */)
     }
@@ -80,6 +80,7 @@ class PeopleHubView(context: Context, attrs: AttributeSet) :
             DataListener<PersonViewModel?> {
 
         override fun onDataChanged(data: PersonViewModel?) {
+            avatarView.visibility = data?.let { View.VISIBLE } ?: View.GONE
             avatarView.setImageDrawable(data?.icon)
             avatarView.setOnClickListener { data?.onClick?.invoke() }
         }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/people/PeopleFakes.kt

@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.notification.people
+
+object EmptySubscription : Subscription {
+    override fun unsubscribe() {}
+}
+
+class FakeDataSource<T>(
+    private val data: T,
+    private val subscription: Subscription = EmptySubscription
+) : DataSource<T> {
+    override fun registerListener(listener: DataListener<T>): Subscription {
+        listener.onDataChanged(data)
+        return subscription
+    }
+}

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/people/PeopleHubViewControllerTest.kt

@@ -30,12 +30,12 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
 import org.mockito.Mockito
-import org.mockito.Mockito.`when`
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.verify
 import org.mockito.junit.MockitoJUnit
 import org.mockito.junit.MockitoRule
 import kotlin.reflect.KClass
+import org.mockito.Mockito.`when` as whenever
 
 @SmallTest
 @RunWith(AndroidTestingRunner::class)
@@ -50,24 +50,19 @@ class PeopleHubViewControllerTest : SysuiTestCase() {
     fun testBindViewModelToViewBoundary() {
         val fakePerson1 = fakePersonViewModel("name")
         val fakeViewModel = PeopleHubViewModel(sequenceOf(fakePerson1), true)
+
+        val mockFactory = mock(PeopleHubViewModelFactory::class.java)
+        whenever(mockFactory.createWithAssociatedClickView(any())).thenReturn(fakeViewModel)
+
+        val mockClickView = mock(View::class.java)
+        whenever(mockViewBoundary.associatedViewForClickAnimation).thenReturn(mockClickView)
+
         val fakePersonViewAdapter1 = FakeDataListener<PersonViewModel?>()
         val fakePersonViewAdapter2 = FakeDataListener<PersonViewModel?>()
-        val mockClickView = mock(View::class.java)
-        `when`(mockViewBoundary.associatedViewForClickAnimation).thenReturn(mockClickView)
-        `when`(mockViewBoundary.personViewAdapters)
+        whenever(mockViewBoundary.personViewAdapters)
                 .thenReturn(sequenceOf(fakePersonViewAdapter1, fakePersonViewAdapter2))
-        val mockFactory = mock(PeopleHubViewModelFactory::class.java)
-        `when`(mockFactory.createWithAssociatedClickView(any())).thenReturn(fakeViewModel)
-        val mockSubscription = mock(Subscription::class.java)
-        val fakeFactoryDataSource = object : DataSource<PeopleHubViewModelFactory> {
-            override fun registerListener(
-                listener: DataListener<PeopleHubViewModelFactory>
-            ): Subscription {
-                listener.onDataChanged(mockFactory)
-                return mockSubscription
-            }
-        }
-        val adapter = PeopleHubViewAdapterImpl(fakeFactoryDataSource)
+
+        val adapter = PeopleHubViewAdapterImpl(FakeDataSource(mockFactory))
 
         adapter.bindView(mockViewBoundary)
 
@@ -77,24 +72,40 @@ class PeopleHubViewControllerTest : SysuiTestCase() {
         verify(mockFactory).createWithAssociatedClickView(mockClickView)
     }
 
+    @Test
+    fun testBindViewModelToViewBoundary_moreDataThanCanBeDisplayed_displaysMostRecent() {
+        val fakePerson1 = fakePersonViewModel("person1")
+        val fakePerson2 = fakePersonViewModel("person2")
+        val fakePerson3 = fakePersonViewModel("person3")
+        val fakePeople = sequenceOf(fakePerson3, fakePerson2, fakePerson1)
+        val fakeViewModel = PeopleHubViewModel(fakePeople, true)
+
+        val mockFactory = mock(PeopleHubViewModelFactory::class.java)
+        whenever(mockFactory.createWithAssociatedClickView(any())).thenReturn(fakeViewModel)
+
+        whenever(mockViewBoundary.associatedViewForClickAnimation)
+                .thenReturn(mock(View::class.java))
+
+        val fakePersonViewAdapter1 = FakeDataListener<PersonViewModel?>()
+        val fakePersonViewAdapter2 = FakeDataListener<PersonViewModel?>()
+        whenever(mockViewBoundary.personViewAdapters)
+                .thenReturn(sequenceOf(fakePersonViewAdapter1, fakePersonViewAdapter2))
+
+        val adapter = PeopleHubViewAdapterImpl(FakeDataSource(mockFactory))
+
+        adapter.bindView(mockViewBoundary)
+
+        assertThat(fakePersonViewAdapter1.lastSeen).isEqualTo(Maybe.Just(fakePerson3))
+        assertThat(fakePersonViewAdapter2.lastSeen).isEqualTo(Maybe.Just(fakePerson2))
+    }
+
     @Test
     fun testViewModelDataSourceTransformsModel() {
         val fakeClickIntent = PendingIntent.getActivity(context, 0, Intent("action"), 0)
         val fakePerson = fakePersonModel("id", "name", fakeClickIntent)
         val fakeModel = PeopleHubModel(listOf(fakePerson))
-        val mockSubscription = mock(Subscription::class.java)
-        val fakeModelDataSource = object : DataSource<PeopleHubModel> {
-            override fun registerListener(listener: DataListener<PeopleHubModel>): Subscription {
-                listener.onDataChanged(fakeModel)
-                return mockSubscription
-            }
-        }
-        val fakeSettingDataSource = object : DataSource<Boolean> {
-            override fun registerListener(listener: DataListener<Boolean>): Subscription {
-                listener.onDataChanged(true)
-                return mockSubscription
-            }
-        }
+        val fakeModelDataSource = FakeDataSource(fakeModel)
+        val fakeSettingDataSource = FakeDataSource(true)
         val factoryDataSource = PeopleHubViewModelFactoryDataSourceImpl(
                 mockActivityStarter,
                 fakeModelDataSource,
@@ -127,19 +138,8 @@ class PeopleHubViewControllerTest : SysuiTestCase() {
         val fakeClickIntent = PendingIntent.getActivity(context, 0, Intent("action"), 0)
         val fakePerson = fakePersonModel("id", "name", fakeClickIntent)
         val fakeModel = PeopleHubModel(listOf(fakePerson))
-        val mockSubscription = mock(Subscription::class.java)
-        val fakeModelDataSource = object : DataSource<PeopleHubModel> {
-            override fun registerListener(listener: DataListener<PeopleHubModel>): Subscription {
-                listener.onDataChanged(fakeModel)
-                return mockSubscription
-            }
-        }
-        val fakeSettingDataSource = object : DataSource<Boolean> {
-            override fun registerListener(listener: DataListener<Boolean>): Subscription {
-                listener.onDataChanged(false)
-                return mockSubscription
-            }
-        }
+        val fakeModelDataSource = FakeDataSource(fakeModel)
+        val fakeSettingDataSource = FakeDataSource(false)
         val factoryDataSource = PeopleHubViewModelFactoryDataSourceImpl(
                 mockActivityStarter,
                 fakeModelDataSource,