phplist3 icon indicating copy to clipboard operation
phplist3 copied to clipboard

Allow CLI enable/disable of plugins

Open hedrickbt opened this issue 4 years ago • 5 comments

In order to support enabling/disabling plugins via CLI for use cases including containers.

Usage:

enable a plugin /usr/bin/phplist -p pluginenable -n CommonPlugin -e true

disable a plugin /usr/bin/phplist -p pluginenable -n CommonPlugin -e false

A non-0 exit code is returned if it fails to perform the requested action.

Description

I keep receiving the "You can't comment at this time." message from github. I have logged out/in and turned off ad blocker - to no avail. So here are my responses

@bramley, I have tested numerous times both enabling and disabling and it has worked without issue for plugins with and without dependencies. I have tested both via the CLI inside the container and via the docker-entrypoint.sh script called as the container starts.

Are there some docs on these? I didn't see anything in /public_html/lists/config/config_extended.php nor in Google. $plugins_autoenable = ['CommonPlugin']; $plugins_disabled = ['SegmentPlugin'];

I did find the relevant code in /public_html/lists/admin/pluginlib.php so I am testing this now.

$plugins_autoenable=[]; DOES solve my issue - enabling plugins in a container by default. It works great. Maybe getting this in the config_extended.php would help others.

Should I create another pull request for changes to config_extended.php?

Related Issue

Screenshots (if appropriate):

hedrickbt avatar Sep 13 '21 21:09 hedrickbt

That's a great contribution, and definitely useful in container context.

I'll review and will request a review from @bramley as well

michield avatar Sep 14 '21 20:09 michield

This looks to be incomplete. Somewhere there is a list of pages that can be called from the command line, so the new page would need to be added to that. Also, the disable processing won't work due to referring to a global variable $plugins without identifying it as a global. @hedrickbt How much testing of this have you done?

A large chunk of this seems to be a modified copy of actions/plugins.php which may create a maintenance problem in the future.

If the intended use is to get a container installation into a known state, I wonder whether it can be simpler by being less sophisticated. Maybe just support enabling a plugin by setting the entry in $plugins and not worrying about dependencies.

Or even specify the plugins to be enabled or disabled in config.php using these variables

$plugins_autoenable = ['CommonPlugin'];
$plugins_disabled = ['SegmentPlugin'];

Rather than true/false I think enable/disable is clearer

usr/bin/phplist -p pluginenable -n CommonPlugin -e enable

bramley avatar Sep 15 '21 03:09 bramley

@bramley, I have tested numerous times both enabling and disabling and it has worked without issue for plugins with and without dependencies. I have tested both via the CLI inside the container and via the docker-entrypoint.sh script called as the container starts.

Are there some docs on these? I didn't see anything in /public_html/lists/config/config_extended.php nor in Google. $plugins_autoenable = ['CommonPlugin']; $plugins_disabled = ['SegmentPlugin'];

I did find the relevant code in /public_html/lists/admin/pluginlib.php so I am testing this now.

$plugins_autoenable=[]; DOES solve my issue - enabling plugins in a container by default. It works great. Maybe getting this in the config_extended.php would help others.

Should I create another pull request for changes to config_extended.php?

hedrickbt avatar Sep 21 '21 13:09 hedrickbt

@hedrickbt Apologies, I had intended to reply sooner. I have tested the change and the two points I made are valid You must have added an entry for the new page to this variable in connect.php. Without that change phplist rejects trying to run the page from the command line

$commandline_pages = array(
    'dbcheck',
    'send',
Error: pluginenable does not process commandline

Also, the function change_plugin_status() references variables $plugins and $disabled_plugins without identifying them as global. With error reporting enabled, php reports undefined variables

PHP Notice: Undefined variable: plugins in /home/duncan/www/lists_3.6.4/admin/pluginenable.php on line 16 The function needs to identify the two variables as global before they are used

global $plugins, $disabled_plugins;

So when a plugin is disabled the code to disable any plugins dependent on it does not happen. But that turns out not to be a problem because there is some similar processing on each page load which will do the same thing. So if you simply look at the Manage Plugins page after disabling a plugin with the new page then it will be shown as disabled anyway.

The processing for disabling could be simplified to remove the unnecessary code in actions/plugins.php and here, but maybe that can be changed later.

After those two changes the new page seemed to work well. Possibly the reporting might be more specific. Whether trying to enable a plugin that is already enabled should be reported as success or failure. I don't have a use for this so don't know which is most useful.

I knew about the $plugins_disabled and $plugins_autoenabled only by seeing them in the code. But $plugins_disabled is mentioned in the old documentation https://resources.phplist.com/system/config/variables

bramley avatar Sep 21 '21 16:09 bramley

@bramley

I can't speak as to why it works fine for me as is - without having to change the commandline_pages variable, but it does. That aside, there is no reason for my pull request given the config settings you mentioned.

$plugins_autoenable=[]; DOES solve my issue - enabling plugins in a container by default. It works great. Maybe getting this in the config_extended.php would help others.

Should I create another pull request for changes to config_extended.php?

hedrickbt avatar Sep 21 '21 18:09 hedrickbt

Due to #901 Github closed this PR. Please re-open it against the main branch.

michield avatar Oct 23 '22 19:10 michield