-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add subscription-restore wide event #7317
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
Add subscription-restore wide event #7317
Conversation
|
Privacy Review task: https://app.asana.com/0/69071770703008/1212352085517594 |
9e8d549 to
dcb39cb
Compare
| private suspend fun onRestoreFlowStarted(restorePlatform: String, purchaseAttempt: Boolean) { | ||
| getCurrentWideEventId()?.let { wideEventId -> | ||
| wideEventClient.flowFinish(wideEventId = wideEventId, status = FlowStatus.Unknown) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing cachedFlowId reset after finishing previous flow
In onRestoreFlowStarted, when a previous flow exists, flowFinish is called on line 94 but cachedFlowId is not set to null afterwards. This differs from the pattern in SubscriptionPurchaseWideEventImpl.onPurchaseFlowStarted() which correctly clears the cache after finishing a previous flow. If flowStart subsequently fails (returns null on line 106), cachedFlowId will still hold the stale ID of the already-finished flow, potentially causing incorrect behavior when success/failure methods are later called.
Please tell me if this was useful or not with a 👍 or 👎.
nalcalag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although when testing Unsuccessful restore using email address, the wide event status came through as UNKNOWN instead of FAILURE.
Also, I noticed in the pixel definition that there are parameters, like context, that aren’t included in the implementation yet. I assume you’re planning to add those later?
dcb39cb to
1cbf653
Compare
| suspend fun onEmailRestoreSuccess() | ||
| suspend fun onGooglePlayRestoreSuccess() | ||
| suspend fun onGooglePlayRestoreFailure(error: String) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing email restore failure handling method
The SubscriptionRestoreWideEvent interface exposes onGooglePlayRestoreFailure(error: String) but has no corresponding onEmailRestoreFailure method. This asymmetry means email restore failures cannot be explicitly reported with a failure reason. The PR test steps expect to verify status=FAILURE for unsuccessful email restores, but the current implementation only handles this through CleanupPolicy.OnProcessStart, which uses the default FlowStatus.Unknown status instead of FlowStatus.Failure. This prevents proper instrumentation of email restore failures.
Please tell me if this was useful or not with a 👍 or 👎.

Task/Issue URL: https://app.asana.com/1/137249556945/project/1205648422731273/task/1212344118527513?focus=true
Description
This PR instruments restore subscription operation with a new wide event.
Steps to test this PR
Prerequisites
Successful restore using Google Play
wide_subscription-restoreevent has been sent withstatus=SUCCESSandrestore_platform=google_playSuccessful restore using email address
wide_subscription-restoreevent has been sent withstatus=SUCCESSandrestore_platform=email_addressUnsuccessful restore using Google Play
wide_subscription-restoreevent has been sent withstatus=FAILUREandrestore_platform=google_playUnsuccessful restore using email address
wide_subscription-restoreevent has been sent withstatus=FAILUREandrestore_platform=email_addressSuccessful restore using Google Play when attempting to purchase new subscription
wide_subscription-restoreevent has been sent withstatus=SUCCESSandrestore_platform=google_playandis_purchase_attempt=trueUnsuccessful restore using Google Play when attempting to purchase new subscription
wide_subscription-restoreevent has been sent withstatus=FAILUREandrestore_platform=google_playandis_purchase_attempt=trueNo UI changes
Note
Instruments the subscription restore flow (Google Play/email) with a new wide event, integrates it in manager and UI, and adds tests and pixel definitions.
PixelDefinitions/pixels/wide_subscription_restore.json5definingsubscription-restorewide pixel withrestore_platform,is_purchase_attempt,restore_latency_ms_bucketed, failure reason, and email-step markers.SubscriptionRestoreWideEvent(SubscriptionRestoreWideEventImpl) to start/finish flows, record latency, and report success/failure.SubscriptionRestoreWideEventintoRealSubscriptionsManagerto track restore during purchase attempts; add helperrecoverSubscriptionFromStoreOnPurchaseAttempt.recoverSubscriptionFromStoreand add feature flagsendSubscriptionRestoreWideEvent()toPrivacyProFeature.RestoreSubscriptionViewModelto emit wide events on store/email restore start and on success/failure.SubscriptionRestoreWideEventTestcovering flow start/success/failure, interval handling, caching, and feature toggle.RealSubscriptionsManagerTestandRestoreSubscriptionViewModelTestto inject and verify new wide event behavior.Written by Cursor Bugbot for commit 1cbf653. This will update automatically on new commits. Configure here.