Skip to content

Commit e705a8a

Browse files
committed
docs: [SDK-5083] correct why the demo extender is conditional
The comment claimed an extender makes the SDK display a data-only push, so installing a no-op one would not be inert. That is not how 5.x behaves. processHandlerResponse gates on canDisplay, a non-empty notification body, before it reaches shouldDisplayNotification, so hasExtender() is never read for a bodyless push and an extender cannot rescue one. The code stays as it is. Setting an extender only when a switch needs one is still right, just for the duller reason that nothing asked for it otherwise. Left uncorrected, a customer reading the demo could design around SDK behavior that does not exist.
1 parent 18bd2bc commit e705a8a

2 files changed

Lines changed: 6 additions & 4 deletions

File tree

examples/build.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ The five behavior switches sit behind a Show options / Hide options row so the s
232232

233233
The channel readout comes from `NotificationCompat.getChannelId(builder.build())` inside the extender, the only place an extension can see the SDK's choice. A restored notification lands on `restored_OS_notifications` no matter what the payload asked for, which the payload alone never shows. `event.restoring` is not on `INotificationReceivedEvent` yet; see the TODO in the class and SDK-5011.
234234

235-
An extender also makes the SDK display a data-only push that carries no `alert` (`NotificationGenerationProcessor.shouldDisplayNotification`), so the class sets one only when a switch needs it.
235+
The class sets an extender only when a switch needs one rather than installing a no-op whenever the extension is on, which is about not doing work nothing asked for. An extender cannot change what displays. `NotificationGenerationProcessor.shouldDisplayNotification` does read `hasExtender()`, but `processHandlerResponse` has already dropped a push with an empty body on `canDisplay` by the time it runs.
236236

237237
---
238238

examples/demo/app/src/main/java/com/onesignal/example/notification/DemoNotificationServiceExtension.kt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,11 @@ class DemoNotificationServiceExtension : INotificationServiceExtension {
5353
return
5454
}
5555

56-
// Set an extender only when a switch needs one. An extender makes the SDK display a
57-
// data-only push that carries no `alert` (NotificationGenerationProcessor
58-
// .shouldDisplayNotification), so an always-installed no-op extender is not free.
56+
// Set an extender only when a switch needs one, rather than installing a no-op
57+
// whenever the extension is on. This is about not doing work nothing asked for.
58+
// An extender cannot change what displays: NotificationGenerationProcessor
59+
// .shouldDisplayNotification does read hasExtender(), but processHandlerResponse
60+
// has already dropped a push with an empty body on canDisplay by the time it runs.
5961
if (options.logDetails || options.applyExtender || options.forceHighImportanceChannel) {
6062
notification.setExtender(buildExtender(event.context, notification, options))
6163
}

0 commit comments

Comments
 (0)