Skip to content

Restore purchases: post product data when posting receipts#2178

Merged
NachoSoto merged 3 commits into
mainfrom
restore-product-data
Dec 27, 2022
Merged

Restore purchases: post product data when posting receipts#2178
NachoSoto merged 3 commits into
mainfrom
restore-product-data

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Dec 20, 2022

Copy link
Copy Markdown
Contributor

Fixes CSDK-604.

When restore receipts, we don't post any price for products because we don't know what the user bought.
As a best-effort approach, this parses the AppleReceipt and looks for products that were purchased.

@NachoSoto NachoSoto added the pr:fix A bug fix label Dec 20, 2022
@NachoSoto NachoSoto requested a review from a team December 20, 2022 19:59
sha1Hash: Data(),
creationDate: Date(),
expirationDate: nil,
inAppPurchases: [

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added a bunch of products to test the sorting / filtering behavior .

Comment on lines +1124 to +1132
guard let receipt = try? self.receiptParser.parse(from: receiptData) else { completion(nil); return }

// Find the latest purchased subscription
guard let productIdentifier = receipt.inAppPurchases
.lazy
.filter({ $0.isActiveSubscription })
.sorted(by: { $0.purchaseDate > $1.purchaseDate })
.first?
.productId else { completion(nil); return }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this feels out of scope for this class, tbh. Like there should either be a separate object for it, or potentially the ReceiptParser itself should be able to answer the latest subscriptions found in the receipt.

Also... This shouldn't be limited to subscriptions, we're really just looking for a set of all product ids the customer has ever purchased

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this feels out of scope for this class, tbh. Like there should either be a separate object for it, or potentially the ReceiptParser itself should be able to answer the latest subscriptions found in the receipt.

Totally good point

This shouldn't be limited to subscriptions

I can remove that filter, but....

we're really just looking for a set of all product ids the customer has ever purchased

But we need to pick one, because we post only 1 ProductRequestData based on a single StoreProduct.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ohh interesting. We should check with the backend team, maybe this is a limitation in khepri that we can work around. If not, then I think the active subscription approach is as good as any

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We do not support multiple productIds by receipt. I think there are a lot of things that would have to change to support more than one product. Receipt and Transaction are laid out to have a single product.
There was talk also to support multi lineItem receipts, but I don't think we have a concrete plan for this.

@aboedo aboedo Dec 21, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@swehner that's good to know. We're not really looking to map this to a purchase of multiple products, really, we're just trying to provide price information for the products that this user has purchased in the past that they might be restoring. Would that be achievable without trying to convert this from a restore event to a purchase event?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@swehner @antoniobg could I get final confirmation that sending the price of the active subscription + the product id in a restore won't negatively affect any endpoints / webhooks?

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.

@aboedo I think it should be fine. It might affect the prices for newer transactions for the receipts for which we didn't have cached prices before, but I think it will be more accurate

@NachoSoto NachoSoto force-pushed the restore-product-data branch from 2fc99f0 to e8f565e Compare December 22, 2022 18:59
@NachoSoto NachoSoto requested a review from aboedo December 22, 2022 19:14
@NachoSoto

Copy link
Copy Markdown
Contributor Author

@aboedo I extracted the logic to AppleReceipt and added a few more tests.

Comment thread Sources/LocalReceiptParsing/BasicTypes/AppleReceipt.swift Outdated
Fixes [CSDK-604].

When restore receipts, we don't post any price for products because we don't know what the user bought.
As a best-effort approach, this parses the `AppleReceipt` and looks for products that were purchased.
@NachoSoto NachoSoto force-pushed the restore-product-data branch from ef7b96e to f6240ce Compare December 27, 2022 17:47
@NachoSoto NachoSoto enabled auto-merge (squash) December 27, 2022 17:48
@NachoSoto NachoSoto merged commit 0819eca into main Dec 27, 2022
@NachoSoto NachoSoto deleted the restore-product-data branch December 27, 2022 18:13
NachoSoto pushed a commit that referenced this pull request Dec 27, 2022
**This is an automatic release.**

### New Features
* Created `ReceiptParser` SPM (#2155) via NachoSoto (@NachoSoto)
* Exposed `PurchasesReceiptParser` and `AppleReceipt` (#2153) via
NachoSoto (@NachoSoto)
### Bugfixes
* `Restore purchases`: post product data when posting receipts (#2178)
via NachoSoto (@NachoSoto)
### Other Changes
* Added `Dictionary.merge` (#2190) via NachoSoto (@NachoSoto)
* `CircleCI`: use Xcode 14.2.0 (#2187) via NachoSoto (@NachoSoto)
* `ReceiptParser`: a few documentation improvements (#2189) via
NachoSoto (@NachoSoto)
* `Purchase Tester`: fixed `TestFlight` deployment (#2188) via NachoSoto
(@NachoSoto)
* `Purchase Tester`: display specific `IntroEligibilityStatus` (#2184)
via NachoSoto (@NachoSoto)
* `Purchase Tester`: fixed `SubscriptionPeriod` (#2185) via NachoSoto
(@NachoSoto)
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.

4 participants