diff options
5 files changed, 165 insertions, 24 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManager.java index b9c8f7223740..c33e8ab8cdd4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManager.java @@ -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); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java index e784ec62c8e6..a6b855f9b838 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java @@ -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) { - if (isEntryTopLevel(entry) || entry.getParent() == null) { - return null; + 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) { - return !isEntryTopLevel(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; } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java index 637637d39957..2e0fa3243435 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java @@ -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 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 55ea31571dfe..65697b736cbd 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,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) @@ -182,7 +188,7 @@ class ConversationCoordinatorTest : SysuiTestCase() { } @Test - fun testInAlertingPeopleSectionWhenThereIsAnImportantChild(){ + fun testInAlertingPeopleSectionWhenThereIsAnImportantChild() { // GIVEN val altChildA = NotificationEntryBuilder().setTag("A") .setImportance(IMPORTANCE_DEFAULT).build() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerTest.kt new file mode 100644 index 000000000000..37ec0e09f841 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerTest.kt @@ -0,0 +1,106 @@ +/* + * 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) + } +} |