From 02ef72f953806269484e165ea311690485c8c4be Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 16 May 2016 17:19:54 +0200 Subject: [PATCH] Bug 16502: Table koha_plugin_com_bywatersolutions_kitchensink_mytable not always dropped after running Plugin.t Content-Type: text/plain; charset=utf-8 If you run Plugin.t, the above table will still be present (when you did not enable UseKohaPlugins). This would trigger a warning when running the test a second time. Why? The uninstall call does its work not completely due to a small inconsistency in Koha::Plugins::Handler::delete when calling run without the enable_plugins parameter. This patch resolves that inconsistency and also removes an unneeded skip in Plugin.t in case the KitchenSink module already exists. Note: This is a small fix. But I wonder if the Handler routines run and delete should not have been implemented in Koha::Plugins::Base. Also note that plugins/plugins-uninstall.pl will not be affacted by this change, since it checks whether the pref is enabled before calling the delete method. Test plan: [1] Do not yet install this patch. [2] Verify that plugins are enabled in koha-conf.xml. [3] Disable UseKohaPlugins in System Preferences! [4] Run t/db_dependent/Plugins.t. [5] Verify that table koha_plugin_com_bywatersolutions_kitchensink_mytable still exists. (It should have been deleted.) Remove it manually. [6] Apply this patch. [7] Run the test again. [8] Verify that the table does not exist. [9] Run the test again (without warnings). --- Koha/Plugins/Handler.pm | 9 ++++++++- t/db_dependent/Plugins.t | 5 ++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Koha/Plugins/Handler.pm b/Koha/Plugins/Handler.pm index 187560d..903b446 100644 --- a/Koha/Plugins/Handler.pm +++ b/Koha/Plugins/Handler.pm @@ -78,11 +78,18 @@ Deletes a plugin sub delete { my ( $class, $args ) = @_; + + return unless ( C4::Context->config("enable_plugins") || $args->{'enable_plugins'} ); + my $plugin_class = $args->{'class'}; my $plugin_dir = C4::Context->config("pluginsdir"); my $plugin_path = "$plugin_dir/" . join( '/', split( '::', $args->{'class'} ) ); - Koha::Plugins::Handler->run( { class => $plugin_class, method => 'uninstall' } ); + Koha::Plugins::Handler->run({ + class => $plugin_class, + method => 'uninstall', + enable_plugins => $args->{enable_plugins}, + }); C4::Context->dbh->do( "DELETE FROM plugin_data WHERE plugin_class = ?", undef, ($plugin_class) ); diff --git a/t/db_dependent/Plugins.t b/t/db_dependent/Plugins.t index 3271b1a..6cff239 100755 --- a/t/db_dependent/Plugins.t +++ b/t/db_dependent/Plugins.t @@ -49,7 +49,7 @@ SKIP: { my $plugins_dir = C4::Context->config("pluginsdir"); skip "plugindir not set", 3 unless defined $plugins_dir; skip "plugindir not writable", 3 unless -w $plugins_dir; - skip "KitchenSink plugin already installed", 3 if (-f "$plugins_dir/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm"); + # no need to skip further tests if KitchenSink would already exist my $ae = Archive::Extract->new( archive => "$Bin/KitchenSinkPlugin.kpz", type => 'zip' ); unless ( $ae->extract( to => $plugins_dir ) ) { @@ -59,7 +59,6 @@ SKIP: { $plugin = Koha::Plugin::Com::ByWaterSolutions::KitchenSink->new({ enable_plugins => 1}); ok( -f $plugins_dir . "/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm", "KitchenSink plugin installed successfully" ); - Koha::Plugins::Handler->delete({ class => "Koha::Plugin::Com::ByWaterSolutions::KitchenSink" }); + Koha::Plugins::Handler->delete({ class => "Koha::Plugin::Com::ByWaterSolutions::KitchenSink", enable_plugins => 1 }); ok( !( -f $plugins_dir . "/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm" ), "Koha::Plugins::Handler::delete works correctly." ); } - -- 1.7.10.4