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

Major Release #1044

Merged
merged 73 commits into from
Jan 6, 2021
Merged

Major Release #1044

merged 73 commits into from
Jan 6, 2021

Conversation

rgomezp
Copy link
Contributor

@rgomezp rgomezp commented Sep 16, 2020

Major Release

This PR upgrades this SDK to use the native SDK versions for iOS & Android of 3.0 & 4.0 respectively.

The migration guide can be found here.

API Additions

  • setAppId function
  • Observers:
    • addPermissionObserver
    • addSubscriptionObserver
    • addEmailSubscriptionObserver
  • setNotificationWillShowInForegroundHandler
  • setNotificationOpenedHandler
  • disablePush
  • getDeviceState
  • deleteTags (iOS) <- this was missed in the iOS bridge in previous releases

API Removals

  • configure (deprecated)
  • init (no longer needed, this is done automatically)
  • addEventListener (replaced by observers)
  • getPermissionSubscriptionState
  • checkPermissions

Other Changes

  • general refactoring
  • clean up of confusing event strings
  • modularization
  • moved code into new src directory
  • created EventManager helper class to manage events, listeners, and handlers related to those events
  • created NotificationReceivedEvent class to encapsulate the event & package it with the platform-specific payload as well as add a complete function prior to passing into the handler for the notificationWillShowInForeground event
  • created helpers file for various miscellaneous helper functions
  • updated example app

Before merging

  • Remove commit intended to facilitate diff-checking index.js since it was moved into a different directory
  • Update deleteTags reference in OneSignal docs

@rgomezp rgomezp force-pushed the major-release branch 4 times, most recently from 5df0a89 to fb25b1e Compare September 17, 2020 23:54
@rgomezp rgomezp force-pushed the major-release branch 6 times, most recently from bbf86b4 to 3fb0bc1 Compare October 1, 2020 00:23
@rgomezp rgomezp force-pushed the major-release branch 4 times, most recently from fc1fd70 to 443b37c Compare October 7, 2020 23:57
@rgomezp rgomezp marked this pull request as ready for review October 8, 2020 00:22
@rgomezp rgomezp requested a review from emawby October 8, 2020 18:41
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Duplicated index.js, some functions need to be cleaned up yet, some questions on implementation choices, and permission naming between iOS and Android.

I left most comments on index.js vs src/index.js but wasn't sure which one we want to keep

examples/RNOneSignal/App.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
ios/RCTOneSignal/RCTOneSignalEventEmitter.m Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@rgomezp rgomezp force-pushed the major-release branch 2 times, most recently from c108157 to 05c71b9 Compare October 8, 2020 23:03
ios/RCTOneSignal/RCTOneSignalEventEmitter.m Outdated Show resolved Hide resolved
ios/RCTOneSignal/RCTOneSignalEventEmitter.m Outdated Show resolved Hide resolved
examples/RNOneSignal/App.js Outdated Show resolved Hide resolved
examples/RNOneSignal/App.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
ios/RCTOneSignal/RCTOneSignal.m Outdated Show resolved Hide resolved
ios/RCTOneSignal/RCTOneSignal.m Outdated Show resolved Hide resolved
ios/RCTOneSignal/RCTOneSignalEventEmitter.m Outdated Show resolved Hide resolved
src/events.js Show resolved Hide resolved
@rgomezp rgomezp force-pushed the major-release branch 3 times, most recently from 86f6426 to dc53fc6 Compare October 16, 2020 00:24
* The `onesignal_app_id` and `onesignal_google_project_number` placehold
values were droped from OneSignal-Android-SDK 4.0.0
…nesignal_manifest_placehold_values

Cleanup - Android - Remove dropped onesignal manifest placeholder values
rgomezp and others added 25 commits December 9, 2020 15:49
Motivation: the native iOS SDK changed the method name `onesignal_Log` to `onesignalLog` for consistency w/the native Android SDK method.
Upgrade to use version 4 of the Android SDK, version 3 of the iOS SDK
Motivation: since we have added user verification to external id setting and modification, we want to surface the error via the function callback by passing it to the JS side in case the error has relevant details.

E.g: "External User Id authentication (auth token) is set to REQUIRED for this application. Please provide an auth token from your backend server or change the setting in the OneSignal dashboard."
Native SDK renamed the outcome event variable to `OSOutcomeEvent`
Motivation: this function was renamed to be clearer
Motivation: this should never have been required
Motivation: tags can be removed by sending a blank string. Here we're updating the logic to allow for that
Motivation: older devices may have older versions of Chrome which don't have support for ES6 syntax
Motivation:
Rename `cancelNotification` to `removeNotification` for clarity
Motivation: update the types to reflect recent changes for user verification for setExternalUserId
Motivation: In the typings, the values for Authorized and Denied were backwards
Platform.OS is assigned to 'ios' in OSNotification.js and it affects all android settings in apps.
Motivation: there are multiple places in native `OneSignalOutcomeEventsController` where we call `success(nil)`. One example is calling `sendUniqueOutcome` multiple times. We should check that the `outcome` object is non-nil before passing it back to JS with the `jsonRepresentation` native method.
Motivation: 
- [Context](https://bit.ly/2WJbxHG)
- Adds some logging in the iOS bridge to address the case that `outcome` object is nil
Motivation: we want Android bridge to match the iOS bridge. For this reason, we are removing the invocation of the callback altogether in the case that the outcome object is null.
* Initial Commit - Entire TS Example App

* Update Yarn Lock to use latest Beta (beta 5)

* Delete Non-TS Example App

Motivation: moving forward we will only maintain the TS version

* Refactor TS Example App

Includes buttons for many functions

* OSConsole to log events and variables

* Finished Setting up Demo App

* Update .gitignore

Motivation: update .gitignore to ignore files from new example app

* Commit XCode Config & Files

* Commit Info.plist, Podfile lock file, and other project files

* Add Error Handling for Getting Trigger Value For Key

Motivation: Example app - add error handling for rejected `getTriggerValueForKey` promise.

Also, remove JSON.stringify around second param to logging function as value is already stringified automatically inside that function.

* Add Clear Handlers to Component Will Unmount

Motivation: to avoid aggregating handlers/listeners, it is good practice to remove them before a component unmounts in the `componentWillUnmount`

* Cross-Platform Conditional Formatting for Console Text

* Add Location Permissions to Config Files

Motivation: we want the example app to work with location out of the box. See reference: https://documentation.onesignal.com/docs/sdk-reference#location-data
* Update package.json files to 4.0.0

* Update Native SDK Versions

* Remove native iOS files

Motivation: these files were previously there in order to support non-cocoapods integrations. We are removing it here in the major release and will no longer be maintained moving forward.

The manual linking process is likely done by a small portion of developers and introduces more room for error. Cocoapods is pretty standard at this point and can always be added back later or via SwiftPM in the future.
@rgomezp rgomezp merged commit 2c78d41 into master Jan 6, 2021
@rgomezp rgomezp deleted the major-release branch August 18, 2023 20:18
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.

6 participants