close
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
Copy link
Copy Markdown
Collaborator

wpmobilebot commented Mar 30, 2026

📊 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)

Copy link
Copy Markdown
Contributor

@epeicher epeicher left a comment

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants