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

Support self-service account closures for Atomic sites #65636

Closed
wants to merge 1 commit into from

Conversation

mpkelly
Copy link
Contributor

@mpkelly mpkelly commented Jul 15, 2022

Fixes #57399

As per @mmtr's comment here, we should allow users with Atomic sites to close them (after removing purchases).

Proposed Changes

Remove the isAtomic-style logic from the Close Account page. This simplifies the logic so there are now only two scenarios:

  1. The user has refundable purchases and is shown the "Manage purchases" button; or
  2. The user doesn't have refundable purchases and is shown the "Close account" button.

Testing Instructions

I have manually tested this and it worked albeit after a manual refresh. I tried to setup a fresh user and ran into some issues where the user account had purchases disabled.

Pre-merge Checklist

@mpkelly mpkelly requested a review from a team July 15, 2022 12:11
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 15, 2022
@matticbot
Copy link
Contributor

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~115 bytes removed 📉 [gzipped])

name           parsed_size           gzip_size
account-close       -822 B  (-0.2%)     -115 B  (-0.1%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@mpkelly
Copy link
Contributor Author

mpkelly commented Jul 16, 2022

It looks like there will need server-side changes to go with this PR. I am still unable to delete an account in one go, even after removing UI restrictions. After confirming delete, I see:
image
The response to the /me/account/close call is:
image
FYI, if you try to delete the account a second time, it works because it is no longer classified as Atomic. I will check what we need to do to make this work in one go. Maybe the fix is just to serialise the API calls.

@mmtr
Copy link
Member

mmtr commented Jul 18, 2022

@mpkelly When you deactivate the plan from an Atomic site, we initiate an async process to revert the site back to Simple. That process takes some time, so you won't be able to immediately close your account until the revert has been completed.

We explored ways to decrease that waiting time under some scenarios (e.g. if you simply want to delete a site or your account) by skipping the lossless import during the revert process. D69024-code added the ability to set a skip_import param to the API request that deactivates the plan, but we should update the UX or add some behavior to the logic to conditionally add that param only if we are certain that the site content doesn't need to be imported to the revert site (see #57273).

@mpkelly
Copy link
Contributor Author

mpkelly commented Jul 18, 2022

Thanks, @mmtr.

I am not sure about the UX for skipping the import step. We could include a checkbox (or something), but it might not always get checked when the user plans to close their account.

My view is that if they choose to close it, it should always work regardless of any pending jobs that might be running to remove the Atomic site. We might not be able to complete the closure internally but externally can still show the account as closed. I think it leads to a broader UX discussion about why the user needs to remove their purchases manually before closing their account, and why closing an account isn't just a one-click operation with some feedback + confirmation steps like we currently have. Is this something that has been discussed before also?

@mmtr
Copy link
Member

mmtr commented Jul 18, 2022

My view is that if they choose to close it, it should always work regardless of any pending jobs that might be running to remove the Atomic site.

Totally agree. One idea is to maybe trigger an async job after the (externally perceived) account closure to remove any Atomic site that is in the process of being reverted. Once all sites have been removed, the same async job can proceed with the (internal) account closure.

Is this something that has been discussed before also?

I couldn't find anything after doing a quick MGS search, but I might have missed something.

@mpkelly
Copy link
Contributor Author

mpkelly commented Jul 19, 2022

After some discussions with @daledupreez about this, I will make two separate PRs.

  1. This one will be modified so that when the user still has an atomic site that is being deleted, they will see a warning and will be told they will need to wait 10-15 minutes before they can close their account.

  2. On the remove purchase screen, show an option below the backup box (see red box below) that tells the user they can speed up removing their atomic site purchase by skipping the import step. For this, the API call will include the skip_import flag mentioned by @mmtr.
    image

This seemed to be the "best" solution after reviewing the async code on the server, which is not straightforward to change and will require more coordination and input.

I will do these two PRs on new clean branches and create new tickets for each.

@mpkelly mpkelly closed this Jul 19, 2022
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 19, 2022
@daledupreez
Copy link
Contributor

  1. This one will be modified so that when the user still has an atomic site that is being deleted, they will see a warning and will be told they will need to wait 10-15 minutes before they can close their account.

Also, as a further clarification to the above, we will also change the logic so that we handle the following two cases separately:

  1. If the user has any cancelable purchases, we will ask them to remove those purchases, regardless of whether they have an Atomic site
  2. If the user doesn't have any cancelable purchases, but they have an Atomic site (which will generally imply they have an Atomic site that is in the process of being reverted to a Simple site), we'll show them a message that we are busy cleaning up one of their sites and they should retry the account closure in 15-30 minutes. If they continue to have issues, they should then reach out to support.

The thinking here is that we can at least get users to remove their purchases as an important first step in the process, and when they get back to this page, they will be in one of the following situations:

  • Their Atomic site has been fully reverted, so they can proceed with the account closure
  • Their Atomic site revert is still in progress, and they can either wait or reach out to support
    = If they wait, the account closure should be OK to proceed relatively soon
    = If they reach out to support, the revert should complete relatively quickly, and the support burden should be lower due to the only complication being the in-progress site revert - their purchases should already have been removed

The second PR will make it possible for the plan cancellation flow to trigger a quicker revert, reducing the likelihood of a revert taking a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Unable to close account due to active subscription.
4 participants