diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt index e59f4a62f9b79..ba88ad7844f1d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt @@ -20,11 +20,13 @@ import com.android.systemui.statusbar.notification.collection.ListEntry import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner import com.android.systemui.statusbar.notification.collection.render.NodeController import com.android.systemui.statusbar.notification.dagger.PeopleHeader import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier +import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.PeopleNotificationType import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_NON_PERSON import com.android.systemui.statusbar.notification.stack.BUCKET_PEOPLE import javax.inject.Inject @@ -48,18 +50,36 @@ class ConversationCoordinator @Inject constructor( val sectioner = object : NotifSectioner("People", BUCKET_PEOPLE) { override fun isInSection(entry: ListEntry): Boolean = - isConversation(entry.representativeEntry!!) + isConversation(entry) override fun getHeaderNodeController() = // TODO: remove SHOW_ALL_SECTIONS, this redundant method, and peopleHeaderController if (RankingCoordinator.SHOW_ALL_SECTIONS) peopleHeaderController else null } + val comparator = object : NotifComparator("People") { + override fun compare(entry1: ListEntry, entry2: ListEntry): Int { + assert(entry1.section === entry2.section) + if (entry1.section?.sectioner !== sectioner) { + return 0 + } + val type1 = getPeopleType(entry1) + val type2 = getPeopleType(entry2) + return type2.compareTo(type1) + } + } + override fun attach(pipeline: NotifPipeline) { pipeline.addPromoter(notificationPromoter) } - private fun isConversation(entry: NotificationEntry): Boolean = - peopleNotificationIdentifier.getPeopleNotificationType(entry) != TYPE_NON_PERSON + private fun isConversation(entry: ListEntry): Boolean = + getPeopleType(entry) != TYPE_NON_PERSON + + @PeopleNotificationType + private fun getPeopleType(entry: ListEntry): Int = + entry.representativeEntry?.let { + peopleNotificationIdentifier.getPeopleNotificationType(it) + } ?: TYPE_NON_PERSON companion object { private const val TAG = "ConversationCoordinator" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt index 757fb5a2fe9a3..850cb4b881545 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt @@ -20,6 +20,7 @@ import com.android.systemui.dump.DumpManager import com.android.systemui.statusbar.notification.NotifPipelineFlags import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner import java.io.FileDescriptor import java.io.PrintWriter @@ -63,6 +64,7 @@ class NotifCoordinatorsImpl @Inject constructor( private val mCoordinators: MutableList = ArrayList() private val mOrderedSections: MutableList = ArrayList() + private val mOrderedComparators: MutableList = ArrayList() /** * Creates all the coordinators. @@ -117,6 +119,9 @@ class NotifCoordinatorsImpl @Inject constructor( mOrderedSections.add(rankingCoordinator.alertingSectioner) // Alerting mOrderedSections.add(rankingCoordinator.silentSectioner) // Silent mOrderedSections.add(rankingCoordinator.minimizedSectioner) // Minimized + + // Manually add ordered comparators + mOrderedComparators.add(conversationCoordinator.comparator) } /** @@ -128,6 +133,7 @@ class NotifCoordinatorsImpl @Inject constructor( c.attach(pipeline) } pipeline.setSections(mOrderedSections) + pipeline.setComparators(mOrderedComparators) } override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifComparator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifComparator.java index 0d150edee1287..f7bbd281ec51a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifComparator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifComparator.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.notification.collection.listbuilder.pluggable; +import androidx.annotation.NonNull; + import com.android.systemui.statusbar.notification.collection.ListEntry; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -39,5 +41,5 @@ public abstract class NotifComparator * @return a negative integer, zero, or a positive integer as the first argument is less than * equal to, or greater than the second (same as standard Comparator<> interface). */ - public abstract int compare(ListEntry o1, ListEntry o2); + public abstract int compare(@NonNull ListEntry o1, @NonNull ListEntry o2); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index b832577c16ac3..25dd23a955e6d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -1968,7 +1968,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { } @Override - public int compare(ListEntry o1, ListEntry o2) { + public int compare(@NonNull ListEntry o1, @NonNull ListEntry o2) { boolean contains1 = mPreferredPackages.contains( o1.getRepresentativeEntry().getSbn().getPackageName()); boolean contains2 = mPreferredPackages.contains( diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt index a46b440028129..8deac94214bdd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt @@ -24,18 +24,24 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder +import com.android.systemui.statusbar.notification.collection.listbuilder.NotifSection +import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner import com.android.systemui.statusbar.notification.collection.render.NodeController import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier +import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_IMPORTANT_PERSON import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_PERSON +import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.withArgCaptor +import com.google.common.truth.Truth.assertThat import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock +import org.mockito.Mockito.never import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations import org.mockito.Mockito.`when` as whenever @@ -47,12 +53,15 @@ class ConversationCoordinatorTest : SysuiTestCase() { // captured listeners and pluggables: private lateinit var promoter: NotifPromoter private lateinit var peopleSectioner: NotifSectioner + private lateinit var peopleComparator: NotifComparator @Mock private lateinit var pipeline: NotifPipeline @Mock private lateinit var peopleNotificationIdentifier: PeopleNotificationIdentifier @Mock private lateinit var channel: NotificationChannel @Mock private lateinit var headerController: NodeController private lateinit var entry: NotificationEntry + private lateinit var entryA: NotificationEntry + private lateinit var entryB: NotificationEntry private lateinit var coordinator: ConversationCoordinator @@ -70,8 +79,15 @@ class ConversationCoordinatorTest : SysuiTestCase() { } peopleSectioner = coordinator.sectioner + peopleComparator = coordinator.comparator entry = NotificationEntryBuilder().setChannel(channel).build() + + val section = NotifSection(peopleSectioner, 0) + entryA = NotificationEntryBuilder().setChannel(channel) + .setSection(section).setTag("A").build() + entryB = NotificationEntryBuilder().setChannel(channel) + .setSection(section).setTag("B").build() } @Test @@ -90,4 +106,36 @@ class ConversationCoordinatorTest : SysuiTestCase() { assertTrue(peopleSectioner.isInSection(entry)) assertFalse(peopleSectioner.isInSection(NotificationEntryBuilder().build())) } + + @Test + fun testComparatorIgnoresFromOtherSection() { + val e1 = NotificationEntryBuilder().setId(1).setChannel(channel).build() + val e2 = NotificationEntryBuilder().setId(2).setChannel(channel).build() + + // wrong section -- never classify + assertThat(peopleComparator.compare(e1, e2)).isEqualTo(0) + verify(peopleNotificationIdentifier, never()).getPeopleNotificationType(any()) + } + + @Test + fun testComparatorPutsImportantPeopleFirst() { + whenever(peopleNotificationIdentifier.getPeopleNotificationType(entryA)) + .thenReturn(TYPE_IMPORTANT_PERSON) + whenever(peopleNotificationIdentifier.getPeopleNotificationType(entryB)) + .thenReturn(TYPE_PERSON) + + // only put people notifications in this section + assertThat(peopleComparator.compare(entryA, entryB)).isEqualTo(-1) + } + + @Test + fun testComparatorEquatesPeopleWithSameType() { + whenever(peopleNotificationIdentifier.getPeopleNotificationType(entryA)) + .thenReturn(TYPE_PERSON) + whenever(peopleNotificationIdentifier.getPeopleNotificationType(entryB)) + .thenReturn(TYPE_PERSON) + + // only put people notifications in this section + assertThat(peopleComparator.compare(entryA, entryB)).isEqualTo(0) + } }