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

Paywalls: don't display "Purchases restored successfully" if nothings was restored #3233

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Sep 20, 2023

We can improve the logic here, but this is a simple improvement for the obvious case where nothing was actually restored.

…ntitlements

We can improve the logic here, but this is a simple improvement for the obvious case where nothing was actually restored.
We don't have an easy way to test this, but at least I added coverage for `onRestoreCompleted` which was missing.
@NachoSoto NachoSoto requested a review from a team September 20, 2023 21:15
Copy link
Member

@aboedo aboedo left a comment

Choose a reason for hiding this comment

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

let's play it safe and generalize to all transactions, not just active entitlements

Comment on lines 172 to 174
if !customerInfo.entitlements.active.isEmpty {
self.displayRestoredAlert = true
}
Copy link
Member

Choose a reason for hiding this comment

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

This might not be accurate - it assumes that the developer is using entitlements, which is encouraged but not required.
We should check nonSubscriptionTransactions as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.

@NachoSoto NachoSoto changed the title Paywalls: don't display "Purchases restored successfully" with no entitlements Paywalls: don't display "Purchases restored successfully" if nothings was restored Sep 20, 2023
@NachoSoto NachoSoto force-pushed the paywalls-restore-purchases-alert branch from 84edd09 to b4da438 Compare September 20, 2023 21:37
Comment on lines +170 to +173
let success = try await self.purchaseHandler.restorePurchases().success
if success {
self.displayRestoredAlert = true
}
Copy link
Member

Choose a reason for hiding this comment

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

this looks good to me as a stopgap, but I think we should try to get copy for saying something about "no purchases found" and localize it.
No-op is kind of an awkward experience to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed a Linear for this.

@NachoSoto NachoSoto enabled auto-merge (squash) September 20, 2023 22:02
@NachoSoto NachoSoto merged commit 5ba2911 into main Sep 20, 2023
19 checks passed
@NachoSoto NachoSoto deleted the paywalls-restore-purchases-alert branch September 20, 2023 22:19
This was referenced Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants