Fix blank site editor when theme name contains a period

[mkaz]

Description

The rest routes do not include a period in the regex so if the theme name includes a period the API requests will fail.

There is still an issue when the route matches and the callback get_item() is passed the theme name as the id. The get_item() method attempts look up global styles using this id but as a string it fails.

@oandregal How should that look up work? Checking for numeric seems to work but feels like it is skipping an important part. I think I figured out the lookup, it got oversimplied but I think the regex might of been from template routes, and didn't need to be as complicated.

I've confirmed via test instructions below and this now seems to work with no errors.

Fixes #37156

How has this been tested?

1. Rename a block theme to include a period, for example twentytwentytwo -> twentytwentytwo-1.0
2. Activate block theme with period in the name.
3. Load site editor confirm error in console.log (404 route not found)
4. Apply patch.
5. Confirm site editor loads and global style changes are saved.

[youknowriad]

Will this change break the endpoint for themes inside subfolders? Did we test that use-case?

[mkaz]

Thanks @youknowriad that explains the regex complexity, I couldn't figure out why.
It almost works 😁

With themes in a sub-directory, the template-parts API doesn't seem to load properly but the other APIs do. I'll troubleshoot some more.

[mkaz]

ok, I added the \/ to the regex to support subdirs and it seems to be working for themes setup in the subdir.

It considers the theme name: subdir/theme so doesn't work for template parts in the theme.
So I think we want to discard the first subdir for it to load properly.

[mkaz]

Should the theme slug include the subdir? There appears to be a conflict.

When looking at the template list it includes the subdir in the slug?
For example: http:https://localhost:8001/wp-admin/themes.php?page=gutenberg-edit-site&postType=wp_template

However, when including from template parts the subdir is not included - since a theme author likely did not create in the same subdirectory.

[mkaz]

There are minor differences between how Core and Gutenberg register API routes, I created a core ticket with a patch to trunk that seems to fix this problem: https://core.trac.wordpress.org/ticket/54596

The same change applied to the Gutenberg plugin still has some minor issues. My environment is a bit of a mess between multiple core versions, themes, modified templates, and other discrepancies. I'll circle back to this one likely tomorrow and test with a fresh setup.

[mkaz]

I've updated this PR to match the core trac ticket and now testing properly against 5.9 source, there are more errors when testing against WP 5.8.2, so we'll need more testing around backwards compatibility if the new plugin version is supposed to work with older WP installs.

Right now it is working functionally as expected. There is a 404 error on the API call when browsing to the listing of templates or template parts page: /wp-admin/themes.php?page=gutenberg-edit-site&postType=wp_template

However, clicking into the templates (or template parts) updating and saving works. It appears the issue on that page is the shortening of the theme name from the directory to the actual name itself, ie. from twentytwentytwo-1.0 -> "Twenty Twenty Two"

[youknowriad]

I'm adding @adamziel as a reviewer here as he worked on similar issues in the endpoint recently.

[mkaz]

This also needs to take into account additional characters.
See: WordPress/wordpress-develop#2034

[Mamaduka]

Hi, @mkaz

It looks like the fix was committed in core - https://core.trac.wordpress.org/changeset/52376.

[youknowriad]

Can we bring the changes to Gutenberg as well (as we need to support older WP versions)

[adamziel]

Maybe we don't even need a separate PR, updating this one should do

[mkaz]

👍 I migrated the changes made in the trac ticket 52376 and everything is now testing well. Please take a look and confirm. Same testing instructions as above but you can also try for additional special characters such as ( ) _ [ ] see trac ticket for additional details

[youknowriad]

There's a failing unit test but otherwise looks good.

[mkaz]

There's a failing unit test but otherwise looks good.

My fault, bad grepping. I saw the test in the trac ticket but searched in the wrong directory for it. There are some tests in core that aren't in the plugin.

I updated the failing test with changes made to core. 👍

[mkaz]

NOTE: Even though the label is there, this does not need to be backported to core, it is an update from core, so the code is already synced.