Merge "Ensure important people conversations appear above their unimportant siblings."
This commit is contained in:
committed by
Android (Google) Code Review
commit
2a736f643e
@@ -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"
|
||||
|
||||
@@ -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<Coordinator> = ArrayList()
|
||||
private val mOrderedSections: MutableList<NotifSectioner> = ArrayList()
|
||||
private val mOrderedComparators: MutableList<NotifComparator> = 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<String>) {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user