Feature/defer cache updates if woken from push notification #288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ier. Added tests to check that caches aren't updated when the SDK is set up with the app running in the background.
aboedo
commented
Jul 20, 2020
| #import <UIKit/UIKit.h> | ||
| #elif TARGET_OS_OSX | ||
| #import <AppKit/AppKit.h> | ||
| #elif TARGET_OS_WATCH | ||
| #import <UIKit/UIKit.h> | ||
| #import <WatchKit/WatchKit.h> |
| #elif TARGET_OS_OSX | ||
| #define IS_APPLICATION_BACKGROUNDED !NSApplication.sharedApplication.isActive | ||
| #elif TARGET_OS_WATCH | ||
| #define IS_APPLICATION_BACKGROUNDED WKExtension.sharedExtension.applicationState == WKApplicationStateBackground |
There was a problem hiding this comment.
Apple's API consistency at its finest 🤪
| if (!self.systemInfo.isApplicationBackgrounded) { | ||
| [self updateAllCachesWithCompletionBlock:callDelegate]; | ||
| } else { | ||
| [self sendCachedPurchaserInfoIfAvailable]; |
There was a problem hiding this comment.
this bit is so that if there's something cached we still call the delegate. I believe this is a departure from Android. @vegaro was there a specific reason not to call the delegate there? I'm worried that not calling it is a bit inconsistent with the philosophy of sending whatever is cached as fast as we can.
There was a problem hiding this comment.
You're right, we should be calling in Android. I think that's what I would expect as a developer.
| @@ -209,8 +209,7 @@ class PurchasesTests: XCTestCase { | |||
| func setupAnonPurchases() { | |||
| Purchases.automaticAppleSearchAdsAttributionCollection = false | |||
| self.identityManager.mockIsAnonymous = true | |||
| let systemInfo = RCSystemInfo(platformFlavor: nil, platformFlavorVersion: nil, finishTransactions: true) | |||
There was a problem hiding this comment.
just cleanup here, this line was duplicated in a couple of places
vegaro
approved these changes
Jul 20, 2020
| #elif TARGET_OS_OSX | ||
| #define IS_APPLICATION_BACKGROUNDED !NSApplication.sharedApplication.isActive | ||
| #elif TARGET_OS_WATCH | ||
| #define IS_APPLICATION_BACKGROUNDED WKExtension.sharedExtension.applicationState == WKApplicationStateBackground |
There was a problem hiding this comment.
Nice! Much easier than in Android!
aboedo
commented
Jul 21, 2020
| #if TARGET_OS_IOS || TARGET_OS_TV | ||
| #define IS_APPLICATION_BACKGROUNDED UIApplication.sharedApplication.applicationState == UIApplicationStateBackground | ||
| #elif TARGET_OS_OSX | ||
| #define IS_APPLICATION_BACKGROUNDED !NSApplication.sharedApplication.isActive |
There was a problem hiding this comment.
in my testing, isActive is returning nil. I'm going to be conservative here and just return NO for mac, since mac apps have a different lifecycle anyway.
… for NSApplication.sharedApplication.isActive is nil for whatever reason.
aboedo
deleted the
feature/defer_cache_updates_if_woken_from_push_notification
branch
vegaro
pushed a commit
that referenced
this pull request
Jul 28, 2020
* Added logic to skip updating cache on setup if app is not active * inverted checks from is app active to !is app backgrounded * made the app backgrounded method an instance method to make tests easier. Added tests to check that caches aren't updated when the SDK is set up with the app running in the background. * updated value of isApplicationBackgrounded for macOS, since the value for NSApplication.sharedApplication.isActive is nil for whatever reason.
Merged
vegaro
added a commit
that referenced
this pull request
Jul 28, 2020
* Feature/defer cache updates if woken from push notification (#288) * Added logic to skip updating cache on setup if app is not active * inverted checks from is app active to !is app backgrounded * made the app backgrounded method an instance method to make tests easier. Added tests to check that caches aren't updated when the SDK is set up with the app running in the background. * updated value of isApplicationBackgrounded for macOS, since the value for NSApplication.sharedApplication.isActive is nil for whatever reason. * Version 3.5.2 Co-authored-by: aboedo <andresboedo@gmail.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates so that when the SDK's
configureis called with the app in the background, it won't try to update offerings or purchaser info.They will, however, be updated if the app becomes active, per the
didBecomeActivenotification (that was already the current behavior).This also ensures that if the app is in the background and there's purchaser info cached, the purchaserInfo delegate gets called with the cached value. I believe this is different than the android behavior.
This behavior ensures that we don't make unnecessary backend calls in the following two scenarios:
Note: this is the iOS equivalent of
RevenueCat/purchases-android#94