Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Benunc opened this issue May 11, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@Benunc
Copy link
Member

commented May 11, 2018

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

  • warn user they are about to update when Give is not on the latest version.
@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented May 23, 2018

@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: #3268 (comment))

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: #3217 (comment)
    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

This comment has been minimized.

Copy link
Member

commented May 23, 2018

@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

This comment has been minimized.

Copy link
Member

commented May 23, 2018

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

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2018

@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

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2018

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 #3217 (comment) comment because our main concern is addon update

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2018

@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

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2018

@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

@impress-org impress-org deleted a comment from raftaar1191 May 24, 2018

@impress-org impress-org deleted a comment from raftaar1191 May 24, 2018

ravinderk added a commit that referenced this issue May 28, 2018

DevinWalker added a commit that referenced this issue May 30, 2018

Merge pull request #3268 from /issues/3217
fix(install): verify addon Give dependency before update #3217
@raftaar1191

This comment has been minimized.

Copy link
Member

commented May 30, 2018

@ravinderk

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

image

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented May 31, 2018

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.