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

Fixing IAM displaying twice on cold start #949

Merged
merged 4 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions iOS_SDK/OneSignalSDK/Source/OSMessagingController.m
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ - (instancetype)init {

- (void)updateInAppMessagesFromCache {
self.messages = [OneSignalUserDefaults.initStandard getSavedCodeableDataForKey:OS_IAM_MESSAGES_ARRAY defaultValue:[NSArray new]];

[self evaluateMessages];
}

Expand Down Expand Up @@ -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) {
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"];
Expand Down Expand Up @@ -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];
}
Expand Down Expand Up @@ -485,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 {
Expand Down
5 changes: 3 additions & 2 deletions iOS_SDK/OneSignalSDK/Source/OSStateSynchronizer.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
Expand Down Expand Up @@ -129,7 +129,7 @@ - (void)registerUserWithState:(OSUserState *)registrationState withSuccess:(OSMu

[OneSignalClient.sharedClient executeSimultaneousRequests:requests withSuccess:^(NSDictionary<NSString *, 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
Expand Down Expand Up @@ -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);
}];
Expand Down
2 changes: 1 addition & 1 deletion iOS_SDK/OneSignalSDK/Source/OSSubscription.m
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
15 changes: 14 additions & 1 deletion iOS_SDK/OneSignalSDK/Source/OneSignal.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -593,6 +598,7 @@ + (void)clearStatics {
_outcomeEventsController = nil;

_registerUserFinished = false;
_registerUserSuccessful = false;

_delayedSMSParameters = nil;
}
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -1928,6 +1940,7 @@ + (void)registerUserInternal {
callbackSet.failureBlock(error);
}
}
[OSMessagingController.sharedInstance updateInAppMessagesFromCache];
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused with one thing since you are checking for user registration to show IAM. At this point, OneSignal.isRegisterUserFinished; will return true? because it will immediateOnSessionRetry, then both _registerUserFinished || isOnSessionSuccessfulForCurrentState will be false right? Am I missing something? Are the cached IAM being displayed? Thanks in advance for clarifying this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya you are right this isn't working we would need a registerUserFailed boolean or settings registerUserFinished to true if it fails (it gets reset when it is called again).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a registerUserSuccessful boolean to be used in place of registerUserFinished in some places. Added comments to make it clear that registerUserFinished is also set to true when the call fails

}];
}

Expand Down
3 changes: 3 additions & 0 deletions iOS_SDK/OneSignalSDK/Source/OneSignalInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<OneSignalNotificationSettings>* _Nonnull osNotificationSettings;
@property (class) OSPermissionState* _Nonnull currentPermissionState;
Expand Down
45 changes: 45 additions & 0 deletions iOS_SDK/OneSignalSDK/UnitTests/InAppMessagingIntegrationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 <NSString *> *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(0, 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;
Expand Down