Fix Incorrect Notification Opened Events (#357)

* Fix Incorrect Notification Opened Events

• The SDK was previously incorrectly sending 'notification opened' HTTP requests when notifications were received
• Fixed the issue so that this event is only triggered when the user opens a notification
• Added a test to ensure it doesn't occur in the future
• In some tests, the SDK was previously checking to make sure the last HTTP request was of a certain type, which caused some race condition issues. Changed it so that it will now check to see if the request was executed recently (since some other event, ie. registering the user, may occur but this shouldn't break the test)

* Fix Notification Opened Issue

• Changes the SDK so that it will correctly send a notification opened HTTP request even if the display type == None and the notification has content.
• However, this change also makes it so that the initialization handleNotificationOpened completion block does not get called

Author: Nightsd01

iOS_SDK/OneSignalSDK/Source/OneSignal.m

@@ -1404,7 +1404,7 @@ + (void)setLastnonActiveMessageId:(NSString*)value { _lastnonActiveMessageId = v
 //  - 2. Notification received
 //    - 2A. iOS 9  - Notification received while app is in focus.
 //    - 2B. iOS 10 - Notification received/displayed while app is in focus.
-+ (void)notificationOpened:(NSDictionary*)messageDict isActive:(BOOL)isActive {
++ (void)notificationReceived:(NSDictionary*)messageDict isActive:(BOOL)isActive wasOpened:(BOOL)opened {
 
     if (!app_id)
         return;
@@ -1439,18 +1439,16 @@ + (void)notificationOpened:(NSDictionary*)messageDict isActive:(BOOL)isActive {
         // App is active and a notification was received without inApp display. Display type is none or notification
         // Call Received Block
         [OneSignalHelper handleNotificationReceived:self.inFocusDisplayType];
-        
-        // Notify backend that user opened the notification
-        NSString *messageId = [customDict objectForKey:@"i"];
-        [OneSignal submitNotificationOpened:messageId];
     } else {
         // Prevent duplicate calls
         let newId = [self checkForProcessedDups:customDict lastMessageId:_lastnonActiveMessageId];
         if ([@"dup" isEqualToString:newId])
             return;
         if (newId)
             _lastnonActiveMessageId = newId;
-        
+    }
+    
+    if (opened) {
         //app was in background / not running and opened due to a tap on a notification or an action check what type
         NSString* actionSelected = NULL;
         OSNotificationActionType type = OSNotificationActionTypeOpened;
@@ -1464,8 +1462,7 @@ + (void)notificationOpened:(NSDictionary*)messageDict isActive:(BOOL)isActive {
         }
 
         // Call Action Block
-        [OneSignalHelper handleNotificationAction:type actionID:actionSelected displayType:OSNotificationDisplayTypeNotification];
-        [OneSignal handleNotificationOpened:messageDict isActive:isActive actionType:type displayType:OSNotificationDisplayTypeNotification];
+        [OneSignal handleNotificationOpened:messageDict isActive:isActive actionType:type displayType:OneSignal.inFocusDisplayType];
     }
 }
 
@@ -1504,7 +1501,11 @@ + (void)handleNotificationOpened:(NSDictionary*)messageDict
 
     //Call Action Block
     [OneSignalHelper lastMessageReceived:messageDict];
-    [OneSignalHelper handleNotificationAction:actionType actionID:actionID displayType:displayType];
+    
+    //ensures that if the app is open and display type == none, the handleNotificationAction block does not get called
+    if (displayType != OSNotificationDisplayTypeNone || (displayType == OSNotificationDisplayTypeNone && !isActive)) {
+        [OneSignalHelper handleNotificationAction:actionType actionID:actionID displayType:displayType];
+    }
 }
 
 + (BOOL)shouldPromptToShowURL {
@@ -1669,9 +1670,13 @@ + (BOOL)remoteSilentNotification:(UIApplication*)application UserInfo:(NSDiction
     // Method was called due to a tap on a notification - Fire open notification
     else if (application.applicationState != UIApplicationStateBackground) {
         [OneSignalHelper lastMessageReceived:userInfo];
+        
         if (application.applicationState == UIApplicationStateActive)
             [OneSignalHelper handleNotificationReceived:OSNotificationDisplayTypeNotification];
-        [OneSignal notificationOpened:userInfo isActive:NO];
+        
+        if (![OneSignalHelper isRemoteSilentNotification:userInfo])
+            [OneSignal notificationReceived:userInfo isActive:NO wasOpened:YES];
+        
         return startedBackgroundJob;
     }
     // content-available notification received in the background - Fire handleNotificationReceived block in app
@@ -1697,7 +1702,7 @@ + (void)processLocalActionBasedNotification:(UILocalNotification*) notification
         return;
 
     let isActive = [[UIApplication sharedApplication] applicationState] == UIApplicationStateActive;
-    [OneSignal notificationOpened:userInfo isActive:isActive];
+    [OneSignal notificationReceived:userInfo isActive:isActive wasOpened:YES];
 
     // Notification Tapped or notification Action Tapped
     if (!isActive)

iOS_SDK/OneSignalSDK/Source/UIApplicationDelegate+OneSignal.m

@@ -40,7 +40,7 @@ + (void) didRegisterForRemoteNotifications:(UIApplication*)app deviceToken:(NSDa
 + (void) handleDidFailRegisterForRemoteNotification:(NSError*)error;
 + (void) updateNotificationTypes:(int)notificationTypes;
 + (NSString*) app_id;
-+ (void) notificationOpened:(NSDictionary*)messageDict isActive:(BOOL)isActive;
++ (void)notificationReceived:(NSDictionary*)messageDict isActive:(BOOL)isActive wasOpened:(BOOL)opened;
 + (BOOL) remoteSilentNotification:(UIApplication*)application UserInfo:(NSDictionary*)userInfo completionHandler:(void (^)(UIBackgroundFetchResult))completionHandler;
 + (void) processLocalActionBasedNotification:(UILocalNotification*) notification identifier:(NSString*)identifier;
 + (void) onesignal_Log:(ONE_S_LOG_LEVEL)logLevel message:(NSString*) message;
@@ -182,7 +182,7 @@ - (void)oneSignalReceivedRemoteNotification:(UIApplication*)application userInfo
     [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"oneSignalReceivedRemoteNotification:userInfo:"];
 
     if ([OneSignal app_id])
-        [OneSignal notificationOpened:userInfo isActive:[application applicationState] == UIApplicationStateActive];
+        [OneSignal notificationReceived:userInfo isActive:[application applicationState] == UIApplicationStateActive wasOpened:true];
 
     if ([self respondsToSelector:@selector(oneSignalReceivedRemoteNotification:userInfo:)])
         [self oneSignalReceivedRemoteNotification:application userInfo:userInfo];
@@ -201,7 +201,7 @@ - (void) oneSignalRemoteSilentNotification:(UIApplication*)application UserInfo:
 
     if ([OneSignal app_id]) {
         if ([UIApplication sharedApplication].applicationState == UIApplicationStateActive && userInfo[@"aps"][@"alert"])
-            [OneSignal notificationOpened:userInfo isActive:YES];
+            [OneSignal notificationReceived:userInfo isActive:YES wasOpened:NO];
         else
             startedBackgroundJob = [OneSignal remoteSilentNotification:application UserInfo:userInfo completionHandler:callExistingSelector ? nil : completionHandler];
     }

iOS_SDK/OneSignalSDK/Source/UNUserNotificationCenter+OneSignal.m

@@ -46,7 +46,7 @@
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
 @interface OneSignal (UN_extra)
-+ (void)notificationOpened:(NSDictionary*)messageDict isActive:(BOOL)isActive;
++ (void)notificationReceived:(NSDictionary*)messageDict isActive:(BOOL)isActive wasOpened:(BOOL)opened;
 @end
 
 // This class hooks into the following iSO 10 UNUserNotificationCenterDelegate selectors:
@@ -159,9 +159,10 @@ - (void)onesignalUserNotificationCenter:(UNUserNotificationCenter *)center
         default: break;
     }
 
-    if ([OneSignal app_id]) {
-        [OneSignal notificationOpened:notification.request.content.userInfo isActive:YES];
-    }
+    let notShown = OneSignal.inFocusDisplayType == OSNotificationDisplayTypeNone && notification.request.content.body != nil;
+    
+    if ([OneSignal app_id])
+        [OneSignal notificationReceived:notification.request.content.userInfo isActive:YES wasOpened:notShown];
 
     // Call orginal selector if one was set.
     if ([self respondsToSelector:@selector(onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler:)])
@@ -239,7 +240,8 @@ + (void) processiOS10Open:(UNNotificationResponse*)response {
     let userInfo = [OneSignalHelper formatApsPayloadIntoStandard:response.notification.request.content.userInfo
                                                       identifier:response.actionIdentifier];
 
-    [OneSignal notificationOpened:userInfo isActive:isActive];
+    [OneSignal notificationReceived:userInfo isActive:isActive wasOpened:YES];
+    
 }
 
 // Calls depercated pre-iOS 10 selector if one is set on the AppDelegate.

iOS_SDK/OneSignalSDK/UnitTests/EmailTests.m

@@ -415,7 +415,6 @@ - (void)testEmailSubscriptionObserver {
 
 //when the user is logged in with email, on_focus requests should be duplicated for the email player id as well
 - (void)testOnFocusEmailRequest {
-    
     [UnitTestCommonMethods runBackgroundThreads];
 
     [self setupEmailTest];
@@ -435,7 +434,7 @@ - (void)testOnFocusEmailRequest {
 
     [UnitTestCommonMethods runBackgroundThreads];
 
-    XCTAssertTrue([OneSignalClientOverrider.lastHTTPRequestType isEqualToString:NSStringFromClass([OSRequestOnFocus class])]);
+    XCTAssertTrue([OneSignalClientOverrider hasExecutedRequestOfType:[OSRequestOnFocus class]]);
     XCTAssertEqual(OneSignalClientOverrider.networkRequestCount, 1);
 
     [OneSignalClientOverrider reset:self];
@@ -469,7 +468,7 @@ - (void)testOnFocusEmailRequest {
     [UnitTestCommonMethods runBackgroundThreads];
 
     // on_focus should fire off two requests, one for the email player ID and one for push player ID
-    XCTAssertTrue([OneSignalClientOverrider.lastHTTPRequestType isEqualToString:NSStringFromClass([OSRequestOnFocus class])]);
+    XCTAssertTrue([OneSignalClientOverrider hasExecutedRequestOfType:[OSRequestOnFocus class]]);
     XCTAssertEqual(OneSignalClientOverrider.networkRequestCount, 2);
 
     [OneSignalClientOverrider setRequiresEmailAuth:false];

iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.h

@@ -41,5 +41,6 @@
 +(void)runBackgroundThreads;
 +(NSString *)lastHTTPRequestType;
 +(void)setRequiresEmailAuth:(BOOL)required;
++(BOOL)hasExecutedRequestOfType:(Class)type;
 @end
 

iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m

@@ -48,6 +48,7 @@ @implementation OneSignalClientOverrider
 static dispatch_queue_t executionQueue;
 static NSString *lastHTTPRequestType;
 static BOOL requiresEmailAuth = false;
+static NSMutableArray<NSString *> *executedRequestTypes;
 
 + (void)load {
     serialMockMainLooper = dispatch_queue_create("com.onesignal.unittest", DISPATCH_QUEUE_SERIAL);
@@ -59,6 +60,7 @@ + (void)load {
 
     executionQueue = dispatch_queue_create("com.onesignal.execution", NULL);
 
+    executedRequestTypes = [[NSMutableArray alloc] init];
 }
 
 - (void)overrideExecuteSimultaneousRequests:(NSDictionary<NSString *, OneSignalRequest *> *)requests withSuccess:(OSMultipleSuccessBlock)successBlock onFailure:(OSMultipleFailureBlock)failureBlock {
@@ -69,6 +71,8 @@ - (void)overrideExecuteSimultaneousRequests:(NSDictionary<NSString *, OneSignalR
     __block NSMutableDictionary<NSString *, NSDictionary *> *results = [NSMutableDictionary new];
 
     for (NSString *key in requests.allKeys) {
+        [executedRequestTypes addObject:NSStringFromClass([requests[key] class])];
+        
         [OneSignalClient.sharedClient executeRequest:requests[key] onSuccess:^(NSDictionary *result) {
             results[key] = result;
             dispatch_semaphore_signal(semaphore);
@@ -90,6 +94,8 @@ - (void)overrideExecuteSimultaneousRequests:(NSDictionary<NSString *, OneSignalR
 }
 
 - (void)overrideExecuteRequest:(OneSignalRequest *)request onSuccess:(OSResultSuccessBlock)successBlock onFailure:(OSFailureBlock)failureBlock {
+    [executedRequestTypes addObject:NSStringFromClass([request class])];
+    
     if (executeInstantaneously) {
         [OneSignalClientOverrider finishExecutingRequest:request onSuccess:successBlock onFailure:failureBlock];
     } else {
@@ -127,6 +133,14 @@ + (void)finishExecutingRequest:(OneSignalRequest *)request onSuccess:(OSResultSu
     }
 }
 
++(BOOL)hasExecutedRequestOfType:(Class)type {
+    for (id requestType in executedRequestTypes)
+        if ([requestType isEqualToString:NSStringFromClass(type)])
+            return true;
+    
+    return false;
+}
+
 +(dispatch_queue_t)getHTTPQueue {
     return executionQueue;
 }
@@ -145,6 +159,7 @@ +(void)reset:(XCTestCase*)testInstance {
     networkRequestCount = 0;
     lastUrl = nil;
     lastHTTPRequest = nil;
+    [executedRequestTypes removeAllObjects];
 }
 
 +(void)setLastHTTPRequest:(NSDictionary*)value {

iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m

@@ -1244,14 +1244,40 @@ - (void)receivedCallbackWithButtonsWithUserInfo:(NSDictionary *)userInfo {
     UNUserNotificationCenter *notifCenter = [UNUserNotificationCenter currentNotificationCenter];
     let notifCenterDelegate = notifCenter.delegate;
 
+    UIApplicationOverrider.currentUIApplicationState = UIApplicationStateInactive;
+    
     //iOS 10 calls UNUserNotificationCenterDelegate method directly when a notification is received while the app is in focus.
     [notifCenterDelegate userNotificationCenter:notifCenter
                         willPresentNotification:[notifResponse notification]
                           withCompletionHandler:^(UNNotificationPresentationOptions options) {}];
 
+    [UnitTestCommonMethods runBackgroundThreads];
+    
     XCTAssertEqual(recievedWasFire, true);
 }
 
+/*
+    There was a bug where receiving notifications would cause OSRequestSubmitNotificationOpened
+    to fire, even though the notification had not been opened
+*/
+- (void)testReceiveNotificationDoesNotSubmitOpenedRequest {
+    [OneSignalClientOverrider reset:self];
+    
+    let newFormat = @{@"aps": @{@"content_available": @1},
+                      @"os_data": @{
+                              @"i": @"b2f7f966-d8cc-11e4-bed1-df8f05be55ba",
+                              @"buttons": @{
+                                      @"m": @"alert body only",
+                                      @"o": @[@{@"i": @"id1", @"n": @"text1"}]
+                                      }
+                              }
+                      };
+    
+    [self receivedCallbackWithButtonsWithUserInfo:newFormat];
+    
+    XCTAssertFalse([OneSignalClientOverrider hasExecutedRequestOfType:[OSRequestSubmitNotificationOpened class]]);
+}
+
 - (void)testReceivedCallbackWithButtonsWithNewFormat {
     let newFormat = @{@"aps": @{@"content_available": @1},
                       @"os_data": @{