Bug 19081 - Plack preventing uninstalled plugins from being removed on the plugins list
Summary: Plack preventing uninstalled plugins from being removed on the plugins list
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Nick Clemens
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-10 18:49 UTC by Lee Jamison
Modified: 2017-10-14 11:56 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19081: Do not list plugins that have been uninstalled (2.07 KB, patch)
2017-08-28 17:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19081: Do not list plugins that have been uninstalled (2.12 KB, patch)
2017-08-28 20:39 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 19081: Remove useless $plugin_file variable (796 bytes, patch)
2017-09-01 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19081: Do not list plugins that have been uninstalled (2.17 KB, patch)
2017-09-01 13:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19081: Remove useless $plugin_file variable (845 bytes, patch)
2017-09-01 13:18 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lee Jamison 2017-08-10 18:49:38 UTC
When uninstalling a plugin with plack enabled, the plugin remains in the list after uninstall has been selected. It only disappears from the list after restarting plack.
Comment 1 Lee Jamison 2017-08-10 18:51:22 UTC
Assigning to Jonathan as instructed.
Comment 2 Lee Jamison 2017-08-11 16:57:28 UTC
Per an email conversation with Kyle Hall, the following is what appears to be the interaction of plack with plugins:

1) Plugin is installed, install() is run ( install is run anytime a plugin is loaded and the data key  "__INSTALLED__" is not set in plugin_data for the plugin
2) Plugin is removed, uninstall is called, which deletes the plugin and tables
3) Page is reloaded, but the plugin is still in memory, install() is called again and tables are recreated, even though the files have been deleted!
Comment 3 Jonathan Druart 2017-08-28 17:21:01 UTC
Created attachment 66561 [details] [review]
Bug 19081: Do not list plugins that have been uninstalled

Under plack, can_load should not check if a package is in cache, but
reload it. Otherwise plugins that have been uninstalled will still get
listed.
The error raised by can_load must only be displayed if the plugin has
been removed.

Test plan:
1/ Upload a plugin
2/ Note the plugin is listed as installed
3/ Modify the package of the plugin to add a compilation error (use
'Foo' for instance)
4/ Reload the page
5/ The plugin is not listed and a warning appear in the logs
6/ Remove the compilation error and uninstall the plugin
7/ The plugin is no longer listed and no warning appear in the logs
Comment 4 Lee Jamison 2017-08-28 20:39:58 UTC
Created attachment 66566 [details] [review]
Bug 19081: Do not list plugins that have been uninstalled

Under plack, can_load should not check if a package is in cache, but
reload it. Otherwise plugins that have been uninstalled will still get
listed.
The error raised by can_load must only be displayed if the plugin has
been removed.

Test plan:
1/ Upload a plugin
2/ Note the plugin is listed as installed
3/ Modify the package of the plugin to add a compilation error (use
'Foo' for instance)
4/ Reload the page
5/ The plugin is not listed and a warning appear in the logs
6/ Remove the compilation error and uninstall the plugin
7/ The plugin is no longer listed and no warning appear in the logs

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 5 Nick Clemens 2017-09-01 12:10:48 UTC
What is the use of $plugin_file?
Comment 6 Jonathan Druart 2017-09-01 12:54:56 UTC
Created attachment 66728 [details] [review]
Bug 19081: Remove useless $plugin_file variable
Comment 7 Jonathan Druart 2017-09-01 12:55:28 UTC
(In reply to Nick Clemens from comment #5)
> What is the use of $plugin_file?

none, it was a previous try.
I will squash the patches later.
Comment 8 Kyle M Hall 2017-09-01 13:18:08 UTC
Created attachment 66736 [details] [review]
Bug 19081: Do not list plugins that have been uninstalled

Under plack, can_load should not check if a package is in cache, but
reload it. Otherwise plugins that have been uninstalled will still get
listed.
The error raised by can_load must only be displayed if the plugin has
been removed.

Test plan:
1/ Upload a plugin
2/ Note the plugin is listed as installed
3/ Modify the package of the plugin to add a compilation error (use
'Foo' for instance)
4/ Reload the page
5/ The plugin is not listed and a warning appear in the logs
6/ Remove the compilation error and uninstall the plugin
7/ The plugin is no longer listed and no warning appear in the logs

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall 2017-09-01 13:18:14 UTC
Created attachment 66737 [details] [review]
Bug 19081: Remove useless $plugin_file variable

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Jonathan Druart 2017-09-01 16:08:36 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 11 Fridolin SOMERS 2017-10-03 10:18:23 UTC
I don't see "Remove useless $plugin_file variable" in current master :o
Comment 12 Jonathan Druart 2017-10-03 13:24:46 UTC
(In reply to Fridolin SOMERS from comment #11)
> I don't see "Remove useless $plugin_file variable" in current master :o

Patches have been squashed.
Comment 13 Fridolin SOMERS 2017-10-10 08:36:55 UTC
Pushed to 17.05.x, will be in 17.05.05.
Comment 14 Katrin Fischer 2017-10-14 11:56:47 UTC
This patch has been pushed to 16.11.x and will be in 16.11.13.