Skip to content

Conversation

@Nightsd01
Copy link
Contributor

@Nightsd01 Nightsd01 commented Mar 12, 2018

• Adds a setting called kOSSettingsKeyPromptBeforeOpeningPushURL which allows developers to control what happens when a user taps a notification with a web URL. If this setting is set to true, the SDK will prompt the user with an alert asking if they want to open the website or not. By default, this setting is false.
• Adds ability for postNotification() to preprocess NSDate objects so that developers can add a delay without having to convert to strings first
• The SDK will now always convert email auth hash tokens to be lowercase for consistency
syncHashedEmail() is now officially deprecated: added a deprecation notice.
• Removed use of private Apple API's which caused problems in rare circumstances. This private API was implemented in order to call deprecated methods for developer convenience. However we have removed this functionality in order to avoid problems with the app review process. The SDK will now print an error to the console if a developer implements these methods.
• Fixes an issue that would have caused problems parsing notifications in a new payload format.
• Fixed some issues that caused crashes in iOS 7.


This change is Reviewable

Nightsd01 added 10 commits March 2, 2018 13:30
• Some users were trying to use the `send_after` field to set delay dates for notifications, and were using NSDate objects to specify the date
• NSJSONSerialization cannot automatically convert dates to strings, so this commit adds automatic date conversion for the `postNotification()` method.
• The UnitTests target did not have the correct file permission to use the new date conversion NSMutableDictionary category...
• Changes the SDK to always convert email auth hash tokens to be lowercase to prevent issues.
• It's been decided to perform case insensitive hash token comparisons on the backend instead of performing this conversion on the SDK.
• Removed the lowercase string conversion. This also broke some tests in the previous commit which weren't changed to convert the hash token to be lowercase before comparing.
• We are deprecating (and will soon remove) the syncHashedEmail() method from the SDK.
• Changed the SDK to ensure that PostNotification() callbacks are always called on the main thread to help developers prevent UI bugs
• Adds a test to verify that the OSEmailSubscriptionState and OSEmailSubscriptionStateChanges instance description strings are properly formatted
* Fixes an issue parsing the new notification payload format
* Adds a new setting, kOSSettingsKeyPromptBeforeOpeningPushURL. This setting determines what happens when the user taps a push notification containing a launch URL. If the setting = true, the SDK will show a popup asking if the user wants to open the URL.
* Fixes an issue with the Swift demo where all tapped notifications would always open the Red view controller (which should only happen in response to tapping a button with id = id1)
• As a convenience, the SDK used to call some deprecated AppDelegate methods (such as application:didReceiveLocalNotification:).
• However some developers have raised concerns that this requires the use of private API's
• Since this should be relatively rare, we are removing this functionality and will print warnings whenever a developer implements these methods
• Fixes the alert so that instead of printing the entire URL address (which could be quite long), it prints only the scheme and the host (ie. http://google.com).
• Using localized strings
@Nightsd01 Nightsd01 requested a review from jkasten2 March 12, 2018 21:53
@jkasten2
Copy link
Member

Review status: 0 of 15 files reviewed at latest revision, 4 unresolved discussions.


iOS_SDK/OneSignalSDK/Source/NSMutableDictionary+OneSignal.h, line 6 at r1 (raw file):

//
//  Created by Brad Hesse on 3/2/18.
//  Copyright © 2018 Hiptic. All rights reserved.

Fix copyright on new files


iOS_SDK/OneSignalSDK/Source/OneSignal.m, line 1397 at r1 (raw file):

    onesignal_Log(ONE_S_LL_VERBOSE, @"notificationOpened:isActive called!");
    
    NSLog(@"Notification opened with messageDict: %@, isActive: %i", messageDict, isActive);

Remove debug logging.


iOS_SDK/OneSignalSDK/Source/OneSignal.m, line 1470 at r1 (raw file):

                     displayType:(OSNotificationDisplayType)displayType {
    NSDictionary* customDict = [messageDict objectForKey:@"os_data"];
    if (customDict == nil)

We can remove these 2 lines now if we are switching to use the elvis operator here.

NSDictionary* customDict = [messageDict objectForKey:@"os_data"];
    if (customDict == nil)

iOS_SDK/OneSignalSDK/Source/OneSignalWebView.m, line 68 at r1 (raw file):

-(void)dismiss:(id)sender {
    NSLog(@"DISMISS CALLED");

Remove debugging


Comments from Reviewable

• Fix NSMutableDictionary category copyright
• Cleans up unnecessary log statements
• Remove an unnecessary log statement
@jkasten2
Copy link
Member

:lgtm:


Reviewed 11 of 15 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.


Comments from Reviewable

@Nightsd01 Nightsd01 merged commit 5ed4ff3 into master Mar 13, 2018
@Jeasmine Jeasmine deleted the sdk_improvements branch May 12, 2020 22:00
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.

3 participants