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: Auto-close paywall activity if restore grants required entitlement identifier #1507

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

tonidero
Copy link
Contributor

@tonidero tonidero commented Nov 30, 2023

Description

Users could potentially restore their purchases and get entitlements granted but the paywall activity was not dismissing. This will add that functionality in the case where users use the requiredEntitlementIdentifier parameter to launch the activity.

This adds a new PaywallResult value, Restored. This will be returned if the last action the user performed was a restore.

@tonidero tonidero added the pr:fix A bug fix label Nov 30, 2023
@tonidero tonidero marked this pull request as ready for review November 30, 2023 12:58
@tonidero tonidero requested a review from a team November 30, 2023 12:58
@tonidero tonidero added pr:RevenueCatUI and removed pr:fix A bug fix labels Nov 30, 2023
@tonidero tonidero changed the title Auto-close paywall activity if restore grants required entitlement identifier Paywalls: Auto-close paywall activity if restore grants required entitlement identifier Nov 30, 2023
finish()
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

hmmm this is making me think... if anyone is already doing this (finishing the activity when they get a PaywallResult.Restored, this will break their implementation. Right? Are we doing this in iOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PaywallResult.Restored is a new sealed class element. Until now, I don't think users could detect whether the activity had restored or not.

As for iOS. I do think we have a didFinishRestoringWith delegate developers can use for this but I don't think it autocloses. That's something developers would need to handle themselves.

Copy link
Contributor

Choose a reason for hiding this comment

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

😮‍💨 ok, should have kept reading

sorry for the noise

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can autoclose and if anyone complains, we can add a setting somehow. I think autoclosing is a better default experience

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, in iOS I think we are not autoclosing for now. Not on a normal purchase nor on a restore. We talked about this before, and we felt it was more in line with iOS to not autodismiss. CC @aboedo @NachoSoto for confirmation.

But in android, since the developer doesn't have good access to the paywall activity, nor a way to commuinicate with it once the paywall activity is launched, we need to handle it for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, there's no way for them to access the activity that was just launched. So even if they wanted to close it themselves, they can't

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6708f96) 84.46% compared to head (3e6047b) 84.46%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1507   +/-   ##
=======================================
  Coverage   84.46%   84.46%           
=======================================
  Files         217      217           
  Lines        7196     7196           
  Branches     1004     1004           
=======================================
  Hits         6078     6078           
  Misses        730      730           
  Partials      388      388           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

offeringId = offering?.identifier,
fontProvider = fontProvider,
shouldDisplayDismissButton = shouldDisplayDismissButton,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought maybe we could return the activity in this function so they could finish it whenever they want, but I don't think there's a way by looking at launch. Devs will probably leak the Activity if we did anyways

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... and we can't really return an activity directly, since we only could have an intent. The system will be in charge of creating the activity.

@tonidero tonidero merged commit 3848f20 into main Nov 30, 2023
9 checks passed
@tonidero tonidero deleted the close-paywall-automatically-upon-restore branch November 30, 2023 14:49
This was referenced Dec 5, 2023
vegaro pushed a commit that referenced this pull request Dec 5, 2023
**This is an automatic release.**

### RevenueCatUI
* Paywalls: Add `PaywallFooterView` (#1509) via Toni Rico (@tonidero)
* Paywalls: Remove `PaywallActivity` theme to pickup application's theme
by default (#1511) via Toni Rico (@tonidero)
* Paywalls: Auto-close paywall activity if restore grants required
entitlement identifier (#1507) via Toni Rico (@tonidero)
### Bugfixes
* Improve pricePerYear price calculation precision (#1515) via Toni Rico
(@tonidero)
* Improve price per month accuracy for weekly subscriptions (#1504) via
Andy Boedo (@aboedo)
### Dependency Updates
* Bump danger from 9.4.0 to 9.4.1 (#1512) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Remove unnecessary appInBackground parameters (#1508) via Cesar de la
Vega (@vegaro)
* Create `PurchasesStateProvider` (#1502) via Cesar de la Vega (@vegaro)

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Copy link
Contributor

@NachoSoto NachoSoto left a comment

Choose a reason for hiding this comment

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

This matches the behavior in iOS 👍🏻

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.

3 participants