Skip to content

Remove old PR preview HTML files and add redirects to new preview modals #2081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

brandonpayton
Copy link
Member

@brandonpayton brandonpayton commented Dec 13, 2024

Motivation for the change, related issues

This PR removes old PR preview pages, adds relevant redirects, and updates old PR preview redirects.

Fixes #1969

Implementation details

In addition to the removals and redirects, this PR:

  • Removes some more complicated redirect-related logic because it is awkward and no longer needed.
  • Adjust old UI links to wordpress.html to link to the new modal instead

Testing Instructions (or ideally a Blueprint)

  • Tested manually on a staging site.
  • Use php -l to ensure there are no syntax errors in custom-redirects-lib.php
  • Deploy and confirm the following paths redirect to the new PR preview modals
    • /wordpress
    • /wordpress.html
    • /gutenberg
    • /gutenberg.html
@brandonpayton
Copy link
Member Author

This PR accidentally contained some export exploration work. Will fix.

@brandonpayton brandonpayton force-pushed the remove-old-pr-preview-html-files-and-add-redirects branch from 6bff7eb to 0ffb8fa Compare December 13, 2024 21:47
@brandonpayton
Copy link
Member Author

This PR accidentally contained some export exploration work. Will fix.

Fixed.

@brandonpayton brandonpayton merged commit a67f08f into trunk Dec 14, 2024
10 checks passed
@brandonpayton brandonpayton deleted the remove-old-pr-preview-html-files-and-add-redirects branch December 14, 2024 02:03
@brandonpayton
Copy link
Member Author

Tests fine after deployment:

% curl -i https://playground.wordpress.net/{wordpress,wordpress.html,gutenberg,gutenberg.html}
HTTP/2 301 
server: nginx
date: Sat, 14 Dec 2024 02:15:29 GMT
content-type: text/html; charset=utf-8
strict-transport-security: max-age=31536000
location: /?modal=preview-pr-wordpress
x-ac: 2.mdw _atomic_dca HIT
alt-svc: h3=":443"; ma=86400

HTTP/2 301 
server: nginx
date: Sat, 14 Dec 2024 02:15:29 GMT
content-type: text/html; charset=utf-8
strict-transport-security: max-age=31536000
location: /?modal=preview-pr-wordpress
x-ac: 2.mdw _atomic_dca HIT
alt-svc: h3=":443"; ma=86400

HTTP/2 301 
server: nginx
date: Sat, 14 Dec 2024 02:15:29 GMT
content-type: text/html; charset=utf-8
strict-transport-security: max-age=31536000
location: /?modal=preview-pr-gutenberg
x-ac: 2.mdw _atomic_dca HIT
alt-svc: h3=":443"; ma=86400

HTTP/2 301 
server: nginx
date: Sat, 14 Dec 2024 02:15:29 GMT
content-type: text/html; charset=utf-8
strict-transport-security: max-age=31536000
location: /?modal=preview-pr-gutenberg
x-ac: 2.mdw _atomic_dca HIT
alt-svc: h3=":443"; ma=86400

@adamziel
Copy link
Collaborator

adamziel commented Dec 14, 2024

Did you test this? When I click on a preview link in wordpress-develop repository, this is what I get instead of a preview:

image

I'll revert this one until we have a fix

adamziel added a commit that referenced this pull request Dec 14, 2024
…view modals" (#2082)

Reverts #2081

The change broke PR preview links. The modal does not yet have feature
parity with the separate HTML pages. It needs to support all the same
user flows and query params before it can replace the existing
previewers. Cc @brandonpayton @ajotka
@brandonpayton
Copy link
Member Author

Did you test this? When I click on a preview link in wordpress-develop repository, this is what I get instead of a preview:
...
I'll revert this one until we have a fix

@adamziel, I did not know about those links from the wordpress-develop repo and thought this was simply replacing a UI that required user interaction. I'm sorry for the oversight. And thank you for the revert!

@brandonpayton
Copy link
Member Author

Instead of getting rid of the standalone pages, maybe we should just change the pages to share lookup logic with the preview modals and otherwise leave the pages as-is. cc @ajotka

@brandonpayton
Copy link
Member Author

Instead of getting rid of the standalone pages, maybe we should just change the pages to share lookup logic with the preview modals and otherwise leave the pages as-is. cc @ajotka

Ah, I see the Playground web app also has core-pr and gutenberg-pr query params, so we could conceivably just make the redirects from this PR use those. I'll reopen #1969, and we can address this as there is time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment