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 3 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)16/46 touched executable lines covered (34.8% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 34.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; |
|
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.
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 +preventDefaultis a no-op; the remaining child stays, including its summary.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 regroup not dismissing. 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. A sibling that displayed earlier in the same pass stays put, and regroup still touches nothing.NotificationRepositoryTestscoversmarkAsDismissedWithoutCancelleaving the shade.NotificationGenerationWorkManagerTestscoversreadRestoreReasonfor the enum, the legacy boolean, and an unknown name.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