Skip to content

[5.0.0] IAM Lifecycle Listener - API update#1257

Merged
emawby merged 12 commits into
5.0.0/notification_permission_observer_apifrom
5.0.0/iam_lifecycle_listener_api
May 1, 2023
Merged

[5.0.0] IAM Lifecycle Listener - API update#1257
emawby merged 12 commits into
5.0.0/notification_permission_observer_apifrom
5.0.0/iam_lifecycle_listener_api

Conversation

@nan-li
Copy link
Copy Markdown
Contributor

@nan-li nan-li commented Apr 28, 2023 •
edited
Loading

Description

One Line Summary

REQUIRED - Very short description that summaries the changes in this PR.

Details

Motivation

REQUIRED - Why is this code change being made? Or what is the goal of this PR? Examples: Fixes a specific bug, provides additional logging to debug future issues, feature to allow X.

Scope

  • Rename OSInAppMessageLifecycleHandler to OSInAppMessageLifecycleListener
  • The methods will receive specific event types such as OSInAppMessageDidDisplayEvent instead of just the OSInAppMessage
  • Add a remove method removeLifecycleListener
  • Due to Obj-C not allowing method overloading, add InAppMessage to method names like the list below, so it doesn't conflict with Notifications displays, but this doesn't apply to Swift as we can refine the swift name.
  • onWillDisplayInAppMessage
  • onDidDisplayInAppMessage
  • onWillDismissInAppMessage
  • onDidDismissInAppMessage
  • I think doesn't support multiple observers, should be modified.

OPTIONAL - Other

OPTIONAL - Feel free to add any other sections or sub-sections that can explain your PR better.

Testing

Unit testing

OPTIONAL - Explain unit tests added, if not clear in the code.

Manual testing

RECOMMEND - OPTIONAL - Explain what scenarios were tested and the environment.
Example: Tested opening a notification while the app was foregrounded, app build with Android Studio 2020.3 with a fresh install of the OneSignal example app on a Pixel 6 with Android 12.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • 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
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • 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.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

nan-li added 2 commits April 28, 2023 02:08
@nan-li
IAM Lifecycle Listener - API update
eb1685a 
* Rename `OSInAppMessageLifecycleHandler` to `OSInAppMessageLifecycleListener`
* The methods will receive specific event types such as `OSInAppMessageDidDisplayEvent` instead of just the `OSInAppMessage`
* Add a remove method `removeLifecycleListener`
* Due to Obj-C not allowing method overloading, add `InAppMessage` to method names, so it doesn't conflict with Notifications displays, but this doesn't apply to Swift as we can refine the swift name.
- onWillDisplayInAppMessage
- onDidDisplayInAppMessage
- onWillDismissInAppMessage
- onDidDismissInAppMessage
@nan-li
Notification Foreground Listener - API update
2cc2c36 
* Rename notificationWillShowInForegroundHandler to OSNotificationLifecycleListener
* It will have the `onWillDisplay` event.
@emawby emawby self-requested a review April 28, 2023 20:24
nan-li and others added 10 commits April 28, 2023 13:37
@nan-li
Push Subscription Observer - API update
c5dad91 
* Rename OSPushSubscriptionStateChanges to OSPushSubscriptionChangedState
* Rename event from onOSPushSubscriptionChanged to onPushSubscriptionDidChange
* Use current and previous instead of to and from
* No other logic changes, only naming
@nan-li
[nit] display event does not need preventDefault state
9d27761 
* It can just use the `wantsToDisplay` property of the notification it owns.
@nan-li
IAM Click Listener - API update
9695f85 
* Rename click block to `OSInAppMessageClickListener`
* Rename `setClickHandler` to  `addClickListener`
* Add a remove method for the click listener
* OSInAppMessageAction has basically been renamed to OSInAppMessageClickResult
* OSInAppMessageClickEvent will be passed to developers instead of OSInAppMessageAction and the event will contain an in app message and a click result.
@nan-li
[nit] remove some old non-existent methods
58cad6f 
OSInAppMessageDelegate  and handleMessageAction not used by anything
@nan-li
make a fix to IAM click listener
f5c34df 
* We have some legacy logic about IAM direct influence but if there are multiple click listeners added, we only want this to update once, and not for every listener.
@nan-li
Notification Click Listener - API update
95efdd1 
* Rename setNotificationOpenedHandler -> addClickListener, removeClickListener
* Rename object `OSNotificationOpenedResult` -> `OSNotificationClickEvent`
* Rename object `OSNotificationAction` -> `OSNotificationClickResult`
* Support having multiple listeners
@emawby
Merge pull request #1262 from OneSignal/5.0.0/notification_click_list…
553d31c 
…ener_api

[5.0.0] Notification Click Listener - API update
@emawby
Merge pull request #1261 from OneSignal/5.0.0/iam_click_listener_api
494bf10 
[5.0.0] IAM Click Listener - API update
@emawby
Merge pull request #1260 from OneSignal/5.0.0/push_subscription_obser…
d3613e2 
…ver_api

[5.0.0] Push Subscription Observer - API update
@emawby
Merge pull request #1258 from OneSignal/5.0.0/notification_foreground…
412e65a 
…_listener_api

[5.0.0] Notification Foreground Listener - API update
@emawby emawby merged commit 58791ef into 5.0.0/notification_permission_observer_api May 1, 2023
@emawby emawby deleted the 5.0.0/iam_lifecycle_listener_api branch May 1, 2023 20:28
@emawby emawby mentioned this pull request May 1, 2023
Merged
@nan-li nan-li mentioned this pull request Jul 19, 2023
Merged
18 tasks
nan-li pushed a commit that referenced this pull request Oct 30, 2023
@emawby @nan-li
Merge pull request #1257 from OneSignal/5.0.0/iam_lifecycle_listener_api
2685944 
[5.0.0] IAM Lifecycle Listener - API update
nan-li pushed a commit that referenced this pull request Oct 30, 2023
@emawby @nan-li
Merge pull request #1257 from OneSignal/5.0.0/iam_lifecycle_listener_api
25099ca 
[5.0.0] IAM Lifecycle Listener - API update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@emawby emawby emawby approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nan-li @emawby