Studio: User can't delete expired demo site from Studio

[katinthehatsite]

Closes https://github.com/Automattic/dotcom-forge/issues/7082

Proposed Changes

This PR adds Clear expired site button to the expired screen under Share tab in Studio:

Testing Instructions

• Pull the changes from this PR
• Start the app with nvm use && npm install && npm start
• Create a demo site
• Force the demo site to be expired by changing the following line in the use-expiration-date.ts:

let isExpired = false;

to true:

let isExpired = true

• Observe Clear expired site button
• Click the button, observe a spinner and confirm that you are seeing the <NoSnapshots> component
• Try testing Clear expired site without internet connection and confirm that the button is disabled in this case

Notes

• There is a loading spinner present which was not mentioned in the initial design. I tried without it but I found the shift in the layout to be too abrupt. Happy to reconsider this approach
• Once you focus on the button to Clear expired site, you will see the borders appear. I am not sure if they should be there according to the design. Happy to remove those as well.

Pre-merge Checklist

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

[wojtekn]

@katinthehatsite I tried to test using steps closer to real-world scenarios - added one of my expired demo sites manually in the appdata-v1.json file and launched Studio. The demo site wasn't displayed at all. I tried to remove demo site using API endpoint directly, restarted Studio, and it disappeared, too. Maybe this is no longer an issue, and we don't need this button anymore?

It happens because we introduced the cleanup feature via clearFloatingSnapshots at https://github.com/Automattic/studio/pull/131.

[katinthehatsite]

I tried to test using steps closer to real-world scenarios - added one of my expired demo sites manually in the appdata-v1.json file and launched Studio. The demo site wasn't displayed at all.

@wojtekn I was trying to test this but I am not completely sure how you did this step? Did you force the site to somehow expire first (e.g. by editing the date in appdata-v1.json) before testing or you already had a site that expired and was present in your snapshots in appdata-v1.json?

It happens because we introduced the cleanup feature via clearFloatingSnapshots at https://github.com/Automattic/studio/pull/131.

It makes sense. I would still like to test in the way that you ran your test but if that's the case, should we still need the UI for the expired demo site view?

[katinthehatsite]

@wojtekn I manually edited the date in the snapshot in the appdata-v1.json file but I am getting an expired view on my end:

How did you trigger the site to be expired exactly? Thanks!

[wojtekn]

@wojtekn I was trying to test this but I am not completely sure how you did this step? Did you force the site to somehow expire first (e.g. by editing the date in appdata-v1.json) before testing or you already had a site that expired and was present in your snapshots in appdata-v1.json?

I deleted it using /jurassic-ninja/delete endpoint on https://developer.wordpress.com/docs/api/console/.

It makes sense. I would still like to test in the way that you ran your test but if that's the case, should we still need the UI for the expired demo site view?

Yes, this is what I meant in my comment above:

Maybe this is no longer an issue, and we don't need this button anymore?

We should test this flow, and we could close the PR if it's no longer an issue.

[jeroenpf]

@katinthehatsite - What I observed is that a deleted/expired site is automatically deleted from the app when you restart it. I think this change is still valid because if you don't restart the app, the demo site will still be there.

However, perhaps one improvement could be to not rely on a hardcoded expiry but instead poll the status endpoint for a site once in a while when the 'share' tab is active. If a site is deleted/expired we could automatically delete it from the local storage, or perhaps show something in the UI to let the user know their previous demo site has expired? In any case, I'm not sure if its a great experience if a demo site suddenly disappears without any feedback to the user that this happened.

[katinthehatsite]

I tried some testing today:

• I created a new site, tried deleting the site from the endpoint and then restarting the app -> it went back to the normal screen to add a new site
• I created a new site, tried deleting the site from the endpoint and then moved around tabs etc. -> the site was still present
• I created a new site, updated isExpired to true, deleted the site from the endpoint, saw the expired screen, moved around tabs etc. -> the expired view was present
• I created a new site, updated isExpired to true, deleted the site from the endpoint, saw the expired screen, restarted the app -> the screen went back to the usual screen to add a site

So I think @jeroenpf is right that the screen would stay until the app restarts. I don't have a strong opinion on how to proceed:

• We can leave the screen without adding any further button since it would be cleared when the app restarts and if the site expires
• We can proceed with the implementation to allow the users who don't restart the app often to clear the site. I would need to refactor a bit the code and remove offline mode handling

Personally, I think it is something that is nice to have but I don't find it to be an important feature because it seems to be a bit of an edge case now that we have clearFloatingSnapshots. I am happy to proceed either way

[wojtekn]

In any case, I'm not sure if its a great experience if a demo site suddenly disappears without any feedback to the user that this happened.

This is a good point, @jeroenpf . What if we go with @katinthehatsite's solution and remove clearFloatingSnapshots()? That way, snapshots wouldn't disappear automatically. We would likely need to change how we delete those snapshots to avoid trying to call API endpoint, though.

[katinthehatsite]

Sounds good @wojtekn - I will adjust the solution based on that and re-request a review again 👍

[katinthehatsite]

@wojtekn @jeroenpf Thanks for suggestions! I have now done the following:

• removed clearFloatingSnapshots to remove the cleanup when the app restarts
• used removeSnapshot instead of deleteSnapshot as it does not call the API and only takes care of the local snapshot
• added some tests

One thing that I have not added is the dialog that asks if the user wants to confirm the clearing of the expired demo site. On one hand, it would make the experience with other destructive buttons in the app and make the UI shift a bit less jarring. On the other hand, it might be a bit of an overkill. Happy to add it if you think we should do it.

[katinthehatsite]

Hey @fluiddot thanks for testing! I tried these steps but I am not seeing the expired site on my end once I click on Add site. Or are you referring to the expired site being in appdata-v1.json?

I compared the behavior to trunk and I see the exactly same experience there as well.

I am wondering though if we should completely remove clearing floating snapshots or leave it for backup. I like having some sort of cleanup job being present.

[fluiddot]

Hey @fluiddot thanks for testing! I tried these steps but I am not seeing the expired site on my end once I click on Add site. Or are you referring to the expired site being in appdata-v1.json?

The expired demo site is displayed after deleting the demo site created after the demo site expires.

I compared the behavior to trunk and I see the exactly same experience there as well.

Ah, true. I've just checked trunk and noticed that the expired snapshot shows up too. Hence, I wouldn't tackle this problem in this PR.

I am wondering though if we should completely remove clearing floating snapshots or leave it for backup. I like having some sort of cleanup job being present.

Good question. I presume that expired demo sites won't be useful for users as they can't access them. So, I think there's no need to keep them as a backup.

[katinthehatsite]

After discussing this with @kozer who initially created clearFloatingSnaphots function, I think that we should keep it. Hence, I have done the following:

• added back clearFloatingSnapshots so that it gets executed when the app restarts if the user has not cleared the snapshot manually. I don't think the change in the UI is too radical for the user for the expired site to be removed when the app restarts so this seems acceptable to me;
• kept the button to Clear expired site to have an option for the user to manually remove the expired snapshot

[katinthehatsite]

Thanks @fluiddot - I have made the adjustments here to fix the issue with the button display: https://github.com/Automattic/studio/pull/218/commits/1fa0063c3bdc8caf7e4e94f745eac374ecb2f148

[fluiddot]

Thanks @fluiddot - I have made the adjustments here to fix the issue with the button display: 1fa0063

I've tested trunk after this PR got merged and confirmed it works as expected. Sorry for the delay in the review 🙇 .