Mark group summaries as interrupted when group alert transfer happens
When the group summary should heads up but the alert is transferred to a child, we want to avoid incorrectly re-alerting on the group on a ranking update because it looks like the group is in a "should alert, hasn't alerted yet" state. Bug: 248325248 Bug: 254472116 Test: HeadsUpCoordinatorTest, manually verified group HUNs seem to behave OK Change-Id: Ia4ada5eeb6abf137b4e16a486d8c7c6152bc57d2
This commit is contained in:
@@ -198,6 +198,13 @@ class HeadsUpCoordinator @Inject constructor(
|
||||
// At this point we just need to initiate the transfer
|
||||
val summaryUpdate = mPostedEntries[logicalSummary.key]
|
||||
|
||||
// Because we now know for certain that some child is going to alert for this summary
|
||||
// (as we have found a child to transfer the alert to), mark the group as having
|
||||
// interrupted. This will allow us to know in the future that the "should heads up"
|
||||
// state of this group has already been handled, just not via the summary entry itself.
|
||||
logicalSummary.setInterruption()
|
||||
mLogger.logSummaryMarkedInterrupted(logicalSummary.key, childToReceiveParentAlert.key)
|
||||
|
||||
// If the summary was not attached, then remove the alert from the detached summary.
|
||||
// Otherwise we can simply ignore its posted update.
|
||||
if (!isSummaryAttached) {
|
||||
|
||||
@@ -69,4 +69,13 @@ class HeadsUpCoordinatorLogger constructor(
|
||||
"updating entry via ranking applied: $str1 updated shouldHeadsUp=$bool1"
|
||||
})
|
||||
}
|
||||
|
||||
fun logSummaryMarkedInterrupted(summaryKey: String, childKey: String) {
|
||||
buffer.log(TAG, LogLevel.DEBUG, {
|
||||
str1 = summaryKey
|
||||
str2 = childKey
|
||||
}, {
|
||||
"marked group summary as interrupted: $str1 for alert transfer to child: $str2"
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -406,6 +406,10 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSummary)
|
||||
verify(mHeadsUpManager).showNotification(mGroupSibling1)
|
||||
|
||||
// In addition make sure we have explicitly marked the summary as having interrupted due
|
||||
// to the alert being transferred
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -424,6 +428,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSummary)
|
||||
verify(mHeadsUpManager).showNotification(mGroupChild1)
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -449,6 +454,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSummary)
|
||||
verify(mHeadsUpManager).showNotification(mGroupSibling1)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling2)
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -474,6 +480,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSummary)
|
||||
verify(mHeadsUpManager).showNotification(mGroupChild1)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupChild2)
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -512,6 +519,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
verify(mHeadsUpManager).showNotification(mGroupPriority)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling1)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling2)
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -548,6 +556,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
verify(mHeadsUpManager).showNotification(mGroupPriority)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling1)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling2)
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -582,6 +591,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
verify(mHeadsUpManager).showNotification(mGroupPriority)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling1)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSibling2)
|
||||
assertTrue(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -671,6 +681,35 @@ class HeadsUpCoordinatorTest : SysuiTestCase() {
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupChild2)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testNoTransfer_groupSummaryNotAlerting() {
|
||||
// When we have a group where the summary should not alert and exactly one child should
|
||||
// alert, we should never mark the group summary as interrupted (because it doesn't).
|
||||
setShouldHeadsUp(mGroupSummary, false)
|
||||
setShouldHeadsUp(mGroupChild1, true)
|
||||
setShouldHeadsUp(mGroupChild2, false)
|
||||
|
||||
mCollectionListener.onEntryAdded(mGroupSummary)
|
||||
mCollectionListener.onEntryAdded(mGroupChild1)
|
||||
mCollectionListener.onEntryAdded(mGroupChild2)
|
||||
val groupEntry = GroupEntryBuilder()
|
||||
.setSummary(mGroupSummary)
|
||||
.setChildren(listOf(mGroupChild1, mGroupChild2))
|
||||
.build()
|
||||
mBeforeTransformGroupsListener.onBeforeTransformGroups(listOf(groupEntry))
|
||||
verify(mHeadsUpViewBinder, never()).bindHeadsUpView(any(), any())
|
||||
mBeforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupEntry))
|
||||
|
||||
verify(mHeadsUpViewBinder, never()).bindHeadsUpView(eq(mGroupSummary), any())
|
||||
finishBind(mGroupChild1)
|
||||
verify(mHeadsUpViewBinder, never()).bindHeadsUpView(eq(mGroupChild2), any())
|
||||
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupSummary)
|
||||
verify(mHeadsUpManager).showNotification(mGroupChild1)
|
||||
verify(mHeadsUpManager, never()).showNotification(mGroupChild2)
|
||||
assertFalse(mGroupSummary.hasInterrupted())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testOnRankingApplied_newEntryShouldAlert() {
|
||||
// GIVEN that mEntry has never interrupted in the past, and now should
|
||||
|
||||
Reference in New Issue
Block a user