Bug 14788: Move opac-topissues.pl code into C4::Circulation

Tested with syspref 'AdvancedSearchTypes' set to itemtypes an ccode (one at a time).
No problems found.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 587f211..43b8f93 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -95,6 +95,7 @@ BEGIN {
                &AnonymiseIssueHistory
         &CheckIfIssuedToPatron
         &IsItemIssued
+        GetTopIssues
        );
 
        # subs to deal with returns
@@ -4037,6 +4038,66 @@ sub GetPendingOnSiteCheckouts {
     |, { Slice => {} } );
 }
 
+sub GetTopIssues {
+    my ($params) = @_;
+
+    my ($count, $branch, $itemtype, $ccode, $newness)
+        = @$params{qw(count branch itemtype ccode newness)};
+
+    my $dbh = C4::Context->dbh;
+    my $query = q{
+        SELECT b.biblionumber, b.title, b.author, bi.itemtype, bi.publishercode,
+          bi.place, bi.publicationyear, b.copyrightdate, bi.pages, bi.size,
+          i.ccode, SUM(i.issues) AS count
+        FROM biblio b
+        LEFT JOIN items i ON (i.biblionumber = b.biblionumber)
+        LEFT JOIN biblioitems bi ON (bi.biblionumber = b.biblionumber)
+    };
+
+    my (@where_strs, @where_args);
+
+    if ($branch) {
+        push @where_strs, 'i.homebranch = ?';
+        push @where_args, $branch;
+    }
+    if ($itemtype) {
+        if (C4::Context->preference('item-level_itypes')){
+            push @where_strs, 'i.itype = ?';
+            push @where_args, $itemtype;
+        } else {
+            push @where_strs, 'bi.itemtype = ?';
+            push @where_args, $itemtype;
+        }
+    }
+    if ($ccode) {
+        push @where_strs, 'i.ccode = ?';
+        push @where_args, $ccode;
+    }
+    if ($newness) {
+        push @where_strs, 'TO_DAYS(NOW()) - TO_DAYS(b.datecreated) <= ?';
+        push @where_args, $newness;
+    }
+
+    if (@where_strs) {
+        $query .= 'WHERE ' . join(' AND ', @where_strs);
+    }
+
+    $query .= q{
+        GROUP BY b.biblionumber
+        HAVING count > 0
+        ORDER BY count DESC
+    };
+
+    $count = int($count);
+    if ($count > 0) {
+        $query .= "LIMIT $count";
+    }
+
+    my $rows = $dbh->selectall_arrayref($query, { Slice => {} }, @where_args);
+
+    return @$rows;
+}
+
 __END__
 
 =head1 AUTHOR

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
index 9538e91..acd67a1 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
@@ -1,4 +1,6 @@
 [% USE Koha %]
+[% USE AuthorisedValues %]
+[% USE ItemTypes %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog &rsaquo; Most popular titles</title>
 [% INCLUDE 'doc-head-close.inc' %]
@@ -20,7 +22,7 @@
         <div class="container-fluid">
             <div class="row-fluid">
                 <div class="span2">
-                    [% IF ( results_loop ) %]
+                    [% IF ( results ) %]
                         <div id="usertopissues">
                             [% INCLUDE 'opac-topissues.inc' %]
                             [% IF ( OpacNav || OpacNavBottom ) %]
@@ -38,7 +40,7 @@
                 <div class="span10">
                     <div id="topissues" class="maincontent">
 
-                        [% IF ( results_loop ) %]
+                        [% IF ( results ) %]
                             <table id="topissuest" class="table table-bordered table-striped">
                                 <caption>
                                     The [% limit %] most checked-out
@@ -49,8 +51,8 @@
                                     at
                                     [% branch %]
                                     [% END %]
-                                    [% IF ( timeLimitFinite ) %]
-                                    in the past [% timeLimitFinite |html %] months
+                                    [% IF ( timeLimit != 999 ) %]
+                                    in the past [% timeLimit |html %] months
                                     [% ELSE %] of all time[% END %]
                                 </caption>
                                 <thead>
@@ -62,33 +64,30 @@
                                     </tr>
                                 </thead>
                                 <tbody>
-                                    [% FOREACH results_loo IN results_loop %]
+                                    [% FOREACH result IN results %]
                                         <tr>
-                                            <td><a class="title" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% results_loo.biblionumber %]">[% results_loo.title |html %]</a><p>[% results_loo.author %]
-                                            [% IF ( results_loo.publishercode ) %]- [% results_loo.publishercode %][% END %] [% IF ( results_loo.seriestitle ) %]([% results_loo.seriestitle %])[% END %]
-                                            [% IF ( results_loo.place ) %][% results_loo.place %][% END %]
-                                            [% IF ( results_loo.publicationyear ) %]
-                                                [% results_loo.publicationyear %]
-                                            [% ELSIF ( results_loo.copyrightdate ) %]
-                                                [% results_loo.copyrightdate %]
+                                            <td><a class="title" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% result.biblionumber %]">[% result.title |html %]</a><p>[% result.author %]
+                                            [% IF ( result.publishercode ) %]- [% result.publishercode %][% END %]
+                                            [% IF ( result.place ) %][% result.place %][% END %]
+                                            [% IF ( result.publicationyear ) %]
+                                                [% result.publicationyear %]
+                                            [% ELSIF ( result.copyrightdate ) %]
+                                                [% result.copyrightdate %]
                                             [% END %]
-                                            [% IF ( results_loo.pages ) %] - [% results_loo.pages %][% END %]
-                                            [% IF ( results_loo.item('size') ) %][% results_loo.item('size') %][% END %]</p>
+                                            [% IF ( result.pages ) %] - [% result.pages %][% END %]
+                                            [% IF ( result.item('size') ) %][% result.item('size') %][% END %]</p>
                                             </td>
                                             <td>
-                                                [% IF ( results_loo.description ) %]
-                                                    <span class="tdlabel">
-                                                        [% IF ( ccodesearch ) %]
-                                                            Collection
-                                                        [% ELSE %]
-                                                            Item type
-                                                        [% END %]:
-                                                    </span>
-                                                    [% results_loo.description %]
-                                                [% END %]
+                                              [% IF Koha.Preference('AdvancedSearchTypes') == 'ccode' %]
+                                                <span class="tdlabel">Collection</span>
+                                                [% AuthorisedValues.GetByCode('ccode', result.ccode, 1) %]
+                                              [% ELSE %]
+                                                <span class="tdlabel">Item type</span>
+                                                [% ItemTypes.GetDescription(result.itemtype) %]
+                                              [% END %]
                                             </td>
-                                            <td><span class="tdlabel">Checkouts: </span> <span title="[% results_loo.tot %]">[% results_loo.tot %]</span></td>
-                                            [% IF Koha.Preference( 'opacuserlogin' ) == 1 %]<td>[% IF Koha.Preference( 'RequestOnOpac' ) == 1 %][% UNLESS ( results_loo.norequests ) %]<a href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% results_loo.biblionumber %]">Place hold</a>[% END %][% END %]</td>[% END %]
+                                            <td><span class="tdlabel">Checkouts: </span> <span title="[% result.count %]">[% result.count %]</span></td>
+                                            [% IF Koha.Preference( 'opacuserlogin' ) == 1 %]<td>[% IF Koha.Preference( 'RequestOnOpac' ) == 1 %][% UNLESS ( result.norequests ) %]<a href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% result.biblionumber %]">Place hold</a>[% END %][% END %]</td>[% END %]
                                             </tr>
                                     [% END %]
                                 </tbody>
@@ -152,7 +151,7 @@
                                     <input type="submit" class="btn" value="Submit" />
                                 </fieldset>
                             </form>
-                       [% END # / IF results_loop %]
+                       [% END # / IF results %]
                     </div> <!-- / #topissues -->
                 </div> <!-- / .span10 -->
             </div> <!-- / .row-fluid -->

diff --git a/opac/opac-topissues.pl b/opac/opac-topissues.pl
index c88cb17..772ec03 100755 (executable)
--- a/opac/opac-topissues.pl
+++ b/opac/opac-topissues.pl
@@ -29,6 +29,7 @@ use C4::Search;
 use C4::Output;
 use C4::Koha;
 use C4::Branch;
+use C4::Circulation;
 use Date::Manip;
 
 =head1 NAME
@@ -73,75 +74,29 @@ my $itemtype = $input->param('itemtype') || '';
 my $timeLimit = $input->param('timeLimit') || 3;
 my $advanced_search_types = C4::Context->preference('AdvancedSearchTypes');
 
-my $whereclause = '';
-$whereclause .= ' AND items.homebranch='.$dbh->quote($branch) if ($branch);
-$whereclause .= ' AND TO_DAYS(NOW()) - TO_DAYS(biblio.datecreated) <= '.($timeLimit*30) if $timeLimit < 999;
-$whereclause =~ s/ AND $// if $whereclause;
-my $query;
+
+my $params = {
+    count => $limit,
+    branch => $branch,
+    newness => $timeLimit < 999 ? $timeLimit * 30 : undef,
+};
 
 if($advanced_search_types eq 'ccode'){
-    $whereclause .= ' AND authorised_values.authorised_value='.$dbh->quote($itemtype) if $itemtype;
-    $query = "SELECT datecreated, biblio.biblionumber, title,
-                    author, sum( items.issues ) AS tot, biblioitems.itemtype,
-                    biblioitems.publishercode, biblioitems.place, biblioitems.publicationyear, biblio.copyrightdate,
-                    authorised_values.lib as description, biblioitems.pages, biblioitems.size
-                    FROM biblio
-                    LEFT JOIN items USING (biblionumber)
-                    LEFT JOIN biblioitems USING (biblionumber)
-                    LEFT JOIN authorised_values ON items.ccode = authorised_values.authorised_value
-                    WHERE 1
-                    $whereclause
-                    AND authorised_values.category = 'ccode' 
-                    GROUP BY biblio.biblionumber
-                    HAVING tot >0
-                    ORDER BY tot DESC
-                    LIMIT ?
-                    ";
+    $params->{ccode} = $itemtype;
     $template->param(ccodesearch => 1);
-}else{
-    if ($itemtype){
-       if (C4::Context->preference('item-level_itypes')){
-           $whereclause .= ' AND items.itype = ' . $dbh->quote($itemtype);
-       }
-       else {
-           $whereclause .= ' AND biblioitems.itemtype='.$dbh->quote($itemtype);
-        }
-    }
-    $query = "SELECT datecreated, biblio.biblionumber, title,
-                    author, sum( items.issues ) AS tot, biblioitems.itemtype,
-                    biblioitems.publishercode, biblioitems.place, biblioitems.publicationyear, biblio.copyrightdate,
-                    itemtypes.description, biblioitems.pages, biblioitems.size
-                    FROM biblio
-                    LEFT JOIN items USING (biblionumber)
-                    LEFT JOIN biblioitems USING (biblionumber)
-                    LEFT JOIN itemtypes ON itemtypes.itemtype = biblioitems.itemtype
-                    WHERE 1
-                    $whereclause
-                    GROUP BY biblio.biblionumber
-                    HAVING tot >0
-                    ORDER BY tot DESC
-                    LIMIT ?
-                    ";
-     $template->param(itemtypesearch => 1);
-}
-
-my $sth = $dbh->prepare($query);
-$sth->execute($limit);
-my @results;
-while (my $line= $sth->fetchrow_hashref) {
-    push @results, $line;
+} else {
+    $params->{itemtype} = $itemtype;
+    $template->param(itemtypesearch => 1);
 }
 
-my $timeLimitFinite = $timeLimit;
-if($timeLimit eq 999){ $timeLimitFinite = 0 };
+my @results = GetTopIssues($params);
 
 $template->param(do_it => 1,
                 limit => $limit,
                 branch => $branches->{$branch}->{branchname},
                 itemtype => $itemtypes->{$itemtype}->{description},
                 timeLimit => $timeLimit,
-                timeLimitFinite => $timeLimitFinite,
-                results_loop => \@results,
+                results => \@results,
                 );
 
 $template->param( branchloop => GetBranchesLoop($branch));