Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 137fadc8 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Ensure important people conversations appear above their unimportant siblings. 

Fixes: 215584507
Test: atest ConversationCoordinatorTest
Change-Id: I23be1854076a18b94c9d200399c56af1471581d6
parent feb567fa

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt

+23 −3
Original line number Diff line number Diff line
@@ -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"

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt

+6 −0
Original line number Diff line number Diff line
@@ -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>) {

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifComparator.java

+3 −1
Original line number Diff line number Diff line
@@ -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);

}

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java

+1 −1
Original line number Diff line number Diff line
@@ -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(

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt

+48 −0
Original line number Diff line number Diff line
@@ -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)

    }

}