Studio: Delete all active demo sites

[kozer]

Related to https://github.com/Automattic/dotcom-forge/issues/6655
Fixes https://github.com/Automattic/dotcom-forge/issues/7081

Proposed Changes

This PR fixes a bug where the user reaches the maximum number of active demo sites. However, if none of the created sites are linked to the demo sites, he can't remove any to continue creating demo sites.

Testing Instructions

Prerequisite: Apply D148749-code

1. Create some sites.
2. Edit appdata-v1.json files, and remove all snapshots
3. Restart the app
4. Go to settings, and ensure that you see a number of active demo sites like the below:
   
5. Press the 3 dots menu, and then press the "Delete all demo sites" button.
6. Ensure that you see a message like below:
   
7. Ensure that pressing cancel doesn't have any effect.
8. Repeat steps from 4-6.
9. On the dialog press "Delete all demo sites"
10. Notice that the sites are deleted, and you end up with 0 active demo sites.

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[sejas]

@kozer , what do you think about moving the heavy logic to the backend? I feel more confident if the App makes only one request to delete all the sites.

[katinthehatsite]

@kozer it seems that you are adding a confirmation window for deleting all sites in this PR. I currently have a similar issue assigned: https://github.com/Automattic/dotcom-forge/issues/7081 - would you like to assign it to yourself if that case since you are covering that functionality?

[kozer]

@kozer it seems that you are adding a confirmation window for deleting all sites in this PR. I currently have a similar issue assigned: https://github.com/Automattic/dotcom-forge/issues/7081 - would you like to assign it to yourself if that case since you are covering that functionality?

I'm sorry, Kat! The ticket I'm working on, mentioned the confirmation in the comments, and I haven't realized, there was a separate issue for that! I can assign it to me, thanks!

[kozer]

@kozer , what do you think about moving the heavy logic to the backend? I feel more confident if the App makes only one request to delete all the sites.

@sejas , I was thinking about that, and I decided not to do that, for the following reasons:

1. We have all that logic in the client already implemented.
2. It wouldn't change anything more, than just moving things around, as still, we will have to loop the sites and call request_site_deletion, on each one of them. Any of those delete requests can fail asynchronously ( as we call jobs in that function, that schedule a job, and call other apis ), so again we could have a network partition.
   So given that, I thought that although seems strange to have multiple requests, I don't see the real value of moving all that complicated logic of handling failures etc, as firstly, it works perfectly as is, and I don't see the clear benefit by doing so.
   What do you think?

[katinthehatsite]

I'm sorry, Kat! The ticket I'm working on, mentioned the confirmation in the comments, and I haven't realized, there was a separate issue for that! I can assign it to me, thanks!

No worries, I have not started yet - so all good! I removed my assignment from that issue 👍

[katinthehatsite]

Hey I am testing this PR now and once I delete all the snapshots and navigate to Settings > three dots and attempt to click on the button to delete all sites, I can't seem to make proceed further as the button appears blocked:

Any ideas what might be happening?

[kozer]

Hey I am testing this PR now and once I delete all the snapshots and navigate to Settings > three dots and attempt to click on the button to delete all sites, I can't seem to make proceed further as the button appears blocked:

Any ideas what might be happening?

Tbh, I had the same problem this morning, @katinthehatsite . I applied the diff, and I didn't have any success.
I restarted the proxy and worked as expected!

To see if it's working or not, open dev tools and check if /usage endpoint resolves. For me it didn't

[katinthehatsite]

Tbh, I had the same problem this morning, @katinthehatsite . I applied the diff, and I didn't have any success.
I restarted the proxy and worked as expected!
To see if it's working or not, open dev tools and check if /usage endpoint resolves. For me it didn't

I see, I did not apply the diff initially because I missed Prerequisite: Apply D148749 🤦‍♀️ However, now it works for me as expected when I follow the testing steps. I only have one comment on my end regarding buttons color.

[wojtekn]

When I tested the feature, I had some floating demo sites in Studio: a few expired ones, and one I removed using Jurassic Ninja Report Card tool. The new feature removed existing sites correctly, but it didn't clear the floating ones. Should it also remove all local demo sites?

[matt-west]

Prerequisite: Apply D148749-code

@kozer I don't have access to that diff to apply it.

[kozer]

Prerequisite: Apply D148749-code

@kozer I don't have access to that diff to apply it.

It's ok Matt! I'm not sure what I need to do for you to obtain access. My guess is that @wojtekn can do that for you.

[sejas]

@kozer , what do you think about moving the heavy logic to the backend? I feel more confident if the App makes only one request to delete all the sites.

@sejas , I was thinking about that, and I decided not to do that, for the following reasons:

1. We have all that logic in the client already implemented.
2. It wouldn't change anything more, than just moving things around, as still, we will have to loop the sites and call request_site_deletion, on each one of them. Any of those delete requests can fail asynchronously ( as we call jobs in that function, that schedule a job, and call other apis ), so again we could have a network partition.
   So given that, I thought that although seems strange to have multiple requests, I don't see the real value of moving all that complicated logic of handling failures etc, as firstly, it works perfectly as is, and I don't see the clear benefit by doing so.
   What do you think?

Thanks for sharing the context! Let's do that 👌

[wojtekn]

Thanks @kozer , looks good:

Could you please fix lint errors?