Support DEFERRED mode by swehner · Pull Request #985 · RevenueCat/purchases-android

swehner · 2023-04-26T16:58:51Z

Support DEFERRED Proration mode.
One problem is that the returned purchase is for the old product, not the new one, so all the callback need to use the replaced productId.

Checklist

If applicable, unit tests
If applicable, create follow-up issues for purchases-ios and hybrids

Description

Use old productId in callbacks for when proration mode is deferred.

dalshamaa

Looks good to my non-expert eye!

MarkVillacampa · 2023-05-04T15:13:11Z

Is this true? Wouldn't the onPurchasesUpdated callback always get called with the old transaction, even if we're using the deprecated flow?

For deferred purchases the purchases list will be empty. For regular purchases or other upgrades, the purchases will contain the new purchase

vegaro · 2023-05-04T17:33:26Z

For deferred purchases the purchases list will be empty. For regular purchases or other upgrades, the purchases will contain the new purchase

vegaro · 2023-05-04T17:35:13Z

Actually tests are failing 😄

MarkVillacampa · 2023-05-05T07:07:58Z

I was getting suspicious by the behaviour of the DEFERRED purchase listener so I dug a bit deeper.

Looks like the behavior for PurchasesUpdatedListener with DEFERRED purchases changed with Billing Library 5.0

According to Google's documentation as of May 23, 2022:

For the deferred replacement mode, your app receives a call to your PurchasesUpdatedListener with an empty list of purchases and a status of whether the upgrade or downgrade was successful.

Which changed to this sometime before July 5, 2022:

For the deferred replacement mode, your app receives a call to your PurchasesUpdatedListener with the purchase of the original subscription plan and a status of whether the upgrade or downgrade was successful.

I could not find an explicit mention of this in the BL5 release notes, but the docs change coincides more or less with the official release of Billing Library 5.0 on May 11, 2022

I'm going to review the places where we may be making the old assumption and make changes accordingly.

MarkVillacampa · 2023-05-06T14:45:42Z

Entirely removed the special handling for the case where purchases are empty:

It was only being handled when using the deprecated purchase methods.

In the new purchase method callback, StoreTransaction is not nullable.

Since Billing Library 5, the transactions list will contain the transaction for the old product. See comment.

We could make the new purchase method callback have a nullable transaction parameter, and handle the case where purchases.isEmpty() for both old and new purchase methods. But supposedly that should not be happening in BL5.

Thoughts?

Hmm in the BillingWrapper class we are still forwarding an empty list of purchases, even though it shouldn't happen in BC5. If that unexpected situation happened, we wouldn't call the callbacks after removing this if I'm not wrong, so it would basically get stuck.

I think we should call the error callback when this happens, at least until we are certain that this won't happen, and at that point, we should remove that code from BillingWrapper

MarkVillacampa · 2023-05-06T14:47:55Z

We had 4 separate but very similar tests for DEFERRED mode, which I'm not entirely sure were even working properly (e.g. ProrationMode.DEFERRED was not even passed).

I simplified them into two tests, each using the old and the new purchase methods respectively.

MarkVillacampa · 2023-05-06T14:53:52Z

Added the same comment in both pieces of code where we do special handling for DEFERRED mode product id. Open to suggestions.

tonidero

Left a comment but looking good!

tonidero · 2023-05-08T13:51:38Z

Maybe we could add a log if this is an unexpected situation now?

After more carefully reading the code, I realized we were already handling this case in the error handling code at the bottom of the method. Simply removing the special case handling means we'll call the error handler as expected.

tonidero · 2023-05-08T14:04:24Z

Hmm in the BillingWrapper class we are still forwarding an empty list of purchases, even though it shouldn't happen in BC5. If that unexpected situation happened, we wouldn't call the callbacks after removing this if I'm not wrong, so it would basically get stuck.

I think we should call the error callback when this happens, at least until we are certain that this won't happen, and at that point, we should remove that code from BillingWrapper

…on stays the same for current enum values.

…ction

…change. Starting with Billing Client 5, the listener will always contain the old transaction.

…erred.

Left only two tests: one using the old deprecated purchase method, and one using the new one.

…se list

…the existing error callback code.

MarkVillacampa · 2023-05-17T22:08:45Z

    @Test
-    fun `purchasesUpdatedCalls are forwarded with empty list if result is ok but with a null purchase`() {
-        val slot = slot<List<StoreTransaction>>()
+    fun `purchasesUpdatedCalls call the error callback if result is ok but with a null purchase`() {


Updated test to reflect the new expected behavior.

tonidero

Looks good! Just left a comment to try to improve the error message in the case we receive a null/empty list of purchases.

…ingResult with a Null purchases list

codecov · 2023-05-23T02:43:52Z

Codecov Report

Merging #985 (40e797c) into main (78cd322) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #985      +/-   ##
==========================================
+ Coverage   85.34%   85.41%   +0.06%     
==========================================
  Files         168      168              
  Lines        5984     5977       -7     
  Branches      835      832       -3     
==========================================
- Hits         5107     5105       -2     
+ Misses        546      544       -2     
+ Partials      331      328       -3

Impacted Files	Coverage Δ
...om/revenuecat/purchases/strings/PurchaseStrings.kt	`0.00% <ø> (ø)`
.../com/revenuecat/purchases/google/BillingWrapper.kt	`80.74% <100.00%> (+0.78%)`	⬆️
...revenuecat/purchases/models/GoogleProrationMode.kt	`71.42% <100.00%> (+9.89%)`	⬆️
.../main/kotlin/com/revenuecat/purchases/Purchases.kt	`82.89% <100.00%> (-0.10%)`	⬇️

... and 2 files with indirect coverage changes

@swehner

**This is an automatic release.** ### New Features * Support DEFERRED mode (#985) via swehner (@swehner) * Add completion callback to syncPurchases API (#1002) via Toni Rico (@tonidero) ### Bugfixes * Workaround bug in android 4 for JSON objects with List<String> (#942) via Andy Boedo (@aboedo) ### Dependency Updates * Bump fastlane-plugin-revenuecat_internal from `fe45299` to `13773d2` (#1015) via dependabot[bot] (@dependabot[bot]) ### Other Changes * Bump dokka to 1.8.10 to support Gradle 8 (#1009) via Toni Rico (@tonidero) * Disable offline entitlements temporarily (#1023) via Toni Rico (@tonidero) * Fix integration tests in CI (#1019) via Toni Rico (@tonidero) * Add offline entitlements integration tests (#1006) via Toni Rico (@tonidero) * Disable offline entitlements in observer mode (#1014) via Toni Rico (@tonidero) * Extracts setup and teardown to BasePurchasesTest (#1011) via Cesar de la Vega (@vegaro) * Support forcing server errors for tests (#1008) via Toni Rico (@tonidero) --------- Co-authored-by: revenuecat-ops <ops@revenuecat.com> Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>

Base automatically changed from add_proration_modes to main April 26, 2023 16:59

swehner force-pushed the support-deferred branch from 42b6d40 to 79f6312 Compare April 26, 2023 17:00

swehner added the pr:feat A new feature label Apr 26, 2023

dalshamaa approved these changes Apr 28, 2023

View reviewed changes

vegaro reviewed Apr 28, 2023

View reviewed changes

Comment thread feature/google/src/main/java/com/revenuecat/purchases/google/BillingWrapper.kt Outdated

Comment thread public/src/main/java/com/revenuecat/purchases/models/GoogleProrationMode.kt Outdated

Comment thread purchases/src/test/java/com/revenuecat/purchases/PurchasesTest.kt Outdated

MarkVillacampa reviewed May 4, 2023

View reviewed changes

MarkVillacampa force-pushed the support-deferred branch from c306a51 to d359fa1 Compare May 4, 2023 16:40

vegaro approved these changes May 4, 2023

View reviewed changes

MarkVillacampa reviewed May 6, 2023

View reviewed changes

tonidero reviewed May 8, 2023

View reviewed changes

swehner and others added 16 commits May 17, 2023 15:39

Support DEFERRED mode

cd4dd65

Fix detekt

f9ba4ef

Use old product id for callbacks

9c9a892

fix detekt

23de410

Revert project changes

8850956

Mock consume call

5336401

Move DEFERRED enum value to the bottom so internal integer repsentati…

0399178

…on stays the same for current enum values.

Fix test

d1efd4e

Remove extra lines

fdd4b99

unify if statements

f933542

test purchase is passed correctly

48d7c35

improve test when making a deferred purchase using the deprecated fun…

c05d8c8

…ction

revert test change

dbe9a85

remove line

54fb833

wait for all transactions before invoking the callback

8bfad8c

Expand comment on special handling for DEFERRED purchases

b7849e2

MarkVillacampa added 6 commits May 17, 2023 15:39

Remove special handling for empty purchases list in DEFERRED product …

f7cf2a4

…change. Starting with Billing Client 5, the listener will always contain the old transaction.

Remove comment about transaction possibly being null if change is def…

c34796e

…erred.

Unify and refactor tests regarding DEFERRED product changes.

ab764df

Left only two tests: one using the old deprecated purchase method, and one using the new one.

update comment

b60c22e

Forward empty transaction list if result is ok but with a null purcha…

8f32486

…se list

Remove special case to forward an empty list of transactions and use …

03f1f91

…the existing error callback code.

MarkVillacampa force-pushed the support-deferred branch from 2391acf to 03f1f91 Compare May 17, 2023 22:05

MarkVillacampa reviewed May 17, 2023

View reviewed changes

tonidero approved these changes May 18, 2023

View reviewed changes

Comment thread feature/google/src/main/java/com/revenuecat/purchases/google/BillingWrapper.kt Outdated

add special error message when onPurchasesUpdated receives an OK Bill…

40e797c

…ingResult with a Null purchases list

MarkVillacampa merged commit 7cb1fc3 into main May 24, 2023

MarkVillacampa deleted the support-deferred branch May 24, 2023 17:13

This was referenced May 24, 2023

[AUTOMATIC] Release/6.3.0 #1022

Closed

[AUTOMATIC] Release/6.3.0 #1025

Merged

Uh oh!

Conversation

swehner commented Apr 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Description

Uh oh!

dalshamaa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vegaro commented May 4, 2023

Uh oh!

MarkVillacampa commented May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkVillacampa May 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented May 23, 2023

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

swehner commented Apr 26, 2023 •

edited

Loading

MarkVillacampa commented May 5, 2023 •

edited

Loading

MarkVillacampa May 6, 2023 •

edited

Loading