Skip to content

PurchasesOrchestrator: fixed callback not invoked regression during downgrades#3028

Merged
NachoSoto merged 1 commit into
mainfrom
revert-purchase-orchestrator-callback-date-hack
Aug 16, 2023
Merged

PurchasesOrchestrator: fixed callback not invoked regression during downgrades#3028
NachoSoto merged 1 commit into
mainfrom
revert-purchase-orchestrator-callback-date-hack

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Fixes #3020.
This essentially reverts #2890.

As shown in #3020, downgrades are reported with the same transaction identifier as the original transaction. Therefore our approach in #2890 is invalid.
We considered checking whether the SKPayment in the SKPaymentTransaction matches the one added to the queue as a way to disambiguate these transactions, but it doesn't. So we have no way of telling the difference between queue transactions and purchases.

I've left the original tests as a way to at least document this behavior.

… downgrades

Fixes #3020.
This essentially reverts #2890.

As shown in #3020, downgrades are reported with the same transaction identifier as the original transaction. Therefore our approach in #2890 is invalid.
We considered checking whether the `SKPayment` in the `SKPaymentTransaction` matches the one added to the queue as a way to disambiguate these transactions, but it doesn't. So we have no way of telling the difference between queue transactions and purchases.

I've left the original tests as a way to at least document this behavior.
@NachoSoto NachoSoto added the pr:fix A bug fix label Aug 16, 2023
@NachoSoto NachoSoto requested a review from a team August 16, 2023 21:48
@NachoSoto NachoSoto enabled auto-merge (squash) August 16, 2023 22:16
@NachoSoto NachoSoto merged commit e8f389a into main Aug 16, 2023
@NachoSoto NachoSoto deleted the revert-purchase-orchestrator-callback-date-hack branch August 16, 2023 23:26
@Abbott-Deng

Copy link
Copy Markdown

@NachoSoto I upgrade to 4.25.5, I get one error with "Value of type 'Purchases' has no member 'storeKit2Setting'", I found the storeKit2Setting is only visible in DEBUG macro. Please help check it.

NachoSoto added a commit that referenced this pull request Aug 17, 2023
Fixes #3028 (comment)
This was wrong in #3032 because `Purchases.storeKit2Setting` is only available in `DEBUG` builds, but we didn't catch it because we have no CI for Xcode 15 yet, which this adds as well.
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Thanks @Abbott-Deng. Sorry for the troubles.

I just fixed that in #3034 and will make a new release. I also added a new CI job that builds the SDK in release mode using Xcode 15 beta so we can catch this in the future.

NachoSoto pushed a commit that referenced this pull request Aug 17, 2023
**This is an automatic release.**

### Bugfixes
* `PurchasesOrchestrator`: fixed callback not invoked regression during
downgrades (#3028) via NachoSoto (@NachoSoto)
* `TransactionPoster`: don't finish transactions for non-subscriptions
if they're not processed (#2841) via NachoSoto (@NachoSoto)
### Performance Improvements
* `StoreKit 2`: only listen to `StoreKit.Transaction.updates` when SK2
is enabled (#3032) via NachoSoto (@NachoSoto)
* `CustomerInfoManager`: post transactions in parallel to POST receipts
only once (#2954) via NachoSoto (@NachoSoto)
### Other Changes
* `PostedTransactionCache`: remove implementation (#3030) via NachoSoto
(@NachoSoto)
* `Integration Tests`: improved `testCanPurchaseMultipleSubscriptions`
(#3025) via NachoSoto (@NachoSoto)
* `GitHub`: improved `ISSUE_TEMPLATE` (#3022) via NachoSoto (@NachoSoto)
* `TransactionPoster`: added transaction ID and Date to log (#3026) via
NachoSoto (@NachoSoto)
* `TransactionPoster`: fix iOS 12 test (#3018) via NachoSoto
(@NachoSoto)
* `SystemInfo`: added `ClockType` (#3014) via NachoSoto (@NachoSoto)
* `Integration Tests`: begin tests with
`UIApplication.willEnterForegroundNotification` to simulate a real app
(#3015) via NachoSoto (@NachoSoto)
* `Integration Tests`: add tests to verify `CustomerInfo`+`Offerings`
request de-dupping (#3013) via NachoSoto (@NachoSoto)
* `SwiftLint`: disable `unneeded_synthesized_initializer` (#3010) via
NachoSoto (@NachoSoto)
* Added `internal`
`NonSubscriptionTransaction.storeTransactionIdentifier` (#3009) via
NachoSoto (@NachoSoto)
* `Integration Tests`: added tests for non-renewing and non-consumable
packages (#3008) via NachoSoto (@NachoSoto)
* Expanded `EnsureNonEmptyArrayDecodable` to
`EnsureNonEmptyCollectionDecodable` (#3002) via NachoSoto (@NachoSoto)
NachoSoto added a commit that referenced this pull request Aug 17, 2023
Fixes
#3028 (comment)
This was wrong in #3032 because `Purchases.storeKit2Setting` is only
available in `DEBUG` builds, but we didn't catch it because we have no
CI for Xcode 15 yet, which this adds as well.
@NachoSoto

Copy link
Copy Markdown
Contributor Author

That's fixed in 4.25.6. Thanks again!

MarkVillacampa pushed a commit that referenced this pull request Sep 6, 2023
… downgrades (#3028)

Fixes #3020.
This essentially reverts #2890.

As shown in #3020, downgrades are reported with the same transaction
identifier as the original transaction. Therefore our approach in #2890
is invalid.
We considered checking whether the `SKPayment` in the
`SKPaymentTransaction` matches the one added to the queue as a way to
disambiguate these transactions, but it doesn't. So we have no way of
telling the difference between queue transactions and purchases.

I've left the original tests as a way to at least document this
behavior.
MarkVillacampa pushed a commit that referenced this pull request Sep 6, 2023
**This is an automatic release.**

### Bugfixes
* `PurchasesOrchestrator`: fixed callback not invoked regression during
downgrades (#3028) via NachoSoto (@NachoSoto)
* `TransactionPoster`: don't finish transactions for non-subscriptions
if they're not processed (#2841) via NachoSoto (@NachoSoto)
### Performance Improvements
* `StoreKit 2`: only listen to `StoreKit.Transaction.updates` when SK2
is enabled (#3032) via NachoSoto (@NachoSoto)
* `CustomerInfoManager`: post transactions in parallel to POST receipts
only once (#2954) via NachoSoto (@NachoSoto)
### Other Changes
* `PostedTransactionCache`: remove implementation (#3030) via NachoSoto
(@NachoSoto)
* `Integration Tests`: improved `testCanPurchaseMultipleSubscriptions`
(#3025) via NachoSoto (@NachoSoto)
* `GitHub`: improved `ISSUE_TEMPLATE` (#3022) via NachoSoto (@NachoSoto)
* `TransactionPoster`: added transaction ID and Date to log (#3026) via
NachoSoto (@NachoSoto)
* `TransactionPoster`: fix iOS 12 test (#3018) via NachoSoto
(@NachoSoto)
* `SystemInfo`: added `ClockType` (#3014) via NachoSoto (@NachoSoto)
* `Integration Tests`: begin tests with
`UIApplication.willEnterForegroundNotification` to simulate a real app
(#3015) via NachoSoto (@NachoSoto)
* `Integration Tests`: add tests to verify `CustomerInfo`+`Offerings`
request de-dupping (#3013) via NachoSoto (@NachoSoto)
* `SwiftLint`: disable `unneeded_synthesized_initializer` (#3010) via
NachoSoto (@NachoSoto)
* Added `internal`
`NonSubscriptionTransaction.storeTransactionIdentifier` (#3009) via
NachoSoto (@NachoSoto)
* `Integration Tests`: added tests for non-renewing and non-consumable
packages (#3008) via NachoSoto (@NachoSoto)
* Expanded `EnsureNonEmptyArrayDecodable` to
`EnsureNonEmptyCollectionDecodable` (#3002) via NachoSoto (@NachoSoto)
MarkVillacampa pushed a commit that referenced this pull request Sep 6, 2023
Fixes
#3028 (comment)
This was wrong in #3032 because `Purchases.storeKit2Setting` is only
available in `DEBUG` builds, but we didn't catch it because we have no
CI for Xcode 15 yet, which this adds as well.
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.

The completion block is not working when downgrade plan

3 participants