site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Bump minimum WordPress version to 5.2

Open felixarntz opened this issue 3 years ago • 11 comments

🚨 Note: do not merge until Release 1.88.0 🚨

The minimum required WordPress version should be bumped from 4.7 to 5.2 as part of the 1.88.0 release (November 21).

The corresponding banner notification to update from prior versions (see #5873) should then be removed.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Throughout the plugin, the minimum WordPress version requirement should be changed from 4.7 to 5.2.
  • The code for the banner notification from #5873 should be completely removed as it will be irrelevant now that the version bump is happening.

Implementation Brief

This can be started, but it must not be merged until develop is open for the 1.88.0 release. When you open a pull request for this issue, please add the same annotation to the top of the PR to minimize the chance of an accidental early merge. :)

  • Remove the notification banner added in the #5873
  • In composer.json:
    • Bump the roots/wordpress and wp-phpunit/wp-phpunit packages to require at least 5.2, and update the lockfile
  • In google-site-kit.php, README.md, readme.txt and .github/PULL_REQUEST_TEMPLATE.md:
    • Replace all occurrences of "4.7" with "5.2"

Test Coverage

PHPCS

In phpcs.xml:

  • Update the minimum_supported_wp_version variable to 5.2, and ensure that no new linter warnings/errors arise

E2E

In tests/e2e/specs/plugin-activation.test.js:

  • Remove the redundant site-kit-by-google plugin activation/deactivation

In tests/e2e/config/wordpress-debug-log/log-ignore-list.js:

  • Remove the now irrelevant array entries for 4.7.19 and 4.9.16

PHPunit

In tests/phpunit/integration/PluginTest.php:

  • Update the test_plugin_data method to check for version 5.2 instead of 4.7.

Actions

Remove the .github/workflows/e2e-tests-wp-4-9-gutenberg.yml action

In .github/workflows/php-tests-wp-4-7-php-5-6.yml:

  • Rename the file to better reflect the version change
  • Replace all references to 4.7 (including "4-7", etc) to 5.2

In .github/workflows/e2e-tests-wp-4-7.yml:

  • Same updates to the file name and 4.7 references as in the action above
  • For the env, set the WP_VERSION variable to the latest available 5.2 release (currently 5.2.16)

QA Brief

  • This PR prevents the Site Kit plugin from working in WordPress versions prior to 5.2.
  • Verify that the reminder notification added in #5873 no longer appears.
  • Verify that trying to activate the plugin in a WordPress site with version prior to 5.2 should display an error Site Kit requires WordPress version 5.2 or higher.
  • If the plugin is already active and the site has WordPress version prior to 5.2, verify that the Site Kit pages do not load.

Changelog entry

  • Raise minimum WordPress version requirement from 4.7 to 5.2.

felixarntz avatar Sep 21 '22 20:09 felixarntz

IB ✔️

eugene-manuilov avatar Sep 23 '22 10:09 eugene-manuilov

Hi @eugene-manuilov does the IB include the removal of the banner notification (from 5873)? Just double checking this is covered!

FlicHollis avatar Sep 25 '22 07:09 FlicHollis

Ouch... I missed that 🤦. Thanks, @FlicHollis. I've updated IB to reflect that.

eugene-manuilov avatar Sep 26 '22 08:09 eugene-manuilov

No problem, thanks so much @eugene-manuilov !

FlicHollis avatar Sep 26 '22 08:09 FlicHollis

I just opened #5926 as a prerequisite here, mostly as part of it should be updated as part of this issue.

aaemnnosttv avatar Sep 27 '22 18:09 aaemnnosttv

@aaemnnosttv @eugene-manuilov Hi! Quick question, should #5873 also be added as a prerequisite/blocker for this issue?

nfmohit avatar Sep 30 '22 06:09 nfmohit

... should #5873 also be added as a prerequisite/blocker for this issue?

@nfmohit, yes, I added it to the blockers list, however we can start working on this ticket earlier and then remove the notification added in #5873 once it is merged into develop.

eugene-manuilov avatar Sep 30 '22 10:09 eugene-manuilov

Thank you @eugene-manuilov!

nfmohit avatar Sep 30 '22 10:09 nfmohit

A few quick questions:

Acceptance criteria

The code for the banner notification from https://github.com/google/site-kit-wp/issues/5873 should be completely removed as it will be irrelevant now that the version bump is happening.

  1. Should we possibly preserve this banner notification for potential future version bumps? If agreed, I think it can stay as it is without any change since:
    • The banner notification will not load if Site Kit meets the minimum WP version requirement.
    • Site Kit will not load at all if it doesn't meet the minimum version requirement.
  2. If we decide to preserve this, should we consider making it autonomous, i.e. check the minimum required WP version from the plugin metadata/GOOGLESITEKIT_WP_MINIMUM constant and show the banner notification if applicable, thus not requiring us to manually update the minimum version in the component?
  3. Should we show an admin notice if the minimum WP version requirement is not met but Site Kit is installed and activated? A probable exemplary scenario could be someone downgrading to a lower than minimum WP version with Site Kit 1.88.0 installed and active prior to the downgrade. The plugin will not load in such a case but without proper context.

CC: @aaemnnosttv

nfmohit avatar Oct 06 '22 06:10 nfmohit

At first I thought it made sense to keep the minimum version component around. But then I thought…

Let's not bother with the second point, because the dates would also change and there's other text in the component that probably makes sense to tailor to each version bump.

And we can always get this file out of our git history, etc. But we won't be doing another version bump for a long time, in which time this component will go entirely unused and untested. Probably best to just create another one in 2+ years if we do this again 😏

So let's remove the file entirely. It won't be used at all so we should get rid of it.


For the last one—will WordPress automatically deactivate plugins that aren't compatible with your version of WordPress? If so let's not do this… in fact, I'd still say it's not worth doing unless we get more than a few support requests about it. Let's skip those plans for now. 🙂 Thanks for checking though, all good points 👍🏻

tofumatt avatar Oct 10 '22 15:10 tofumatt

Understood, thank you for the confirmation, @tofumatt!

nfmohit avatar Oct 11 '22 05:10 nfmohit

QA Update: ⚠️

@nfmohit @felixarntz @aaemnnosttv if I have Site Kit setup, and on a version of WordPress prior to 5.2, and I upgrade to Site Kit v 1.88, I am not going to see any message informing me that Site Kit is no longer compatible. Site Kit just disappears from the WordPress menu. I know they have had the banner with an opportunity to upgrade, but it doesn't feel like a good user experience. I feel we should at least have a message in the dashboard saying that they need to upgrade.

Interested in your thoughts.

wpdarren avatar Nov 07 '22 15:11 wpdarren

if I have Site Kit setup, and on a version of WordPress prior to 5.2, and I upgrade to Site Kit v 1.88, I am not going to see any message informing me that Site Kit is no longer compatible.

@wpdarren WordPress won't surface the update for Site Kit for users who are on an older version of WP; in my understanding, the only way this is possible would be to manually upgrade. I agree that it would be good to leave a notification in this situation, but this would make more sense as a follow-up issue as this is of course out of scope with the AC defined for this issue.

aaemnnosttv avatar Nov 07 '22 15:11 aaemnnosttv

@wpdarren @aaemnnosttv This is currently expected, but I don't think we need to implement a manual notification. The limitation right now only occurs because the plugin isn't released on wordpress.org yet in that version. Once 1.88.0 is on wordpress.org, sites with an older WP version than 5.2 will not even be able to update the plugin, and there will be a warning in all relevant update UIs in WordPress explaining why - this is a feature part of WordPress core and wordpress.org.

Of course you can still manually update by uploading the ZIP, but that is such an edge case at the grand scheme of things that I don't think we need to specifically cater for that with an extra message.

So from my perspective this is probably good to go.

felixarntz avatar Nov 07 '22 15:11 felixarntz

QA Update: ✅

Thanks @felixarntz @aaemnnosttv 👍

Verified:

  • the reminder notification no longer appears.
  • when trying to activate the plugin in a WordPress site with version prior to 5.2 an error is displayed stating that Site Kit requires WordPress version 5.2 or higher.
  • if the plugin is already active and the site has WordPress version prior to 5.2, the Site Kit pages do not load.
sk

wpdarren avatar Nov 07 '22 15:11 wpdarren