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

Commit c4e61e34 authored by Ioana Alexandru's avatar Ioana Alexandru Committed by Android (Google) Code Review
Browse files

Merge "Fix GroupMembershipManager implementation again." into main

parents e0cc1121 ecf7481e
Loading
Loading
Loading
Loading
+13 −2
Original line number Diff line number Diff line
@@ -104,7 +104,18 @@ public class GroupExpansionManagerImpl implements GroupExpansionManager, Dumpabl

    @Override
    public void setGroupExpanded(NotificationEntry entry, boolean expanded) {
        final NotificationEntry groupSummary = mGroupMembershipManager.getGroupSummary(entry);
        NotificationEntry groupSummary = mGroupMembershipManager.getGroupSummary(entry);
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)
                && entry.getParent() == null) {
            if (expanded) {
                throw new IllegalArgumentException("Cannot expand group that is not attached");
            } else {
                // The entry is no longer attached, but we still want to make sure we don't have
                // a stale expansion state.
                groupSummary = entry;
            }
        }

        boolean changed;
        if (expanded) {
            changed = mExpandedGroups.add(groupSummary);
+4 −4
Original line number Diff line number Diff line
@@ -25,18 +25,18 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import java.util.List;

/**
 * Helper that determines the group states (parent, summary, children) of a notification.
 * Helper that determines the group states (parent, summary, children) of a notification. This
 * generally assumes that the notification is attached (aka its parent is not null).
 */
public interface GroupMembershipManager {
    /**
     * @return whether a given notification is a top level entry or is the summary in a group which
     * has children
     * @return whether a given notification is the summary in a group which has children
     */
    boolean isGroupSummary(@NonNull NotificationEntry entry);

    /**
     * Get the summary of a specified status bar notification. For an isolated notification this
     * returns itself.
     * returns null, but if called directly on a summary it returns itself.
     */
    @Nullable
    NotificationEntry getGroupSummary(@NonNull NotificationEntry entry);
+21 −18
Original line number Diff line number Diff line
@@ -22,7 +22,7 @@ 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.FeatureFlagsClassic;
import com.android.systemui.flags.Flags;
import com.android.systemui.statusbar.notification.collection.GroupEntry;
import com.android.systemui.statusbar.notification.collection.ListEntry;
@@ -38,47 +38,50 @@ import javax.inject.Inject;
 */
@SysUISingleton
public class GroupMembershipManagerImpl implements GroupMembershipManager {
    FeatureFlags mFeatureFlags;
    FeatureFlagsClassic mFeatureFlags;

    @Inject
    public GroupMembershipManagerImpl(FeatureFlags featureFlags) {
    public GroupMembershipManagerImpl(FeatureFlagsClassic featureFlags) {
        mFeatureFlags = featureFlags;
    }

    @Override
    public boolean isGroupSummary(@NonNull NotificationEntry entry) {
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {
            if (entry.getParent() == null) {
                // The entry is not attached, so it doesn't count.
                return false;
            }
            // If entry is a summary, its parent is a GroupEntry with summary = entry.
            return entry.getParent().getSummary() == entry;
        } else {
            return getGroupSummary(entry) == entry;
        }
    }

    @Nullable
    @Override
    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) {
        if (isTopLevelEntry(entry) || entry.getParent() == null) {
            return null;
        }
        }

        return entry.getParent().getRepresentativeEntry();
        return entry.getParent().getSummary();
    }

    @Override
    public boolean isChildInGroup(@NonNull NotificationEntry entry) {
        if (mFeatureFlags.isEnabled(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE)) {
            return !isEntryTopLevel(entry) && entry.getParent() != null;
            // An entry is a child if it's not a summary or top level entry, but it is attached.
            return !isGroupSummary(entry) && !isTopLevelEntry(entry) && entry.getParent() != null;
        } else {
            return !isEntryTopLevel(entry);
            return !isTopLevelEntry(entry);
        }
    }

    @Override
    public boolean isOnlyChildInGroup(@NonNull NotificationEntry entry) {
        if (entry.getParent() == null) {
            return false;
            return false; // The entry is not attached.
        }

        return !isGroupSummary(entry) && entry.getParent().getChildren().size() == 1;
@@ -103,7 +106,7 @@ public class GroupMembershipManagerImpl implements GroupMembershipManager {
        return null;
    }

    private boolean isEntryTopLevel(@NonNull NotificationEntry entry) {
    private boolean isTopLevelEntry(@NonNull NotificationEntry entry) {
        return entry.getParent() == ROOT_ENTRY;
    }
}
+17 −0
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.mock
import com.android.systemui.util.mockito.withArgCaptor
import com.google.common.truth.Truth.assertThat
import org.junit.Assert.assertThrows
import org.junit.Before
import org.junit.Test
import org.mockito.Mockito.never
@@ -125,6 +126,22 @@ class GroupExpansionManagerTest : SysuiTestCase() {
        assertThat(listenerCalledCount).isEqualTo(5)
    }

    @Test
    fun testExpandUnattachedEntry() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        // First, expand the entry when it is attached.
        gem.setGroupExpanded(summary1, true)
        assertThat(gem.isGroupExpanded(summary1)).isTrue()

        // Un-attach it, and un-expand it.
        NotificationEntryBuilder.setNewParent(summary1, null)
        gem.setGroupExpanded(summary1, false)

        // Expanding again should throw.
        assertThrows(IllegalArgumentException::class.java) { gem.setGroupExpanded(summary1, true) }
    }

    @Test
    fun testSyncWithPipeline() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
+83 −16
Original line number Diff line number Diff line
@@ -58,6 +58,35 @@ class GroupMembershipManagerTest : SysuiTestCase() {
        val noParentEntry = NotificationEntryBuilder().setParent(null).build()
        assertThat(gmm.isChildInGroup(noParentEntry)).isFalse()
    }
    @Test
    fun testIsChildInGroup_summary_old() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.isChildInGroup(summary)).isTrue()
    }

    @Test
    fun testIsChildInGroup_summary_new() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.isChildInGroup(summary)).isFalse()
    }

    @Test
    fun testIsChildInGroup_child() {
@@ -67,40 +96,78 @@ class GroupMembershipManagerTest : SysuiTestCase() {
    }

    @Test
    fun testIsGroupSummary() {
    fun testIsGroupSummary_topLevelEntry() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
        val entry = NotificationEntryBuilder().setGroupSummary(mContext, true).build()
        assertThat(gmm.isGroupSummary(entry)).isTrue()
        val entry = NotificationEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).build()
        assertThat(gmm.isGroupSummary(entry)).isFalse()
    }

    @Test
    fun testGetGroupSummary() {
    fun testIsGroupSummary_summary() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, "group")
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        val groupEntry =
            GroupEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).setSummary(summary).build()
        val entry =
            NotificationEntryBuilder().setGroup(mContext, "group").setParent(groupEntry).build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

        assertThat(gmm.getGroupSummary(entry)).isEqualTo(summary)
        assertThat(gmm.isGroupSummary(summary)).isTrue()
    }

    @Test
    fun testGetGroupSummary_isSummary_old() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, false)
        val entry = NotificationEntryBuilder().setGroupSummary(mContext, true).build()
    fun testIsGroupSummary_child() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        val entry = NotificationEntryBuilder().setGroup(mContext, groupKey).build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).addChild(entry).build()

        assertThat(gmm.isGroupSummary(entry)).isFalse()
    }

    @Test
    fun testGetGroupSummary_topLevelEntry() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
        val entry = NotificationEntryBuilder().setParent(GroupEntry.ROOT_ENTRY).build()
        assertThat(gmm.getGroupSummary(entry)).isNull()
    }

    @Test
    fun testGetGroupSummary_isSummary_new() {
    fun testGetGroupSummary_summary() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).build()

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

    @Test
    fun testGetGroupSummary_child() {
        featureFlags.set(Flags.NOTIFICATION_GROUP_EXPANSION_CHANGE, true)
        val entry = NotificationEntryBuilder().setGroupSummary(mContext, true).build()
        assertThat(gmm.getGroupSummary(entry)).isEqualTo(entry)

        val groupKey = "group"
        val summary =
            NotificationEntryBuilder()
                .setGroup(mContext, groupKey)
                .setGroupSummary(mContext, true)
                .build()
        val entry = NotificationEntryBuilder().setGroup(mContext, groupKey).build()
        GroupEntryBuilder().setKey(groupKey).setSummary(summary).addChild(entry).build()

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