impress-org / givewp

GiveWP - The #1 Donation Plugin for WordPress. Easily accept donations and fundraise using your WordPress website.
https://givewp.com/
GNU General Public License v3.0
344 stars 191 forks source link

feat(admin): add notice to add-ons warning to always check if Give Core is up-to-date. #3217

Closed Benunc closed 6 years ago

Benunc commented 6 years ago

User Story

As a site administrator, I want to be aware that Give core needs to be updated before any add-ons so that after performing updates all of my add-ons remain active.

Every major release, the support team gets a handful of tickets from customers panicking because their forms are broken, when in reality it is simply that they updated add-ons before updating core, and the add-ons deactivate themselves.

Current Behavior

I currently click through and update the plugins, and if I go alphabetically and I am using the Give Authorize.net add-on, it updates before the Give Donation plugin and deactivates itself if it doesn't pass the minimum give version test.

Expected Behavior

I expect to be warned if a new update to Give core is necessary before updating an add-on.

Possible Solution

Since the minimum give version check technically happens in the add-on code, this might require issues in each add-on repo, but I was thinking about a way to add aWarning any time Give Core is not on the latest stable version to the top of the plugins page, or to the top of each add-on's listing (search for give- in the list?) warning to update core first.

Steps to Reproduce

  1. Downgrade Give to 1.8.x
  2. Try to update the recurring add-on to the latest.
  3. It'll deactivate.

Tasks

ravinderk commented 6 years ago

@DevinWalker Here are my suggestion to resolve this issue:

I think we do not need to add code to each addon to show warnings it can be done by core only. To achieve that I need following data:

  1. Minimum Give version requirement
  2. Tested up to which Give the version

We can insert this information into readme.txt file in plugin header with the following tag: Give requires at least Give tested up to Requires Give: 2.1 (as suggested by @DevinWalker here: https://github.com/WordImpress/Give/pull/3268#discussion_r191266081)

I will prefer to show two notice:

  1. It will appear when new Give version release. The program will check add-ons readme.txt file and warn admin about there addon compatibility with the latest release. Reason: https://github.com/WordImpress/Give/issues/3217#issuecomment-391562853 Example from Woocommerce wordpress-plugin-update-notice-1920x634

  2. It will appear when any addon update release. We will check readme.txt file of coming release and show notice if required. image

Note: We have to release the update to all addon with there update readme.txt to see the effect of this feature.

kevinwhoffman commented 6 years ago

@ravinderk This looks good to me. The add-on notice example shown under Give - Email Reports will be a welcome addition.

But I have some concerns about the Give core notice demonstrated here:

image

My understanding is the notice would alert the user to a list of add-ons with Give tested up to in the file header that is less than the available update version. So let's think about a typical update to Give core...

A Give core update is usually accompanied by a few add-on updates, however the majority of add-ons are not updated alongside every Give core update (which means their Give tested up to will be behind). I'm concerned that every Give core update will show a long list of add-ons that "have not been tested" with the available core update just because their Give tested up to has not been updated. This could result in concerned users who are wary of core updates or get a bad first impression rather than excitement over a new release.

DevinWalker commented 6 years ago

This also doesn't resolve the issue of people updating add-ons remotely and never seeing the notice. @ravinderk let's discuss when you get online. Remember, the major issue here is people updating add-ons that require a higher give version that they are not running and then the add-on deactivating. That's what needs to be resolved.

ravinderk commented 6 years ago

@Devinwalker I got you.

I researched on how WordPress updates plugin by ajax.

image

We can use upgrade_pre_install filter to set custom error message.

image

To review how we can remotely update the plugin, I reviewed https://wordpress.org/plugins/shift8-remote-update/ plugin and found that they are following WordPress logic:

image

By using upgrade_pre_install filter we can stop plugin update if a custom check fail. Let me know if you have access to any hosting service where we can test implementation.

ravinderk commented 6 years ago

My understanding is the notice would alert the user to a list of add-ons with Give tested up to in the file header that is less than the available update version. So let's think about a typical update to Give core...

A Give core update is usually accompanied by a few add-on updates, however the majority of add-ons are not updated alongside every Give core update (which means their Give tested up to will be behind). I'm concerned that every Give core update will show a long list of add-ons that "have not been tested" with the available core update just because their Give tested up to has not been updated. This could result in concerned users who are wary of core updates or get a bad first impression rather than excitement over a new release.

@Devinwalker, @mathetos, @kevinwhoffman I disagree with my first point in https://github.com/WordImpress/Give/issues/3217#issuecomment-391318862 comment because our main concern is addon update

ravinderk commented 6 years ago

@rafftar We do not need to create API for that. we have access to readme.txt which we can read. https://givewp.com/downloads/plugins/give-fee-recovery/readme.txt

ravinderk commented 6 years ago

@Devinwalker @mathetos I ran test on infinitewp to check if we will get failed plugin update notice or not and I found that we will get notice for plugin update notice.

Onsite update: image

Remote update: image

Testing site: http://recurring.givewp.com/wp-admin/plugins.php?plugin_status=upgrade

raftaar1191 commented 6 years ago

@ravinderk

Can we do the Give Minimum version check before WordPress Enabling Maintenance mode

image

ravinderk commented 6 years ago

@raftaar1191 We can not do this here because WordPress fire bulk update hook here, in iframe even we are updating a single plugin and WordPress enables maintenance mode when we updating the plugin in bulk.

for ref: https://github.com/WordPress/WordPress/blob/master/wp-admin/update-core.php#L729 https://github.com/WordPress/WordPress/blob/master/wp-admin/update.php#L47 https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-plugin-upgrader.php#L220