Offline Entitlements: get SK2 purchase information #2352

aboedo · 2023-03-17T21:44:27Z

Just started work on this. Early draft getting purchase information from StoreKit 2 so we can create entitlements from it.

addresses sdk-2978

RevenueCat-Danger-Bot · 2023-03-17T21:44:43Z

	1 Error
🚫	Label the PR using one of the change type labels

Label	Description
breaking	Changes that are breaking
build	Changes that affect the build system
ci	Changes to our CI configuration files and scripts
docs	Documentation only changes
feat	A new feature
fix	A bug fix
perf	A code change that improves performance
refactor	A code change that neither fixes a bug nor adds a feature
style	Changes that don't affect the meaning of the code (white-space, formatting, missing semi-colons, etc
test	Adding missing tests or correcting existing tests
next_release	Preparing a new release
dependencies	Updating a dependency
phc_dependencies	Updating purchases-hybrid-common dependency

Generated by 🚫 Danger

NachoSoto · 2023-03-17T21:57:57Z

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift

+@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
+struct PurchasedProductsManager {
+
+    func fetchPurchasedProducts() async throws -> [PurchasedSK2Product] {


This would probably be static, right?

I went with instance out of sheer habit. Based on the env detector there, though, it looks like current style is protocol + static + initializer with default param? Happy to go with that instead

Oh wait, this is a "manager" ignore my comment. I was thinking this was a product type.

so... to be clear, do you like this style better than the other one? I'm happy to go with either.

Yeah this makes sense 👍🏻
Both are the same thing TBH, but this makes mocking a bit easier.

Actually this is a struct, so it can't be mocked. So might as well make this static. But this is fine for now, let's see how we need to use this and/or mock it and then we can change it.

NachoSoto · 2023-03-17T21:58:25Z

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift

+/// a Factory pattern to store the logic in one place
+@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
+struct PurchasedSK2Product {
+    let productIdentifier: String


Nit: line break inside types (it took me a while to get used to this but now it's hard not to hehe)

I like it 💪🏻 thanks! FWIW, expect to see some of the ugliest code ever while this is in draft

NachoSoto · 2023-03-21T15:33:29Z

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift

+                                  billingIssueDetectedAt: purchasedSK2Product.billingIssueDetectedAt,
+                                  ownershipType: purchasedSK2Product.ownershipType,
+                                  verification: purchasedSK2Product.verification),
+                  rawData: [:]) // todo: should we just have the data be the transaction's raw data?


Yeah ideally this matches the same format as our normal EntitlementInfos, but that's going to be hard to guarantee. Given that we don't document it, I think it would be safe to have something else, like the transaction's raw data as you said 👍🏻

NachoSoto · 2023-03-21T15:35:30Z

Sources/Purchasing/EntitlementInfo.swift

@@ -246,6 +246,14 @@ extension PeriodType: DefaultValueProvider {
        self.rawData = entitlement.rawData
    }

+    init(contents: EntitlementInfo.Contents,


I'm surprised this compiles. Can you make the other constructor convenience and delegate it to this one now?

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift

NachoSoto · 2023-03-21T16:29:08Z

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift

+            self.periodType = .normal
+        }
+
+        self.isActive = expirationDate == nil || expirationDate! > Date() // todo: check what we usually do for non-subs


To avoid the force unwrap maybe:

Suggested change

self.isActive = expirationDate == nil || expirationDate! > Date() // todo: check what we usually do for non-subs

self.isActive = expirationDate.map { $0 > Date() } ?? false // todo: check what we usually do for non-subs

NachoSoto · 2023-03-21T16:29:51Z

Sources/Logging/Strings/OfflineEntitlementsStrings.swift

+@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
+enum OfflineEntitlementsStrings {
+
+    case found_unverified_transactions_in_sk2(transaction: StoreKit.Transaction, error: Error)


Nit: I think these parameter names don't add anything useful that's not already encoded in the types:

Suggested change

case found_unverified_transactions_in_sk2(transaction: StoreKit.Transaction, error: Error)

case found_unverified_transactions_in_sk2(StoreKit.Transaction, Error)

NachoSoto · 2023-03-21T16:30:36Z

Sources/Logging/Strings/OfflineEntitlementsStrings.swift

+
+    var description: String {
+        switch self {
+        case .found_unverified_transactions_in_sk2(let transaction, let error):


We don't have anything in the style guide, but if you agree with this maybe we should:

Suggested change

case .found_unverified_transactions_in_sk2(let transaction, let error):

case let .found_unverified_transactions_in_sk2(transaction, error):

One let instead of multiple?

NachoSoto · 2023-03-21T16:31:06Z

This is a great start 💪🏻 about to branch off to work on exposing the CustomerInfo too.

aboedo · 2023-03-21T21:07:54Z

Sources/Logging/Strings/OfflineEntitlementsStrings.swift

+    var description: String {
+        switch self {
+        case let .found_unverified_transactions_in_sk2(transaction, error):
+            return """
+                Found an unverified transaction. It will be ignored and will not be a part of CustomerInfo.
+                    Details:
+                    Error: \(error.localizedDescription)
+                    Transaction: \(transaction.debugDescription)
+            """


@NachoSoto I also updated this to multi-line string, I figured it's much easier to read now. WDTY?

NachoSoto · 2023-03-21T23:07:19Z

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift

+}
+
+@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
+extension PurchasedSK2Product {


I think we need a different representation here, so we can use the same code that constructs EntitlementInfos from the response data. This is useful for creating an individual EntitlementInfo, but we'd have to change a bunch more code. Instead, we can use the types in CustomerInfoResponse so that at the top level we can construct a CustomerInfo the same way.

Might not make a lot of sense, but I'll have a PR ready to show what I mean.

My new suggested type:

struct PurchasedSK2Product { let subscription: CustomerInfoResponse.Subscription let transaction: CustomerInfoResponse.Transaction let entitlement: CustomerInfoResponse.Entitlement }

NachoSoto · 2023-03-22T20:07:17Z

Continued on #2358

…ts (#2358) Based on #2352. Depends on #2351 and #2365. --------- Co-authored-by: Andy Boedo <andresboedo@gmail.com> Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>

WIP, creating class to get the info we need from StoreKit.Transaction.

b8ef951

addresses sdk-2978

aboedo self-assigned this Mar 17, 2023

NachoSoto reviewed Mar 17, 2023

View reviewed changes

aboedo added 5 commits March 17, 2023 19:49

added a couple of todos

832d484

cleaned up comments to showcase the idea

7b0df32

updated to fill missing fields

df9d836

mapped missing fields

2282472

some cleanup, added logging

5bb8337

NachoSoto reviewed Mar 21, 2023

View reviewed changes

Sources/OfflineEntitlements/PurchasedProductsFetcher.swift Show resolved Hide resolved

NachoSoto reviewed Mar 21, 2023

View reviewed changes

NachoSoto mentioned this pull request Mar 21, 2023

Offline Entitlements: create CustomerInfo from offline entitlements #2358

Merged

cleanup from PR comments

b8d5148

aboedo commented Mar 21, 2023

View reviewed changes

NachoSoto reviewed Mar 21, 2023

View reviewed changes

NachoSoto closed this Mar 23, 2023

NachoSoto deleted the andy/sdk-2978-write-code-to-list-purchases-made-by-the branch March 23, 2023 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Offline Entitlements: get SK2 purchase information #2352

Offline Entitlements: get SK2 purchase information #2352

aboedo commented Mar 17, 2023

RevenueCat-Danger-Bot commented Mar 17, 2023

NachoSoto Mar 17, 2023

aboedo Mar 20, 2023

NachoSoto Mar 20, 2023

aboedo Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 17, 2023

aboedo Mar 20, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto commented Mar 21, 2023

aboedo Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 21, 2023

NachoSoto Mar 22, 2023

NachoSoto commented Mar 22, 2023

	self.isActive = expirationDate == nil \|\| expirationDate! > Date() // todo: check what we usually do for non-subs
	self.isActive = expirationDate.map { $0 > Date() } ?? false // todo: check what we usually do for non-subs

	case found_unverified_transactions_in_sk2(transaction: StoreKit.Transaction, error: Error)
	case found_unverified_transactions_in_sk2(StoreKit.Transaction, Error)

	case .found_unverified_transactions_in_sk2(let transaction, let error):
	case let .found_unverified_transactions_in_sk2(transaction, error):

Offline Entitlements: get SK2 purchase information #2352

Offline Entitlements: get SK2 purchase information #2352

Conversation

aboedo commented Mar 17, 2023

RevenueCat-Danger-Bot commented Mar 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NachoSoto commented Mar 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NachoSoto commented Mar 22, 2023