`Configuration`: log warning if attempting to use observer mode with StoreKit 2 by NachoSoto · Pull Request #3066 · RevenueCat/purchases-ios

NachoSoto · 2023-08-23T19:13:31Z

See also https://github.com/RevenueCat/revenuecat-docs/pull/299.

Since #3032 developers using observer mode need to configure the SDK with the correct StoreKit 2 setting.

codecov · 2023-08-23T20:47:41Z

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.05% ⚠️

Comparison is base (461111b) 85.93% compared to head (5e7f906) 85.89%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3066      +/-   ##
==========================================
- Coverage   85.93%   85.89%   -0.05%     
==========================================
  Files         233      233              
  Lines       16654    16662       +8     
==========================================
  Hits        14312    14312              
- Misses       2342     2350       +8

Files Changed	Coverage Δ
Sources/Misc/Deprecations.swift	`37.85% <ø> (ø)`
Sources/Purchasing/Purchases/Purchases.swift	`76.75% <ø> (ø)`
Sources/Logging/Strings/ConfigureStrings.swift	`82.88% <100.00%> (+0.31%)`	⬆️
Sources/Purchasing/Configuration.swift	`82.07% <100.00%> (+1.07%)`	⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tonidero

I'm wondering about whether we need to undeprecate the storekit 2 setting... But I understand linking observer mode to the storeKit2 setting, so I think this is probably better

tonidero · 2023-08-24T08:05:14Z

I'm wondering whether we really need to keep useStoreKit2IfAvailable deprecated... Considering we still need to use it here. Also, devs can potentially do .with(observerMode: false, storeKit2: true), to bypass the deprecation and achieve the same result, so it's pretty much as if it wasn't deprecated...

I'm wondering whether we really need to keep useStoreKit2IfAvailable deprecated...

@aboedo and I talked about this yesterday. We think it makes sense to keep it deprecated until we finish supporting SK2 in the backend, at which point we'll at least be following Apple's guidance.

Also, devs can potentially do .with(observerMode: false, storeKit2: true)

Yeah maybe the method should have never been .with(observerMode: Bool) but instead .enableObserverMode().

Feels a little odd to have to select "storekit 2 false" now, like, I'm not sure that tells you all that much about what's happening.

How about

enum StoreKitVersion { case StoreKit1, StoreKit2 } .with(observerMode: Bool, storeKitVersion: StoreKitVersion) -> Builder {

(well, with changes for objc compatibility for the enum and stuff)

Isn't that the same for usesStoreKit2IfAvailable? We made that a Bool too.
While we're at it, we could add an overload of that one to use this new enum. What do you think?

I wouldn't add a new API that's already deprecated when we add it.
I was only suggesting that

.with(observerMode: false, storeKitVersion: storeKit1)

might be slightly more expressive than

.with(observerMode: false, storeKit2: false)

But I don't feel strongly about it

That way the docstring can read something like

* Set `observerMode` with a corresponding `StoreKit` implementation * - Parameter storeKitVersion: Set this to the StoreKit implementation * your app uses for purchases. * I.e.: if your app uses StoreKit 1, set it to `.storeKit1`, * and if your app uses StoreKit 2, set it to `.storeKit2`. * Apps using StoreKit 1 use `SKPaymentQueue` for transactions, * whereas StoreKit 2 uses `StoreKit.Product.Purchase`.

Added some more text in case folks don't know which is which. Apple removed many of the SK1 docs, so I think it's likely that folks will start getting confused about this over time as SK1 slowly fades away

I'd avoid adding a new enum + docs + Obj-C equivalent etc just for one API.
But I do think it makes sense to change the deprecated method (which arguably won't be once we fully support SK2) to use it as well.

@aboedo changed!

I'd avoid adding a new enum + docs + Obj-C equivalent etc just for one API.
But I do think it makes sense to change the deprecated method (which arguably won't be once we fully support SK2) to use it as well.

I missed this comment but that makes a lot of sense

tonidero · 2023-08-24T08:10:49Z

I wonder if we should deprecate this method and ask people to use the other one to make it more explicit...

I think that would be a good way to notify developers that are currently using this function

Yeah I'll do that.

If a developer is currently using this function, it's almost certain that they're using SK1, though, right? Like, it seems fairly unlikely that you:

have an existing SK1 implementation that you don't want to touch, but you want RC, so you add observer mode

decide that you do want to move into SK2, but still want to keep RC in observer mode

Right?

So I'd vote to just keep it without deprecation

vegaro · 2023-08-24T13:37:48Z

I think that would be a good way to notify developers that are currently using this function

NachoSoto · 2023-08-24T18:43:31Z

This was missing from the API tester.

aboedo · 2023-08-24T20:23:43Z

That way the docstring can read something like

* Set `observerMode` with a corresponding `StoreKit` implementation * - Parameter storeKitVersion: Set this to the StoreKit implementation * your app uses for purchases. * I.e.: if your app uses StoreKit 1, set it to `.storeKit1`, * and if your app uses StoreKit 2, set it to `.storeKit2`. * Apps using StoreKit 1 use `SKPaymentQueue` for transactions, * whereas StoreKit 2 uses `StoreKit.Product.Purchase`.

Added some more text in case folks don't know which is which. Apple removed many of the SK1 docs, so I think it's likely that folks will start getting confused about this over time as SK1 slowly fades away

NachoSoto · 2023-08-29T20:18:59Z

Planning to ship this with the paywalls release since they both require a minor.

NachoSoto · 2023-08-31T15:48:21Z

Updated this per our conversation:

Not deprecated .with(observerMode:)
Not deprecated .with(observerMode:usesStoreKit2:)
Deprecated .with(usesStoreKit2IfAvailable:)

NachoSoto · 2023-08-31T15:49:19Z

 public extension Configuration.Builder {

-    /// Set `usesStoreKit2IfAvailable`. If `true`, the SDK will use StoreKit 2 APIs internally. If disabled, it will use StoreKit 1 APIs instead.
+    /// Set `storeKit2Setting`. If `true`, the SDK will use StoreKit 2 APIs internally. If disabled, it will use StoreKit 1 APIs instead.


This is the name of the property in Configuration.Builder.

should we use double backticks to create a code reference then, so mismatches get caught at the docs generation step?

NachoSoto · 2023-08-31T17:04:19Z

Changing this since we can't support observer mode + SK2.

…StoreKit 2 See also https://github.com/RevenueCat/revenuecat-docs/pull/299. Since #3032 developers using observer mode need to configure the SDK with the correct StoreKit 2 setting. This new API makes that explicit, while leaving `with(usesStoreKit2IfAvailable:)` still deprecated.

slavasemeniuk · 2023-09-18T17:23:01Z

Changing this since we can't support observer mode + SK2.

Hey, is there any insights on availability of SK 2 support for the observer mode?

Also is there any way to just manually call something like ‘trackPurchase(transaction)’?

Thanks

NachoSoto · 2023-09-18T21:04:21Z

Also is there any way to just manually call something like ‘trackPurchase(transaction)’?

@slavasemeniuk
We are indeed looking into adding that exact API in our continuous effort to better support StoreKit 2 :)

aboedo

do we plan on doing the syncObserverModePurchase thing? Is this an intermediate step?

aboedo · 2023-09-19T20:16:52Z

 public extension Configuration.Builder {

-    /// Set `usesStoreKit2IfAvailable`. If `true`, the SDK will use StoreKit 2 APIs internally. If disabled, it will use StoreKit 1 APIs instead.
+    /// Set `storeKit2Setting`. If `true`, the SDK will use StoreKit 2 APIs internally. If disabled, it will use StoreKit 1 APIs instead.


should we use double backticks to create a code reference then, so mismatches get caught at the docs generation step?

NachoSoto · 2023-09-19T20:20:44Z

do we plan on doing the syncObserverModePurchase thing? Is this an intermediate step?

Yup that's the plan (@MarkVillacampa do you think you can take on that? I think you're pretty familiar with the setup now, and I'm happy to support you in the process :))

This is an intermediate step. Once the API is available we can remove the warning and update the documentation

NachoSoto added the pr:feat A new feature label Aug 23, 2023

NachoSoto requested a review from a team August 23, 2023 19:13

tonidero approved these changes Aug 24, 2023

View reviewed changes

vegaro approved these changes Aug 24, 2023

View reviewed changes

NachoSoto force-pushed the observer-mode-parameter branch from 90cabe0 to 8f5f709 Compare August 24, 2023 18:41

NachoSoto changed the title ~~Configuration: added observerMode overload that allows configuring SK2 setting~~ Configuration: changed observerMode method to select SK2 setting Aug 24, 2023

NachoSoto commented Aug 24, 2023

View reviewed changes

NachoSoto requested a review from aboedo August 24, 2023 18:59

aboedo approved these changes Aug 24, 2023

View reviewed changes

NachoSoto force-pushed the observer-mode-parameter branch 2 times, most recently from 2b43259 to af0ea6c Compare August 29, 2023 20:10

NachoSoto requested a review from aboedo August 29, 2023 20:11

NachoSoto force-pushed the observer-mode-parameter branch from af0ea6c to d5d78b3 Compare August 29, 2023 20:14

NachoSoto changed the title ~~Configuration: changed observerMode method to select SK2 setting~~ Configuration: new .with(observerMode:storeKitVersion:) Aug 31, 2023

NachoSoto commented Aug 31, 2023

View reviewed changes

NachoSoto added the HOLD label Aug 31, 2023

NachoSoto force-pushed the observer-mode-parameter branch from 47e015f to 75063bd Compare September 14, 2023 19:17

NachoSoto changed the title ~~Configuration: new .with(observerMode:storeKitVersion:)~~ Configuration: log warning if attempting to use observer mode with StoreKit 2 Sep 14, 2023

NachoSoto assigned NachoSoto and unassigned NachoSoto Sep 14, 2023

NachoSoto added refactor and removed HOLD pr:feat A new feature labels Sep 14, 2023

NachoSoto force-pushed the observer-mode-parameter branch 3 times, most recently from 4fd073c to 17fce11 Compare September 14, 2023 19:20

NachoSoto requested review from tonidero and vegaro September 15, 2023 19:33

NachoSoto force-pushed the observer-mode-parameter branch from 17fce11 to 6a82362 Compare September 15, 2023 19:33

NachoSoto force-pushed the observer-mode-parameter branch from 6a82362 to 5e7f906 Compare September 15, 2023 19:33

tonidero approved these changes Sep 18, 2023

View reviewed changes

aboedo reviewed Sep 19, 2023

View reviewed changes

NachoSoto merged commit 7fa0b26 into main Sep 19, 2023

NachoSoto deleted the observer-mode-parameter branch September 19, 2023 20:21

This was referenced Sep 20, 2023

[AUTOMATIC] Release/4.26.1 #3231

Closed

[AUTOMATIC] Release/4.26.1 #3232

Closed

[AUTOMATIC] Release/4.26.1 #3234

Closed

[AUTOMATIC] Release/4.26.1 #3236

Merged

NachoSoto mentioned this pull request Dec 6, 2023

[SK2] Add Configuration.with(storeKitVersion:) to select the version of StoreKit to use #3487

Merged

Conversation

NachoSoto commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

Choose a reason for hiding this comment

Uh oh!

aboedo Aug 24, 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!

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!

aboedo Aug 30, 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Aug 29, 2023

Uh oh!

NachoSoto commented Aug 31, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Aug 31, 2023

Uh oh!

slavasemeniuk commented Sep 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NachoSoto commented Sep 18, 2023

Uh oh!

aboedo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Sep 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

NachoSoto commented Aug 23, 2023 •

edited

Loading

codecov Bot commented Aug 23, 2023 •

edited

Loading

aboedo Aug 24, 2023 •

edited

Loading

aboedo Aug 30, 2023 •

edited

Loading

slavasemeniuk commented Sep 18, 2023 •

edited

Loading