From 9cf6d292b23b9613145df12cb471fb3bb588ee97 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Thu, 14 Dec 2023 08:02:01 +0000
Subject: [PATCH] Bug 35536: Add RemovePlugins calls in plugin unit tests
Content-Type: text/plain; charset=utf-8

[1] Replace Methods->delete by RemovePlugins.
    git grep -l "Plugins::Methods->delete" | xargs sed -i -e's/Plugins::Methods->delete/Plugins->RemovePlugins/g'
[2] Replace $schema->resultset('PluginData')->delete by destructive parameter.
[3] Add RemovePlugins too in Handler->delete too. Note that this call
    might be better off with disable? Added a comment.

Test plan:
prove $(git grep -l Koha::Plugin | grep -P "^t\/db")

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 Koha/Plugins/Handler.pm                       |  9 ++++++---
 t/db_dependent/Koha/Plugins/Account_hooks.t   |  2 +-
 t/db_dependent/Koha/Plugins/BackgroundJob.t   |  2 +-
 .../Koha/Plugins/Barcode_transform_hooks.t    |  2 +-
 .../Plugins/Biblio_and_Items_plugin_hooks.t   |  2 +-
 .../Koha/Plugins/Circulation_hooks.t          |  2 +-
 t/db_dependent/Koha/Plugins/Holds_hooks.t     |  6 +++---
 t/db_dependent/Koha/Plugins/Patron.t          |  2 +-
 t/db_dependent/Koha/Plugins/Plugins.t         | 19 +++++++------------
 t/db_dependent/Koha/Plugins/Recall_hooks.t    |  3 +--
 t/db_dependent/Koha/Plugins/authority_hooks.t |  2 +-
 .../Koha/REST/Plugin/PluginRoutes.t           |  2 +-
 12 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/Koha/Plugins/Handler.pm b/Koha/Plugins/Handler.pm
index a32487d4ad..b1a3831a8d 100644
--- a/Koha/Plugins/Handler.pm
+++ b/Koha/Plugins/Handler.pm
@@ -25,6 +25,7 @@ use File::Path qw( remove_tree );
 use Module::Load qw( load );
 
 use C4::Context;
+use Koha::Plugins;
 use Koha::Plugins::Methods;
 
 BEGIN {
@@ -76,7 +77,9 @@ sub run {
 
 =item delete
 
-Deletes a plugin
+Deletes a plugin.
+
+Called in plugins/plugins-uninstall.pl.
 
 =cut
 
@@ -101,8 +104,8 @@ sub delete {
         enable_plugins => $args->{enable_plugins},
     });
 
-    C4::Context->dbh->do( "DELETE FROM plugin_data WHERE plugin_class = ?", undef, ($plugin_class) );
-    Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete();
+    # TODO Would next call be a candidate for replacing destructive by disable ?
+    Koha::Plugins->RemovePlugins( { plugin_class => $plugin_class, destructive => 1 } );
 
     unlink "$plugin_path.pm" or warn "Could not unlink $plugin_path.pm: $!";
     remove_tree($plugin_path);
diff --git a/t/db_dependent/Koha/Plugins/Account_hooks.t b/t/db_dependent/Koha/Plugins/Account_hooks.t
index 3d27dc1be8..4eade79abc 100755
--- a/t/db_dependent/Koha/Plugins/Account_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Account_hooks.t
@@ -69,6 +69,6 @@ subtest 'Koha::Account tests' => sub {
     qr/after_account_action called with action: add_credit, type: payment, ref: Koha::Account::Line/,
       '->add_credit calls the after_account_action hook with type payment';
 
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
     $schema->storage->txn_rollback;
 };
diff --git a/t/db_dependent/Koha/Plugins/BackgroundJob.t b/t/db_dependent/Koha/Plugins/BackgroundJob.t
index 26139ddeb4..a4c8e58e29 100755
--- a/t/db_dependent/Koha/Plugins/BackgroundJob.t
+++ b/t/db_dependent/Koha/Plugins/BackgroundJob.t
@@ -85,5 +85,5 @@ subtest 'background_tasks() hooks tests' => sub {
     is_deeply( $tasks, $bj->core_types_to_classes, 'Only core mapping returned when plugins disabled' );
 
     $schema->storage->txn_rollback;
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
 };
diff --git a/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t b/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t
index ff990be9f1..202f1bfdd9 100755
--- a/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t
@@ -100,5 +100,5 @@ subtest 'patron_barcode_transform() and item_barcode_transform() hook tests' =>
         'Koha::Item->store calls the item_barcode_transform hook';
 
     $schema->storage->txn_rollback;
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
 };
diff --git a/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t b/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t
index 4f29aae60f..1ca1d5116e 100755
--- a/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t
@@ -84,5 +84,5 @@ subtest 'after_biblio_action() and after_item_action() hooks tests' => sub {
             'DelBiblio calls the hook with action=delete biblio_id passed';
 
     $schema->storage->txn_rollback;
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
 };
diff --git a/t/db_dependent/Koha/Plugins/Circulation_hooks.t b/t/db_dependent/Koha/Plugins/Circulation_hooks.t
index 89587f5de7..02911a01c5 100755
--- a/t/db_dependent/Koha/Plugins/Circulation_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Circulation_hooks.t
@@ -121,5 +121,5 @@ subtest 'after_circ_action() hook tests' => sub {
     };
 
     $schema->storage->txn_rollback;
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
 };
diff --git a/t/db_dependent/Koha/Plugins/Holds_hooks.t b/t/db_dependent/Koha/Plugins/Holds_hooks.t
index d80d73c489..90d1df0c28 100755
--- a/t/db_dependent/Koha/Plugins/Holds_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Holds_hooks.t
@@ -80,7 +80,7 @@ subtest 'after_hold_create() hook tests' => sub {
           qr/after_hold_create called with parameter Koha::Hold/ ],
           'AddReserve calls the after_hold_create hook, deprecation warning found';
 
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
     $schema->storage->txn_rollback;
 };
 
@@ -124,7 +124,7 @@ subtest 'after_hold_action (placed) hook tests' => sub {
     }
     [ qr/after_hold_create is deprecated and will be removed soon/, qr/after_hold_action called with action: place, ref: Koha::Hold/,  ], 'AddReserve calls the after_hold_action hook';
 
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
     $schema->storage->txn_rollback;
 };
 
@@ -205,6 +205,6 @@ subtest 'Koha::Hold tests' => sub {
     qr/after_hold_action called with action: cancel, ref: Koha::Old::Hold/,
       '->cancel calls the after_hold_action hook';
 
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
     $schema->storage->txn_rollback;
 };
diff --git a/t/db_dependent/Koha/Plugins/Patron.t b/t/db_dependent/Koha/Plugins/Patron.t
index a818cdeb45..363a2fa0e3 100755
--- a/t/db_dependent/Koha/Plugins/Patron.t
+++ b/t/db_dependent/Koha/Plugins/Patron.t
@@ -88,7 +88,7 @@ subtest 'check_password hook tests' => sub {
     ok( $patron->set_password({ password => 'explosion', skip_validation => 1}), 'Patron password updated via skip validation');
 
     $schema->storage->txn_rollback;
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
 };
 
 1;
diff --git a/t/db_dependent/Koha/Plugins/Plugins.t b/t/db_dependent/Koha/Plugins/Plugins.t
index 3037ed801e..3838d46cbe 100755
--- a/t/db_dependent/Koha/Plugins/Plugins.t
+++ b/t/db_dependent/Koha/Plugins/Plugins.t
@@ -57,8 +57,7 @@ subtest 'call() tests' => sub {
 
     $schema->storage->txn_begin;
     # Temporarily remove any installed plugins data
-    Koha::Plugins::Methods->delete;
-    $schema->resultset('PluginData')->delete();
+    Koha::Plugins->RemovePlugins( { destructive => 1 } );
 
     t::lib::Mocks::mock_config('enable_plugins', 1);
     my $plugins = Koha::Plugins->new({ enable_plugins => 1 });
@@ -95,8 +94,7 @@ subtest 'more call() tests' => sub {
 
     $schema->storage->txn_begin;
     # Temporarily remove any installed plugins data
-    Koha::Plugins::Methods->delete;
-    $schema->resultset('PluginData')->delete();
+    Koha::Plugins->RemovePlugins( { destructive => 1 } );
 
     t::lib::Mocks::mock_config('enable_plugins', 1);
     my $plugins = Koha::Plugins->new({ enable_plugins => 1 });
@@ -138,8 +136,7 @@ subtest 'feature_enabled tests' => sub {
     $schema->storage->txn_begin;
 
     # Temporarily remove any installed plugins data
-    Koha::Plugins::Methods->delete;
-    $schema->resultset('PluginData')->delete();
+    Koha::Plugins->RemovePlugins( { destructive => 1 } );
 
     t::lib::Mocks::mock_config( 'enable_plugins', 0 );
     my $enabled = Koha::Plugins->feature_enabled('check_password');
@@ -169,8 +166,8 @@ subtest 'GetPlugins() tests' => sub {
     plan tests => 3;
 
     $schema->storage->txn_begin;
-    # Temporarily remove any installed plugins data
-    Koha::Plugins::Methods->delete;
+    # Temporarily remove any installed plugins data (FIXME not done)
+    Koha::Plugins->RemovePlugins;
 
     my $plugins = Koha::Plugins->new({ enable_plugins => 1 });
 
@@ -215,8 +212,7 @@ subtest 'is_enabled() tests' => sub {
     $schema->storage->txn_begin;
 
     # Make sure there's no previous installs or leftovers on DB
-    Koha::Plugins::Methods->delete;
-    $schema->resultset('PluginData')->delete;
+    Koha::Plugins->RemovePlugins( { destructive => 1 } );
 
     my $plugin = Koha::Plugin::Test->new({ enable_plugins => 1, cgi => CGI->new });
     ok( $plugin->is_enabled, 'Plugins enabled by default' );
@@ -234,8 +230,7 @@ subtest 'is_enabled() tests' => sub {
 
 subtest 'Koha::Plugin::Test' => sub {
     $schema->storage->txn_begin;
-    Koha::Plugins::Methods->delete;
-    $schema->resultset('PluginData')->delete;
+    Koha::Plugins->RemovePlugins( { destructive => 1 } );
 
     warning_is { Koha::Plugins->new( { enable_plugins => 1 } )->InstallPlugins(); } undef;
 
diff --git a/t/db_dependent/Koha/Plugins/Recall_hooks.t b/t/db_dependent/Koha/Plugins/Recall_hooks.t
index 680aaa1362..0ab79e0517 100755
--- a/t/db_dependent/Koha/Plugins/Recall_hooks.t
+++ b/t/db_dependent/Koha/Plugins/Recall_hooks.t
@@ -26,7 +26,6 @@ use t::lib::TestBuilder;
 use Koha::Database;
 use C4::Circulation qw();
 use Koha::CirculationRules;
-use Koha::Plugins::Methods;
 use Koha::Recalls;
 
 BEGIN {
@@ -95,6 +94,6 @@ subtest 'after_recall_action hook' => sub {
     qr/after_recall_action called with action: add, ref: Koha::Recall/,
       '->add_recall calls the after_recall_action hook with action add';
 
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
     $schema->storage->txn_rollback;
 };
diff --git a/t/db_dependent/Koha/Plugins/authority_hooks.t b/t/db_dependent/Koha/Plugins/authority_hooks.t
index 96e4b6b562..464fb16262 100755
--- a/t/db_dependent/Koha/Plugins/authority_hooks.t
+++ b/t/db_dependent/Koha/Plugins/authority_hooks.t
@@ -66,5 +66,5 @@ subtest 'after_authority_action hook' => sub {
             'DelAuthority calls the hook with action=delete, id passed';
 
     $schema->storage->txn_rollback;
-    Koha::Plugins::Methods->delete;
+    Koha::Plugins->RemovePlugins;
 };
diff --git a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
index 66746508ed..140b10f93b 100755
--- a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
+++ b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
@@ -234,7 +234,7 @@ subtest 'needs_install use case tests' => sub {
 
     t::lib::Mocks::mock_preference('Version', '3.0.0');
 
-    $schema->resultset('PluginData')->delete;
+    Koha::Plugins->RemovePlugins( { destructive => 1 } );    # FIXME destructive seems not to be needed here
     $plugins->InstallPlugins;
 
     # re-initialize Koha::REST::V1 after mocking
-- 
2.30.2