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

[User Model] Fixes #626

Merged
merged 18 commits into from
Aug 17, 2023
Merged

[User Model] Fixes #626

merged 18 commits into from
Aug 17, 2023

Conversation

shepherd-l
Copy link
Contributor

@shepherd-l shepherd-l commented Jul 27, 2023

Description

One Line Summary

Fixes for user model for GA

Details

  • Removes SetLaunchURLsInApp iOS feature
  • Updates native Android and iOS SDKs to 5.0
  • Removes async from Location request permission and renames method name to RequestPermission
  • Fixes outcome method calls on Android to call down to native methods correctly
  • Fixes completion check for the "Copy Android plugin to Assets" setup step
  • Updates default OneSignal Android notification icons to new logo
  • Updates broken documentation links to v11
  • Bump license copyright year to 2023
  • Updates guide for minimum requirements to Android 5 and iOS 11

Motivation

Update native iOS SDK to 5.0.1 and match API removal of launch urls in app from beta-06. Removal explained here

Update native Android SDK to 5.0.0 to fix outcomes

Testing

Manual testing

Tested OneSignal initialization, app build with Unity 2021.3.25f1 of the OneSignal example app on a emulated Pixel 4 with Android 12 and physical iPhone 12 with iOS 15.5.

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
  • 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

@@ -147,7 +147,6 @@ The SDK is still accessible via a `OneSignal` static class, it provides access t
| `void Initialize(string appId)` | *Initialize the OneSignal SDK. This should be called during startup of the application.* |
| `void Login(string externalId, string jwtBearerToken = null)` | *Login to OneSignal under the user identified by the [externalId] provided. The act of logging a user into the OneSignal SDK will switch the [user] context to that specific user.*<br><br>- *If the [externalId] exists the user will be retrieved and the context set from that user information. If operations have already been performed under a guest user, they* ***will not*** *be applied to the now logged in user (they will be lost).*<br>- *If the [externalId] does not exist the user will be created and the context set from the current local state. If operations have already been performed under a guest user those operations* ***will*** *be applied to the newly created user.*<br><br>***Push Notifications and In App Messaging***<br>*Logging in a new user will automatically transfer push notification and in app messaging subscriptions from the current user (if there is one) to the newly logged in user. This is because both Push and IAM are owned by the device.* |
| `void Logout()` | *Logout the user previously logged in via [login]. The [user] property now references a new device-scoped user. A device-scoped user has no user identity that can later be retrieved, except through this device as long as the app remains installed and the app data is not cleared.* |
| `void SetLaunchURLsInApp(bool launchInApp)` | ***Note:*** *This method is for iOS only<br>This method can be used to set if launch URLs should be opened in Safari or within the application. Set to true to launch all notifications with a URL in the app instead of the default web browser. Make sure to call SetLaunchURLsInApp before the initialize call.* |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets also call out this removal somewhere in the guide. In the iOS guide I said this in the initialization section

Remove any usages of setLaunchURLsInApp as the method and functionality has been removed.

@shepherd-l shepherd-l requested a review from emawby August 11, 2023 19:15
@shepherd-l shepherd-l changed the title [User Model] Remove SetLaunchURLsInApp [User Model] Fixes Aug 14, 2023
@shepherd-l shepherd-l merged commit 8da0fc1 into user-model/main Aug 17, 2023
1 check passed
@shepherd-l shepherd-l deleted the removeSetLaunchUrlsInApp branch August 17, 2023 17:58
jinliu9508 pushed a commit that referenced this pull request Feb 23, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Mar 1, 2024
jinliu9508 pushed a commit that referenced this pull request Mar 4, 2024
jinliu9508 pushed a commit that referenced this pull request Mar 4, 2024
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