Removed preventPurchasePopupCallFromTriggeringCacheRefresh, update caches on willEnterForeground#2623
Merged
NachoSoto merged 2 commits intoJun 12, 2023
Conversation
e5312ce to
56b7cb4
Compare
preventPurchasePopupCallFromTriggeringCacheRefresh with checking purchase in progresspreventPurchasePopupCallFromTriggeringCacheRefresh with checking purchase in progress
5083bfe to
cb946b8
Compare
preventPurchasePopupCallFromTriggeringCacheRefresh with checking purchase in progresspreventPurchasePopupCallFromTriggeringCacheRefresh, update caches on willEnterForeground
NachoSoto
commented
Jun 12, 2023
Contributor
Author
There was a problem hiding this comment.
This is no longer needed, we won't update caches in the middle of a purchase.
…caches on `willEnterForeground` This method was a hack to deal with apps getting `didBecomeActive` notifications in the middle of a purchase when the popups were displayed. This had several problems: - It was only being used for StoreKit 1 purchases - It wasn't used for StoreKit 2 purchases through the SDK - It wasn't used for purchases initiated outside the SDK (like using the upcoming paywall screens) - It updated the cache timestamp to "now", including offerings, despite not actually updating data - It was an indirect and implicit way of solving a problem instead of an explicit one This solves the same by observing `willEnterForeground`, which only happens when the app actually comes back from the background, and not when it simply loses focus.
cb946b8 to
c0538c8
Compare
Member
|
I went through the trouble of spelunking through RC history to see when that was added, to see if we were missing anything... Jan 2019 🤯 64e59c9 |
Member
Could you clarify this? Why would SK2 not trigger didBecomeActive after the modal? Or other stuff? They all show the same UI |
aboedo
approved these changes
Jun 12, 2023
Comment on lines
-38
to
+42
| func testSubscribesToUIApplicationDidBecomeActive() throws { | ||
| func testSubscribesToUIApplicationWillEnterForeground() throws { | ||
| expect(self.notificationCenter.observers).to(haveCount(2)) | ||
|
|
||
| let (_, _, name, _) = try XCTUnwrap(self.notificationCenter.observers.first) | ||
| expect(name) == SystemInfo.applicationDidBecomeActiveNotification | ||
| expect(name) == SystemInfo.applicationWillEnterForegroundNotification |
Member
There was a problem hiding this comment.
maybe we can also test "does not subscribe to app did become active", since doing so would actually introduce a regression where we double-post?
Contributor
Author
StoreKit 2 purchases wasn't calling |
This was referenced Jun 13, 2023
NachoSoto
pushed a commit
that referenced
this pull request
Jun 13, 2023
**This is an automatic release.** ### New Features * New `DebugViewController`: UIKit counterpart for SwiftUI's `debugRevenueCatOverlay` (#2631) via NachoSoto (@NachoSoto) * Created `PaywallExtensions`: `StoreView` and `SubscriptionStoreView` overloads for `Offering` (#2593) via NachoSoto (@NachoSoto) * Introduced `debugRevenueCatOverlay()`: new SwiftUI debug overlay (#2567) via NachoSoto (@NachoSoto) ### Bugfixes * Removed `preventPurchasePopupCallFromTriggeringCacheRefresh`, update caches on `willEnterForeground` (#2623) via NachoSoto (@NachoSoto) * Fixed `Catalyst` build with `Xcode 15 beta 1` (#2586) via NachoSoto (@NachoSoto) ### Dependency Updates * Bump danger from 9.3.0 to 9.3.1 (#2592) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `StoreTransaction`: added new `Storefront` to API testers (#2634) via NachoSoto (@NachoSoto) * `DebugView`: added snapshot tests (#2630) via NachoSoto (@NachoSoto) * `verifyNoUnfinishedTransactions`/`verifyUnfinishedTransaction`: added missing `#file` parameter (#2625) via NachoSoto (@NachoSoto) * `PostReceiptDataOperation`: clean up cache key (#2628) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: also get `Storefront` from SK1 (#2629) via NachoSoto (@NachoSoto) * `CI`: disable iOS 17 for now (#2627) via NachoSoto (@NachoSoto) * `Tests`: fixed crash on iOS 13 (#2624) via NachoSoto (@NachoSoto) * `StoreTransaction`: read `Storefront` from `StoreKit.Transaction` (#2611) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`/`BaseStoreKitIntegrationTests`: also clear transactions after every test (#2616) via NachoSoto (@NachoSoto) * `ErrorCode.networkError`: improved description (#2610) via NachoSoto (@NachoSoto) * `PurchaseTester`: make CI job always point to current version (#2622) via NachoSoto (@NachoSoto) * Improved `finishAllUnfinishedTransactions` (#2615) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`: improved `waitForStoreKitTestIfNeeded` (#2614) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`: set `continueAfterFailure` to `false` (#2617) via NachoSoto (@NachoSoto) * `PaywallExtensions`: fixed compilation (#2613) via NachoSoto (@NachoSoto) * `CI`: added `iOS 17` job (#2591) via NachoSoto (@NachoSoto) * `Encodable.jsonEncodedData`: fixed tests on iOS 17 due to inconsistent key ordering (#2607) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: added ability to display new `SubscriptionStoreView` (#2595) via NachoSoto (@NachoSoto) * Refactor: extracted all log strings (#2600) via NachoSoto (@NachoSoto) * Changed tests to work around `URL` decoding differences in `iOS 17` (#2605) via NachoSoto (@NachoSoto) * Removed unnecessary `Strings.trimmedOrError` (#2601) via NachoSoto (@NachoSoto) * Fixed test compilation with `Xcode 15` (#2602) via NachoSoto (@NachoSoto) * Tests: added `iOS 17` snapshots (#2603) via NachoSoto (@NachoSoto) * `StoreProductDiscount`: added `description` (#2604) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay` improvements (#2594) via NachoSoto (@NachoSoto) * `Xcode 15`: fixed all documentation warnings (#2596) via NachoSoto (@NachoSoto) * `StoreKitObserverModeIntegrationTests`: fixed and disabled SK2 `testPurchaseInDevicePostsReceipt` (#2589) via NachoSoto (@NachoSoto) * `StoreKit2TransactionListener`: added log when receiving `Transactions.Updates` (#2588) via NachoSoto (@NachoSoto) * `Dictionary.MergeStrategy`: simplify implementation (#2587) via NachoSoto (@NachoSoto) * `Configuration.Builder`: fixed doc reference (#2583) via NachoSoto (@NachoSoto) * `APITesters`: available since iOS 11 (#2581) via NachoSoto (@NachoSoto)
NachoSoto
added a commit
that referenced
this pull request
Aug 14, 2023
…p launch Possibly as a consequence of #2623 we were calling `updateAllCachesIfNeeded` on app launch together with `updateCachesIfInForeground`, meaning that we were updating all caches twice. Thanks to our de-duping logic this is normally not a big issue, but this issue combined with what #2954 fixes, means that if there are pending transactions, opening the app would probably end up posting receipts duplicated times. This adds a new log as well to detect this: > [purchases] DEBUG: ℹ️ Throttling cache update, only 0.8 seconds elapsed And a new integration test that posts `UIApplication.willEnterForegroundNotification` to test this.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This method was a hack to deal with apps getting
didBecomeActivenotifications in the middle of a purchase when the popups were displayed.This had several problems:
This solves the same by observing
willEnterForeground, which only happens when the app actually comes back from the background, and not when it simply loses focus.