Email #336
Merged
Email #336
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
• Begins to add support for email in the iOS SDK
• Changed the SDK so that for certain HTTP requests (ie. registering a user or sending location), the SDK will send two HTTP requests (separate for Push user ID and Email user ID). • The method to execute multiple HTTP requests will wait until both finish to trigger a callback. Each request is passed in with a key string. The callback returns key/value pairs where the key is associated with the same key. • Added tests covering the additional functionality
• Included app_id as a parameter for the email logout request • Added a test for email logout • Fixed an issue where setEmail: did not correctly set the current subscription state email user ID
• Got rid of a thread synchronization line that wasn't necessary and slowed down unit tests by nearly 2x.
• Began (but did not finish) the work of splitting up unit tests so that they are no longer all in one giant file/class and are a bit more modular • Fixed the Create/Edit device REST requests in the application which were using incorrect 'email' vs. 'identifier' parameter banes
• This implements a system so that the SDK will reattempt failed network requests under two possible situations: 1. The HTTP status code is 500+, indicating a server error. 2. The HTTP status code is 0, which in iOS indicates no internet connection • The re-attempt is only made one time, after waiting 15 seconds. • The success/fail callbacks are not called until the re-attempt is made. If the second attempt fails, the failure callback is called.
• The re-attempt define statements were incorrect due to debugging, fixed them to be the correct values.
• Changes the SDK so that multiple re-attempts can be made for failed HTTP requests • Currently, all failed requests (HTTP code 500+) will be attempted 3 times total.
• Built out most of the Email feature for the SDK • Improved the failed HTTP request re-attempt code to be a bit more readable • Put a lot of commonly used strings into a single CommonDefines header file • Made it so that calls to `setEmail:` will be delayed under two conditions: 1. The player_id is not set, meaning `registerUserInternal()` has not yet finished 2. No email_auth_code was passed in as a parameter to `setEmail:` and the GetIOSParams request has not yet finished.
• Added convenience methods to set/logout/etc. from email without completion blocks • Added another test to cover unauthenticated email • Fixed some errors with tests • Changed verbose logging with HTTP requests to log HTTP parameters as pretty-printed JSON for easier debugging with rest clients • Persists the email_auth_token and email address in NSUserDefaults for use to authenticate in future sessions • This commit now correctly sends in the email_auth_hash in the correct HTTP requests (send location, send tags, etc) • The iOS Params file is now downloaded at launch in all cases (since it now effects email)
• Made some changes to parameter names for a few endpoints to match what the backend requires • The backend needs the client to call UpdateDeviceToken even when the user just receives a new email player_id. Added this HTTP request in the setEmail: method. • Adjusted unit tests to account for the fact that all successful calls to setEmail: will end with a call to UpdateDeviceToken in one way or another • Added some more checks to the unit tests for Email to ensure it is correctly setting the email and also logs out correctly • Adjusted the dev app to include an email text field and Set Email + Logout Email buttons to make testing easier.
• Added "carrier" as a parameter on registration. Uses reflection to access the Core Telephony framework (if available) and uses it to attach the carrier name to the body of the request. • Fixes an issue with unauthenticated email users where some requests (on_session, update location, etc) weren't being called for Email • Added emailUserId and emailAddress as parameters in the dictionary & string representations of OSSubscriptionState
• Changed the OSSubscriptionState `description` property to include Email & Email player id in the previous commit. • This broke a test checking the `description` property. • Removed this description value as it seems somewhat superfluous to test.
• Because some developers will probably make UI adjustments when the completion block for setEmail() is called, I have changed the SDK to always call the success/fail completion blocks on the main thread.
• Adds Email functionality to the Swift demo project, both the ability to setUnauthenticatedEmail() and logoutEmail() • Fixes some minor compiler warnings due to optional string variables • Adds warning to the syncEmail() method in the SDK so that it will print a warning if the email is invalid or nil. Otherwise developers may not notice that the email isn't actually getting synced since no error is returned.
• Checks to make sure that the SDK rejects invalid emails.
• Added tests to ensure the SDK correctly handles the require_email_auth parameter from iOS params • If require_email_auth == true, the SDK should reject attempts to use `setUnauthenticatedEmail:`
• Puts commonly used email test setup into its own method to avoid duplication • Adds more conditions to the testRequiresEmailAuth: test
• Unit tests used hardcoded URL's for checking HTTP request correctness • Unit tests now build use the same compiler definition to get the server URL & api version as the SDK does. • Moved the URL & API version definitions into the Common Definitions header file
• Removed email-related properties and functionality from OSSubscriptionState and created a separate Email Subscription Observer (OSEmailSubscription) • Created a new protocol so that developers can add email observers and get updated whenever the values change. • Added tests for the new email subscription observer • Fixed an issue where a change in the email player ID in response to registration/device token update would not have been reflected in the SDK
• Checks to make sure the SKPaymentQueue class responds to the canMakePayments() selector before calling it. • Cleans up email-related OneSignal.h header declarations, adds some comments, and moves unnecessary typedefs to a more appropriate header file (OneSignalClient.h)
• Mistakenly committed staging server as the OneSignal server URL in common defines
• Adds an Email subscription state property to the result of getPermissionSubscriptionState() • This allows developers to ask if the user is subscribed to onesignal email.
• If setEmail() is called, and then gets called later on with the same parameters (same email address), there is no need to perform another HTTP request • This was implemented in a previous commit but would have failed if using email in an unauthenticated state, since emailAuthToken is nil. • The NSString comparison would return false if one/both strings were nil • Fixed it so that if both strings are nil (unauthenticated state), it will correctly identify it as a duplicate request if the emails match (existing email and email passed in as parameter to setEmail())
• The SDK was calling a method to download the iOS parameters file whenever init() was called on the OneSignal SDK • Since some wrapper SDK's will call init() twice, this caused an unnecessary duplicate call to downloadIOSParams • Resolved by using a static boolean flag to prevent duplicate calls • Adjusted some of the tests to reflect this change.
Member
Review status: 0 of 40 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed. iOS_SDK/OneSignalSDK/Source/OneSignal.m, line 519 at r1 (raw file):
Lets use our iOS_SDK/OneSignalSDK/Source/OneSignal.m, line 529 at r1 (raw file):
Remove logging iOS_SDK/OneSignalSDK/Source/OSEmailSubscription.h, line 6 at r1 (raw file):
Change Copyright OneSignal, matching our other files. Comments from Reviewable |
• We've decided to rename setUnauthenticatedEmail: to setEmail: (but without an emailAuthToken parameter)
• Some wrapper SDK's pass in NSNull instead of nil, which caused an issue with the setEmail: method. • Added a check for this condition.
• Check to make sure the SDK is correctly sending on_focus events for both email and push player ID's
• Adds tests to ensure that: • The register user request will send out two requests, one for email player ID (if it exists) and one for push • on_focus events also send out two requests, one for email & push
• CI build appears to be failing for a strange issue on line 805 of OneSignal.m callbackSet.failureBlock(error); • Added some parentheses to ensure the correct codepath is being followed.
Contributor
Author
|
Added tests, changed SendPurchases to also send these events to the email Player ID as well (if it exists) Comments from Reviewable |
• Removes extraneous log statements • Fixes header comments for OSEmailSubscription class
• Travis build is failing due to an 'undeclared identifier' error. However this doesn't appear to be correct, attempting to change Travis yml settings. • Remove an unnecessary log statement
• Travis doesn't appear to have support for running tests targeted at iOS 11.2. Changed config file back to iOS 11.0
• Remove the 'error' variable entirely. • Travis CI complains this variable is undeclared.....
Member
|
Reviewed 31 of 40 files at r1, 1 of 2 files at r2, 9 of 9 files at r3. Comments from Reviewable |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
• Adds email to the iOS SDK
• Adds
setEmail:,setUnauthenticatedEmail:,logoutEmail:, and some convenience methods• Adds a separate email observer class & subscription state model
• Adds tests to cover email functionality
• Adds HTTP request re-attempts when receiving status code 500~
• Adds email to the Swift demo
• Adds 'carrier name' as a parameter on registration using reflection to access core telephony framework if available
This change is