OneSignal / OneSignal-iOS-SDK

OneSignal is a free push notification service for mobile apps. This plugin makes it easy to integrate your native iOS app with OneSignal. https://onesignal.com
Other
496 stars 263 forks source link

[Bug] The user executor needs to uncache before other executors #1465

Closed nan-li closed 3 months ago

nan-li commented 3 months ago

Description

One Line Summary

The User Executor is no longer a static class, and will finish uncaching before other executors uncache.

Details

User executor is no longer a static class

Motivation

Bug Fix

Scope

Testing

Unit testing

Manual testing

Device: iPhone 13 on iOS 17.5.1 Reproduce original wrong behavior:

  1. Start with an anonymous user and turn off internet connection
  2. Login to userA and add tag for userA
  3. Login to userB and add tag for userB
  4. Note this will enqueue an IdentifyUser userA, CreateUser userB in the user executor
  5. Note this will enqueue two UpdateUser in the property executor
  6. Swipe away the app, turn on internet, and re-open app
  7. The user executor does not finish uncaching before the property executor uncaches
  8. Therefore, UpdateUser userA is dropped as the property executor cannot connect the Identity model reference (needed to know when the OSID is hydrated)

After changes in this PR:

Affected code checklist

Checklist

Overview

Testing

Final pass


This change is Reviewable

nan-li commented 3 months ago

Looks like some of the switch user integration tests failed in the CI. When I ran it locally they didn't fail but other tests are failing every time

A live activity executor test, right?

emawby commented 3 months ago

Looks like some of the switch user integration tests failed in the CI. When I ran it locally they didn't fail but other tests are failing every time

A live activity executor test, right?

No it is a notification enters foreground test. It seems like they are probably all unrelated to this PR and are just flaky