Merge "Fix ranking of people notifications and peoplehub"

5 files changed, 60 insertions, 100 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt
index 8bce528bab8c..48a4882bcf82 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt
@@ -16,37 +16,29 @@
 
 package com.android.systemui.statusbar.notification.collection
 
-import android.app.Notification
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
 import android.app.NotificationManager.IMPORTANCE_HIGH
 import android.app.NotificationManager.IMPORTANCE_LOW
 import android.app.NotificationManager.IMPORTANCE_MIN
-import android.app.Person
 import android.service.notification.NotificationListenerService.Ranking
 import android.service.notification.NotificationListenerService.RankingMap
 import android.service.notification.StatusBarNotification
 import com.android.internal.annotations.VisibleForTesting
-
 import com.android.systemui.statusbar.NotificationMediaManager
 import com.android.systemui.statusbar.notification.NotificationFilter
 import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager
 import com.android.systemui.statusbar.notification.logging.NotifEvent
 import com.android.systemui.statusbar.notification.logging.NotifLog
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_PEOPLE
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_SILENT
 import com.android.systemui.statusbar.phone.NotificationGroupManager
 import com.android.systemui.statusbar.policy.HeadsUpManager
-
+import dagger.Lazy
 import java.util.Objects
-import java.util.ArrayList
-
 import javax.inject.Inject
 
-import kotlin.Comparator
-
-import dagger.Lazy
-
 private const val TAG = "NotifRankingManager"
 
 /**
@@ -64,7 +56,8 @@ open class NotificationRankingManager @Inject constructor(
     private val headsUpManager: HeadsUpManager,
     private val notifFilter: NotificationFilter,
     private val notifLog: NotifLog,
-    sectionsFeatureManager: NotificationSectionsFeatureManager
+    sectionsFeatureManager: NotificationSectionsFeatureManager,
+    private val peopleNotificationIdentifier: PeopleNotificationIdentifier
 ) {
 
     var rankingMap: RankingMap? = null
@@ -79,6 +72,9 @@ open class NotificationRankingManager @Inject constructor(
         val aRank = a.ranking.rank
         val bRank = b.ranking.rank
 
+        val aIsPeople = a.isPeopleNotification()
+        val bIsPeople = b.isPeopleNotification()
+
         val aMedia = isImportantMedia(a)
         val bMedia = isImportantMedia(b)
 
@@ -88,25 +84,19 @@ open class NotificationRankingManager @Inject constructor(
         val aHeadsUp = a.isRowHeadsUp
         val bHeadsUp = b.isRowHeadsUp
 
-        if (usePeopleFiltering && a.isPeopleNotification() != b.isPeopleNotification()) {
-            if (a.isPeopleNotification()) -1 else 1
-        } else if (aHeadsUp != bHeadsUp) {
-            if (aHeadsUp) -1 else 1
-        } else if (aHeadsUp) {
+        when {
+            usePeopleFiltering && aIsPeople != bIsPeople -> if (aIsPeople) -1 else 1
+            aHeadsUp != bHeadsUp -> if (aHeadsUp) -1 else 1
             // Provide consistent ranking with headsUpManager
-            headsUpManager.compare(a, b)
-        } else if (aMedia != bMedia) {
+            aHeadsUp -> headsUpManager.compare(a, b)
             // Upsort current media notification.
-            if (aMedia) -1 else 1
-        } else if (aSystemMax != bSystemMax) {
+            aMedia != bMedia -> if (aMedia) -1 else 1
             // Upsort PRIORITY_MAX system notifications
-            if (aSystemMax) -1 else 1
-        } else if (a.isHighPriority != b.isHighPriority) {
-            -1 * java.lang.Boolean.compare(a.isHighPriority, b.isHighPriority)
-        } else if (aRank != bRank) {
-            aRank - bRank
-        } else {
-            nb.notification.`when`.compareTo(na.notification.`when`)
+            aSystemMax != bSystemMax -> if (aSystemMax) -1 else 1
+            a.isHighPriority != b.isHighPriority ->
+                -1 * a.isHighPriority.compareTo(b.isHighPriority)
+            aRank != bRank -> aRank - bRank
+            else -> nb.notification.`when`.compareTo(na.notification.`when`)
         }
     }
 
@@ -138,10 +128,9 @@ open class NotificationRankingManager @Inject constructor(
         val c = entry.channel
         val n = entry.sbn.notification
 
-        if (((n.isForegroundService && entry.ranking.importance >= IMPORTANCE_LOW) ||
-            n.hasMediaSession() ||
-            n.hasPerson() ||
-            n.hasStyle(Notification.MessagingStyle::class.java))) {
+        if ((n.isForegroundService && entry.ranking.importance >= IMPORTANCE_LOW) ||
+                n.hasMediaSession() ||
+                entry.isPeopleNotification()) {
             // Users who have long pressed and demoted to silent should not see the notification
             // in the top section
             if (c != null && c.hasUserSetImportance()) {
@@ -204,7 +193,7 @@ open class NotificationRankingManager @Inject constructor(
         isMedia: Boolean,
         isSystemMax: Boolean
     ) {
-        if (usePeopleFiltering && entry.hasAssociatedPeople()) {
+        if (usePeopleFiltering && entry.isPeopleNotification()) {
             entry.bucket = BUCKET_PEOPLE
         } else if (isHeadsUp || isMedia || isSystemMax || entry.isHighPriority) {
             entry.bucket = BUCKET_ALERTING
@@ -235,6 +224,11 @@ open class NotificationRankingManager @Inject constructor(
             }
         }
     }
+
+    private fun NotificationEntry.isPeopleNotification() =
+            sbn.isPeopleNotification()
+    private fun StatusBarNotification.isPeopleNotification() =
+            peopleNotificationIdentifier.isPeopleNotification(this)
 }
 
 // Convenience functions
@@ -245,16 +239,3 @@ private fun NotificationEntry.isSystemMax(): Boolean {
 private fun StatusBarNotification.isSystemNotification(): Boolean {
     return "android" == packageName || "com.android.systemui" == packageName
 }
-
-private fun Notification.hasPerson(): Boolean {
-    val people: ArrayList<Person> =
-            (extras?.getParcelableArrayList(Notification.EXTRA_PEOPLE_LIST)) ?: ArrayList()
-    return people.isNotEmpty()
-}
-
-private fun Notification.hasStyle(targetStyleClass: Class<*>): Boolean {
-    return targetStyleClass == notificationStyle
-}
-
-private fun NotificationEntry.isPeopleNotification(): Boolean =
-        sbn.notification.hasStyle(Notification.MessagingStyle::class.java)
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
index fdd51e9e7790..8e9a051e7d43 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
@@ -318,7 +318,7 @@ public class NotificationSectionsManager implements StackScrollAlgorithm.Section
                 }
                 mParent.addView(mPeopleHubView, targetIndex);
                 return true;
-            } else if (currentHubIndex != targetIndex - 1) {
+            } else if (currentHubIndex != targetIndex) {
                 if (currentHubIndex < targetIndex) {
                     targetIndex--;
                 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java
index a98945f49b99..e0dfe7e66ad5 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java
@@ -85,6 +85,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationRowBin
 import com.android.systemui.statusbar.notification.collection.NotificationRowBinderImpl;
 import com.android.systemui.statusbar.notification.logging.NotifLog;
 import com.android.systemui.statusbar.notification.logging.NotificationLogger;
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
 import com.android.systemui.statusbar.notification.row.NotificationGutsManager;
 import com.android.systemui.statusbar.notification.row.RowInflaterTask;
@@ -228,7 +229,8 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                         mHeadsUpManager,
                         mock(NotificationFilter.class),
                         mNotifLog,
-                        mock(NotificationSectionsFeatureManager.class)),
+                        mock(NotificationSectionsFeatureManager.class),
+                        mock(PeopleNotificationIdentifier.class)),
                 mEnvironment
         );
         Dependency.get(InitController.class).executePostInitTasks();
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
index 01b2f8932d9b..cda1538e899d 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
@@ -20,7 +20,6 @@ import android.app.Notification
 import android.app.NotificationChannel
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
 import android.app.NotificationManager.IMPORTANCE_LOW
-import android.app.Person
 import android.service.notification.NotificationListenerService.RankingMap
 import android.service.notification.StatusBarNotification
 import android.testing.AndroidTestingRunner
@@ -36,6 +35,7 @@ import com.android.systemui.statusbar.NotificationMediaManager
 import com.android.systemui.statusbar.notification.NotificationFilter
 import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager
 import com.android.systemui.statusbar.notification.logging.NotifLog
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_SILENT
 import com.android.systemui.statusbar.phone.NotificationGroupManager
@@ -52,62 +52,41 @@ import org.mockito.Mockito.mock
 
 @SmallTest
 @RunWith(AndroidTestingRunner::class)
-class NotificationRankingManagerTest
-    : SysuiTestCase() {
+class NotificationRankingManagerTest : SysuiTestCase() {
 
-    private var lazyMedia: Lazy<NotificationMediaManager> = Lazy {
-        mock<NotificationMediaManager>(NotificationMediaManager::class.java)
+    private val lazyMedia: Lazy<NotificationMediaManager> = Lazy {
+        mock(NotificationMediaManager::class.java)
     }
-
-    private val rankingManager = TestableNotificationRankingManager(
-            lazyMedia,
-            mock<NotificationGroupManager>(NotificationGroupManager::class.java),
-            mock<HeadsUpManager>(HeadsUpManager::class.java),
-            mock<NotificationFilter>(NotificationFilter::class.java),
-            mock<NotifLog>(NotifLog::class.java),
-            mock<NotificationSectionsFeatureManager>(NotificationSectionsFeatureManager::class.java)
-    )
+    private lateinit var personNotificationIdentifier: PeopleNotificationIdentifier
+    private lateinit var rankingManager: TestableNotificationRankingManager
 
     @Before
     fun setup() {
+        personNotificationIdentifier =
+                mock(PeopleNotificationIdentifier::class.java)
+        rankingManager = TestableNotificationRankingManager(
+                lazyMedia,
+                mock(NotificationGroupManager::class.java),
+                mock(HeadsUpManager::class.java),
+                mock(NotificationFilter::class.java),
+                mock(NotifLog::class.java),
+                mock(NotificationSectionsFeatureManager::class.java),
+                personNotificationIdentifier
+        )
     }
 
     @Test
     fun testPeopleNotification_isHighPriority() {
-        val person = Person.Builder()
-                .setName("name")
-                .setKey("abc")
-                .setUri("uri")
-                .setBot(true)
-                .build()
-
         val notification = Notification.Builder(mContext, "test")
-                .addPerson(person)
                 .build()
 
         val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
                 notification, mContext.user, "", 0)
 
-        val e = NotificationEntryBuilder()
-                .setNotification(notification)
-                .setSbn(sbn)
-                .build()
-
-        assertTrue(rankingManager.isHighPriority2(e))
-    }
-
-    @Test
-    fun messagingStyleHighPriority() {
-
-        val notif = Notification.Builder(mContext, "test")
-                .setStyle(Notification.MessagingStyle(""))
-                .build()
-
-        val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
-                notif, mContext.getUser(), "", 0)
+        `when`(personNotificationIdentifier.isPeopleNotification(sbn)).thenReturn(true)
 
         val e = NotificationEntryBuilder()
-                .setNotification(notif)
+                .setNotification(notification)
                 .setSbn(sbn)
                 .build()
 
@@ -117,7 +96,7 @@ class NotificationRankingManagerTest
     @Test
     fun lowForegroundHighPriority() {
         val notification = mock(Notification::class.java)
-        `when`<Boolean>(notification.isForegroundService).thenReturn(true)
+        `when`(notification.isForegroundService).thenReturn(true)
 
         val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
                 notification, mContext.user, "", 0)
@@ -136,15 +115,7 @@ class NotificationRankingManagerTest
 
     @Test
     fun userChangeTrumpsHighPriorityCharacteristics() {
-        val person = Person.Builder()
-                .setName("name")
-                .setKey("abc")
-                .setUri("uri")
-                .setBot(true)
-                .build()
-
         val notification = Notification.Builder(mContext, "test")
-                .addPerson(person)
                 .setStyle(Notification.MessagingStyle(""))
                 .setFlag(Notification.FLAG_FOREGROUND_SERVICE, true)
                 .build()
@@ -152,6 +123,8 @@ class NotificationRankingManagerTest
         val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
                 notification, mContext.user, "", 0)
 
+        `when`(personNotificationIdentifier.isPeopleNotification(sbn)).thenReturn(true)
+
         val channel = NotificationChannel("a", "a", IMPORTANCE_LOW)
         channel.lockFields(NotificationChannel.USER_LOCKED_IMPORTANCE)
 
@@ -297,14 +270,16 @@ class NotificationRankingManagerTest
         headsUpManager: HeadsUpManager,
         filter: NotificationFilter,
         notifLog: NotifLog,
-        sectionsFeatureManager: NotificationSectionsFeatureManager
+        sectionsFeatureManager: NotificationSectionsFeatureManager,
+        peopleNotificationIdentifier: PeopleNotificationIdentifier
     ) : NotificationRankingManager(
         mediaManager,
         groupManager,
         headsUpManager,
         filter,
         notifLog,
-        sectionsFeatureManager
+        sectionsFeatureManager,
+        peopleNotificationIdentifier
     ) {
 
         fun isHighPriority2(e: NotificationEntry): Boolean {
@@ -315,4 +290,4 @@ class NotificationRankingManagerTest
             rankingMap = r
         }
     }
-}
\ No newline at end of file
+}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
index d20a37a18d2e..d2b4a20ca3a9 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
@@ -74,6 +74,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 import com.android.systemui.statusbar.notification.collection.NotificationRankingManager;
 import com.android.systemui.statusbar.notification.logging.NotifLog;
 import com.android.systemui.statusbar.notification.people.PeopleHubSectionFooterViewAdapter;
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
 import com.android.systemui.statusbar.notification.row.FooterView;
 import com.android.systemui.statusbar.notification.row.NotificationBlockingHelperManager;
@@ -164,7 +165,8 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
                         mHeadsUpManager,
                         mock(NotificationFilter.class),
                         mock(NotifLog.class),
-                        mock(NotificationSectionsFeatureManager.class)
+                        mock(NotificationSectionsFeatureManager.class),
+                        mock(PeopleNotificationIdentifier.class)
                 ),
                 mock(NotificationEntryManager.KeyguardEnvironment.class));
         mDependency.injectTestDependency(NotificationEntryManager.class, mEntryManager);