Bug 21628: Simplify holds awating pickup report

To test:
1 - Place a number of holds
2 - Check in the items and confirm the holds
3 - Visit /cgi-bin/koha/circ/waitingreserves.pl
4 - View the report
5 - Apply patch
6 - Confirm that the report looks the same
7 - Confirm cancelling holds works as before

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Followed test plan, patch worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl
index aae9cbe..c78884f 100755 (executable)
--- a/circ/waitingreserves.pl
+++ b/circ/waitingreserves.pl
@@ -81,69 +81,30 @@ if ( C4::Context->preference('IndependentBranches') ) {
 }
 $template->param( all_branches => 1 ) if $all_branches;
 
-my (@reservloop, @overloop);
-my ($reservcount, $overcount);
+my (@reserve_loop, @over_loop);
 # FIXME - Is priority => 0 useful? If yes it must be moved to waiting, otherwise we need to remove it from here.
 my $holds = Koha::Holds->waiting->search({ priority => 0, ( $all_branches ? () : ( branchcode => $default ) ) }, { order_by => ['waitingdate'] });
+
 # get reserves for the branch we are logged into, or for all branches
 
 my $today = Date_to_Days(&Today);
-my $max_pickup_delay = C4::Context->preference('ReservesMaxPickUpDelay');
 
 while ( my $hold = $holds->next ) {
     next unless ($hold->waitingdate && $hold->waitingdate ne '0000-00-00');
 
-    my $item = $hold->item;
-    my $patron = $hold->borrower;
-    my $biblio = $item->biblio;
-    my $holdingbranch = $item->holdingbranch;
-    my $homebranch = $item->homebranch;
-
-    my %getreserv = (
-        title             => $biblio->title,
-        itemnumber        => $item->itemnumber,
-        waitingdate       => $hold->waitingdate,
-        reservedate       => $hold->reservedate,
-        borrowernum       => $patron->borrowernumber,
-        biblionumber      => $biblio->biblionumber,
-        barcode           => $item->barcode,
-        homebranch        => $homebranch,
-        holdingbranch     => $item->holdingbranch,
-        itemcallnumber    => $item->itemcallnumber,
-        enumchron         => $item->enumchron,
-        copynumber        => $item->copynumber,
-        borrowername      => $patron->surname, # FIXME Let's send $patron to the template
-        borrowerfirstname => $patron->firstname,
-        borrowerphone     => $patron->phone,
-    );
-
-    my $itemtype = Koha::ItemTypes->find( $item->effective_itemtype );
     my ( $expire_year, $expire_month, $expire_day ) = split (/-/, $hold->expirationdate);
     my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day );
 
-    $getreserv{'itemtype'}       = $itemtype->description; # FIXME Should not it be translated_description?
-    $getreserv{'subtitle'}       = GetRecordValue(
-        'subtitle',
-        GetMarcBiblio({ biblionumber => $biblio->biblionumber }),
-        $biblio->frameworkcode);
-    if ( $homebranch ne $holdingbranch ) {
-        $getreserv{'dotransfer'} = 1;
-    }
-
-    $getreserv{patron} = $patron;
-
     if ($today > $calcDate) {
         if ($cancelall) {
-            my $res = cancel( $item->itemnumber, $patron->borrowernumber, $holdingbranch, $homebranch, !$transfer_when_cancel_all );
+            my $res = cancel( $hold->item->itemnumber, $hold->borrowernumber, $hold->item->holdingbranch, $hold->item->homebranch, !$transfer_when_cancel_all );
             push @cancel_result, $res if $res;
             next;
         } else {
-            push @overloop,   \%getreserv;
-            $overcount++;
+            push @over_loop, $hold;
         }
     }else{
-        push @reservloop, \%getreserv;
-        $reservcount++;
+        push @reserve_loop, $hold;
     }
     
 }
@@ -151,12 +112,11 @@ while ( my $hold = $holds->next ) {
 $template->param(cancel_result => \@cancel_result) if @cancel_result;
 
 $template->param(
-    reserveloop => \@reservloop,
-    reservecount => $reservcount,
-    overloop    => \@overloop,
-    overcount   => $overcount,
+    reserveloop => \@reserve_loop,
+    reservecount => scalar @reserve_loop,
+    overloop    => \@over_loop,
+    overcount   => scalar @over_loop,
     show_date   => output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }),
-    ReservesMaxPickUpDelay => $max_pickup_delay,
     tab => $tab,
 );
 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc
new file mode 100644 (file)
index 0000000..b5b74c0
--- /dev/null
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc
@@ -0,0 +1,51 @@
+[% USE ItemTypes %]
+<table id="[% table_name %]">
+<thead><tr>
+    <th class="title-string">Waiting since</th>
+    <th class="title-string">Date hold placed</th>
+    <th class="anti-the">Title</th>
+    <th>Patron</th>
+    <th>Home branch</th>
+    <th>Current location</th>
+    <th>Call number</th>
+    <th>Copy number</th>
+    <th>Enumeration</th>
+    <th>Action</th>
+</tr></thead>
+<tbody>[% FOREACH reserveloo IN reserveloop %]
+<tr>
+    <td><span title="[% reserveloo.waitingdate | html %]">[% reserveloo.waitingdate | $KohaDates %]</span></td>
+    <td><span title="[% reserveloo.reservedate | html %]">[% reserveloo.reservedate | $KohaDates %]</span></td>
+    <td>[% INCLUDE 'biblio-default-view.inc' biblionumber = reserveloo.biblionumber %]
+        [% reserveloo.biblio.title | html %] [% FOREACH subtitl IN reserveloo.biblio.subtitles %] [% subtitl.subfield | html %][% END %]
+        </a>
+            [% UNLESS ( item_level_itypes ) %][% IF ( ItemTypes.GetDescription(reserveloo.item.effective_itemtype) ) %]&nbsp; (<b>[% ItemTypes.GetDescription(reserveloo.item.effective_itemtype) | html %]</b>)[% END %][% END %]
+            <br />Barcode: [% reserveloo.item.barcode | html %]
+    </td>
+    <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% reserveloo.borrower.borrowernumber | uri %]">[% reserveloo.borrower.surname | html %], [% reserveloo.borrower.firstname | html %]</a>
+        [% IF ( reserveloo.borrower.phone ) %]<br />[% reserveloo.borrower.phone | html %][% END %]
+        [% IF ( reserveloo.borrower.first_valid_email_address ) %]<br /><a href="mailto:[% reserveloo.borrower.first_valid_email_address | uri %]?subject=Hold waiting: [% reserveloo.biblio.title | uri %]">
+        [% reserveloo.borrower.first_valid_email_address | html %]</a>[% END %]
+    </td>
+    <td>[% Branches.GetName( reserveloo.item.homebranch ) | html %]</td>
+    <td>[% Branches.GetName( reserveloo.item.holdingbranch ) | html %]</td>
+    <td>[% reserveloo.item.itemcallnumber | html %]</td>
+    <td>[% reserveloo.item.copynumber | html %]</td>
+    <td>[% reserveloo.item.enumchron | html %]</td>
+    <td>
+        <form name="cancelReserve" action="waitingreserves.pl" method="post">
+            <input type="hidden" name="borrowernumber" value="[% reserveloo.borrower.borrowernumber | html %]" />
+            <input type="hidden" name="itemnumber" value="[% reserveloo.item.itemnumber | html %]" />
+            <input type="hidden" name="fbr" value="[% reserveloo.item.holdingbranch | html %]" />
+            <input type="hidden" name="tbr" value="[% reserveloo.item.homebranch | html %]" />
+            <input type="hidden" name="tab" value="holdswaiting">
+            [% IF ( reserveloo.item.homebranch != reserveloo.item.holdingbranch ) %]
+            <input type="submit" value="Cancel hold and return to : [% Branches.GetName( reserveloo.item.homebranch ) | html %]" />
+            [% ELSE %]
+            <input type="submit" value="Cancel hold" />
+            [% END %]
+       </form>
+    </td>
+</tr>
+[% END %]</tbody>
+</table>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt
index c7850d4..1ba62df 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt
@@ -63,62 +63,13 @@
                 <li><a href="#holdswaiting">Holds waiting: [% reservecount | html %]</a></li>
                 <li>
                     <a href="#holdsover">
-                        Holds waiting over [% ReservesMaxPickUpDelay | html %] days: [% overcount | html %]
+                        Holds waiting over [% Koha.Preference('ReservesMaxPickUpDelay') | html %] days: [% overcount | html %]
                     </a>
                 </li>
             </ul>
             <div id="holdswaiting">
-            [% IF ( reserveloop ) %]
-               <table id="holdst">
-               <thead><tr>
-                    <th class="title-string">Waiting since</th>
-                    <th class="title-string">Date hold placed</th>
-                    <th class="anti-the">Title</th>
-                    <th>Patron</th>
-                    <th>Home branch</th>
-                    <th>Current location</th>
-                    <th>Call number</th>
-                    <th>Copy number</th>
-                    <th>Enumeration</th>
-                    <th>Action</th>
-               </tr></thead>
-               <tbody>[% FOREACH reserveloo IN reserveloop %]
-                <tr>
-                    <td><span title="[% reserveloo.waitingdate | html %]">[% reserveloo.waitingdate | $KohaDates %]</span></td>
-                    <td><span title="[% reserveloo.reservedate | html %]">[% reserveloo.reservedate | $KohaDates %]</span></td>
-                    <td>[% INCLUDE 'biblio-default-view.inc' biblionumber = reserveloo.biblionumber %]
-                        [% reserveloo.title | html %] [% FOREACH subtitl IN reserveloo.subtitle %] [% subtitl.subfield | html %][% END %]
-                        </a>
-                            [% UNLESS ( item_level_itypes ) %][% IF ( reserveloo.itemtype ) %]&nbsp; (<b>[% reserveloo.itemtype | html %]</b>)[% END %][% END %]
-                            <br />Barcode: [% reserveloo.barcode | html %]
-                    </td>
-                    <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% reserveloo.patron.borrowernumber | uri %]">[% reserveloo.patron.surname | html %], [% reserveloo.patron.firstname | html %]</a>
-                        [% IF ( reserveloo.patron.phone ) %]<br />[% reserveloo.patron.phone | html %][% END %]
-                        [% IF ( reserveloo.patron.first_valid_email_address ) %]<br /><a href="mailto:[% reserveloo.patron.first_valid_email_address | uri %]?subject=Hold waiting: [% reserveloo.title | uri %]">
-                        [% reserveloo.patron.first_valid_email_address | html %]</a>[% END %]
-                    </td>
-                    <td>[% Branches.GetName( reserveloo.homebranch ) | html %]</td>
-                    <td>[% Branches.GetName( reserveloo.holdingbranch ) | html %]</td>
-                    <td>[% reserveloo.itemcallnumber | html %]</td>
-                    <td>[% reserveloo.copynumber | html %]</td>
-                    <td>[% reserveloo.enumchron | html %]</td>
-                    <td>
-                        <form name="cancelReserve" action="waitingreserves.pl" method="post">
-                            <input type="hidden" name="borrowernumber" value="[% reserveloo.borrowernum | html %]" />
-                            <input type="hidden" name="itemnumber" value="[% reserveloo.itemnumber | html %]" />
-                            <input type="hidden" name="fbr" value="[% reserveloo.holdingbranch | html %]" />
-                            <input type="hidden" name="tbr" value="[% reserveloo.homebranch | html %]" />
-                            <input type="hidden" name="tab" value="holdswaiting">
-                            [% IF ( reserveloo.dotransfer ) %]
-                            <input type="submit" value="Cancel hold and return to : [% Branches.GetName( reserveloo.homebranch ) | html %]" />
-                            [% ELSE %]
-                            <input type="submit" value="Cancel hold" />
-                            [% END %]
-                       </form>
-                    </td>
-                </tr>
-                [% END %]</tbody>
-        </table>
+        [% IF ( reserveloop ) %]
+            [% INCLUDE waiting_holds.inc table_name='holdst' reserveloop=reserveloop %]
         [% ELSE %]
             <div class="dialog message">No holds found.</div>
         [% END %]
@@ -140,54 +91,7 @@
                     Only items that need not be transferred will be cancelled (TransferWhenCancelAllWaitingHolds sypref)
                [% END %]
 
-               <table id="holdso">
-               <thead><tr>
-                    <th class="title-string">Waiting since</th>
-                    <th class="title-string">Date hold placed</th>
-                    <th class="anti-the">Title</th>
-                    <th>Patron</th>
-                    <th>Home branch</th>
-                    <th>Current location</th>
-                    <th>Call number</th>
-                    <th>Copy number</th>
-                    <th>Enumeration</th>
-                    <th>Action</th>
-               </tr></thead>
-               <tbody>[% FOREACH overloo IN overloop %]
-                    <tr>
-                        <td><span title="[% overloo.waitingdate | html %]">[% overloo.waitingdate | $KohaDates %]</span></td>
-                        <td><span title="[% overloo.reservedate | html %]">[% overloo.reservedate | $KohaDates %]</span></td>
-                        <td>[% INCLUDE 'biblio-default-view.inc' biblionumber = overloo.biblionumber %][% overloo.title | html %]
-                            [% FOREACH subtitl IN overloo.subtitle %] [% subtitl.subfield | html %][% END %]
-                        </a>
-                            [% UNLESS ( item_level_itypes ) %][% IF ( overloo.itemtype ) %]&nbsp; (<b>[% overloo.itemtype | html %]</b>)[% END %][% END %]
-                        <br />Barcode: [% overloo.barcode | html %]
-                    </td>
-                    <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% overloo.patron.borrowernumber | uri %]">[% overloo.patron.surname | html %], [% overloo.patron.firstname | html %]</a><br />[% overloo.patron.phone | html %]<br />
-                        [% IF ( overloo.patron.first_valid_email_address ) %]<a href="mailto:[% overloo.patron.first_valid_email_address | uri %]?subject=Reservation: [% overloo.title |url %]">
-        [% overloo.patron.first_valid_email_address | html %]</a>[% END %]
-                    </td>
-                    <td>[% Branches.GetName( overloo.homebranch ) | html %]</td>
-                    <td>[% Branches.GetName( overloo.holdingbranch ) | html %]</td>
-                    <td>[% overloo.itemcallnumber | html %]</td>
-                    <td>[% overloo.copynumber | html %]</td>
-                    <td>[% overloo.enumchron | html %]</td>
-                    <td><form name="cancelReserve" action="waitingreserves.pl" method="post">
-                            <input type="hidden" name="borrowernumber" value="[% overloo.borrowernum | html %]" />
-                            <input type="hidden" name="itemnumber" value="[% overloo.itemnumber | html %]" />
-                            <input type="hidden" name="fbr" value="[% overloo.holdingbranch | html %]" />
-                            <input type="hidden" name="tbr" value="[% overloo.homebranch | html %]" />
-                            <input type="hidden" name="tab" value="holdsover">
-                            [% IF ( overloo.dotransfer ) %]
-                            <input type="submit" value="Cancel hold and return to : [% Branches.GetName( overloo.homebranch ) | html %]" />
-                            [% ELSE %]
-                            <input type="submit" value="Cancel hold" />
-                            [% END %]
-                       </form>
-                    </td>
-                </tr>
-                [% END %]</tbody>
-        </table>
+               [% INCLUDE waiting_holds.inc table_name='holdso' reserveloop=overloop %]
         [% ELSE %]
             <div class="dialog message">No holds found.</div>
                 [% END %]