From 493292b36c1513a90415d144ec72c10e36ff7968 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Mon, 17 Jun 2019 08:07:03 -0300
Subject: [PATCH] Bug 21073: (QA follow-up) Simplify logic

This patch simplifies the logic inside GetPlugins so:
- It uses Koha::Plugins::Methods instead of plain SQL
- It doesn't do more DB calls than needed, by filtering on method in the
  initial query to Koha::Plugins::Methods.

It also relies on the (newly introduced) ->is_enabled method in
Koha::Plugins::Base, for better readability.

To test:
- Run the tests and notice no behaviour changes are introduced.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/Plugins.pm | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm
index be1d091631..9dc42c5155 100644
--- a/Koha/Plugins.pm
+++ b/Koha/Plugins.pm
@@ -70,33 +70,33 @@ If you pass multiple keys in the metadata hash, all keys must match.
 
 sub GetPlugins {
     my ( $self, $params ) = @_;
-    my $method = $params->{method};
+
+    my $method       = $params->{method};
     my $req_metadata = $params->{metadata} // {};
 
-    my $dbh = C4::Context->dbh;
-    my $plugin_classes = $dbh->selectcol_arrayref('SELECT DISTINCT(plugin_class) FROM plugin_methods');
+    my $filter = ( $method ) ? { plugin_method => $method } : undef;
+
+    my $plugin_classes = Koha::Plugins::Methods->search(
+        $filter,
+        {   columns  => 'plugin_class',
+            distinct => 1
+        }
+    )->_resultset->get_column('plugin_class');
+
     my @plugins;
 
     # Loop through all plugins that implement at least a method
-    foreach my $plugin_class (@$plugin_classes) {
-        # filter the plugin out by method
-        next
-            if $method
-            && !Koha::Plugins::Methods->search(
-            { plugin_class => $plugin_class, plugin_method => $method } )->count;
+    while ( my $plugin_class = $plugin_classes->next ) {
 
         load $plugin_class;
-        my $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} });
+        my $plugin = $plugin_class->new({
+            enable_plugins => $self->{'enable_plugins'}
+                # loads even if plugins are disabled
+                # FIXME: is this for testing without bothering to mock config?
+        });
 
-        my $plugin_enabled = $plugin->retrieve_data('__ENABLED__');
-        $plugin->{enabled} = $plugin_enabled;
-
-        # Want all plugins. Not only enabled ones.
-        if ( defined($params->{all}) && $params->{all} ) {
-            $plugin_enabled = 1;
-        }
-
-        next unless $plugin_enabled;
+        next unless $plugin->is_enabled or
+                    defined($params->{all}) && $params->{all};
 
         # filter the plugin out by metadata
         my $plugin_metadata = $plugin->get_metadata;
@@ -120,6 +120,9 @@ This method iterates through all plugins physically present on a system.
 For each plugin module found, it will test that the plugin can be loaded,
 and if it can, will store its available methods in the plugin_methods table.
 
+NOTE: We re-load all plugins here as a protective measure in case someone
+has removed a plugin directly from the system without using the UI
+
 =cut
 
 sub InstallPlugins {
-- 
2.39.5