Site Settings: Make it impossible for users to delete a site with active subscriptions #1246

drewblaisdell · 2015-12-03T20:16:41Z

Currently, it is possible to delete a site that has active subscriptions by either:

Visiting /settings/general/:site and clicking the 'Delete Site' link from DeleteSiteOptions before the purchases have been fetched from the API.
Visiting /settings/delete-site/:site directly.

This PR addresses this by:

Not rendering DeleteSiteOptions until the purchases have been loaded from the API (this is how this component used to behave).
Returning an error from the site deletion endpoint if there are active subscriptions for the given site.
Updating NoticesList so that it renders even when passed an empty array of notices, ensuring that DeleteSiteNotices is rendered (this is how the this component used to behave).

@johnHackworth I believe the inverse change of this commit was added by you a couple months ago. Do you know of any issues with removing that check in NoticesList?

Testing

Apply the patch.
Checkout this branch.
Navigate to /settings/general/:site for a site with an active subscription (e.g. a premium plan).
Assert that clicking the 'Delete Site' link at the bottom of the page brings up a dialog prompting you to remove your active subscription.
Navigate to /settings/delete-site/:site for a site with an active subscription.
Assert that an error notice appears when you attempt to delete a site.

scruffian · 2015-12-03T23:45:19Z

This looks good to me. It works as advertised and I don't have any comments on the code. I'm happy for you to merge, but I'll look more in detail tomorrow if you'd rather.

drewblaisdell · 2015-12-03T23:58:13Z

@scruffian Thanks for taking a look. Either works for me.

scruffian · 2015-12-04T11:19:21Z

Not rendering DeleteSiteOptions until the purchases have been loaded from the API (this is how this component used to behave).

Would a placeholder be better?

scruffian · 2015-12-04T11:43:35Z

Code looks good, and the server side patch can be deployed. My only concern is that by not showing the delete section until the endpoint has loaded, so users might get confused.

johnHackworth · 2015-12-04T14:21:06Z

        if ( ! noticesRaw.length ) {
            return null;
        }

To be honest, I don't remember why I needed that ... I guess I added it when I did all the pinned notices stuff, but I've just tested your branch and everything seems to be working as usual

/shrug

johnHackworth · 2015-12-04T14:29:51Z

client/lib/sites-list/actions.js

 		Dispatcher.handleViewAction( {
 			type: 'DELETE_SITE',
 			site: site
 		} );

 		debug( 'Deleting site', site );
-		wpcom.deleteSite( site.ID, SitesListActions.receiveDeletedSite );
+
+		wpcom.deleteSite( site.ID, function( error, data ) {


It would be so nice if we could do all this wpcom methods to return promises someday :/

johnHackworth · 2015-12-04T14:37:43Z

I left a minor comment, but codewise looks good to me too

drewblaisdell · 2015-12-04T18:19:07Z

Would a placeholder be better?

Yes, but that is out of scope for this PR, which just fixes the existing behavior. There are multiple settings on that page that "pop in" after a response from the API. I'll open an issue for it after this is merged if one doesn't already exist.

scruffian · 2015-12-04T21:01:33Z

Ok sounds good. In that case 🚢

…d from the API This was the original behavior of `DeleteSite`, and prevents users from deleting sites with active subscriptions.

…rns an error

Otherwise, `<DeleteSiteNotices />` isn't rendered.

…of `DeleteSite`

…ptions Site Settings: Make it impossible for users to delete a site with active subscriptions

scruffian · 2015-12-04T23:56:33Z

I'll open an issue for it after this is merged if one doesn't already exist.

Did you do this?

drewblaisdell · 2015-12-05T00:04:58Z

Whoops, here it is: #1291

Also, I discovered that this PR fixed another issue: #158

scruffian · 2015-12-05T12:19:13Z

Also, I discovered that this PR fixed another issue: #158

Nice! Good work @drewblaisdell!

drewblaisdell added [Type] Bug [Feature] Site Settings All other general site settings. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 3, 2015

drewblaisdell self-assigned this Dec 3, 2015

drewblaisdell added this to the Purchases: v1 milestone Dec 3, 2015

drewblaisdell force-pushed the update/site-deletion-subscriptions branch 2 times, most recently from 48050a2 to c7c7189 Compare December 3, 2015 20:17

drewblaisdell mentioned this pull request Dec 3, 2015

Purchases: It is possible to delete a site with active subscriptions #1192

Closed

scruffian added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 4, 2015

johnHackworth reviewed Dec 4, 2015
View reviewed changes

drewblaisdell force-pushed the update/site-deletion-subscriptions branch from c7c7189 to f3c01be Compare December 4, 2015 18:15

drewblaisdell added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Dec 4, 2015

scruffian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 4, 2015

drewblaisdell added 4 commits December 4, 2015 14:10

Site Settings: Hide DeleteSite until the purchases have been fetche…

bad18c9

…d from the API This was the original behavior of `DeleteSite`, and prevents users from deleting sites with active subscriptions.

Site Settings: Add an action for when the site deletion endpoint retu…

9fff28a

…rns an error

Notices: Render NoticesList when the list of notices is empty

ff90769

Otherwise, `<DeleteSiteNotices />` isn't rendered.

Site Settings: Reset scroll position in the stats controller instead …

6d4711d

…of `DeleteSite`

drewblaisdell force-pushed the update/site-deletion-subscriptions branch from f3c01be to 6d4711d Compare December 4, 2015 22:10

drewblaisdell added a commit that referenced this pull request Dec 4, 2015

Merge pull request #1246 from Automattic/update/site-deletion-subscri…

dcc3794

…ptions Site Settings: Make it impossible for users to delete a site with active subscriptions

drewblaisdell merged commit dcc3794 into master Dec 4, 2015

drewblaisdell removed the [Status] Ready to Merge label Dec 4, 2015

drewblaisdell deleted the update/site-deletion-subscriptions branch December 4, 2015 22:19

drewblaisdell mentioned this pull request Dec 5, 2015

Settings: delete-site-notices are not getting displayed #158

Closed

fabianapsimoes modified the milestone: Purchases: Future Feb 25, 2016

This was referenced Nov 18, 2020

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#35

Open

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#49

Open

This was referenced Sep 15, 2021

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#63

Open

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#97

Open

ajesse11x mentioned this pull request Nov 4, 2022

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#83

Open

awaisab172 mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#134

Open

ajesse11x mentioned this pull request Dec 25, 2022

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#87

Open

awaisab172 mentioned this pull request Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#141

Open

awaisab172 mentioned this pull request Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#152

Open

awaisab172 mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities awaisab172/wp-calypso#196

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Site Settings: Make it impossible for users to delete a site with active subscriptions #1246

Site Settings: Make it impossible for users to delete a site with active subscriptions #1246

drewblaisdell commented Dec 3, 2015

scruffian commented Dec 3, 2015

drewblaisdell commented Dec 3, 2015

scruffian commented Dec 4, 2015

scruffian commented Dec 4, 2015

johnHackworth commented Dec 4, 2015

johnHackworth Dec 4, 2015

johnHackworth commented Dec 4, 2015

drewblaisdell commented Dec 4, 2015

scruffian commented Dec 4, 2015

scruffian commented Dec 4, 2015

drewblaisdell commented Dec 5, 2015

scruffian commented Dec 5, 2015

Site Settings: Make it impossible for users to delete a site with active subscriptions #1246

Site Settings: Make it impossible for users to delete a site with active subscriptions #1246

Conversation

drewblaisdell commented Dec 3, 2015

scruffian commented Dec 3, 2015

drewblaisdell commented Dec 3, 2015

scruffian commented Dec 4, 2015

scruffian commented Dec 4, 2015

johnHackworth commented Dec 4, 2015

johnHackworth Dec 4, 2015

Choose a reason for hiding this comment

johnHackworth commented Dec 4, 2015

drewblaisdell commented Dec 4, 2015

scruffian commented Dec 4, 2015

scruffian commented Dec 4, 2015

drewblaisdell commented Dec 5, 2015

scruffian commented Dec 5, 2015