Skip to content

Support product_plan_identifier for purchased subscriptions from Google Play#2654

Merged
joshdholtz merged 4 commits into
mainfrom
support-product_plan_identifier-in-subscriptions
Jun 15, 2023
Merged

Support product_plan_identifier for purchased subscriptions from Google Play#2654
joshdholtz merged 4 commits into
mainfrom
support-product_plan_identifier-in-subscriptions

Conversation

@joshdholtz

Copy link
Copy Markdown
Member

Motivation

Addresses RevenueCat/purchases-flutter#692

Description

Shows the product id as <product>:<base_plan> if the subscription was purchased from Google Play. This uses the product_plan_identifier that is returned with the subscription.

@NachoSoto

Copy link
Copy Markdown
Contributor

Ooooh this makes a lot of sense 👍🏻

@NachoSoto NachoSoto added the pr:fix A bug fix label Jun 14, 2023
@joshdholtz joshdholtz force-pushed the support-product_plan_identifier-in-subscriptions branch from 1cd4b48 to ba179d2 Compare June 14, 2023 20:39
@joshdholtz joshdholtz requested a review from NachoSoto June 14, 2023 20:39
@joshdholtz joshdholtz marked this pull request as ready for review June 14, 2023 20:40
@codecov

codecov Bot commented Jun 14, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2654 (e1785c7) into main (7f875ae) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2654      +/-   ##
==========================================
+ Coverage   86.37%   86.39%   +0.01%     
==========================================
  Files         207      207              
  Lines       14632    14652      +20     
==========================================
+ Hits        12639    12658      +19     
- Misses       1993     1994       +1     
Impacted Files Coverage Δ
...es/Networking/Responses/CustomerInfoResponse.swift 100.00% <ø> (ø)
Sources/Identity/CustomerInfo+ActiveDates.swift 100.00% <100.00%> (ø)
Sources/Purchasing/EntitlementInfo.swift 80.64% <100.00%> (+0.42%) ⬆️

... and 3 files with indirect coverage changes

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this makes sense. My only important question is regarding the need to expose EntitlementInfo.productPlanIdentifier.

Comment thread Sources/Identity/CustomerInfo+ActiveDates.swift Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add a note that this is just for Android? In fact I wonder if this even needs to be exposed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that if we do need to expose it you need to change this from fix to feat and add this new method to the API testers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made it internal! Doesn't need to be public at all yet (unless some users think it should be 🤷‍♂️)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍🏻

Comment on lines 136 to 137

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was worried that this kinda breaks product lookup, because these products would no longer exist in iOS. But I guess these refer to Android products anyway, right? (/cc @aboedo @tonidero)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But the more I think about it, I do worry about breaking the semantics of activeSubscriptions and allPurchasedProductIdentifiers. They'd no longer be "product identifiers" as their documented to be.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So I wonder if the solution would be to have a separate property instead that combines both. At the end of the day this is something that users of the iOS SDK won't care about, only hybrids, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah yeah, this is generally something that hybrids or people using native iOS and Android will care about. I'd ideally like this to be modeled in a new way in somehow in a future version of the SDK because I don't really love this 😛 But 🤷‍♂️

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay I think I'm okay with this implementation then since it only affects Android products, and those can't be fetched on iOS anyway.
But can you add a comment here explaining that? 🙏🏻

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done!

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome 🚀

@NachoSoto NachoSoto changed the title Support product_plan_identifier for purchased subscriptions from Google Play Support product_plan_identifier for purchased subscriptions from Google Play Jun 15, 2023
@joshdholtz joshdholtz force-pushed the support-product_plan_identifier-in-subscriptions branch from 2fcc0d9 to e1785c7 Compare June 15, 2023 22:23
@joshdholtz joshdholtz merged commit dececcb into main Jun 15, 2023
@joshdholtz joshdholtz deleted the support-product_plan_identifier-in-subscriptions branch June 15, 2023 22:54
NachoSoto pushed a commit that referenced this pull request Jun 22, 2023
**This is an automatic release.**

### Bugfixes
* `PurchasesOrchestrator`: update `CustomerInfoManager` cache after
processing transactions (#2676) via NachoSoto (@NachoSoto)
* `ErrorResponse`: drastically improved error messages, no more "unknown
error"s (#2660) via NachoSoto (@NachoSoto)
* `PaywallExtensions`: post purchases with `Offering` identifier (#2645)
via NachoSoto (@NachoSoto)
* Support `product_plan_identifier` for purchased subscriptions from
`Google Play` (#2654) via Josh Holtz (@joshdholtz)
### Performance Improvements
* `copy(with: VerificationResult)`: optimization to avoid copies (#2639)
via NachoSoto (@NachoSoto)
### Other Changes
* `ETagManager`: refactored e-tag creation and tests (#2671) via
NachoSoto (@NachoSoto)
* `getPromotionalOffer`: return `ErrorCode.ineligibleError` if receipt
is not found (#2678) via NachoSoto (@NachoSoto)
* `TimingUtil`: removed slow purchase logs (#2677) via NachoSoto
(@NachoSoto)
* `CI`: changed `Codecov` to `informational` (#2670) via NachoSoto
(@NachoSoto)
* `LoadShedderIntegrationTests`: verify requests are actually handled by
load shedder (#2663) via NachoSoto (@NachoSoto)
* `ETagManager.httpResultFromCacheOrBackend`: return response headers
(#2666) via NachoSoto (@NachoSoto)
* `Integration Tests`: added tests to verify 304 behavior (#2659) via
NachoSoto (@NachoSoto)
* `HTTPClient`: disable `URLSession` cache (#2668) via NachoSoto
(@NachoSoto)
* Documented `HTTPStatusCode.isSuccessfullySynced` (#2661) via NachoSoto
(@NachoSoto)
* `NetworkError.signatureVerificationFailed`: added status code to error
`userInfo` (#2657) via NachoSoto (@NachoSoto)
* `HTTPClient`: improved log for failed requests (#2669) via NachoSoto
(@NachoSoto)
* `ETagManager`: added new verbose logs (#2656) via NachoSoto
(@NachoSoto)
* `Signature Verification`: added test-only log for debugging invalid
signatures (#2658) via NachoSoto (@NachoSoto)
* Fixed `HTTPResponse.description` (#2664) via NachoSoto (@NachoSoto)
* Changed `Logger` to use `os_log` (#2608) via NachoSoto (@NachoSoto)
* `MainThreadMonitor`: increased threshold (#2662) via NachoSoto
(@NachoSoto)
* `debugRevenueCatOverlay`: display `receiptURL` (#2652) via NachoSoto
(@NachoSoto)
* `PurchaseTester`: added ability to display `debugRevenueCatOverlay`
(#2653) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: ability to close on `macOS`/`Catalyst`
(#2649) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: added support for `macOS` (#2648) via
NachoSoto (@NachoSoto)
* `LoadShedderIntegrationTests`: enable signature verification (#2655)
via NachoSoto (@NachoSoto)
* `ImageSnapshot`: fixed Xcode 15 compilation (#2651) via NachoSoto
(@NachoSoto)
* `OfferingsManager`: don't clear offerings cache timestamp when request
fails (#2359) via NachoSoto (@NachoSoto)
* `StoreKitObserverModeIntegrationTests`: added test for posting
renewals (#2590) via NachoSoto (@NachoSoto)
* Always initialize `StoreKit2TransactionListener` even on SK1 mode
(#2612) via NachoSoto (@NachoSoto)
* `ErrorUtils.missingReceiptFileError`: added receipt URL `userInfo`
context (#2650) via NachoSoto (@NachoSoto)
* Added `.xcprivacy` for Xcode 15 (#2619) via NachoSoto (@NachoSoto)
* `Trusted Entitlements`: added debug log with
`ResponseVerificationMode` (#2647) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: simplified title (#2641) via NachoSoto
(@NachoSoto)
* Simplified `Purchases.updateAllCachesIfNeeded` (#2626) via NachoSoto
(@NachoSoto)
* `HTTPResponseTests`: fixed disabled test (#2643) via NachoSoto
(@NachoSoto)
* Add `InternalDangerousSettings.forceSignatureFailures` (#2635) via
NachoSoto (@NachoSoto)
* `IntegrationTests`: explicit `StoreKit 1` mode (#2636) via NachoSoto
(@NachoSoto)
* `Signing`: removed API for loading key from a file (#2638) via
NachoSoto (@NachoSoto)
tonidero added a commit that referenced this pull request Jun 26, 2023
### Description 
Reported in RevenueCat/purchases-flutter#738. 

After #2654, we were
using the new Google Play identifiers that include the plan id in
expiration dates, but we didn't change the system that parses purchase
dates. Later on, when we map purchases, we iterate over the expiration
dates map, and since that id wasn't found in the purchase dates, it was
null, causing the issue in flutter.

This makes the purchase date calculation use the same algorithm that we
use for expiration dates to get the new google play product identifiers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants