Fix notification grouping and section bug
Update HighPriorityProvider to check whether a group summary has a child
with high priority - if so, consider the summary high priority. This was
implemented for the new pipelne but not for the old pipeline grouping
structure. This retains the functionality pre-R where a group of low
priority and high priority notifications gets sorted into the "Alerting"
section instead of "Silent".
In NVHM, guard against summaries appearing after its children in the
sorted list of notifications returned by NEM.getVisibleNotifications.
Previously, summaries appearing after its children would remove the
children from the final notification shade list.
Test: atest SystemUITests
Fixes: 156407436
Change-Id: Id023047360249370e94085d62705d2977b03056e
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
index 3dda15b..7b60d51 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
@@ -61,7 +61,11 @@
private final Handler mHandler;
- /** Re-usable map of top-level notifications to their sorted children if any.*/
+ /**
+ * Re-usable map of top-level notifications to their sorted children if any.
+ * If the top-level notification doesn't have children, its key will still exist in this map
+ * with its value explicitly set to null.
+ */
private final HashMap<NotificationEntry, List<NotificationEntry>> mTmpChildOrderMap =
new HashMap<>();
@@ -215,10 +219,19 @@
}
orderedChildren.add(ent);
} else {
- // Top-level notif
- mTmpChildOrderMap.put(ent, null);
+ // Top-level notif (either a summary or single notification)
+
+ // A child may have already added its summary to mTmpChildOrderMap with a
+ // list of children. This can happen since there's no guarantee summaries are
+ // sorted before its children.
+ if (!mTmpChildOrderMap.containsKey(ent)) {
+ // mTmpChildOrderMap's keyset is used to iterate through all entries, so it's
+ // necessary to add each top-level notif as a key
+ mTmpChildOrderMap.put(ent, null);
+ }
toShow.add(ent.getRow());
}
+
}
ArrayList<ExpandableNotificationRow> viewsToRemove = new ArrayList<>();
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java
index df3609b..1c076c4 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/provider/HighPriorityProvider.java
@@ -23,6 +23,9 @@
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
+import com.android.systemui.statusbar.phone.NotificationGroupManager;
+
+import java.util.List;
import javax.inject.Inject;
import javax.inject.Singleton;
@@ -36,10 +39,14 @@
@Singleton
public class HighPriorityProvider {
private final PeopleNotificationIdentifier mPeopleNotificationIdentifier;
+ private final NotificationGroupManager mGroupManager;
@Inject
- public HighPriorityProvider(PeopleNotificationIdentifier peopleNotificationIdentifier) {
+ public HighPriorityProvider(
+ PeopleNotificationIdentifier peopleNotificationIdentifier,
+ NotificationGroupManager groupManager) {
mPeopleNotificationIdentifier = peopleNotificationIdentifier;
+ mGroupManager = groupManager;
}
/**
@@ -74,13 +81,25 @@
private boolean hasHighPriorityChild(ListEntry entry) {
+ List<NotificationEntry> children = null;
+
if (entry instanceof GroupEntry) {
- for (NotificationEntry child : ((GroupEntry) entry).getChildren()) {
+ // New notification pipeline
+ children = ((GroupEntry) entry).getChildren();
+ } else if (entry.getRepresentativeEntry() != null
+ && mGroupManager.isGroupSummary(entry.getRepresentativeEntry().getSbn())) {
+ // Old notification pipeline
+ children = mGroupManager.getChildren(entry.getRepresentativeEntry().getSbn());
+ }
+
+ if (children != null) {
+ for (NotificationEntry child : children) {
if (isHighPriority(child)) {
return true;
}
}
}
+
return false;
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java
index 78e9b33..2b12c22 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/HighPriorityProviderTest.java
@@ -38,6 +38,7 @@
import com.android.systemui.statusbar.RankingBuilder;
import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider;
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
+import com.android.systemui.statusbar.phone.NotificationGroupManager;
import org.junit.Before;
import org.junit.Test;
@@ -45,16 +46,22 @@
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
+import java.util.ArrayList;
+import java.util.Arrays;
+
@SmallTest
@RunWith(AndroidTestingRunner.class)
public class HighPriorityProviderTest extends SysuiTestCase {
@Mock private PeopleNotificationIdentifier mPeopleNotificationIdentifier;
+ @Mock private NotificationGroupManager mGroupManager;
private HighPriorityProvider mHighPriorityProvider;
@Before
public void setup() {
MockitoAnnotations.initMocks(this);
- mHighPriorityProvider = new HighPriorityProvider(mPeopleNotificationIdentifier);
+ mHighPriorityProvider = new HighPriorityProvider(
+ mPeopleNotificationIdentifier,
+ mGroupManager);
}
@Test
@@ -166,6 +173,22 @@
}
@Test
+ public void testIsHighPriority_checkChildrenToCalculatePriority() {
+ // GIVEN: a summary with low priority has a highPriorityChild and a lowPriorityChild
+ final NotificationEntry summary = createNotifEntry(false);
+ final NotificationEntry lowPriorityChild = createNotifEntry(false);
+ final NotificationEntry highPriorityChild = createNotifEntry(true);
+ when(mGroupManager.isGroupSummary(summary.getSbn())).thenReturn(true);
+ when(mGroupManager.getChildren(summary.getSbn())).thenReturn(
+ new ArrayList<>(Arrays.asList(lowPriorityChild, highPriorityChild)));
+
+ // THEN the summary is high priority since it has a high priority child
+ assertTrue(mHighPriorityProvider.isHighPriority(summary));
+ }
+
+ // Tests below here are only relevant to the NEW notification pipeline which uses GroupEntry
+
+ @Test
public void testIsHighPriority_summaryUpdated() {
// GIVEN a GroupEntry with a lowPrioritySummary and no children
final GroupEntry parentEntry = new GroupEntry("test_group_key");
@@ -186,7 +209,7 @@
}
@Test
- public void testIsHighPriority_checkChildrenToCalculatePriority() {
+ public void testIsHighPriority_checkChildrenToCalculatePriorityOf() {
// GIVEN:
// GroupEntry = parentEntry, summary = lowPrioritySummary
// NotificationEntry = lowPriorityChild
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
index 1c47131..82a7774 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
@@ -77,7 +77,8 @@
mock(NotificationEntryManagerLogger::class.java),
sectionsManager,
personNotificationIdentifier,
- HighPriorityProvider(personNotificationIdentifier)
+ HighPriorityProvider(personNotificationIdentifier,
+ mock(NotificationGroupManager::class.java))
)
}