Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Add] notification permission prompting callback and In-App Message support #1607

Merged
merged 3 commits into from
Jun 28, 2022

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Jun 24, 2022

Description

One Line Summary

Adds optional notification permission prompting callback to promptForPushNotifications and adds support to prompt from In-App Messages as well.

Details

Motivation

Since Android 13 now requires permission just like iOS we are want the features set to match 1-to-1 with our OneSignal-iOS-SDK.

Scope

Only affects notification permission prompting and prompting from In-App Messages.

Testing

Manual testing

Tested on Android 13 Beta 3 (API 33) Emulator to ensure the callback fires when it should.
New IAM notification prompt not tested, however it is essentially behind a feature flag. If something is wrong we can omit this specific SDK version and make another PR to address any bug fixes.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
    • Prompting
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

jkasten2 added 3 commits June 26, 2022 14:19
Added new public method overload for promptForPushNotifications to
accept a new optional PromptForPushNotificationPermissionResponseHandler
type. This fires when the user accepts or declines the notification
permission, includes handling of the fallbackToSettings logic as well.
We support multiple callbacks for location so adding this to
notifications to match. This is to ensure events are not missed for any
reason.
Add support for notification permission prompting to In-App Messaging.
@jkasten2 jkasten2 force-pushed the add/notification_permission_prompting_callback_and_iam branch from 4ea9662 to 550caa3 Compare June 26, 2022 21:20
@jkasten2 jkasten2 changed the title WIP - [Add] notification permission prompting callback and In-App Message support [Add] notification permission prompting callback and In-App Message support Jun 26, 2022
@jkasten2 jkasten2 requested review from emawby and nan-li June 26, 2022 21:24
Base automatically changed from add/notification_permission_prompting to main June 28, 2022 19:57
@jkasten2 jkasten2 merged commit d400283 into main Jun 28, 2022
@jkasten2 jkasten2 deleted the add/notification_permission_prompting_callback_and_iam branch June 28, 2022 19:58
@jkasten2 jkasten2 mentioned this pull request Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants