Skip to content
This repository was archived by the owner on May 29, 2020. It is now read-only.

Added a filter so users can not enable auto updates via action if plugin has disabled auto-updates. #10

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

seb86
Copy link

@seb86 seb86 commented Feb 26, 2020

This PR allows plugins to remove the option for users to enable auto updates for a specific plugin on the plugins table.

Since the table columns can not be filtered and only allows you to add content, this allows the action to have a condition for a specific plugin.

Example of use in a plugin

function disable_auto_updates_action( $status, $plugin_file ) {
	if ( $plugin_file === 'cocart-pro/cocart-pro.php' ) {
		return false;
	}

	return $status;
}
add_filter( 'wp_plugin_allows_auto_update', 'disable_auto_updates_action', 10, 2 );

@audrasjb
Copy link
Owner

Hi, thank you!

This feature already exists today in WordPress Core and already works fine with this Feature Plugin.

For example, you can use:

function auto_update_specific_plugins ( $update, $item ) {
    // Array of plugin slugs that must NOT auto-update
    $plugins = array (
        'akismet',
        'buddypress',
    );
    if ( in_array( $item->slug, $plugins ) ) {
         // Do NOT update plugins in this array
        return false;
    } else {
        // Else, use the normal API response to decide whether to update or not
        return $update;
    }
}
add_filter( 'auto_update_plugin', 'auto_update_specific_plugins', 10, 2 );

Cheers,
Jb

@seb86
Copy link
Author

seb86 commented Feb 26, 2020

@audrasjb Yes but it does not prevent the actions in the column to hide. The action is still there even if the plugin allows auto-updates or not. That's the purpose of this filter.

@seb86 seb86 changed the title Added a filter so plugins can decide if auto updates should be an option. Added a filter so users can not enable auto updates via action if plugin has disabled auto-updates. Feb 26, 2020
@seb86
Copy link
Author

seb86 commented Feb 26, 2020

Updated title of issue.

@seb86
Copy link
Author

seb86 commented Feb 26, 2020

@audrasjb It's really a UI filter than actually preventing auto-updates.

@audrasjb
Copy link
Owner

Ah ok. Thank you for clarifying.
Will review which approach is the best in our case, thanks!

@audrasjb audrasjb self-requested a review February 26, 2020 17:15
@audrasjb audrasjb self-assigned this Feb 26, 2020
@@ -9,7 +9,7 @@
Tested up to: 5.3
Author: The WordPress Team
Author URI: https://wordpress.org
Contributors: wordpressdotorg, audrasjb, whodunitagency, desrosj, xkon, karmatosed
Contributors: wordpressdotorg, audrasjb, whodunitagency, desrosj, xkon, karmatosed, sebd86
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don’t directly add your name to the contributors on the plugin header.
You can add your name on the readme.md file and I will take care your name is added to the props list of WordPress 5.5, but the plugin’s list of contributors is updated manually.

Thanks for your comprehension :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood @audrasjb

@seb86
Copy link
Author

seb86 commented Mar 11, 2020

@audrasjb Pull request updated.

@pbiron
Copy link
Collaborator

pbiron commented Mar 13, 2020

@seb86 thanx for raising this.

It makes me wonder whether the UI (i.e., the presence of the enable/disable links) should be keyed on the return value from core's auto_update_plugin.

That is, something like:

// remove the callback we hook to the filter.
remove_filter( 'auto_update_plugin', 'wp_autoupdates_selected_plugins' );

// note: in a real implementation, the 2nd param to the filter would have
//       to have all fields that `WP_Automatic_Updater` would pass in the object.
$auto_update = apply_filters( 'auto_update_plugin', null, (object) array( 'plugin' => $plugin_file ) );
if ( null === $auto_update ) {
	// output the enable/disabled links as currently done.
}
elseif ( $auto_update ) {
	printf( __( 'Auto-updates always enabled for this plugin.', 'wp-autoupdates' ) );
}
else {
	printf( __( 'Auto-updates always disbled for this plugin.', 'wp-autoupdates' ) );
}

// add the callback again.
add_filter( 'auto_update_plugin', 'wp_autoupdates_selected_plugins', 10, 2 );

@pbiron
Copy link
Collaborator

pbiron commented Mar 13, 2020

Related: #16

@seb86 seb86 requested a review from audrasjb March 14, 2020 15:56
@seb86
Copy link
Author

seb86 commented Mar 14, 2020

@seb86 thanx for raising this.

It makes me wonder whether the UI (i.e., the presence of the enable/disable links) should be keyed on the return value from core's auto_update_plugin.

That is, something like:

// remove the callback we hook to the filter.
remove_filter( 'auto_update_plugin', 'wp_autoupdates_selected_plugins' );

// note: in a real implementation, the 2nd param to the filter would have
//       to have all fields that `WP_Automatic_Updater` would pass in the object.
$auto_update = apply_filters( 'auto_update_plugin', null, (object) array( 'plugin' => $plugin_file ) );
if ( null === $auto_update ) {
	// output the enable/disabled links as currently done.
}
elseif ( $auto_update ) {
	printf( __( 'Auto-updates always enabled for this plugin.', 'wp-autoupdates' ) );
}
else {
	printf( __( 'Auto-updates always disbled for this plugin.', 'wp-autoupdates' ) );
}

// add the callback again.
add_filter( 'auto_update_plugin', 'wp_autoupdates_selected_plugins', 10, 2 );

I did at first think about this but then plugins that sell like me would require requirements to be filled such:

  • An active licence or
  • Need to check if a specific plugin it supports i.e. WooCommerce has been tested up to specific version.

@pbiron
Copy link
Collaborator

pbiron commented Mar 16, 2020

I did at first think about this but then plugins that sell like me would require requirements to be filled such:

  • An active licence or
  • Need to check if a specific plugin it supports i.e. WooCommerce has been tested up to specific version.

Can you expand on the above please? I'm not sure I understand what you're saying, since the example code you have above doesn't test either of those things. If you'd need to test them in a callback hooked to auto_update_plugin wouldn't you also need to test them in a callback of the UI-only hook this PR adds?

@seb86
Copy link
Author

seb86 commented Mar 16, 2020

For me personally, I don't want the user option to enable or disable plugin updates automatically which is why with the example above, I then hook into auto_update_plugin to run my check.

So when my plugin has an update available, it fetches the data and passes the parameters I require to check against.

If the requirement is good then I enable the plugin to auto update.

/**
 * Enable auto updates for CoCart Pro if latest release supports 
 * the current installed version of WooCommerce.
 *
 * @access public
 * @param  bool   $should_update Should this plugin auto update.
 * @param  object $plugin Plugin being checked.
 * @return bool   $should_update Returns the new status if plugin should auto update.
 */
public function auto_update_plugin( $should_update, $plugin ) {
	if ( ! isset( $plugin->slug ) ) {
		return $should_update;
	}

	// If the plugin is not CoCart Pro then just return original status.
	if ( $this->config['file'] !== $plugin->plugin ) {
		return $should_update;
	}

	/**
	 * Check to see if the current installed WooCommerce version 
	 * is less than a version required or more than a tested up to.
	 */
	if (
		version_compare( WC_VERSION, $this->config['wc_requires'], '<' ) ||
		version_compare( WC_VERSION, $this->config['wc_tested_up_to'], '>' )
	) {
		return false;
	}

	return $should_update;
} // END auto_update_plugin()

Hope that helps.

@pbiron
Copy link
Collaborator

pbiron commented Mar 16, 2020

That's what I thought you meant. And that's why in the sample code I added the comment:

// note: in a real implementation, the 2nd param to the filter would have
// to have all fields that WP_Automatic_Updater would pass in the object.

so that when the existing core filter were applied to decide whether to display the UI you'd have all the info you'd get when it is applied by WP_Automatic_Updater.

So, in the case where your callback returns false, the UI would display "Auto-updates always disbled for this plugin.". You'd just need to change your callback to explicitly return true when it should auto-update (instead of assuming that it gets passed true, which I think you'd want to do anyway)...in which case the UI would display "Auto-updates always enabled for this plugin.".

Note that I'm not completely sold on the idea of using the existing core filter to decide whether to display the UI, I'm just trying to figure out how that would be any different than a new filter whose only purpose were to control whether the UI displays or not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants