Consolidate the logic for displaying WP & PHP compatibility messages for themes

[SergeyBiryukov]

Background: #48491

As part of the changes on [48636-48638,48640], these messages are duplicated in at least seven different places:

• "This theme doesn’t work with your versions of WordPress and PHP. Please update WordPress, and then learn more about updating PHP."
• "This theme doesn’t work with your version of WordPress. Please update WordPress."
• "This theme doesn’t work with your version of PHP. Learn more about updating PHP."

See comment:36:ticket:48491 for the list of relevant areas.

It would be nice to consolidate the logic in one place.

[hellofromTonya]

Per Sergey during today's scrub:

Needs a patch, I'll try to get to it before beta, or punt otherwise.

[afragen]

50787.diff​ is an initial pass at this ticket.

[hellofromTonya]

A couple of code review notes:

Function naming

For the new function name, it's not clear to me what the function will do when invoked. I can infer. But I'm wondering if it would be more clear if action-based such as

wp_get_compatibility_string

to tell us that it "gets" and returns a single string. What do you think @afragen ?

Performance

Each time the function is called, the $strings array rebuilds. This means each __() has to run again, ie there are 12 of them. Is this necessary? Do we need to retranslate and rebuild the array each time the function is invoked?

We can use memoization for a tiny peformance boost.

[hellofromTonya]

Revising my performance note above. ​See slack conversation.

The last 3 string elements are dynamic, i.e. build from the injected $name. if no name is passed to the function, then we can skip building those last 3 elements.

For the non-dynamic strings, we can leverage memoization to generate the $strings array once and then reuse the same array each time the function is invoked.

It's a tiny tiny performance boost.

[afragen]

rename function and use static variable for performance

[hellofromTonya]

@SergeyBiryukov Can you please do a final review on the latest patch Andy and I did? What do you think?

[afragen]

@SergeyBiryukov I think we're ready for a final review.

Thanks @hellofromTonya 🤗

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

Props: @afragen

[SergeyBiryukov]

Thanks for the patch!

Didn't get a chance to review this in time for 5.6 Beta 1, but I think it can still go in before RC, since it's more of a cleanup than new functionality. Converting to a task for now.

[SergeyBiryukov]

Didn't get a chance to review this in time for 5.6 RC, moving to the next release for now.

[hellofromTonya]

This ticket is ready for code review.

[hellofromTonya]

For manually testing this patch:

• Get a plugin or theme that is not compatible with the WP version running in your local env
• Check that incompatible message appears

For example, plugin incompatible message:

This plugin doesn't work with your versions of WordPress and PHP.

[hellofromTonya]

Before: On "Add Plugins" UI with PHP 5.6 and with WooCommerce (which requires PHP 7+)

[hellofromTonya]

After: On "Add Plugins" UI with PHP 5.6 and with WooCommerce (which requires PHP 7+)

[SergeyBiryukov]

Thanks for the patch!

Just noting that 50787.2.diff​ is a good start, as it touches all the relevant areas, but is not quite what I envisioned for this ticket.

I would like to consolidate not just the strings (that doesn't seem to reduce much code), but also the repeated logic for these messages, i.e. various conditionals for adjusting them based on user capabilities and what actually needs updating. Sorry if that was not clear from the description.

[hellofromTonya]

Before: Theme PHP incompatibility when on PHP 5.6 and theme requires at least PHP 7

[hellofromTonya]

After: Theme PHP incompatibility when on PHP 5.6 and theme requires at least PHP 7

[hellofromTonya]

PHP Incompatibility Testing Strategy

How to test the PR

Pull the PR down to your local machine

git fetch origin pull/607/head:pr607
git checkout pr607

Step 2: Change the LOCAL_PHP=5.6-fpm in .env.

Step 3: Reprovision

Step 4: Log into the admin area as an admin

Testing the plugin's PHP incompatibility message:

We'll attempt to add a plugin that requires PHP 7+ while your local dev environment is running PHP 5.6.

Step 5: Go to the "Plugins" and then "Add Plugins" screen

Step 6: Type "WooCommerce" in the search plugins field. Press enter/return.

The incompatibility notice should appear across the top of the plugin:

This plugin doesn’t work with your version of PHP. Learn more about updating PHP.

Testing the theme's PHP incompatibility message:

Let's repeat this test with a theme that requires at least PHP 7.

Step 7: Go to the "Themes" and then "Add Theme" screen

Step 8: Type "Charitas Lite" in the search themes field. Press enter/return.

The incompatibility notice should appear across the top of the plugin:

This theme doesn’t work with your version of PHP. Learn more about updating PHP.

[hellofromTonya]

Removing has-patch and has-unit-tests and adding needs-patch to continue iterating on the solution per Sergey's review.

Replying to SergeyBiryukov:

Thanks for the patch!

Just noting that 50787.2.diff​ is a good start, as it touches all the relevant areas, but is not quite what I envisioned for this ticket.

I would like to consolidate not just the strings (that doesn't seem to reduce much code), but also the repeated logic for these messages, i.e. various conditionals for adjusting them based on user capabilities and what actually needs updating. Sorry if that was not clear from the description.

[hellofromTonya]

With 5.7 RC1 landing tomorrow and no activity on this ticket for a new patch, punting this ticket to Future Release.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.