Skip to content

PaymentQueueWrapper: also conform to SKPaymentTransactionObserver to fix promoted purchases#1962

Merged
NachoSoto merged 1 commit into
mainfrom
payment-queue-wrapper-transaction-observer
Oct 7, 2022
Merged

PaymentQueueWrapper: also conform to SKPaymentTransactionObserver to fix promoted purchases#1962
NachoSoto merged 1 commit into
mainfrom
payment-queue-wrapper-transaction-observer

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Follow up to #1901. That fix was incomplete. I missed it because the protocols were mixed up (fixed in #1961). Now PaymentQueueWrapper, when set up as the delegate (only when SK2 is enabled and SK1 is disabled) will be added as a transaction observer.

@NachoSoto NachoSoto added the pr:fix A bug fix label Sep 30, 2022
@NachoSoto NachoSoto requested a review from a team September 30, 2022 19:37
@NachoSoto NachoSoto force-pushed the payment-queue-wrapper-transaction-observer branch from 4eedf0d to 688b5cf Compare September 30, 2022 19:37

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 is equivalent, but avoids duplicating the expectation in the if and the assert.

@NachoSoto NachoSoto force-pushed the payment-queue-wrapper-transaction-observer branch 2 times, most recently from 9d67a6c to 8e43fe8 Compare September 30, 2022 20:42

extension PaymentQueueWrapper: SKPaymentQueueDelegate {

}

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.

Do we need this then?

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.

Ugh I did this a week ago and now I'm confusing myself. My best guess is I forgot something, one sec.

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 was in the process of implementing this protocol too, when I was convinced I already did it...

Yup, that's in #1963 🤦🏻‍♂️

Base automatically changed from store-kit-wrapper-delegates to main October 7, 2022 14:31
… to fix promoted purchases

Follow up to #1901. That fix was incomplete. I missed it because the protocols were mixed up (fixed in #1961).
Now `PaymentQueueWrapper`, when set up as the delegate (only when SK2 is enabled and SK1 is disabled) will be added as a transaction observer.
@NachoSoto NachoSoto force-pushed the payment-queue-wrapper-transaction-observer branch from 8e43fe8 to 3ef87d2 Compare October 7, 2022 14:31
@NachoSoto NachoSoto merged commit 7dd98ee into main Oct 7, 2022
@NachoSoto NachoSoto deleted the payment-queue-wrapper-transaction-observer branch October 7, 2022 14:54
NachoSoto added a commit that referenced this pull request Oct 7, 2022
Follow up to #1962. This is also needed when `StoreKit1Wrapper` is not
setup (see #1882).
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.

2 participants