Sync pending purchases before getting customer info#1073
Conversation
There was a problem hiding this comment.
This is the main change of this PR
There was a problem hiding this comment.
Do we need the IfNeeded? At the end of the day this is just syncing pending purchases right and not really checking if it's needed right?
There was a problem hiding this comment.
This is another change I would like to note. This made it so, in the beginning of the SDK initialization, the billing client would likely be disconnected, so it usually failed. The queryPurchases method already has a mechanism to call the callbacks once the billing client is connected, so this seemed unnecesarily. The only thing that slightly concerns me is that this method may take longer now, since it needs to wait for the billing client to connect during SDK initialization. I thought it was ok though. Thoughts?
Codecov Report
@@ Coverage Diff @@
## main #1073 +/- ##
==========================================
+ Coverage 85.84% 85.86% +0.01%
==========================================
Files 179 179
Lines 6336 6359 +23
Branches 876 876
==========================================
+ Hits 5439 5460 +21
- Misses 556 557 +1
- Partials 341 342 +1
|
f46db41 to
d3cdbcc
Compare
1ab2c4a to
3c59515
Compare
There was a problem hiding this comment.
Do we need the IfNeeded? At the end of the day this is just syncing pending purchases right and not really checking if it's needed right?
There was a problem hiding this comment.
Maybe: "CustomerInfo updated after syncing pending purchases."
There was a problem hiding this comment.
Well, it's not technically after, it's more "from" syncing pending purchases. As in, if there are pending purchases, we will use the customer info returned by the last request that syncs those pending purchases and we won't do the "get customer info" request after. But yeah I can add that.
d3cdbcc to
4a44b45
Compare
3c59515 to
65becd2
Compare
### Description An integration test started failing due to #1073. The error was that the schemaVersion field in the customer info didn't match between the fetched and cached customer info. It was actually luck that made it work before 😬 . Before that change, this test tried to get customer info twice: On sdk configuration and the first part of the test. Since both of these happen pretty quickly, we only made the request once and called both callbacks with the same response. However, the first request actually modified the JSONObject, adding the schema version and some other fields in order to cache it in the device. This made it so the JSONObject actually had those fields in the second callback, causing the schemaVersion to have a value. Then, the value returned from cache already has the schemaVersion field set so the test passed. After the change in #1073, the test still tries to get customer info twice, but the raw json sharing didn't happen, since when we call getCustomerInfo we now need to check first for unsynced purchases, causing us to not share the callback, so the schema version that was set in the first callback wasn't shared with the second one, causing the schemaVersion field to be 0, causing the test to fail. After talking about it, we think defaulting the schema version to the latest known version by the SDK is probably expected behavior, instead of defaulting to 0 before it's been saved. Followup to #1078
**This is an automatic release.** ### Bugfixes * Default customer info schema version to latest known by SDK (#1080) via Toni Rico (@tonidero) * Handle other diagnostics-related exceptions (#1076) via Toni Rico (@tonidero) * Return error in queryPurchases if error connecting to billing client (#1072) via Toni Rico (@tonidero) ### Other Changes * Fix offline entitlements integration tests (#1085) via Toni Rico (@tonidero) * Add defaultsRelease variant tests run configuration (#1074) via Toni Rico (@tonidero) * Compose sample app: move to gradle catalog (#1081) via Toni Rico (@tonidero) * Compose sample app: automate builds (#1082) via Toni Rico (@tonidero) * Compose sample app (#1056) via Toni Rico (@tonidero) * Migrate to Gradle version catalog (#1059) via Cesar de la Vega (@vegaro) * Trusted entitlements: Add logs with verification mode (#1067) via Toni Rico (@tonidero) * Sync pending purchases before getting customer info (#1073) via Toni Rico (@tonidero) * Refactor syncing pending transactions logic out of `Purchases` (#1058) via Toni Rico (@tonidero) * Refactor CustomerInfo listener and cache logic into CustomerInfoUpdater (#1052) via Toni Rico (@tonidero) * Trusted entitlements: Add integration tests (#1071) via Toni Rico (@tonidero) * Trusted entitlements: Add internal mechanism to force signing errors for tests (#1070) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
### Description In some situations, we weren't refreshing the customer info on app open. This was caused by #1073. In that PR we changed to fetch current purchases before getting customer info, so we would sync purchases before getting customer info. However, when we fetch customer info during the app open's `onAppForeground` callback, the `billing.purchasesUpdatedListener` was not set. Currently, if that listener isn't set, we would just [skip the query](https://github.com/RevenueCat/purchases-android/blob/ef34062996a17aa6a47d6eb2ca0903d66a51ab5e/purchases/src/main/kotlin/com/revenuecat/purchases/google/BillingWrapper.kt#L142) altogether so we would never get a response. Thanks @joshdholtz for bringing it up!
Description
Followup to #1052 and #1058
This changes the behavior of fetching customer info. Now, when fetching customer info, we will first try to sync any pending purchases if any. If there are pending purchases, we will return the customer info from the last request sent. If there aren't, we will then hit the current customer info endpoint to get the latest customer info.