feat: [SDK-5011] detect restored notifications and stop them re-alerting - #2723
feat: [SDK-5011] detect restored notifications and stop them re-alerting#2723nan-li wants to merge 6 commits into
Conversation
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)21/57 touched executable lines covered (36.8% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 36.8% (minimum 80%). |
SDK review — request changesRestore-reason split, sticky shade dismiss, and re-asserting the restore channel after the extender all look right. Grouped restore + mixed suppress.
Nits: log the suppress-branch outcome; |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
nan-li
changed the title
|
Both suppress branches log their outcome now. The badge call is deliberate, and it now carries a comment saying why. One correction on the window, though. |
Apps could not tell a restore from a new push, so once-only work such as analytics and network calls ran again every time Android cleared the shade and the SDK re-posted its saved notifications. v3 exposed a restoring flag and the v4 refactor dropped it. The property is read-only and defaulted, so existing implementations still compile. It reports true for a shade restore and for a group collapsing to one member, since both mean the app has already seen this notification.
…nnel An extender calling setChannelId moved a restored notification off restored_OS_notifications and onto the app's own channel. The channel governs alerting on Android O+, so a high-importance one turned every restore into a heads-up banner. The displayer now records the channel OneSignal picked and re-asserts it after the extender runs. That also explains the dead code this removes. removeNotifyOptions strips flags and sound to keep restores quiet, which predates notification channels and cannot win against one on O+. NotificationGenerationJob loses orgFlags, written after every extender and read nowhere. Its sibling orgSound stays, still needed by the pre-Android 7.0 grouped-sound workaround in SummaryNotificationDisplayer.
preventDefault() during a restore did nothing durable. markNotificationAsDismissed bailed out unless isNotificationToDisplay was set, which never happens when display is suppressed, so the row kept dismissed = 0 and the restore query picked it up again on every later cold start. A suppressed shade restore now marks the row dismissed without clearing the shade, since the restore pass can include notifications still showing on API 21 and 22, and whenever getActiveNotifications fails. It then updates the dependent group summary the same way a real dismissal does, so a suppressed child cannot orphan its summary or leave it counting a notification that is gone. Telling the two restore triggers apart is what makes that safe, so the internal isRestoring boolean becomes a NotificationRestoreReason. A shade restore is gone from the shade and should stay gone. A group collapsing to one member is still on screen and the user never dismissed it, so suppressing that rebuild leaves it alone. The work manager still reads the legacy is_restoring boolean from work enqueued by an older version.
The suppress branch treated GROUP_REGROUP as always posted. That premise breaks during a restore pass. When a suppressed child drops its group to one member before the restore loop reaches the sibling, restoreSummary enqueues that sibling as GROUP_REGROUP first, and the work manager then drops the sibling's own SHADE_RESTORE, since unique work is keyed on the OS id alone and the in-memory map keeps the first enqueue. The sibling runs as a regroup while nothing is posted, a suppression no-ops, and the row comes back on the next restore. A user swiping a redisplayed child mid-pass reaches the same state through the dismissal broadcast. A suppressed regroup now asks the notification manager whether the notification is actually posted. Posted keeps the old behavior, since the user never dismissed it. Not posted takes the same path as a suppressed shade restore, marking the row dismissed without touching the shade and reconciling the dependent group summary. Below Android M there is no API to ask, so it is assumed posted and API 21 and 22 keep the old behavior. This makes blanket preventDefault(true) safe for both restore reasons, so the public KDoc now promises a suppressed restore stays gone without qualifying which kind. Both suppress branches log the dismissal outcome, so an already-dismissed row is distinguishable from a fresh one in logcat.
restoreSummary re-enqueued the last remaining group child unconditionally. During a restore pass that child may not have reached its own SHADE_RESTORE enqueue yet, and a GROUP_REGROUP enqueued first wins the unique-work slot, converting the child's restore into a regroup. The previous commit makes that survivable; this one stops it happening from the summary path. The 1-remaining rebuild now only fires for a child that is actually posted. That keeps the case it exists for, a sibling that displayed earlier in the pass and whose summary still counts the dismissed row. A child that is not posted is mid restore, and its own shade work displays or dismisses it.
readRestoreReason mapped an unknown reason name to null, so work written by a newer SDK and drained after a downgrade was processed as a fresh push. In practice the duplicate check drops it and the row heals on the next restore, but the failure direction was still towards alerting. An unknown name now reads as SHADE_RESTORE with a warning, which keeps it quiet and restorable.
1 participant
Description
One Line Summary
(API Addition)
Let apps detect restored notifications, and stop restored ones from re-alerting or reappearing after they are suppressed.
Details
Motivation
When Android clears the shade (reboot, app update, force-stop) the SDK re-posts saved notifications and re-fires
onNotificationReceived. Three problems fell out of that:restoringflag; it was dropped in the v4 refactor.preventDefault()during a restore did not stick.markNotificationAsDismissedbailed out unlessisNotificationToDisplaywas set, which never happens when display is suppressed, so the row was never marked dismissed and the notification returned on every later restore.setChannelIdmoved a restored notification off the silentrestored_OS_notificationschannel. The channel governs alerting on Android O+, soremoveNotifyOptions()could not keep it quiet and it arrived as a heads-up banner.Scope
INotificationReceivedEventgains a read-onlyrestoring: Boolean, defaulted so existing implementations still compile. True for both a shade restore and a group collapsing to one member.preventDefault(true)marks the row dismissed, leaves the shade alone, and updates the dependent group summary the same way a real dismissal would. A suppressed last child consumes its orphaned summary row, and a suppressed sibling refreshes the summary count. Group collapse +preventDefaultleaves a still-posted child alone, including its summary. One that is not posted is dismissed like a shade restore, so suppression sticks for both reasons.restore_reasonname, possible when queued work is drained after a downgrade, now reads as a shade restore with a warning instead of presenting as a fresh push.NotificationGenerationJob.orgFlags, written after every extender and read nowhere. Its siblingorgSoundis kept, still needed by the pre-Android 7.0 grouped-sound workaround.Testing
Unit testing
NotificationDisplayerTestscovers the restore channel, an extender trying to override it, an extender keeping its channel on a normal notification, and the payload-vs-extender sound split thatorgSoundexists for.NotificationGenerationProcessorTestscoversrestoringfor both reasons, shadepreventDefault(true), no-arg shadepreventDefault(), and the regroup suppress split, where a posted child is left alone and an unposted one is dismissed like a shade restore. It also covers the summary follow-up. A suppressed shade restore reconciles the dependent summary. A repeat suppression on an already-dismissed row does not, for either reason. A sibling that displayed earlier in the same pass stays put.NotificationSummaryManagerTestscovers the regroup guard. The 1-remaining rebuild fires for a posted child and is skipped for one that is mid restore.NotificationRepositoryTestscoversmarkAsDismissedWithoutCancelleaving the shade while refreshing the badge.NotificationGenerationWorkManagerTestscoversreadRestoreReasonfor the enum, the legacy boolean, and an unknown name failing closed as a shade restore.JaCoCo does not attribute Robolectric-executed lines, so this PR uses Skip Coverage Check. The processor and work-manager cases are plain JVM and do count.
Manual testing
Confirmed behavior before and after with force kill
Affected code checklist
Checklist
Overview
Testing
Final pass