Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NonSubscriptionTransaction: expose storeTransactionIdentifier #3639

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

NachoSoto
Copy link
Contributor

Fixes #3636.

This was added in #3009.
It was not made public because it was only used for #2841, but as explained in #3636 it can be useful as public.

Fixes #3636.

This was added in #3009.
It was not made `public` because it was only used for #2841, but as explained in #3636 it can be useful as `public`.
@NachoSoto NachoSoto added the pr:feat A new feature label Feb 6, 2024
@NachoSoto NachoSoto requested a review from a team February 6, 2024 16:57
@NachoSoto
Copy link
Contributor Author

@aboedo you pointed out in #3009 (comment) that you wouldn't make it public. Thoughts on this now?

You mentioned:

a) we don't currently expose it for subscriptions, so maybe it's better to avoid confusion

But we do expose it when purchasing through StoreTransaction.

Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to wait for @aboedo's review, since he had concerns on making this public. But LGTM!

@@ -32,7 +32,7 @@ public final class NonSubscriptionTransaction: NSObject {
@objc public let transactionIdentifier: String

/// The unique identifier for the transaction created by the Store.
@objc internal let storeTransactionIdentifier: String
@objc public let storeTransactionIdentifier: String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only slightly concerned about the confussion of having both ids... but I think it shoud be ok?

@NachoSoto NachoSoto merged commit 51fd49e into main Feb 6, 2024
24 checks passed
@NachoSoto NachoSoto deleted the non-sub-transaction-store-id branch February 6, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:feat A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NonSubscriptionTransaction.transactionIdentifier Does not match StoreTransaction.identifier
3 participants