Skip to content

PurchaseTesterSwiftUI: allow configuring API key at runtime#1999

Merged
NachoSoto merged 3 commits into
mainfrom
purchase-tester-api-key-2
Nov 3, 2022
Merged

PurchaseTesterSwiftUI: allow configuring API key at runtime#1999
NachoSoto merged 3 commits into
mainfrom
purchase-tester-api-key-2

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Oct 25, 2022

Copy link
Copy Markdown
Contributor

Fixes CSDK-456.

Configuration screen:

Simulator Screen Shot - iPhone 13 Pro Max - 2022-10-25 at 15 02 26

Displaying errors:

_Note that if you enter an invalid API key and there's a credentials issue, you can tap on "reconfigure" to start over:

Simulator Screen Shot - iPhone 13 Pro - 2022-10-26 at 13 35 42

@NachoSoto NachoSoto added the pr:dependencies Changes on external dependencies label Oct 25, 2022
@NachoSoto NachoSoto requested review from a team October 25, 2022 22:04
@NachoSoto NachoSoto changed the title PurchaseTesterSwiftUI: allow configuring APIKey at runtime PurchaseTesterSwiftUI: allow configuring API key at runtime Oct 25, 2022
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Actually I'm gonna save it in user defaults so it remembers the last used one.

@NachoSoto NachoSoto force-pushed the purchase-tester-api-key branch from e1b2431 to f1458de Compare October 26, 2022 18:27
@NachoSoto NachoSoto force-pushed the purchase-tester-api-key-2 branch from d45eed4 to 6cb1fc6 Compare October 26, 2022 18:27
Base automatically changed from purchase-tester-api-key to main October 28, 2022 19:17
@NachoSoto NachoSoto force-pushed the purchase-tester-api-key-2 branch from 00ab935 to e24e7ce Compare October 28, 2022 19:19

@tonidero tonidero 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.

Just some questions but looking good!


func purchases(_ purchases: Purchases, readyForPromotedProduct product: StoreProduct, purchase makeDeferredPurchase: @escaping StartPurchaseBlock) {
makeDeferredPurchase { (transaction, customerInfo, error, success) in
print("Yay")

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.

Should we print something more meaningful? I guess this will not be called usually but something more meaningful would be better :P

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 agree, I just copied the existing implementation. There's tons we could improve but this PR focuses on just this feature.

.toolbar {
ToolbarItem(placement: .navigationBarLeading) {
Button {
self.configuration = nil

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.

Does the iOS SDK keep some sort of shared instance of Purchases? Just wondering if this is enough to reset it.

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.

Technically not, but it will be re-created when it gets re-configured.

}
}
.onChange(of: self.data) { _ in
self.saveData()

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.

A UX question, should we only save data when pressing the "Continue" button? I think it shouldn't matter much, but seems more intuitive for me... For example, if a user wants to reconfigure the sdk, starts changing the data but then restarts the app, I would expect to still have the old information... NABD though.

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.

Yeah, I could see either way. I don't think this is that "wrong", so if you don't mind I'm going to leave it as is so I don't have to re-test this 9-day old PR.

@NachoSoto NachoSoto merged commit 2773783 into main Nov 3, 2022
@NachoSoto NachoSoto deleted the purchase-tester-api-key-2 branch November 3, 2022 21:44
NachoSoto pushed a commit that referenced this pull request Nov 9, 2022
**This is an automatic release.**

### Bugfixes
* `ISO8601DateFormatter.withMilliseconds`: fixed iOS 11 crash (#2037)
via NachoSoto (@NachoSoto)
* Changed `StoreKit2Setting.default` back to
`.enabledOnlyForOptimizations` (#2022) via NachoSoto (@NachoSoto)
### Other Changes
* `Integration Tests`: changed weekly to monthly subscriptions to work
around 0-second subscriptions (#2042) via NachoSoto (@NachoSoto)
* `Integration Tests`: fixed `testPurchaseWithAskToBuyPostsReceipt`
(#2040) via NachoSoto (@NachoSoto)
* `ReceiptRefreshPolicy.retryUntilProductIsFound`: default to returning
"invalid" receipt (#2024) via NachoSoto (@NachoSoto)
* `CachingProductsManager`: use partial cached products (#2014) via
NachoSoto (@NachoSoto)
* Added `BackendErrorCode.purchasedProductMissingInAppleReceipt` (#2033)
via NachoSoto (@NachoSoto)
* `PurchaseTesterSwiftUI`: replaced `Purchases` dependency with `SPM`
(#2027) via NachoSoto (@NachoSoto)
* `Integration Tests`: changed log output to `raw` (#2031) via NachoSoto
(@NachoSoto)
* `Integration Tests`: run on iOS 16 (#2035) via NachoSoto (@NachoSoto)
* CI: fixed `iOS 14` tests Xcode version (#2030) via NachoSoto
(@NachoSoto)
* `Async.call`: added non-throwing overload (#2006) via NachoSoto
(@NachoSoto)
* Documentation: Fixed references in `V4_API_Migration_guide.md` (#2018)
via NachoSoto (@NachoSoto)
* `eligiblePromotionalOffers`: don't log error if response is ineligible
(#2019) via NachoSoto (@NachoSoto)
* Runs push-pods after make-release (#2025) via Cesar de la Vega
(@vegaro)
* Some updates on notify-on-non-patch-release-branches: (#2026) via
Cesar de la Vega (@vegaro)
* Deploy `PurchaseTesterSwiftUI` to TestFlight (#2003) via NachoSoto
(@NachoSoto)
* `PurchaseTesterSwiftUI`: added "logs" screen (#2012) via NachoSoto
(@NachoSoto)
* `PurchaseTesterSwiftUI`: allow configuring API key at runtime (#1999)
via NachoSoto (@NachoSoto)
NachoSoto added a commit that referenced this pull request Dec 20, 2022
…tances in memory

We allow calling `Purchases.configure` multiple times. Likely a programmer error, but it's common in tests, or for example in `Purchase Tester` where users can re-configure the SDK at runtime (#1999).
When this happens, the SDK was creating the new instance before clearing the old one from memory (clear thanks to #2082, notice how the first `deinit` happens after the second `init`):
```
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600003ca4200)
[Purchases] - INFO: ℹ️ Purchases instance already set. Did you mean to configure two Purchases objects?
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600003c8a100)
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600003ca4200)
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600003c8a100)
```

With this change, we avoid that, which could also avoid issues with concurrent requests ([CSDK-517]):
```
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600002b6e600)
[Purchases] - INFO: ℹ️ Purchases instance already set. Did you mean to configure two Purchases objects?
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600002b6e600)
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600002b68100)
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600002b68100)
```
NachoSoto added a commit that referenced this pull request Dec 21, 2022
…tances in memory (#2180)

We allow calling `Purchases.configure` multiple times. Likely a
programmer error, but it's common in tests, or for example in `Purchase
Tester` where users can re-configure the SDK at runtime (#1999), which
is where I saw this issue.

When this happens, the SDK was creating the new instance before clearing
the old one from memory (clear thanks to #2082). Notice how the first
`deinit` happens **after** the second `init`:
```
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600003ca4200)
[Purchases] - INFO: ℹ️ Purchases instance already set. Did you mean to configure two Purchases objects?
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600003c8a100)
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600003ca4200)
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600003c8a100)
```

With this change, we avoid that, which could also avoid issues with
concurrent requests ([CSDK-517]):
```
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600002b6e600)
[Purchases] - INFO: ℹ️ Purchases instance already set. Did you mean to configure two Purchases objects?
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600002b6e600)
[Purchases] - VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x0000600002b68100)
[Purchases] - VERBOSE: Purchases.deinit: Purchases (0x0000600002b68100)
```

Unfortunately we don't have an easy way to test this, especially
considering that we prohibit multiple instances in tests (#2100).

[CSDK-517]:
https://revenuecats.atlassian.net/browse/CSDK-517?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:dependencies Changes on external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants