Do not show snooze button for cancelled notifications
Cancelled notifications should not be snoozable. Test: 1) receive a message (from an app that cancels on send, e.g. Gmail Chat) 2) Inline reply to the message 3) Snooze button is not visible Test: atest PreparationCoordinatorTest Bug: 237288843 Change-Id: Ib1712cf55bb8f7914f2f340b73edafdae015f656
This commit is contained in:
committed by
Iavor-Valentin Iftime
parent
20b2302e13
commit
c5a1730b46
@@ -307,7 +307,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
|
||||
}
|
||||
|
||||
entriesToLocallyDismiss.add(entry);
|
||||
if (!isCanceled(entry)) {
|
||||
if (!entry.isCanceled()) {
|
||||
// send message to system server if this notification hasn't already been cancelled
|
||||
mBgExecutor.execute(() -> {
|
||||
try {
|
||||
@@ -387,7 +387,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
|
||||
entry.setDismissState(DISMISSED);
|
||||
mLogger.logNotifDismissed(entry);
|
||||
|
||||
if (isCanceled(entry)) {
|
||||
if (entry.isCanceled()) {
|
||||
canceledEntries.add(entry);
|
||||
} else {
|
||||
// Mark any children as dismissed as system server will auto-dismiss them as well
|
||||
@@ -396,7 +396,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
|
||||
if (shouldAutoDismissChildren(otherEntry, entry.getSbn().getGroupKey())) {
|
||||
otherEntry.setDismissState(PARENT_DISMISSED);
|
||||
mLogger.logChildDismissed(otherEntry);
|
||||
if (isCanceled(otherEntry)) {
|
||||
if (otherEntry.isCanceled()) {
|
||||
canceledEntries.add(otherEntry);
|
||||
}
|
||||
}
|
||||
@@ -523,7 +523,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
|
||||
+ logKey(entry)));
|
||||
}
|
||||
|
||||
if (!isCanceled(entry)) {
|
||||
if (!entry.isCanceled()) {
|
||||
throw mEulogizer.record(
|
||||
new IllegalStateException("Cannot remove notification " + logKey(entry)
|
||||
+ ": has not been marked for removal"));
|
||||
@@ -587,7 +587,7 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
|
||||
private void applyRanking(@NonNull RankingMap rankingMap) {
|
||||
ArrayMap<String, NotificationEntry> currentEntriesWithoutRankings = null;
|
||||
for (NotificationEntry entry : mNotificationSet.values()) {
|
||||
if (!isCanceled(entry)) {
|
||||
if (!entry.isCanceled()) {
|
||||
|
||||
// TODO: (b/148791039) We should crash if we are ever handed a ranking with
|
||||
// incomplete entries. Right now, there's a race condition in NotificationListener
|
||||
@@ -815,15 +815,6 @@ public class NotifCollection implements Dumpable, PipelineDumpable {
|
||||
return ranking;
|
||||
}
|
||||
|
||||
/**
|
||||
* True if the notification has been canceled by system server. Usually, such notifications are
|
||||
* immediately removed from the collection, but can sometimes stick around due to lifetime
|
||||
* extenders.
|
||||
*/
|
||||
private boolean isCanceled(NotificationEntry entry) {
|
||||
return entry.mCancellationReason != REASON_NOT_CANCELED;
|
||||
}
|
||||
|
||||
private boolean cannotBeLifetimeExtended(NotificationEntry entry) {
|
||||
final boolean locallyDismissedByUser = entry.getDismissState() != NOT_DISMISSED;
|
||||
final boolean systemServerReportedUserCancel =
|
||||
|
||||
@@ -321,6 +321,15 @@ public final class NotificationEntry extends ListEntry {
|
||||
mDismissState = requireNonNull(dismissState);
|
||||
}
|
||||
|
||||
/**
|
||||
* True if the notification has been canceled by system server. Usually, such notifications are
|
||||
* immediately removed from the collection, but can sometimes stick around due to lifetime
|
||||
* extenders.
|
||||
*/
|
||||
public boolean isCanceled() {
|
||||
return mCancellationReason != REASON_NOT_CANCELED;
|
||||
}
|
||||
|
||||
@Nullable public NotifFilter getExcludingFilter() {
|
||||
return getAttachState().getExcludingFilter();
|
||||
}
|
||||
|
||||
@@ -46,7 +46,7 @@ class NotifUiAdjustmentProvider @Inject constructor(
|
||||
private val userTracker: UserTracker
|
||||
) {
|
||||
private val dirtyListeners = ListenerSet<Runnable>()
|
||||
private var isSnoozeEnabled = false
|
||||
private var isSnoozeSettingsEnabled = false
|
||||
|
||||
/**
|
||||
* Update the snooze enabled value on user switch
|
||||
@@ -95,7 +95,7 @@ class NotifUiAdjustmentProvider @Inject constructor(
|
||||
}
|
||||
|
||||
private fun updateSnoozeEnabled() {
|
||||
isSnoozeEnabled =
|
||||
isSnoozeSettingsEnabled =
|
||||
secureSettings.getIntForUser(SHOW_NOTIFICATION_SNOOZE, 0, UserHandle.USER_CURRENT) == 1
|
||||
}
|
||||
|
||||
@@ -118,7 +118,7 @@ class NotifUiAdjustmentProvider @Inject constructor(
|
||||
smartActions = entry.ranking.smartActions,
|
||||
smartReplies = entry.ranking.smartReplies,
|
||||
isConversation = entry.ranking.isConversation,
|
||||
isSnoozeEnabled = isSnoozeEnabled,
|
||||
isSnoozeEnabled = isSnoozeSettingsEnabled && !entry.isCanceled,
|
||||
isMinimized = isEntryMinimized(entry),
|
||||
needsRedaction = lockscreenUserManager.needsRedaction(entry),
|
||||
)
|
||||
|
||||
@@ -16,14 +16,18 @@
|
||||
|
||||
package com.android.systemui.statusbar.notification.collection.coordinator;
|
||||
|
||||
import static android.provider.Settings.Secure.SHOW_NOTIFICATION_SNOOZE;
|
||||
|
||||
import static com.android.systemui.statusbar.notification.collection.GroupEntry.ROOT_ENTRY;
|
||||
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyInt;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.spy;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||
@@ -31,6 +35,7 @@ import static org.mockito.Mockito.when;
|
||||
|
||||
import static java.util.Objects.requireNonNull;
|
||||
|
||||
import android.database.ContentObserver;
|
||||
import android.os.Handler;
|
||||
import android.os.RemoteException;
|
||||
import android.testing.AndroidTestingRunner;
|
||||
@@ -294,6 +299,42 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
|
||||
assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testEntryCancellationWillRebindViews() {
|
||||
// Configure NotifUiAdjustmentProvider to set up SHOW_NOTIFICATION_SNOOZE value
|
||||
mEntry = spy(mEntry);
|
||||
mAdjustmentProvider.addDirtyListener(mock(Runnable.class));
|
||||
when(mSecureSettings.getIntForUser(eq(SHOW_NOTIFICATION_SNOOZE), anyInt(), anyInt()))
|
||||
.thenReturn(1);
|
||||
ArgumentCaptor<ContentObserver> contentObserverCaptor = ArgumentCaptor.forClass(
|
||||
ContentObserver.class);
|
||||
verify(mSecureSettings).registerContentObserverForUser(eq(SHOW_NOTIFICATION_SNOOZE),
|
||||
contentObserverCaptor.capture(), anyInt());
|
||||
ContentObserver contentObserver = contentObserverCaptor.getValue();
|
||||
contentObserver.onChange(false);
|
||||
|
||||
// GIVEN an inflated notification
|
||||
mCollectionListener.onEntryInit(mEntry);
|
||||
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
|
||||
verify(mNotifInflater).inflateViews(eq(mEntry), any(), any());
|
||||
mNotifInflater.invokeInflateCallbackForEntry(mEntry);
|
||||
|
||||
// Verify that snooze is initially enabled: from Settings & notification is not cancelled
|
||||
assertTrue(mAdjustmentProvider.calculateAdjustment(mEntry).isSnoozeEnabled());
|
||||
|
||||
// WHEN notification is cancelled, rebind views because snooze enabled value changes
|
||||
when(mEntry.isCanceled()).thenReturn(true);
|
||||
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
|
||||
|
||||
assertFalse(mAdjustmentProvider.calculateAdjustment(mEntry).isSnoozeEnabled());
|
||||
|
||||
// THEN we rebind it
|
||||
verify(mNotifInflater).rebindViews(eq(mEntry), any(), any());
|
||||
|
||||
// THEN we do not filter it because it's not the first inflation.
|
||||
assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testDoesntFilterInflatedNotifs() {
|
||||
// GIVEN an inflated notification
|
||||
|
||||
Reference in New Issue
Block a user