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

Query only active purchases when generating offline entitlements customer info #1003

Merged

Conversation

tonidero
Copy link
Contributor

Description

We were querying for the whole history of purchases when generating the offline entitlements customer info. This was done to get more accurate data in that situation. However, it adds complexity for a non-critical use case. This PR removes that functionality, simplifying the use cases for offline entitlements.

@tonidero tonidero added the refactor A code change that neither fixes a bug nor adds a feature label May 15, 2023
@@ -7,7 +7,6 @@ data class PurchasedProduct(
val productIdentifier: String,
val basePlanId: String?,
val storeTransaction: StoreTransaction,
val isActive: Boolean,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now all these will be active, so I just removed this property.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was a very confusing property too

val activePurchases = activePurchasesByHashedToken.values.toList()
val purchasedProducts = activePurchases.map {
createPurchasedProduct(it, productEntitlementMapping)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I'm assuming there can't be more than 1 active purchase with the same product id. I think that's safe to assume but lmk if I'm wrong.

)

mockAllPurchases(purchaseRecordMonthly, purchaseRecordAnnual)
mockActivePurchases(activePurchase)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed a couple of these tests were wrong, it was actually only mocking a single active purchase, and multiple non-active. They are fixed on this PR.

@@ -175,10 +175,13 @@ fun stubPurchaseHistoryRecord(
fun stubStoreTransactionFromGooglePurchase(
productIds: List<String>,
purchaseTime: Long,
purchaseToken: String = "abcdefghijipehfnbbnldmai.AO-J1OxqriTepvB7suzlIhxqPIveA0IHtX9amMedK0KK9CsO0S3Zk5H6gdwvV" +
"7HzZIJeTzqkY4okyVk8XKTmK1WZKAKSNTKop4dgwSmFnLWsCxYbahUmADg"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to add this as a parameter to allow having multiple purchases in the map returned by queryPurchases

@tonidero tonidero marked this pull request as ready for review May 15, 2023 12:21
@tonidero tonidero requested review from vegaro and a team May 15, 2023 12:21
@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Merging #1003 (bd17b2f) into main (82e7c20) will increase coverage by 0.27%.
The diff coverage is 90.21%.

@@            Coverage Diff             @@
##             main    #1003      +/-   ##
==========================================
+ Coverage   85.07%   85.35%   +0.27%     
==========================================
  Files         165      168       +3     
  Lines        5824     5983     +159     
  Branches      801      834      +33     
==========================================
+ Hits         4955     5107     +152     
+ Misses        550      546       -4     
- Partials      319      330      +11     
Impacted Files Coverage Δ
...enuecat/purchases/common/networking/HTTPRequest.kt 100.00% <ø> (ø)
...evenuecat/purchases/strings/CustomerInfoStrings.kt 0.00% <ø> (ø)
...at/purchases/strings/OfflineEntitlementsStrings.kt 0.00% <ø> (ø)
...at/purchases/google/BillingFlowParamsExtensions.kt 70.00% <25.00%> (-30.00%) ⬇️
...revenuecat/purchases/models/GoogleProrationMode.kt 61.53% <58.33%> (-38.47%) ⬇️
...ava/com/revenuecat/purchases/common/ReceiptInfo.kt 83.33% <66.66%> (-1.42%) ⬇️
...in/java/com/revenuecat/purchases/common/Backend.kt 83.98% <73.07%> (-0.54%) ⬇️
...enuecat/purchases/utils/ParcelizationTestHelper.kt 89.47% <80.00%> (-4.28%) ⬇️
.../main/kotlin/com/revenuecat/purchases/Purchases.kt 82.98% <80.55%> (-2.16%) ⬇️
.../com/revenuecat/purchases/google/BillingWrapper.kt 79.95% <85.71%> (-0.70%) ⬇️
... and 24 more

... and 5 files with indirect coverage changes

@@ -18,7 +18,7 @@ class PurchasedProductsFetcher(
private val dateProvider: DateProvider = DefaultDateProvider()
) {

fun queryPurchasedProducts(
fun queryActiveProducts(
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it

@tonidero tonidero merged commit 7aba9b4 into main May 16, 2023
7 checks passed
@tonidero tonidero deleted the toniricodiez/sdk-3111-android-query-only-active-purchases-when branch May 16, 2023 08:13
@tonidero tonidero mentioned this pull request May 16, 2023
5 tasks
tonidero added a commit that referenced this pull request May 18, 2023
**This is an automatic release.**

### New Features
* CAT-859 Expose whether or not a SubscriptionOption is Prepaid in the
SDK (#1005) via Deema AlShamaa (@dalshamaa)
### Bugfixes
* [CF-1324] Fix personalizedPrice defaulting to false (#952) via beylmk
(@beylmk)
### Performance Improvements
* Store and return ETag last refresh time header (#978) via Toni Rico
(@tonidero)
### Dependency Updates
* Bump fastlane-plugin-revenuecat_internal from `3b03efa` to `fe45299`
(#991) via dependabot[bot] (@dependabot[bot])
* Bump danger from 9.2.0 to 9.3.0 (#981) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane-plugin-revenuecat_internal from `8482a43` to `3b03efa`
(#974) via dependabot[bot] (@dependabot[bot])
* Bump fastlane from 2.212.1 to 2.212.2 (#973) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane-plugin-revenuecat_internal from `9255366` to `8482a43`
(#961) via dependabot[bot] (@dependabot[bot])
### Other Changes
* Add proration modes to post to backend (#977) via swehner (@swehner)
* Added ENTITLEMENTS_COMPUTED_ON_DEVICE (#939) via Cesar de la Vega
(@vegaro)
* Fix flaky test in OfflineCustomerInfoCalculatorTest (#997) via Cesar
de la Vega (@vegaro)
* Fix `OfflineCustomerInfoCalculatorTest` `Unresolved reference:
ProducType` (#995) via Cesar de la Vega (@vegaro)
* Add support for product_plan_identifier for offline customer info
(#959) via Cesar de la Vega (@vegaro)
* Add non-subscriptions support to offline customer info (#958) via
Cesar de la Vega (@vegaro)
* Query only active purchases when generating offline entitlements
customer info (#1003) via Toni Rico (@tonidero)
* Fix `PurchasesIntegrationTest` building issue (#996 into main) (#998)
via Cesar de la Vega (@vegaro)
* Fail offline entitlements computation if product entitlement mapping
not available (#999) via Toni Rico (@tonidero)
* Fix  build_magic_weather lane (#993) via Cesar de la Vega (@vegaro)
* Add backend integration tests and test product entitlement mapping
endpoint (#988) via Toni Rico (@tonidero)
* Fix purchases integration tests (#980) via Toni Rico (@tonidero)
* Disable offline entitlements if active inapp purchases exist (#983)
via Toni Rico (@tonidero)
* Clear cached customer info upon entering offline entitlements mode
(#989) via Toni Rico (@tonidero)
* Update product entitlement mapping request to new format (#976) via
Toni Rico (@tonidero)
* Support enabling/disabling offline entitlements (#964) via Toni Rico
(@tonidero)
* Add back integration tests automation (#972) via Toni Rico (@tonidero)
* Upgrade to AGP 8.0 (#975) via Toni Rico (@tonidero)
* Extract post receipt logic to PostReceiptHelper (#967) via Toni Rico
(@tonidero)
* Add isServerDown to error callback for postReceipt and getCustomerInfo
requests (#963) via Toni Rico (@tonidero)
* Add back integration test flavors (#962) via Toni Rico (@tonidero)
* Fix storing test results (#966) via Cesar de la Vega (@vegaro)
* Extract detekt job from test job (#965) via Cesar de la Vega (@vegaro)


[CF-1324]:
https://revenuecats.atlassian.net/browse/CF-1324?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>
tonidero added a commit that referenced this pull request May 23, 2023
### Description
This removes the flag that was disabling offline entitlements, so we
actually start using offline entitlements

#### TODO
- [x] Hold until making sure it all works correctly
- [x] Hold until #983 is merged
- [x] Hold until #959 is merged
- [x] Hold until
#999 is merged
- [x] Hold until #1003 is merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants