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

Commit bf9a93a5 authored by Ioana Alexandru's avatar Ioana Alexandru
Browse files

Make GroupMembershipManager implementation match docs. 

Also added nullability annotations to better surface NPEs. Made all the
entries passed to methods @NonNull since the methods would've crashed
if given a null entry anyway. Eventually we might consider moving these
methods directly to ListEntry.,

Fix: 293434635
Test: GroupMembershipManagerTest
Flag: NOTIFICATION_GROUP_EXPANSION_CHANGE
Change-Id: I53754d6dbb0b82d3d1a4c4572d79066d41ab017e
parent 6cac1114

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManager.java

+9 −6
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.statusbar.notification.collection.render;



import android.annotation.NonNull;

import android.annotation.Nullable;



import com.android.systemui.statusbar.notification.collection.ListEntry;
@@ -31,13 +32,14 @@ public interface GroupMembershipManager { 
     * @return whether a given notification is a top level entry or is the summary in a group which

     * has children

     */

    boolean isGroupSummary(NotificationEntry entry);

    boolean isGroupSummary(@NonNull NotificationEntry entry);



    /**

     * Get the summary of a specified status bar notification. For an isolated notification this

     * returns itself.

     */

    NotificationEntry getGroupSummary(NotificationEntry entry);

    @Nullable

    NotificationEntry getGroupSummary(@NonNull NotificationEntry entry);



    /**

     * Similar to {@link #getGroupSummary(NotificationEntry)} but doesn't get the visual summary
@@ -46,19 +48,20 @@ public interface GroupMembershipManager { 
     * TODO: remove this when migrating to the new pipeline, this is taken care of in the

     * dismissal logic built into NotifCollection

     */

    default NotificationEntry getLogicalGroupSummary(NotificationEntry entry) {

    @Nullable

    default NotificationEntry getLogicalGroupSummary(@NonNull NotificationEntry entry) {

        return getGroupSummary(entry);

    }



    /**

     * @return whether a given notification is a child in a group

     */

    boolean isChildInGroup(NotificationEntry entry);

    boolean isChildInGroup(@NonNull NotificationEntry entry);



    /**

     * Whether this is the only child in a group

     */

    boolean isOnlyChildInGroup(NotificationEntry entry);

    boolean isOnlyChildInGroup(@NonNull NotificationEntry entry);



    /**

     * Get the children that are in the summary's group, not including those isolated.
@@ -67,5 +70,5 @@ public interface GroupMembershipManager { 
     * @return list of the children

     */

    @Nullable

    List<NotificationEntry> getChildren(ListEntry summary);

    List<NotificationEntry> getChildren(@NonNull ListEntry summary);

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java

+40 −11
Original line number Diff line number Diff line
@@ -18,40 +18,65 @@ package com.android.systemui.statusbar.notification.collection.render; 


import static com.android.systemui.statusbar.notification.collection.GroupEntry.ROOT_ENTRY;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;



import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.flags.FeatureFlags;

import com.android.systemui.flags.Flags;

import com.android.systemui.statusbar.notification.collection.GroupEntry;

import com.android.systemui.statusbar.notification.collection.ListEntry;

import com.android.systemui.statusbar.notification.collection.NotificationEntry;



import java.util.List;



import javax.inject.Inject;



/**

 * ShadeListBuilder groups notifications from system server. This manager translates

 * ShadeListBuilder's method of grouping to be used within SystemUI.

 */

@SysUISingleton

public class GroupMembershipManagerImpl implements GroupMembershipManager {

    FeatureFlags mFeatureFlags;



    @Inject

    public GroupMembershipManagerImpl(FeatureFlags featureFlags) {

        mFeatureFlags = featureFlags;

    }



    @Override

    public boolean isGroupSummary(NotificationEntry entry) {

    public boolean isGroupSummary(@NonNull NotificationEntry entry) {

        return getGroupSummary(entry) == entry;

    }



    @Nullable

    @Override

    public NotificationEntry getGroupSummary(NotificationEntry entry) {

    public NotificationEntry getGroupSummary(@NonNull NotificationEntry entry) {

        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {

            if (!isChildInGroup(entry)) {

                return entry.getRepresentativeEntry();

            }

        } else {

            if (isEntryTopLevel(entry) || entry.getParent() == null) {

                return null;

            }

        }



        return entry.getParent().getRepresentativeEntry();

    }



    @Override

    public boolean isChildInGroup(NotificationEntry entry) {

    public boolean isChildInGroup(@NonNull NotificationEntry entry) {

        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {

            return !isEntryTopLevel(entry) && entry.getParent() != null;

        } else {

            return !isEntryTopLevel(entry);

        }

    }



    @Override

    public boolean isOnlyChildInGroup(NotificationEntry entry) {

    public boolean isOnlyChildInGroup(@NonNull NotificationEntry entry) {

        if (entry.getParent() == null) {

            return false;

        }
@@ -61,20 +86,24 @@ public class GroupMembershipManagerImpl implements GroupMembershipManager { 


    @Nullable

    @Override

    public List<NotificationEntry> getChildren(ListEntry entry) {

    public List<NotificationEntry> getChildren(@NonNull ListEntry entry) {

        if (entry instanceof GroupEntry) {

            return ((GroupEntry) entry).getChildren();

        }



        if (isGroupSummary(entry.getRepresentativeEntry())) {

        NotificationEntry representativeEntry = entry.getRepresentativeEntry();

        if (representativeEntry != null && isGroupSummary(representativeEntry)) {

            // maybe we were actually passed the summary

            return entry.getRepresentativeEntry().getParent().getChildren();

            GroupEntry parent = representativeEntry.getParent();

            if (parent != null) {

                return parent.getChildren();

            }

        }



        return null;

    }



    private boolean isEntryTopLevel(NotificationEntry entry) {

    private boolean isEntryTopLevel(@NonNull NotificationEntry entry) {

        return entry.getParent() == ROOT_ENTRY;

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java

+2 −5
Original line number Diff line number Diff line
@@ -148,11 +148,8 @@ public interface NotificationsModule { 
    }



    /** Provides an instance of {@link GroupMembershipManager} */

    @SysUISingleton

    @Provides

    static GroupMembershipManager provideGroupMembershipManager() {

        return new GroupMembershipManagerImpl();

    }

    @Binds

    GroupMembershipManager provideGroupMembershipManager(GroupMembershipManagerImpl impl);



    /** Provides an instance of {@link GroupExpansionManager} */

    @Binds

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

+8 −2
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import android.testing.AndroidTestingRunner 
import android.testing.TestableLooper

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.flags.FakeFeatureFlagsClassic

import com.android.systemui.statusbar.notification.collection.GroupEntry

import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder

import com.android.systemui.statusbar.notification.collection.NotifPipeline
@@ -77,13 +78,18 @@ class ConversationCoordinatorTest : SysuiTestCase() { 


    private lateinit var coordinator: ConversationCoordinator



    private val featureFlags = FakeFeatureFlagsClassic()



    @Before

    fun setUp() {

        MockitoAnnotations.initMocks(this)

        coordinator = ConversationCoordinator(

            peopleNotificationIdentifier,

            conversationIconManager,

            HighPriorityProvider(peopleNotificationIdentifier, GroupMembershipManagerImpl()),

            HighPriorityProvider(

                peopleNotificationIdentifier,

                GroupMembershipManagerImpl(featureFlags)

            ),

            headerController

        )

        whenever(channel.isImportantConversation).thenReturn(true)

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerTest.kt

0 → 100644
+106 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 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.collection.render



import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.flags.FakeFeatureFlagsClassic

import com.android.systemui.flags.Flags

import com.android.systemui.statusbar.notification.collection.GroupEntry

import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder

import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder

import com.google.common.truth.Truth.assertThat

import org.junit.Before

import org.junit.Test



@SmallTest

class GroupMembershipManagerTest : SysuiTestCase() {

    private lateinit var gmm: GroupMembershipManagerImpl



    private val featureFlags = FakeFeatureFlagsClassic()



    @Before

    fun setUp() {

        gmm = GroupMembershipManagerImpl(featureFlags)

    }



    @Test

    fun testIsChildInGroup_topLevel() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        val topLevelEntry = NotificationEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).build()

        assertThat(gmm.isChildInGroup(topLevelEntry)).isFalse()

    }



    @Test

    fun testIsChildInGroup_noParent_old() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        val noParentEntry = NotificationEntryBuilder().setParent(null).build()

        assertThat(gmm.isChildInGroup(noParentEntry)).isTrue()

    }



    @Test

    fun testIsChildInGroup_noParent_new() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val noParentEntry = NotificationEntryBuilder().setParent(null).build()

        assertThat(gmm.isChildInGroup(noParentEntry)).isFalse()

    }



    @Test

    fun testIsChildInGroup_child() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        val childEntry = NotificationEntryBuilder().build()

        assertThat(gmm.isChildInGroup(childEntry)).isTrue()

    }



    @Test

    fun testIsGroupSummary() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val entry = NotificationEntryBuilder().setGroupSummary(mContext, true).build()

        assertThat(gmm.isGroupSummary(entry)).isTrue()

    }



    @Test

    fun testGetGroupSummary() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)



        val summary =

            NotificationEntryBuilder()

                .setGroup(mContext, "group")

                .setGroupSummary(mContext, true)

                .build()

        val groupEntry =

            GroupEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).setSummary(summary).build()

        val entry =

            NotificationEntryBuilder().setGroup(mContext, "group").setParent(groupEntry).build()



        assertThat(gmm.getGroupSummary(entry)).isEqualTo(summary)

    }



    @Test

    fun testGetGroupSummary_isSummary_old() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        val entry = NotificationEntryBuilder().setGroupSummary(mContext, true).build()

        assertThat(gmm.getGroupSummary(entry)).isNull()

    }



    @Test

    fun testGetGroupSummary_isSummary_new() {

        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val entry = NotificationEntryBuilder().setGroupSummary(mContext, true).build()

        assertThat(gmm.getGroupSummary(entry)).isEqualTo(entry)

    }

}