Skip to content

Clearing expired preview site shows unnecessary error - #2943

Merged
katinthehatsite merged 3 commits into
trunkfrom
fix/expired-preview-site
Apr 1, 2026
Merged

Clearing expired preview site shows unnecessary error#2943
katinthehatsite merged 3 commits into
trunkfrom
fix/expired-preview-site

Conversation

@katinthehatsite

Copy link
Copy Markdown
Contributor

Related issues

Fixes STU-1471

How AI was used in this PR

I debated two different solutions with AI before picking an option.

Proposed Changes

This PR ensures that when an expired preview site is to be Clear, we are not making the API request.

Testing Instructions

I was not able to reproduce the exact error shown in the issue but I think this approach should handle the issue:

  • First create a preview site
  • Next, navigate to https://developer.wordpress.com/docs/api/console/
  • Run wpcom/v2, POST, /jurassic-ninja, delete
  • Fill in the site id for the preview site (you can find the site id in https://mc.a8c.com/tools/jn-report-card/ )
  • Delete the preview site
  • Navigate to the preview site in Studio
  • Observe that it is now marked as expired
  • Open the Network tab in the console
  • Click on Clear
  • Confirm that there are no errors and that the site clears
  • Confirm that there is no network request
  • Confirm that the expires site is removed from CLI config

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@katinthehatsite katinthehatsite self-assigned this Mar 30, 2026
@katinthehatsite
katinthehatsite requested a review from a team March 30, 2026 08:57
@wpmobilebot

wpmobilebot commented Mar 30, 2026

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 21be50d vs trunk

app-size

Metric trunk 21be50d Diff Change
App Size (Mac) 1271.88 MB 1271.28 MB 0.60 MB ⚪ 0.0%

site-editor

Metric trunk 21be50d Diff Change
load 1922 ms 1593 ms 329 ms 🟢 -17.1%

site-startup

Metric trunk 21be50d Diff Change
siteCreation 8265 ms 8216 ms 49 ms ⚪ 0.0%
siteStartup 4949 ms 4950 ms +1 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@epeicher epeicher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @katinthehatsite for solving this! I have not been able to reproduce the issue, as I think the error is correctly handled here. This code prevents a request from being made if the site has already expired, so we are improving with this change. I am accepting this, and it can be landed. LGTM!

I have committed two lint fixes so once the CI passes, this could be merged.

Comment thread apps/cli/commands/preview/delete.ts Outdated
Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>
@katinthehatsite

Copy link
Copy Markdown
Contributor Author

Thanks for cleaning up the logs @epeicher !

@katinthehatsite
katinthehatsite merged commit 57b0753 into trunk Apr 1, 2026
10 checks passed
@katinthehatsite
katinthehatsite deleted the fix/expired-preview-site branch April 1, 2026 07:42
fredrikekelund added a commit that referenced this pull request Apr 2, 2026
* Ensure that expires screenshots don't hit the api

* Fix lint errors

* Delete debug log

Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>

---------

Co-authored-by: Kateryna Kodonenko <kateryna@automattic.com>
Co-authored-by: Roberto Aranda <roberto.aranda@automattic.com>
Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants