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>
This commit is contained in:
Jonathan Druart 2016-11-25 11:12:20 +01:00 committed by Kyle M Hall
parent 3de12fdeed
commit 3a41dffaf3
2 changed files with 11 additions and 95 deletions

View File

@ -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);

View File

@ -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" );