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

Allowing opened push event to be transmitted when SDK is not initialized #181

Merged
merged 5 commits into from
May 31, 2024

Conversation

ajaysubra
Copy link
Contributor

@ajaysubra ajaysubra commented May 30, 2024

Description

Background

The SDK can be in three states of initialization -

  • Initialized - The SDK is initialized and any requests it gets is queued immediately and flushed at certain intervals.
  • Initializing - The SDK is initializing (reading from disk etc) and any requests that the SDK gets is held in memory and enqueued once the SDK is fully initialized.
  • un-initialized - The SDK will ignore the request and log a warning message to the developer (which they probably won't see if the app is terminated when testing locally).

The issue with react native is that by the time react native is booted from the native code the opened event request has already hit our SDK and the SDK is in the un-initialized state so the request is simply ignored. This logic has been in place since Dec 2023 on iOS and is part of requirement at that point and also now that developers should always initialize the SDK before interacting with it. When we implemented initialize in react native we didn't test this edge case.

While we still want our devs to initialize the SDK before calling any of it's methods. As long as they stick to interfacing with the SDK on just one of the layers (native or RN) they shouldn't encounter any issues. There is an exception to this with calling the SDK when a user opens push and this has to happen on the native side because of the application delegation pattern on iOS.

We want to keep the SDK simple in terms of logic and not queue up things which we don't know which company it belong to. For instance, if we allow devs to set profile and then initialize there is a chance that the profile wasn't intended to be associated with the company that was set later. In short, it keeps us from making too many assumptions on the SDK.

Finally, since we are the ones queueing up the opened push event when the devs calls our SDK to handle a push opened action from the user, it seems reasonable make an exception just in this case since we know for react native it is possible that the SDK may not be initialized in time to handle sending opened push event.

Solution

When the SDK is un-initialized and receives an opened push event, keep it in state until the SDK is initialized and then queue it up.

Check List

  • Are you changing anything with the public API?
  • Have you tested this change on real device?
  • Are your changes backwards compatible with previous SDK Versions?
  • Have you added unit test coverage for your changes?
  • Have you verified that your changes are compatible with all the operating system version this SDK currently supports?

Manual Test Plan

  1. Test all SDK interactions w/o initializing it locally: This should not enqueue any request other than opened push event ✅
  2. Test all SDK interactions after initializing and make sure this change didn't impact any existing use cases ✅

@ajaysubra ajaysubra requested a review from a team as a code owner May 30, 2024 17:03
@ajaysubra ajaysubra requested a review from ndurell May 30, 2024 17:03
@@ -116,7 +116,7 @@ class StateManagementEdgeCaseTests: XCTestCase {
// MARK: - Set Email

@MainActor
func testSetEmailUninitialized() async throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just updated some test names to indicate the outcome we expect

}

@MainActor
func testEnqueueNonOpenedPushEventUninitializedDoesNotAddToPendingRequest() async throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test goes through all other event metrics other than opened push and makes sure that they don't get through the SDK when it's not initialized.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice.

@@ -369,15 +369,30 @@ class StateManagementEdgeCaseTests: XCTestCase {
// MARK: - set enqueue event uninitialized

@MainActor
func testEnqueueEventUninitialized() async throws {
func testOpenedPushEventUninitializedAddsToPendingRequests() async throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests opened push event is added to pending requests when SDK isn't initialized.

@ajaysubra ajaysubra requested a review from evan-masseau May 30, 2024 17:06
@ajaysubra ajaysubra merged commit 9b08d8f into master May 31, 2024
9 checks passed
@ajaysubra ajaysubra deleted the as/chnl-9098-opened-event-when-not-inited branch May 31, 2024 16:11
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