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