Attachment 57904 Details for Bug 17678 – Bug 17678: C4::Circulation - Replace GetIssues with Koha::Issues

[patch] Bug 17678: C4::Circulation - Replace GetIssues with Koha::Issues

Bug-17678-C4Circulation---Replace-GetIssues-with-K.patch (text/plain), 5.56 KB, created by Marcel de Rooy on 2016-12-02 09:10:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2016-12-02 09:10:24 UTC

Size: 5.56 KB

patch

obsolete

>From 9cf10e36be743832d556f1502f02525710a39e76 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
>Content-Type: text/plain; charset=utf-8
>
>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>
>---
> C4/Circulation.pm                                  | 83 +++-------------------
> .../{GetIssues.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 b505031..cf0c3cf 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1064,14 +1064,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;
>             }
>         }
>@@ -2506,73 +2510,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 f271414..30aadcb 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.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17678: 57767 | 57768 | 57777 | 57778 | 57903 | 57904 | 57905 | 58294