Custom Template Parts are Duplicated, rather than being updated (PHP 5.6)

[jonmackintosh]

I actually logged this in the forums initially, ​https://wordpress.org/support/topic/site-template-issue-in-php-5-6/

Attempts to edit site template parts in WordPress 6.0.1 running on PHP 5.6.40 using the Twentytwentytwo theme result in duplicate template part being created as opposed to the actual template part being edited. This has a knock on impact. You can edit a template part but any subsequent attempt to edit the template part loses the previous changes, reverting the template part to its original state.

The issue cannot be recreated on PHP 7.4, and the functionality works as expected.

Image provided to illustrate PHP 5.6.40 outcome when attempting to change the background colour on the default header.

​https://i.imgur.com/sHXrvAw.jpg

[uofaberdeendarren]

Output on PHP 5.6.40

[uofaberdeendarren]

The issue comes from this line here: ​https://github.com/WordPress/WordPress/blob/master/wp-includes/block-template-utils.php#L699

$query_result is an array of WP_Block_Template objects. The array_columns tries to "pluck" out the IDs for each of the template parts, to compare the current file-based template id to the database ones to see if the template already exists.

There is a difference in how array_columns works between PHP 5 and PHP 7. PHP 7 works fine when using objects, but PHP 5 does not. See: ​https://stackoverflow.com/a/23335938

That's why it works fine in PHP 7 because the return from array_columns does correctly contain the IDs of the templates, which will then stop the duplication. In PHP 5, the return is always an empty array, so the following conditionals don't think there are any database template parts, and therefore can duplicate them.

[uofaberdeendarren]

Working on a PR to resolve the issue.

An issue has been reported whereby template parts can be duplicated when running on PHP 5.6.

This has been tracked down to the use of array_column on an array of WP_Block_Template. Whilst this works correctly in PHP 7, it will always return an empty array in PHP 5.6. See: ​https://stackoverflow.com/a/23335938

I have converted the array_column call to array_map to ensure that this will continue to work with backwards compatibility.

Full details of the issue and the analysis can be found in the trac ticket.

Trac ticket: https://core.trac.wordpress.org/ticket/56271

[rudlinkon]

@uofaberdeendarren please update the PR with the Suggested changes which is reviewed by @costdev

[uofaberdeendarren]

@rudlinkon changes are now made as requested.

[rudlinkon]

Replying to uofaberdeendarren:

@rudlinkon changes are now made as requested.

Thanks for your PR. It will be reviewed soon by @costdev

The change looks good to me. It would be great to add tests to tests/phpunit/tests/block-template-utils.php to prevent future regressions here.

The change looks good to me. It would be great to add tests to tests/phpunit/tests/block-template-utils.php to prevent future regressions here.

Thanks, @swissspidy - Totally agree re unit tests, however, I suspect that @costdev (or indeed anyone else) would be quicker at adding them - I'm not too familiar with the codebase, let alone its tests. Happy to have a look, but I wouldn't count on me!

I'm working on unit tests for this issue.

This PR aims to fix a bug in the get_block_templates() method.
array_column doesn't work correctly with an array of objects on PHP <= 5.6. So, under certain circumstances, get_block_templates() could return entities with the same ID (slug).
A regression unit test (included in this PR) should prevent this from happening in the future.

Trac ticket: https://core.trac.wordpress.org/ticket/56271

I've just submitted a ​new PR with the unit tests and the fix.
I propose to close this PR in favor ​of the new PR as it has unit tests.
Props to @darrencoutts118 for fixing the issue!

[antonvlasenko]

Steps to test ​https://github.com/WordPress/wordpress-develop/pull/3051:

1. Make sure you are on PHP 5.6.x.
2. Open the Editor and go to the list of templates/template parts.
3. Modify any template/template part. Save it.
4. Open the list of templates/template parts again.

Expected result:
The template/template part you've just edited should be displayed once.
Actual result:
The template/template part you've just edited will be displayed twice.

After the fix the template/template part will displayed once.

Thanks @anton-vlasenko! This review is targeted at formatting and organisation of methods.

Thanks for the review, @costdev!
I've replied to your comments.

Thanks for putting the tests together @anton-vlasenko!

Slightly cheeky question... given I found the issues/came up with the original solution... will I still get credit on the contributor's page? 😄

First-time contributor, but would be nice to get my name there!

Thanks for putting the tests together @anton-vlasenko!

Slightly cheeky question... given I found the issues/came up with the original solution... will I still get credit on the contributor's page? 😄

First-time contributor, but would be nice to get my name there!

Not a cheeky question 🙂

As you submitted the original patch, and the Trac ticket includes this in one of the comments:

Props to @darrencoutts118 for fixing the issue!

So you'll be included in the credits when this is committed! See ​the Handbook entry on Commit Props (and the "Guidelines" section below it) for more information on how props are determined.

I'd suggest that you register an account on WordPress.org so that the link in the "Credits" page can link to your WordPress.org profile. You can also optionally add your GitHub username to your WordPress.org profile so others can find you here too 🙂

Thanks for contributing to WordPress Core! There is more information in the ​FAQ for New Contributors, should you need it! 🙂

Thanks for putting the tests together @anton-vlasenko!

Slightly cheeky question... given I found the issues/came up with the original solution... will I still get credit on the contributor's page? 😄

First-time contributor, but would be nice to get my name there!

That's an entirely valid question, @darrencoutts118.
I'm not a release lead, so I can't tell for sure if you will get credit on the contributor's page. But I think you should get it.
I had to create a new PR only because I do not have write access to your repository.
I will modify the description section of my PR to make your contribution more obvious.

[ironprogrammer]

Test Report

Patch tested: ​PR 3051 - LGTM 👍🏻

Environment

• OS: macOS 12.5
• Browser: Safari 15.6
• Server: nginx/1.23.1
• PHP: 5.6.40
• WordPress: 6.0.2-alpha-53697-src
• Theme: twentytwentytwo v1.2

Actual Results

• ✅ After patching, customized Templates and Template Parts are no longer displayed in duplicate.

Additional Notes

• Before patch is applied, custom templates are repeatedly added between lists when switching between Templates and Template Parts. ​See this video demonstration.
• Also verified that after upgrading from PHP 5.6 to 7.4, templates were not shown in duplicate.

Supplemental Artifacts

Figures 1-2: Templates and Template Parts screens listing customized templates multiple times.

Figures 3-4: Templates and Template Parts screens displaying non-matching template types between screens.

Figures 5-6: AFTER PATCH. Templates and Template Parts screens listing customized templates only once. ✅

[ironprogrammer]

get_block_templates() was ​introduced in 5.8. As this is a display issue, removed performance focus and adjusted severity.

[rudlinkon]

Replying to ironprogrammer:

Test Report

Patch tested: ​PR 3051 - LGTM 👍🏻

Environment

• OS: macOS 12.5
• Browser: Safari 15.6
• Server: nginx/1.23.1
• PHP: 5.6.40
• WordPress: 6.0.2-alpha-53697-src
• Theme: twentytwentytwo v1.2

Actual Results

• ✅ After patching, customized Templates and Template Parts are no longer displayed in duplicate.

Additional Notes

• Before patch is applied, custom templates are repeatedly added between lists when switching between Templates and Template Parts. ​See this video demonstration.
• Also verified that after upgrading from PHP 5.6 to 7.4, templates were not shown in duplicate.

Thank you so much for your detailed test report, so it may be resolved now.

[SergeyBiryukov]

In 53927:

Editor: Ensure get_block_templates() returns unique templates or template parts.

The function was using the array_column() PHP function on an array of objects, which works as expected on PHP 7 or later, but not on PHP 5.6.

This resulted in customized templates being listed multiple times on the Templates and Template Parts screens, and being repeatedly added between lists when switching between the screens.

The issue is now resolved by replacing array_column() with the wp_list_pluck() WordPress core function, which provides consistent behavior beetween PHP versions.

Reference: ​PHP 7.0 Upgrade Notes: array_column().

Props uofaberdeendarren, antonvlasenko, ironprogrammer, jonmackintosh, costdev, hellofromTonya, swissspidy, rudlinkon.
Fixes #56271.

[SergeyBiryukov]

In 53928:

Editor: Ensure get_block_templates() returns unique templates or template parts.

The function was using the array_column() PHP function on an array of objects, which works as expected on PHP 7 or later, but not on PHP 5.6.

This resulted in customized templates being listed multiple times on the Templates and Template Parts screens, and being repeatedly added between lists when switching between the screens.

The issue is now resolved by replacing array_column() with the wp_list_pluck() WordPress core function, which provides consistent behavior beetween PHP versions.

Reference: ​PHP 7.0 Upgrade Notes: array_column().

Props uofaberdeendarren, antonvlasenko, ironprogrammer, jonmackintosh, costdev, hellofromTonya, swissspidy, rudlinkon.
Merges [53927] to the 6.0 branch.
Fixes #56271.

Thanks for the PR! Merged in r53927, backported to the 6.0 branch in r53928.

Thanks for the PR! Merged in r53927, backported to the 6.0 branch in r53928.