From 31704702131fec6f2fd821581c67116a4996fc38 Mon Sep 17 00:00:00 2001 From: emawby Date: Tue, 15 Jun 2021 14:23:47 -0700 Subject: [PATCH 1/4] Checking OneSignal.isRegiusterUserFinished before displaying IAMs If we display IAMs prior to receiving the response to on session, we could be displaying cached IAMs that have been paused, or edited. Additionally this is causing an issue where we display IAMs twice because of the self.messages = newMessages line in updateInAppMessagesFromOnSession. We overwrite the seenInSession variable of the old object with the new one. By waiting to display IAMs until onSession has returned we fix this issue. To avoid a case where cached IAMs aren't shown when onSession fails I am now calling updateInAppMessagesFromCache in the failure block of registerUserInternal. --- iOS_SDK/OneSignalSDK/Source/OSMessagingController.m | 5 ++--- iOS_SDK/OneSignalSDK/Source/OneSignal.m | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m b/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m index b56206f76..ad4b6661a 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m +++ b/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m @@ -160,7 +160,6 @@ - (instancetype)init { - (void)updateInAppMessagesFromCache { self.messages = [OneSignalUserDefaults.initStandard getSavedCodeableDataForKey:OS_IAM_MESSAGES_ARRAY defaultValue:[NSArray new]]; - [self evaluateMessages]; } @@ -241,8 +240,9 @@ - (void)presentInAppMessage:(OSInAppMessage *)message { [self.messageDisplayQueue addObject:message]; // Return early if an IAM is already showing - if (self.isInAppMessageShowing) + if (self.isInAppMessageShowing || !OneSignal.isRegisterUserFinished) { return; + } // Return early if the app is not active if (![UIApplication applicationIsActive]) { [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"Pause IAMs display due to app inactivity"]; @@ -286,7 +286,6 @@ - (void)displayMessage:(OSInAppMessage *)message { [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"In app messages will not show while paused"]; return; } - self.isInAppMessageShowing = true; [self showMessage:message]; } diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m index d0fd9fe55..45189fdb8 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.m +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.m @@ -1928,6 +1928,8 @@ + (void)registerUserInternal { callbackSet.failureBlock(error); } } + + [OSMessagingController.sharedInstance updateInAppMessagesFromCache]; }]; } From f53be87b7e5a804b2458cee43f9eabb89e64de47 Mon Sep 17 00:00:00 2001 From: emawby Date: Thu, 17 Jun 2021 10:28:39 -0700 Subject: [PATCH 2/4] unit test to verify that IAMs are not displayed until registerUser is finished --- .../InAppMessagingIntegrationTests.m | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m b/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m index 8e6917ee5..5d5c22892 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m @@ -439,6 +439,51 @@ - (void)testIAMWithRedisplay { XCTAssertTrue(secondLastDisplayTime - firstInterval >= delay); } +- (void)testCachedIAMsDonotDisplayUntilRegisterUser { + let limit = 5; + let delay = 30; + + let message = [OSInAppMessageTestHelper testMessageWithRedisplayLimit:limit delay:@(delay)]; + //Time interval mock + NSDateComponents* comps = [[NSDateComponents alloc]init]; + comps.year = 2019; + comps.month = 6; + comps.day = 10; + comps.hour = 10; + comps.minute = 1; + + NSCalendar* calendar = [NSCalendar currentCalendar]; + NSDate* date = [calendar dateFromComponents:comps]; + NSTimeInterval firstInterval = [date timeIntervalSince1970]; + NSMutableSet *seenMessages = [NSMutableSet new]; + [OSMessagingControllerOverrider setSeenMessages:seenMessages]; + + message.displayStats.lastDisplayTime = firstInterval - delay; + // Save IAM to messages cache + [OneSignalUserDefaults.initStandard saveCodeableDataForKey:OS_IAM_MESSAGES_ARRAY withValue:@[message]]; + + [OSMessagingControllerOverrider setMockDateGenerator: ^NSTimeInterval(void) { + return firstInterval; + }]; + + [OneSignal setAppId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba"]; + + [[OSMessagingController sharedInstance] updateInAppMessagesFromCache]; + + [OneSignal pauseInAppMessages:false]; + + // IAM should not be shown since we have not initialized OneSignal + XCTAssertEqual(1, OSMessagingControllerOverrider.messageDisplayQueue.count); + XCTAssertFalse(OSMessagingControllerOverrider.isInAppMessageShowing); + + [self initOneSignalWithInAppMessage:message]; + XCTAssertEqual(1, OSMessagingControllerOverrider.messageDisplayQueue.count); + XCTAssertTrue(OSMessagingControllerOverrider.isInAppMessageShowing); + [OSMessagingControllerOverrider dismissCurrentMessage]; + XCTAssertEqual(0, OSMessagingControllerOverrider.messageDisplayQueue.count); + XCTAssertFalse(OSMessagingControllerOverrider.isInAppMessageShowing); +} + - (void)testIAMClickLaunchesAPIRequestMultipleTimes_Redisplay { let limit = 5; let delay = 60; From e5d4c79e6ab5812dab5ce9e54869e5d93a1d5ecc Mon Sep 17 00:00:00 2001 From: emawby Date: Thu, 17 Jun 2021 11:32:34 -0700 Subject: [PATCH 3/4] Checking for isRegisterUserFinished before adding to queue This avoids a potential issue where an IAM is added to the queue, but then removed from the messages list after receiving the new messages from the server. --- iOS_SDK/OneSignalSDK/Source/OSMessagingController.m | 5 +++-- .../OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m b/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m index ad4b6661a..4e1233d08 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m +++ b/iOS_SDK/OneSignalSDK/Source/OSMessagingController.m @@ -240,7 +240,7 @@ - (void)presentInAppMessage:(OSInAppMessage *)message { [self.messageDisplayQueue addObject:message]; // Return early if an IAM is already showing - if (self.isInAppMessageShowing || !OneSignal.isRegisterUserFinished) { + if (self.isInAppMessageShowing) { return; } // Return early if the app is not active @@ -484,7 +484,8 @@ - (BOOL)hasMessageTriggerChanged:(OSInAppMessage *)message { - (BOOL)shouldShowInAppMessage:(OSInAppMessage *)message { return ![self.seenInAppMessages containsObject:message.messageId] && [self.triggerController messageMatchesTriggers:message] && - ![message isFinished]; + ![message isFinished] && + OneSignal.isRegisterUserFinished; } - (void)handleMessageActionWithURL:(OSInAppMessageAction *)action { diff --git a/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m b/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m index 5d5c22892..ac4ab7c6f 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m @@ -473,7 +473,7 @@ - (void)testCachedIAMsDonotDisplayUntilRegisterUser { [OneSignal pauseInAppMessages:false]; // IAM should not be shown since we have not initialized OneSignal - XCTAssertEqual(1, OSMessagingControllerOverrider.messageDisplayQueue.count); + XCTAssertEqual(0, OSMessagingControllerOverrider.messageDisplayQueue.count); XCTAssertFalse(OSMessagingControllerOverrider.isInAppMessageShowing); [self initOneSignalWithInAppMessage:message]; From 419df982bd8e80b7a44022a2b5b2b63b97f0f03c Mon Sep 17 00:00:00 2001 From: emawby Date: Tue, 29 Jun 2021 10:10:01 -0700 Subject: [PATCH 4/4] adding a registerUserSuccessful. registerUserFinished also represents fail --- iOS_SDK/OneSignalSDK/Source/OSStateSynchronizer.m | 5 +++-- iOS_SDK/OneSignalSDK/Source/OSSubscription.m | 2 +- iOS_SDK/OneSignalSDK/Source/OneSignal.m | 15 +++++++++++++-- iOS_SDK/OneSignalSDK/Source/OneSignalInternal.h | 3 +++ 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OSStateSynchronizer.m b/iOS_SDK/OneSignalSDK/Source/OSStateSynchronizer.m index ec5537be4..04af6ce1e 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSStateSynchronizer.m +++ b/iOS_SDK/OneSignalSDK/Source/OSStateSynchronizer.m @@ -48,7 +48,7 @@ + (void)setEmailUserId:(NSString *)emailUserId; + (void)saveExternalIdAuthToken:(NSString *)hashToken; + (void)saveSMSNumber:(NSString *)smsNumber withAuthToken:(NSString *)smsAuthToken userId:(NSString *)smsPlayerId; + (void)registerUserFinished; - ++ (void)registerUserSuccessful; @end @interface OSStateSynchronizer () @@ -129,7 +129,7 @@ - (void)registerUserWithState:(OSUserState *)registrationState withSuccess:(OSMu [OneSignalClient.sharedClient executeSimultaneousRequests:requests withSuccess:^(NSDictionary *results) { [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:[NSString stringWithFormat:@"on_session result: %@", results]]; - [OneSignal registerUserFinished]; + [OneSignal registerUserSuccessful]; // If the external user ID was sent as part of this request, we need to save it // Cache the external id if it exists within the registration payload @@ -186,6 +186,7 @@ - (void)registerUserWithState:(OSUserState *)registrationState withSuccess:(OSMu for (NSString *key in @[OS_PUSH, OS_EMAIL]) [OneSignal onesignal_Log:ONE_S_LL_ERROR message:[NSString stringWithFormat: @"Encountered error during %@ registration with OneSignal: %@", key, errors[key]]]; + [OneSignal registerUserFinished]; if (failureBlock) failureBlock(errors); }]; diff --git a/iOS_SDK/OneSignalSDK/Source/OSSubscription.m b/iOS_SDK/OneSignalSDK/Source/OSSubscription.m index 57931ee12..216e64544 100644 --- a/iOS_SDK/OneSignalSDK/Source/OSSubscription.m +++ b/iOS_SDK/OneSignalSDK/Source/OSSubscription.m @@ -181,7 +181,7 @@ + (void)fireChangesObserver:(OSSubscriptionState*)state { OSSubscriptionStateChanges* stateChanges = [OSSubscriptionStateChanges alloc]; stateChanges.from = OneSignal.lastSubscriptionState; stateChanges.to = [state copy]; - if (OneSignal.isRegisterUserFinished) { + if (OneSignal.isRegisterUserSuccessful) { BOOL hasReceiver = [OneSignal.subscriptionStateChangesObserver notifyChange:stateChanges]; if (hasReceiver) { diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m index 45189fdb8..7996507b4 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.m +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.m @@ -471,10 +471,15 @@ + (NSString*)mUserId { + (void)setUserId:(NSString *)userId { self.currentSubscriptionState.userId = userId; } - +// This is set to true even if register user fails + (void)registerUserFinished { _registerUserFinished = true; } +// If successful then register user is also finished ++ (void)registerUserSuccessful { + _registerUserSuccessful = true; + [OneSignal registerUserFinished]; +} + (NSString *)mEmailAuthToken { return self.currentEmailSubscriptionState.emailAuthCode; @@ -593,6 +598,7 @@ + (void)clearStatics { _outcomeEventsController = nil; _registerUserFinished = false; + _registerUserSuccessful = false; _delayedSMSParameters = nil; } @@ -1653,6 +1659,11 @@ + (BOOL)isRegisterUserFinished { return _registerUserFinished || isOnSessionSuccessfulForCurrentState; } +static BOOL _registerUserSuccessful = false; ++ (BOOL)isRegisterUserSuccessful { + return _registerUserSuccessful || isOnSessionSuccessfulForCurrentState; +} + + (BOOL)shouldRegisterNow { // return if the user has not granted privacy permissions if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:nil]) @@ -1822,6 +1833,7 @@ + (OSUserState *)createUserState { + (void)registerUserInternal { [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"registerUserInternal"]; _registerUserFinished = false; + _registerUserSuccessful = false; // return if the user has not granted privacy permissions if ([self shouldLogMissingPrivacyConsentErrorWithMethodName:nil]) @@ -1928,7 +1940,6 @@ + (void)registerUserInternal { callbackSet.failureBlock(error); } } - [OSMessagingController.sharedInstance updateInAppMessagesFromCache]; }]; } diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignalInternal.h b/iOS_SDK/OneSignalSDK/Source/OneSignalInternal.h index ee22a596f..3c3f4cd2b 100644 --- a/iOS_SDK/OneSignalSDK/Source/OneSignalInternal.h +++ b/iOS_SDK/OneSignalSDK/Source/OneSignalInternal.h @@ -79,7 +79,10 @@ @property (class, readonly) BOOL didCallDownloadParameters; @property (class, readonly) BOOL downloadedParameters; +//Indicates we have attempted to register the user and it has succeeded or failed @property (class, readonly) BOOL isRegisterUserFinished; +//Indicates that registering the user was successful +@property (class, readonly) BOOL isRegisterUserSuccessful; @property (class) NSObject* _Nonnull osNotificationSettings; @property (class) OSPermissionState* _Nonnull currentPermissionState;