From 3a41dffaf3908f2397bc662e2a9d77cff6521bd0 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 25 Nov 2016 11:12:20 +0100
Subject: [PATCH] Bug 17678: C4::Circulation - Replace GetIssues with
 Koha::Issues

The C4::Circulation::GetIssues subroutine is only called once and can be
replaced with a call to Koha::Isues->search with a join on items.

Test plan:
- Apply first patch and make sure the tests pass
- Apply second patch and make sure the tests still pass

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/Circulation.pm                             | 83 +++----------------
 ...etIssues.t => GetPendingOnSiteCheckouts.t} | 23 +----
 2 files changed, 11 insertions(+), 95 deletions(-)
 rename t/db_dependent/Circulation/{GetIssues.t => GetPendingOnSiteCheckouts.t} (69%)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index d4f3cdb2a2..4325801d09 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1066,14 +1066,18 @@ sub CanBookBeIssued {
         require C4::Serials;
         my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber);
         unless ($is_a_subscription) {
-            my $issues = GetIssues( {
-                borrowernumber => $borrower->{borrowernumber},
-                biblionumber   => $biblionumber,
-            } );
-            my @issues = $issues ? @$issues : ();
+            my $issues = Koha::Issues->search(
+                {
+                    borrowernumber => $borrower->{borrowernumber},
+                    biblionumber   => $biblionumber,
+                },
+                {
+                    join => 'item',
+                }
+            );
             # if we get here, we don't already have a loan on this item,
             # so if there are any loans on this bib, ask for confirmation
-            if (scalar @issues > 0) {
+            if ( $issues->count ) {
                 $needsconfirmation{BIBLIO_ALREADY_ISSUED} = 1;
             }
         }
@@ -2508,73 +2512,6 @@ sub GetOpenIssue {
 
 }
 
-=head2 GetIssues
-
-    $issues = GetIssues({});    # return all issues!
-    $issues = GetIssues({ borrowernumber => $borrowernumber, biblionumber => $biblionumber });
-
-Returns all pending issues that match given criteria.
-Returns a arrayref or undef if an error occurs.
-
-Allowed criteria are:
-
-=over 2
-
-=item * borrowernumber
-
-=item * biblionumber
-
-=item * itemnumber
-
-=back
-
-=cut
-
-sub GetIssues {
-    my ($criteria) = @_;
-
-    # Build filters
-    my @filters;
-    my @allowed = qw(borrowernumber biblionumber itemnumber);
-    foreach (@allowed) {
-        if (defined $criteria->{$_}) {
-            push @filters, {
-                field => $_,
-                value => $criteria->{$_},
-            };
-        }
-    }
-
-    # Do we need to join other tables ?
-    my %join;
-    if (defined $criteria->{biblionumber}) {
-        $join{items} = 1;
-    }
-
-    # Build SQL query
-    my $where = '';
-    if (@filters) {
-        $where = "WHERE " . join(' AND ', map { "$_->{field} = ?" } @filters);
-    }
-    my $query = q{
-        SELECT issues.*
-        FROM issues
-    };
-    if (defined $join{items}) {
-        $query .= q{
-            LEFT JOIN items ON (issues.itemnumber = items.itemnumber)
-        };
-    }
-    $query .= $where;
-
-    # Execute SQL query
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare($query);
-    my $rv = $sth->execute(map { $_->{value} } @filters);
-
-    return $rv ? $sth->fetchall_arrayref({}) : undef;
-}
-
 =head2 GetItemIssues
 
   $issues = &GetItemIssues($itemnumber, $history);
diff --git a/t/db_dependent/Circulation/GetIssues.t b/t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
similarity index 69%
rename from t/db_dependent/Circulation/GetIssues.t
rename to t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
index f271414649..30aadcb160 100644
--- a/t/db_dependent/Circulation/GetIssues.t
+++ b/t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 10;
+use Test::More tests => 2;
 use Test::MockModule;
 use t::lib::TestBuilder;
 
@@ -72,27 +72,6 @@ AddIssue($borrower, '0101');
 AddIssue($borrower, '0203');
 
 # Begin tests...
-my $issues;
-$issues = C4::Circulation::GetIssues({biblionumber => $biblionumber1});
-is(scalar @$issues, 1, "Biblio $biblionumber1 has 1 item issued");
-is($issues->[0]->{itemnumber}, $itemnumber1, "First item of biblio $biblionumber1 is issued");
-
-$issues = C4::Circulation::GetIssues({biblionumber => $biblionumber2});
-is(scalar @$issues, 1, "Biblio $biblionumber2 has 1 item issued");
-is($issues->[0]->{itemnumber}, $itemnumber3, "First item of biblio $biblionumber2 is issued");
-
-$issues = C4::Circulation::GetIssues({borrowernumber => $borrowernumber});
-is(scalar @$issues, 2, "Borrower $borrowernumber checked out 2 items");
-
-$issues = C4::Circulation::GetIssues({borrowernumber => $borrowernumber, biblionumber => $biblionumber1});
-is(scalar @$issues, 1, "One of those is an item from biblio $biblionumber1");
-
-$issues = C4::Circulation::GetIssues({borrowernumber => $borrowernumber, biblionumber => $biblionumber2});
-is(scalar @$issues, 1, "The other is an item from biblio $biblionumber2");
-
-$issues = C4::Circulation::GetIssues({itemnumber => $itemnumber2});
-is(scalar @$issues, 0, "No one has issued the second item of biblio $biblionumber2");
-
 my $onsite_checkouts = GetPendingOnSiteCheckouts;
 is( scalar @$onsite_checkouts, 0, "No pending on-site checkouts" );
 
-- 
2.39.5