Prevent BP from being updated when minimum requirements are not met

[boonebgorges]

If you're running PHP 5.2, and BP 2.8 won't support 5.2, we should take steps to prevent the user from upgrading, since upgrading will either cause fatal errors, or cause BP to bail out completely (see #7195).

The main thing here is probably to hook into the list of plugins at plugins.php and make sure that (1) the Update plugin is hidden, and (2) BP can't be selected as part of bulk updates. This will need some reexamination after WP 4.6, which is changing some of the update UI ("shiny updates"). It'll still be possible to update via wp-cli, direct file upload, etc, but we're mainly trying to protect naive users using the native WP interface.

[boonebgorges]

Not sure whether it's worth a separate ticket, but we should probably introduce some messaging - an admin notice, or maybe a new admin tab (it's that important!) - that explains the situation to affected users. Ideally we'd link to some codex documentation that gives more explanation, and gives some language that the users can send to their webhosts.

This stuff needs to go into 2.7, so that we can protect against bad 2.8 updates.

[boonebgorges]

@dcavins points out that we may want to do the same thing for unsupported WP versions.

[boonebgorges]

bump

[boonebgorges]

7196.diff​ is an attempt to prevent plugin updates through WP's native UIs.

It's not easy to do, and it's not beautiful. In the case of the "update_row" on plugins.php, there's an action that allows me to unhook WP's native notice and replace it with my own. In the case of the bulk update checkboxes, the only option I could find was to disable with JavaScript. It's not foolproof, but it's not meant to be: our primary goal here is to dissuade the unknowing from breaking their site.

The message that I added in the plugin row is very general:

printf( esc_html__( 'A BuddyPress update is available, but your system is not compatible. See %s for more information.', 'buddypress' ), '<a href="#">some link</a>' );

where "some link" will presumably be a Codex page explaining the situation. I think we can craft some sort of brief message to use here and also in an admin notice (not implemented here yet), and then link off to a Codex page that explains in some more detail.

I can work on some of this documentation, and cleaning up the patch, but I first wanted to run the technique by the group in case anyone had a better idea of how to do it.

[boonebgorges]

It's probably obvious that you'll have to futz with the numbers in the patch to see it at work :-D I posted Screenshot_2016-09-16_20-51-27.png​ so that you can see what it looks like without adding lots of !s.

[r-a-y]

02.patch disables bulk activation in two ways:

1. For older WordPress installs that do not support Shiny Updates - bp_core_admin_disable_bulk_activate()
2. For WordPress installs that support Shiny Updates - bp_core_admin_disable_shiny_bulk_update()

To show the notice for testing purposes:

• In bp-loader.php, the version is downgraded to BP 2.5.
• In bp_core_admin_is_running_php53_or_greater(), I bumped the PHP requirement function to PHP7.
• In bp_core_admin_php52_plugin_row(), I commented out the BP 2.8 check.

The notice also needs better styling for older versions of WordPress. I'll look into this a bit later on.

[boonebgorges]

Thank you, @r-a-y!

What about update-core.php? You removed my crummy core_upgrade_preamble bit, but I don't think any of your new Shiny stuff covers that case. Or am I wrong?

It dawned on my after my last post that the update-core.php part of my patch didn't check for the BP version, so it would block 2.7.x in addition to 2.8; this suggests that we might need to use jQuery, which I'd been avoiding, or check the site transient to get the available update info.

Your trick in bp_core_admin_disable_bulk_activate() is less ugly codewise, than disabling the checkbox, but it seems like suboptimal UX, since people might get confused about the fact that they're checking BuddyPress but it's not being updated. To be clear: I don't think it's worth spending a ton of time or effort making the experience seamless in these cases. But I mainly wanted to check to see if you'd removed this for a principled reason, or just because it was ugly :-D

[r-a-y]

What about update-core.php?

Forgot about update-core.php!

You removed my crummy core_upgrade_preamble bit, but I don't think any of your new Shiny stuff covers that case. Or am I wrong?

bp_core_admin_disable_shiny_bulk_update() should cover WP installs that support Shiny Updates.

However, this doesn't cover non-Shiny installs.

Maybe we can use your preamble code for non-Shiny installs, since the remove checkbox idea is better than my bp_core_admin_disable_bulk_activate() method.

[r-a-y]

03.patch includes support to remove BuddyPress from the update plugins list on the update-core.php page. This removes the need to disable the checkbox via the 'core_upgrade_preamble' hook in 7196.diff.

However, I left the checkbox code in bp_core_admin_php52_plugin_row() and removed my bp_core_admin_disable_bulk_activate() method, since you make a good point about UX.

I've also removed bp_core_admin_disable_shiny_bulk_update(). It does act as a safeguard just in case a savvy dev re-enables the checkbox via their browser's web inspector. But, if a dev is going to go through this trouble, the person would probably already upload the new version of BP via FTP or what have you :)

[boonebgorges]

I've drafted a Codex page for us to link to in various places: ​https://codex.buddypress.org/getting-started/php-version-support/buddypress-2-8-will-require-php-5-3/ Feedback/edits are welcome. I think we need an admin notice for this. For BP 2.7, it can be dismissable, but for 2.8 we should probably make it persistent. Does that sound right to others?

7196.03.patch​ is looking nice. The site transient filter is a better version of what I was trying to do.

The patch will need to be updated once we have finalized on a URL for the codex page linked above. @hnla @mercime Do you have thoughts about where such a page should live in the hierarchy?

@r-a-y Can you take point on committing your patch once you feel it's ready? I am traveling for the next few days, but this really should be part of any release candidate.

[hnla]

I think that doc as it stands is more developer focussed so the main copy should live under dev docs. In the 'Getting Started' page we mention the minimum php/MySQL requirements so we'll update the min php, keeping the recommendation but adding a sub note that as of ... an absolute minimum exists and add link to the 'will-require.5.3' page.

@mercime Wondering whether we should have a broader page under dev docs for general requirements with this new page a section in that?

[mercime]

@boonebgorges @hnla imho the articles for "PHP version support" and "BuddyPress 2.8 will require PHP 5.3+" are fine under the "Getting Started" section where the article for "WordPress version compatibility" also is under. I think even new BP users will appreciate learning about the requirements before installing BP. Thank you.

[boonebgorges]

Thanks @hnla and @mercime - I've moved the page to ​https://codex.buddypress.org/getting-started/buddypress-2-8-will-require-php-5-3/.

[boonebgorges]

7196.2.diff​ adds a dismissible admin notice (with a tiny little API for these notices, which we may want to use in the future, but don't have to). It also fixes a translation issue, and fixes placeholder links to point to the actual Codex page.

[r-a-y]

In 11174:

Admin: Block updates to BuddyPress 2.8 if PHP version is < 5.3.

BuddyPress 2.8 is going to require PHP 5.3+. To prepare for this, we are
adding some functionality to block updates to BP 2.8 in the WP admin
dashboard for BP 2.7.0.

Props boonebgorges, r-a-y, hnla, mercime.

See #7196.

[r-a-y]

In 11175:

Admin: Add dismissable notice for BuddyPress 2.8 PHP requirement.

Props boonebgorges.

See #7196.

[r-a-y]

I've committed the patches and made some minor changes so:

• The plugin row notice looks okay on < WP 4.6
• The codex URL isn't added to the translation string
• The dismissable notice is also displayed in the network admin dashboard

One last, minor thing, should the dismissable notice be shown on all pages in the the admin dashboard? I think the Plugins page should be sufficient.

Feel free to close if the notice should be shown on all pages!

[DJPaul]

I think just Plugins make sense - if something isn't seem to work, I would guess where that's where most people go to check if the plugin's active

[hnla]

Might be a silly question, might be case of go read the code but if we're adding this check before allowing the update to run through and adding it to 2.7 what happens to people that leap frog over releases, say from 2.6 jumping straight to 2.8, do they just crash and burn?

As this is an important issue I wouldn't be adverse to showing the admin notice across all admin screens, at least it isn't simply a piece of banner advertising hooked into the message api.

[DJPaul]

@hnla About people skipping 2.7 -- we don't want crashing :) but we'll probably do something like check the PHP version in the plugin loader, and simply return out if it's too old; maybe add a UI notification of some kind. Good topics of discussion for 2.8. :)

[r-a-y]

what happens to people that leap frog over releases, say from 2.6 jumping straight to 2.8, do they just crash and burn?

Good point, indeed!

Something like what @DJPaul mentioned is what we'll probably have to do, but it kind of circumvents all the code that has been committed in this ticket 😄

Maybe we can have a long release cycle for BP 2.8 in the hopes that people upgrade to BP 2.7! 🕰

[hnla]

I can see an endless loop of version checks :)

[boonebgorges]

I agree with the above - for 2.7-skippers (as well as new installations) we will probably have a PHP 5.2-compatible loader file that throws some sort of admin notice. What's included in the current ticket is an attempt to prevent users from breaking their sites in as many cases as we can, but there are some cases we'll simply be unable to help with.

Showing the admin notice only on plugins.php seems OK to me.

[r-a-y]

In 11179:

Admin: Only show dismissable notice on the Plugins page.

See #7196.

[r-a-y]

I think we've done enough for this ticket.

I've opened #7277 for what we've been talking about since comment:24.