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.

Sign up for GitHub

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

Jump to bottom

Fixing IAM displaying twice on cold start #949

Merged
merged 4 commits into from
Jun 29, 2021
Merged

Fixing IAM displaying twice on cold start #949

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3170470
Checking OneSignal.isRegiusterUserFinished before displaying IAMs
emawby Jun 15, 2021
f53be87
unit test to verify that IAMs are not displayed until registerUser is…
emawby Jun 17, 2021
e5d4c79
Checking for isRegisterUserFinished before adding to queue
emawby Jun 17, 2021
419df98
adding a registerUserSuccessful. registerUserFinished also represents…
emawby Jun 29, 2021
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
5 changes: 2 additions & 3 deletions iOS_SDK/OneSignalSDK/Source/OSMessagingController.m
View file
Open in desktop
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 || !OneSignal.isRegisterUserFinished) {
Copy link
Contributor

@Jeasmine Jeasmine Jun 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we can add a comment regarding this conditional addition. I imagine you are not putting it on [self evaluateMessages]; because even if the user is not registered, some triggers might have happened that make an IAM appear, but we need to wait for the user registration. Is that correct?

Can it happen that after the session call, the IAM added to the queue is deleted from the Server? Might messageDisplayQueue end in a weird consistency situation? because messageDisplayQueue is not used until evaluateMessageDisplayQueue is called

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 thats a good point I think I should check earlier so that the message doesn't enter the queue. I don't want the message in the queue and not in the messages array

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
2 changes: 2 additions & 0 deletions iOS_SDK/OneSignalSDK/Source/OneSignal.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1928,6 +1928,8 @@ + (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