Attachment 60008 Details for Bug 18079 – Bug 18079: Holds to pull cleanup

[patch] Bug 18079: Holds to pull cleanup

Bug-18079-Holds-to-pull-cleanup.patch (text/plain), 10.97 KB, created by Josef Moravec on 2017-02-08 09:20:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Josef Moravec

Created: 2017-02-08 09:20:59 UTC

Size: 10.97 KB

patch

obsolete

>From 4f9ee85a27cf3a557b53ad1acdb23223b6b1afe8 Mon Sep 17 00:00:00 2001
>From: Josef Moravec <josef.moravec@gmail.com>
>Date: Wed, 8 Feb 2017 08:49:46 +0000
>Subject: [PATCH] Bug 18079: Holds to pull cleanup
>
>Changes made:
>
>  - remove obsolete comment in pendingreserves.pl
>  - use Modern::Perl in circ/pendingreserves.pl
>  - get rid of unusable param run_report - followup for bug 8454
>  - get rid of references to hold status - followup for bug 9320
>  - remove unused data from SQL and reservedata structure
>
>Test plan:
>1) Apply patch from bug 18073
>2) Apply patch on this bug
>3) Enable on shelf hold in administration -> circulation and fines rules
>4) Create some holds on avalaible items
>5) Confirm that circulation -> holds to pull page works as expected
>6) Try to find any regression
>---
> circ/pendingreserves.pl                            | 144 +++++++++------------
> .../prog/en/modules/circ/pendingreserves.tt        |   5 +-
> 2 files changed, 61 insertions(+), 88 deletions(-)
>
>diff --git a/circ/pendingreserves.pl b/circ/pendingreserves.pl
>index f601659..83ca8f3 100755
>--- a/circ/pendingreserves.pl
>+++ b/circ/pendingreserves.pl
>@@ -17,13 +17,7 @@
> # You should have received a copy of the GNU General Public License
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
>-# Modification by D.Ulm, actually works (as long as indep. branches not turned on)
>-#		Someone let me know what indep. branches is supposed to do and I'll make that part work too
>-#
>-# 		The reserve pull lists *works* as long as not for indepencdant branches, I can fix!
>-
>-use strict;
>-#use warnings; FIXME - Bug 2505
>+use Modern::Perl;
> 
> use constant PULL_INTERVAL => 2;
> 
>@@ -37,9 +31,8 @@ use Koha::DateUtils;
> use DateTime::Duration;
> 
> my $input = new CGI;
>-my $startdate=$input->param('from');
>-my $enddate=$input->param('to');
>-my $run_report = ( not defined $input->param('run_report') ) ? 1 : $input->param('run_report');
>+my $startdate = $input->param('from');
>+my $enddate = $input->param('to');
> 
> my $theme = $input->param('theme');    # only used if allowthemeoverride is set
> 
>@@ -55,12 +48,10 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
> );
> 
> my $today = dt_from_string;
>-$startdate =~ s/^\s+//;
>-$startdate =~ s/\s+$//;
>-$enddate =~ s/^\s+//;
>-$enddate =~ s/\s+$//;
> 
> if ( $startdate ) {
>+    $startdate =~ s/^\s+//;
>+    $startdate =~ s/\s+$//;
>     $startdate = eval{dt_from_string( $startdate )};
> }
> unless ( $startdate ){
>@@ -70,6 +61,8 @@ unless ( $startdate ){
> }
> 
> if ( $enddate ) {
>+    $enddate =~ s/^\s+//;
>+    $enddate =~ s/\s+$//;
>     $enddate = eval{dt_from_string( $enddate )};
> }
> unless ( $enddate ) {
>@@ -78,32 +71,31 @@ unless ( $enddate ) {
> }
> 
> my @reservedata;
>-if ( $run_report ) {
>-    my $dbh    = C4::Context->dbh;
>-    my $sqldatewhere = "";
>-    my $startdate_iso = output_pref({ dt => $startdate, dateformat => 'iso', dateonly => 1 });
>-    my $enddate_iso   = output_pref({ dt => $enddate, dateformat => 'iso', dateonly => 1 });
>-    $debug and warn $startdate_iso. "\n" . $enddate_iso;
>-    my @query_params = ();
>-    if ($startdate_iso) {
>-        $sqldatewhere .= " AND reservedate >= ?";
>-        push @query_params, $startdate_iso;
>-    }
>-    if ($enddate_iso) {
>-        $sqldatewhere .= " AND reservedate <= ?";
>-        push @query_params, $enddate_iso;
>-    }
>+my $dbh = C4::Context->dbh;
>+my $sqldatewhere = "";
>+my $startdate_iso = output_pref({ dt => $startdate, dateformat => 'iso', dateonly => 1 });
>+my $enddate_iso   = output_pref({ dt => $enddate, dateformat => 'iso', dateonly => 1 });
> 
>-    my $strsth =
>+$debug and warn $startdate_iso. "\n" . $enddate_iso;
>+
>+my @query_params = ();
>+
>+if ($startdate_iso) {
>+    $sqldatewhere .= " AND reservedate >= ?";
>+    push @query_params, $startdate_iso;
>+}
>+if ($enddate_iso) {
>+    $sqldatewhere .= " AND reservedate <= ?";
>+    push @query_params, $enddate_iso;
>+}
>+
>+my $strsth =
>     "SELECT min(reservedate) as l_reservedate,
>             reserves.borrowernumber as borrowernumber,
>             GROUP_CONCAT(DISTINCT items.holdingbranch 
>                     ORDER BY items.itemnumber SEPARATOR '|') l_holdingbranch,
>             reserves.biblionumber,
>-            reserves.branchcode,
>-            GROUP_CONCAT(DISTINCT reserves.branchcode 
>-                    ORDER BY items.itemnumber SEPARATOR ', ') l_branch,
>-            items.holdingbranch as branch,
>+            reserves.branchcode as l_branch,
>             GROUP_CONCAT(DISTINCT items.itype 
>                     ORDER BY items.itemnumber SEPARATOR '|') l_itype,
>             GROUP_CONCAT(DISTINCT items.location 
>@@ -114,11 +106,6 @@ if ( $run_report ) {
>                     ORDER BY items.itemnumber SEPARATOR '<br/>') l_enumchron,
>             GROUP_CONCAT(DISTINCT items.copynumber
>                     ORDER BY items.itemnumber SEPARATOR '<br/>') l_copynumber,
>-            items.itemnumber,
>-            notificationdate,
>-            reminderdate,
>-            max(priority) as priority,
>-            reserves.found,
>             biblio.title,
>             biblio.author,
>             count(DISTINCT items.itemnumber) as icount,
>@@ -135,7 +122,7 @@ if ( $run_report ) {
>     $sqldatewhere
>     AND (reserves.itemnumber IS NULL OR reserves.itemnumber = items.itemnumber)
>     AND items.itemnumber NOT IN (SELECT itemnumber FROM branchtransfers where datearrived IS NULL)
>-    AND items.itemnumber NOT IN (select itemnumber FROM reserves where found='W')
>+    AND items.itemnumber NOT IN (select itemnumber FROM reserves where found IS NOT NULL)
>     AND issues.itemnumber IS NULL
>     AND reserves.priority <> 0 
>     AND reserves.suspend = 0
>@@ -145,59 +132,48 @@ if ( $run_report ) {
>     #    multiple patrons have a hold on an item
> 
> 
>-    if (C4::Context->preference('IndependentBranches')){
>-        $strsth .= " AND items.holdingbranch=? ";
>-        push @query_params, C4::Context->userenv->{'branch'};
>-    }
>-    $strsth .= " GROUP BY reserves.biblionumber ORDER BY biblio.title ";
>+if (C4::Context->preference('IndependentBranches')){
>+    $strsth .= " AND items.holdingbranch=? ";
>+    push @query_params, C4::Context->userenv->{'branch'};
>+}
>+$strsth .= " GROUP BY reserves.biblionumber ORDER BY biblio.title ";
> 
>-    my $sth = $dbh->prepare($strsth);
>-    $sth->execute(@query_params);
>+my $sth = $dbh->prepare($strsth);
>+$sth->execute(@query_params);
> 
>-    while ( my $data = $sth->fetchrow_hashref ) {
>-        my $record = Koha::Biblios->find($data->{biblionumber});
>-        if ($record){
>-            $data->{subtitle} = [ $record->subtitles ];
>-        }
>-        push(
>-            @reservedata,
>-            {
>-                reservedate     => $data->{l_reservedate},
>-                priority        => $data->{priority},
>-                name            => $data->{l_patron},
>-                title           => $data->{title},
>-                subtitle        => $data->{subtitle},
>-                author          => $data->{author},
>-                borrowernumber  => $data->{borrowernumber},
>-                itemnum         => $data->{itemnumber},
>-                phone           => $data->{phone},
>-                email           => $data->{email},
>-                biblionumber    => $data->{biblionumber},
>-                statusw         => ( $data->{found} eq "W" ),
>-                statusf         => ( $data->{found} eq "F" ),
>-                holdingbranches => [split('\|', $data->{l_holdingbranch})],,
>-                branch          => $data->{l_branch},
>-                itemcallnumber  => $data->{l_itemcallnumber},
>-                enumchron       => $data->{l_enumchron},
>-                copyno          => $data->{l_copynumber},
>-                notificationdate=> $data->{notificationdate},
>-                reminderdate    => $data->{reminderdate},
>-                count           => $data->{icount},
>-                rcount          => $data->{rcount},
>-                pullcount       => $data->{icount} <= $data->{rcount} ? $data->{icount} : $data->{rcount},
>-                itypes          => [split('\|', $data->{l_itype})],
>-                locations       => [split('\|', $data->{l_location})],
>-            }
>-        );
>+while ( my $data = $sth->fetchrow_hashref ) {
>+    my $record = Koha::Biblios->find($data->{biblionumber});
>+    if ($record){
>+        $data->{subtitle} = [ $record->subtitles ];
>     }
>-    $sth->finish;
>+    push(
>+        @reservedata, {
>+            reservedate     => $data->{l_reservedate},
>+            name            => $data->{l_patron},
>+            title           => $data->{title},
>+            subtitle        => $data->{subtitle},
>+            author          => $data->{author},
>+            borrowernumber  => $data->{borrowernumber},
>+            biblionumber    => $data->{biblionumber},
>+            holdingbranches => [split('\|', $data->{l_holdingbranch})],
>+            branch          => $data->{l_branch},
>+            itemcallnumber  => $data->{l_itemcallnumber},
>+            enumchron       => $data->{l_enumchron},
>+            copyno          => $data->{l_copynumber},
>+            count           => $data->{icount},
>+            rcount          => $data->{rcount},
>+            pullcount       => $data->{icount} <= $data->{rcount} ? $data->{icount} : $data->{rcount},
>+            itypes          => [split('\|', $data->{l_itype})],
>+            locations       => [split('\|', $data->{l_location})],
>+        }
>+    );
> }
>+$sth->finish;
> 
> $template->param(
>     todaysdate          => $today,
>     from                => $startdate,
>     to                  => $enddate,
>-    run_report          => $run_report,
>     reserveloop         => \@reservedata,
>     "BiblioDefaultView".C4::Context->preference("BiblioDefaultView") => 1,
>     HoldsToPullStartDate => C4::Context->preference('HoldsToPullStartDate') || PULL_INTERVAL,
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
>index 7bda261..27c08a2 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
>@@ -85,8 +85,7 @@ $(document).ready(function() {
>     <div id="yui-main">
>     <div class="yui-b">
> 
>-<h2>Holds to pull[% IF ( run_report ) %] placed between [% from | $KohaDates %] and [% to | $KohaDates %][% END %]</h2>
>-[% IF ( run_report ) %]
>+<h2>Holds to pull placed between [% from | $KohaDates %] and [% to | $KohaDates %]</h2>
> <h3>Reported on [% todaysdate | $KohaDates %]</h3>
> <p>The following holds have not been filled. Please retrieve them and check them in.</p>
> <div class="searchresults">
>@@ -149,7 +148,6 @@ $(document).ready(function() {
>         </td>
>         <td>
>             <span title="[% reserveloo.reservedate %]">[% reserveloo.reservedate | $KohaDates %] in [% Branches.GetName ( reserveloo.branch ) %]</span>
>-            [% IF ( reserveloo.statusw ) %]<p>Waiting</p>[% END %][% IF ( reserveloo.statusf ) %]<p>Fulfilled</p>[% END %]
>         </td>
>         </tr>
>         [% END %]
>@@ -175,7 +173,6 @@ $(document).ready(function() {
>         <b>No items found.</b>
>     [% END %]
> </div>
>-[% END %]
> 
> </div>
> </div>
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 18079: 60008 | 60038 | 60070 | 60078 | 61189