Making WordPress.org

Opened 6 months ago

Closed 6 months ago

#7382 closed enhancement (fixed)

Add Playground links to themes trac tickets

Reported by: acosmin's profile acosmin Owned by:
Milestone: Priority: normal
Component: Theme Directory Keywords: has-patch
Cc:

Description

Based on our Slack discussion, it would be nice to have playground links on every theme ticket on themes Trac. This will make the theme review process simpler.

The links should load the version mentioned in the ticket comment in which they are included. They can be added somewhere in the description https://imgur.com/cCpQVuJ. Additionally, the WP instances should be pre-installed with a few things, such as:

cc @zieladam

Change History (12)

#1 @acosmin
6 months ago

  • Summary changed from Add Plaground links to themes trac tickets to Add Playground links to themes trac tickets

#2 @zieladam
6 months ago

Here's the PR I just started for the WordPress.org repository:

https://github.com/WordPress/wordpress.org/pull/186

cc @dd32

#3 @dd32
6 months ago

  • Keywords needs-patch 2nd-opinion removed

Copying my comment from Github over here, because the PR didn't auto-link for some reason, and as a result didn't sync over here... Fixed via #7388

And as a bonus, I incorrectly linked the ticket in the Commit and forgot props :facepalm:.

In [13080]:

Theme Directory: Review: Add a live preview to review trac tickets.

See https://meta.trac.wordpress.org/ticket/7382
Closes https://github.com/WordPress/wordpress.org/pull/186

Last edited 6 months ago by dd32 (previous) (diff)

This ticket was mentioned in PR #186 on WordPress/wordpress.org by @zieladam.


6 months ago
#4

  • Keywords has-patch added

Adds a "Live preview" link to the automated trac ticket comment that gets posted when a new theme version is submitted for a review. See an example of the automated comment.

The link leads to a WordPress Playground instance set up with:

  • The version of the theme listed in the comment
  • Imported theme unit test data
  • The Theme Check plugin installed and activated
  • WP_DEBUG enabled

## Caveats

### Zip URL

The zip file URL used in the comment follows this pattern:

https://wordpress.org/themes/download/gutenify-shoppe.1.0.0.zip?nostats=1

However, I used the following URL for this PR:

https://downloads.wordpress.org/theme/gutenify-shoppe.1.0.0.zip?nostats=1

Why?

  1. The former URL redirects to the latter, even for themes that are not live yet
  2. Playground uses fetch() and can only interact with URLs that provide appropriate Access-Control-* headers. Only the latter URL does that.

### Missing images in the theme unit test data

The theme unit test data references images from a wordpress.com site that doesn't provide the Access-Control-* headers and thus can't be downloaded. A fix would involve updating the following XML file to load images from raw.githubusercontent.com: https://raw.githubusercontent.com/WordPress/theme-test-data/master/themeunittestdata.wordpress.xml

This is outside of scope for this PR and can be addressed separately if needed.

## Testing instructions

I am not sure how to test this PR 😆 Any suggestions @dd32 @acosmin?

## Implementation

The following Blueprint is used to achieve this setup (https://playground.wordpress.net/builder/builder.html#{%22preferredVersions%22:{%22php%22:%227.4%22,%22wp%22:%22latest%22},%22steps%22:[{%22step%22:%22login%22,%22username%22:%22admin%22,%22password%22:%22password%22},{%22step%22:%22defineWpConfigConsts%22,%22consts%22:{%22WP_DEBUG%22:true}},{%22step%22:%22importFile%22,%22file%22:{%22resource%22:%22url%22,%22url%22:%22https://raw.githubusercontent.com/WordPress/theme-test-data/master/themeunittestdata.wordpress.xml%22,%22caption%22:%22Downloading%20theme%20testing%20content%22},%22progress%22:{%22caption%22:%22Installing%20theme%20testing%20content%22}},{%22step%22:%22installPlugin%22,%22pluginZipFile%22:{%22resource%22:%22wordpress.org/plugins%22,%22slug%22:%22theme-check%22},%22options%22:{%22activate%22:true}},{%22step%22:%22installTheme%22,%22themeZipFile%22:{%22resource%22:%22url%22,%22url%22:%22https://downloads.wordpress.org/theme/pendant.1.0.15.zip%22,%22caption%22:%22Downloading%20the%20theme%22}}} preview in the Blueprints builder]):

{
  "preferredVersions": {
    "php": "7.4",
    "wp": "latest"
  },
  "steps": [
    {
      "step": "login",
      "username": "admin",
      "password": "password"
    },
    {
      "step": "defineWpConfigConsts",
      "consts": {
        "WP_DEBUG": true
      }
    },
    {
      "step": "importFile",
      "file": {
        "resource": "url",
        "url": "https://raw.githubusercontent.com/WordPress/theme-test-data/master/themeunittestdata.wordpress.xml",
        "caption": "Downloading theme testing content"
      },
      "progress": {
        "caption": "Installing theme testing content"
      }
    },
    {
      "step": "installPlugin",
      "pluginZipFile": {
        "resource": "wordpress.org/plugins",
        "slug": "theme-check"
      },
      "options": {
        "activate": true
      }
    },
    {
      "step": "installTheme",
      "themeZipFile": {
        "resource": "url",
        "url": "https://downloads.wordpress.org/theme/pendant.1.0.15.zip",
        "caption": "Downloading the theme"
      }
    }
  ]
}

@dd32 commented on PR #186:


6 months ago
#5

I am not sure how to test this PR 😆

Testing the Trac integrations is never easy :) because of that I don't usually actually test it.. I just do it live 🤠

The part that can be tested though is the PHP in a standalone file, and a Trac admin can edit the existing trac tickets/comments with the new generated text to see how it works.

Unfortunately, the link in the PR as-is is a bit.. finicky with Trac.. the Trac linking code gets a bit confused by the { and [ in the link.

The zip file URL used in the comment follows this pattern:

https://wordpress.org/themes/download/...

Ha.. there's no reason that specific link format was used here, other than legacy reasons, it pre-dates downloads.wordpress.org being a "thing", so I'll change that as part of this.

Missing images in the theme unit test data
This is outside of scope for this PR and can be addressed separately if needed.

Agreed, this is a change that should be made, but it doesn't need to be done before this is implemented.

I'm not 100% sure on the blueprint, the PR used one which doesn't import the test data, so I've used the one from the description, which seems to work.

I'm also not 100% sure that hard-coding the blueprint into the Trac tickets is the right way to go, we might be better off having a redirect location like https://wordpress.org/themes/wp-json/trt/live-preview?slug={$slug}&version={$version}, but I guess in a way it's good that it's hard-coded, it means that future PHP version changes in the blueprint won't change the url in older tickets, etc.. so I'm just going to hard-code it for now.

#6 @dd32
6 months ago

Well, that didn't work :(

Example: https://themes.trac.wordpress.org/ticket/160029#comment:1

That raises two issues:

  1. The payload being passed didn't work
  2. There needs to be an installParent step too.

#7 @dd32
6 months ago

In 13082:

Theme Directory: Review: Don't encode the JSON for live preview, and use 'WikiCreole' syntax to avoid issues with ] and } in the links.

See #7382.

#8 @dd32
6 months ago

In 13083:

Theme Directory: Review: Install the parent theme in live previews.

See #7382.

#9 @zieladam
6 months ago

Yay, brilliant! I just checked this ticket, and it has a Live Preview link that actually works!

https://themes.trac.wordpress.org/ticket/159444

@acosmin @kafleg – is that what you had in mind? Or would you like the Live Preview to do something more?

#10 @acosmin
6 months ago

:-) Looks great!

Wished WP didn't display so many deprecated/warning messages :) in the latest WP versions. E.g:

Deprecated: The PSR-0 `Requests_...` class names in the Requests library are deprecated. Switch to the PSR-4 `WpOrg\Requests\...` class names at your earliest convenience. in /wordpress/wp-includes/Requests/src/Autoload.php on line 2

Warning: Cannot modify header information - headers already sent by (output started at /wordpress/wp-includes/Requests/src/Autoload.php:2) in /wordpress/wp-admin/includes/misc.php on line 87

Warning: Cannot modify header information - headers already sent by (output started at /wordpress/wp-includes/Requests/src/Autoload.php:2) in /wordpress/wp-includes/functions.php on line 140

Warning: Cannot modify header information - headers already sent by (output started at /wordpress/wp-includes/Requests/src/Autoload.php:2) in /wordpress/wp-admin/admin-header.php on line 2

Warning: Cannot modify header information - headers already sent by (output started at /wordpress/wp-includes/Requests/src/Autoload.php:2) in /wordpress/wp-includes/option.php on line 14

Warning: Cannot modify header information - headers already sent by (output started at /wordpress/wp-includes/Requests/src/Autoload.php:2) in /wordpress/wp-includes/option.php on line 14

Also, loading PHP 8.1+ with WP_DEBUG is a wonderful experience :D

We might consider creating a new theme unit test data file... with some basic posts/pages. The current probably takes more time to import into the playground.

Anyway, we'll have enough time to play around with this after the holidays.

Happy holidays!

#11 @dd32
6 months ago

Deprecated: The PSR-0 Requests_... class names in the Requests library are deprecated. Switch to the PSR-4 WpOrg\Requests\... class names at your earliest convenience. in /wordpress/wp-includes/Requests/src/Autoload.php on line 2

This is actually a playground-specific deprecated warning, in WP 6.2+. I've filed a bug for this here: https://github.com/WordPress/wordpress-playground/issues/922

#12 @dd32
6 months ago

  • Resolution set to fixed
  • Status changed from new to closed

Also, loading PHP 8.1+ with WP_DEBUG is a wonderful experience :D

:horrified-face:

:)

Now that the above PSR-0 warning is gone, I'm marking this as fixed.

For any playground-specific WordPress issues, open a ticket in https://github.com/WordPress/wordpress-playground

For any enhancements to the live-preview links here, such as different unit test data, open a new ticket here.

For live-previews in the front-end / wp-themes.com check out #7309 before making a new ticket.

Note: See TracTickets for help on using tickets.