From 8f3d88fe9b06179b248f03a686096ce6bf0987d6 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 18 Nov 2020 11:14:45 +0100
Subject: [PATCH] Bug 24488: Simplify structure passed to the template

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
---
 circ/pendingreserves.pl                       | 36 ++++++-----
 .../prog/en/modules/circ/pendingreserves.tt   | 64 ++++++++++---------
 2 files changed, 53 insertions(+), 47 deletions(-)

diff --git a/circ/pendingreserves.pl b/circ/pendingreserves.pl
index 9cad99ab8f..69477bf6da 100755
--- a/circ/pendingreserves.pl
+++ b/circ/pendingreserves.pl
@@ -222,10 +222,11 @@ my $patrons_count = {
     }
 };
 
-# make final reserves hash and fill with info
-my $reserves;
+# make final holds_info array and fill with info
+my @holds_info;
 foreach my $bibnum ( @biblionumbers ){
 
+    my $hold_info;
     my $items = $all_items->{$bibnum};
 
     # get available item types for each biblio
@@ -240,39 +241,38 @@ foreach my $bibnum ( @biblionumbers ){
             }
         )->get_column('itemtype');
     }
-    $reserves->{$bibnum}->{itemtypes} = \@res_itemtypes;
+    $hold_info->{itemtypes} = \@res_itemtypes;
 
     # get available locations for each biblio
-    $reserves->{$bibnum}->{locations} = [ uniq map { defined $_->location ? $_->location : () } @$items ];
+    $hold_info->{locations} = [ uniq map { defined $_->location ? $_->location : () } @$items ];
 
     # get available callnumbers for each biblio
-    $reserves->{$bibnum}->{callnumbers} = [ uniq map { defined $_->itemcallnumber ? $_->itemcallnumber : () } @$items ];
+    $hold_info->{callnumbers} = [ uniq map { defined $_->itemcallnumber ? $_->itemcallnumber : () } @$items ];
 
     # get available enumchrons for each biblio
-    $reserves->{$bibnum}->{enumchrons} = [ uniq map { defined $_->enumchron ? $_->enumchron : () } @$items ];
+    $hold_info->{enumchrons} = [ uniq map { defined $_->enumchron ? $_->enumchron : () } @$items ];
 
     # get available copynumbers for each biblio
-    $reserves->{$bibnum}->{copynumbers} = [ uniq map { defined $_->copynumber ? $_->copynumber : () } @$items ];
+    $hold_info->{copynumbers} = [ uniq map { defined $_->copynumber ? $_->copynumber : () } @$items ];
 
     # get available barcodes for each biblio
-    $reserves->{$bibnum}->{barcodes} = [ uniq map { defined $_->barcode ? $_->barcode : () } @$items ];
+    $hold_info->{barcodes} = [ uniq map { defined $_->barcode ? $_->barcode : () } @$items ];
 
     # get available holding branches for each biblio
-    $reserves->{$bibnum}->{holdingbranches} = [ uniq map { defined $_->holdingbranch ? $_->holdingbranch : () } @$items ];
+    $hold_info->{holdingbranches} = [ uniq map { defined $_->holdingbranch ? $_->holdingbranch : () } @$items ];
 
     # items available
     my $items_count = scalar @$items;
-    $reserves->{$bibnum}->{items_count} = $items_count;
+    $hold_info->{items_count} = $items_count;
 
     # patrons with holds
     $hold_info->{patrons_count} = $patrons_count->{$bibnum};
 
     my $pull_count = $items_count <= $patrons_count->{$bibnum} ? $items_count : $patrons_count->{$bibnum};
     if ( $pull_count == 0 ) {
-        delete($reserves->{$bibnum});
         next;
     }
-    $reserves->{$bibnum}->{pull_count} = $pull_count;
+    $hold_info->{pull_count} = $pull_count;
 
     # get other relevant information
     my $res_info = Koha::Holds->search(
@@ -282,17 +282,19 @@ foreach my $bibnum ( @biblionumbers ){
           alias => 'reserve'
         }
     )->next; # get first item in results
-    $reserves->{$bibnum}->{borrower} = $res_info->borrower;
-    $reserves->{$bibnum}->{item} = $res_info->item;
-    $reserves->{$bibnum}->{biblio} = $res_info->biblio;
-    $reserves->{$bibnum}->{reserve} = $res_info;
+    $hold_info->{patron} = $res_info->patron;
+    $hold_info->{item}   = $res_info->item;
+    $hold_info->{biblio} = $res_info->biblio;
+    $hold_info->{hold}   = $res_info;
+
+    push @holds_info, $hold_info;
 }
 
 $template->param(
     todaysdate          => $today,
     from                => $startdate,
     to                  => $enddate,
-    reserves            => $reserves,
+    holds_info          => \@holds_info,
     "BiblioDefaultView".C4::Context->preference("BiblioDefaultView") => 1,
     HoldsToPullStartDate => C4::Context->preference('HoldsToPullStartDate') || PULL_INTERVAL,
     HoldsToPullEndDate  => C4::Context->preference('ConfirmFutureHolds') || 0,
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 221dbddc28..fa9e239784 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
@@ -47,7 +47,7 @@
 <h3>Reported on [% todaysdate | $KohaDates %]</h3>
 <p>The following holds have not been filled. Please retrieve them and check them in.</p>
 <div id="searchresults">
-    [% IF ( reserves ) %]
+    [% IF holds_info %]
     <table id="holdst">
     <thead>
         <tr>
@@ -68,20 +68,24 @@
         </tr>
     </thead>
     <tbody>
-        [% FOREACH reserve IN reserves %]
+        [% FOREACH hold_info IN holds_info %]
         <tr>
-        [% IF ( reserve.value.borrower ) %]
-            <td><p><strong>[% reserve.value.pull_count | html %]</strong></p></td>
-            <td>[% reserve.value.items_count | html %]</td>
-            <td>[% reserve.value.patrons_count | html %]</td>
-            <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% reserve.value.borrower.borrowernumber | uri %]">[% reserve.value.borrower.firstname | html %] [% reserve.value.borrower.surname | html %]</a></td>
+        [% SET patron = hold_info.patron %]
+        [% SET item = hold_info.item %]
+        [% SET hold = hold_info.hold %]
+        [% IF patron %]
+            [% SET biblio = hold_info.biblio %]
+            <td><p><strong>[% hold_info.pull_count | html %]</strong></p></td>
+            <td>[% hold_info.items_count | html %]</td>
+            <td>[% hold_info.patrons_count | html %]</td>
+            <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %]</a></td>
             <td>
                 <p>
-                    [% INCLUDE 'biblio-title.inc' biblio=reserve.value.biblio link = 1 %]
+                    [% INCLUDE 'biblio-title.inc' biblio=biblio link = 1 %]
                 </p>
-                [% IF ( reserve.value.biblio.author ) %]<p> by [% reserve.value.biblio.author | html %]</p>[% END %]
-                [% IF ( reserve.value.biblio.biblioitem.editionstatement ) %]<p>[% reserve.value.biblio.biblioitem.editionstatement | html %]</p>[% END %]
-                [% IF ( reserve.value.biblio.biblioitem.publicationyear ) %]<p>[% reserve.value.biblio.biblioitem.publicationyear | html %]</p>[% END %]
+                [% IF ( biblio.author ) %]<p> by [% biblio.author | html %]</p>[% END %]
+                [% IF ( biblio.biblioitem.editionstatement ) %]<p>[% biblio.biblioitem.editionstatement | html %]</p>[% END %]
+                [% IF ( biblio.biblioitem.publicationyear ) %]<p>[% biblio.biblioitem.publicationyear | html %]</p>[% END %]
             </td>
         [% ELSE %]
             <td>"</td>
@@ -91,24 +95,24 @@
             <td>"</td>
         [% END %]
         <td>
-            [% IF ( reserve.value.holdingbranches.size ) %]
+            [% IF ( hold_info.holdingbranches.size ) %]
             <ul>
-                [% FOREACH holdingbranch IN reserve.value.holdingbranches %]
+                [% FOREACH holdingbranch IN hold_info.holdingbranches %]
                     <li>[% Branches.GetName ( holdingbranch ) | html %]</li>
                 [% END %]
             </ul>
             [% END %]
         </td>
         <td>
-            [% IF ( reserve.value.barcodes.size ) %]
-                [% SET barcode = reserve.value.barcodes.first %]
-                [% IF ( reserve.value.itemnumber ) %]Only [% barcode | html %][% ELSE %][% barcode | html %] or any available.[% END %]
+            [% IF ( hold_info.barcodes.size ) %]
+                [% SET barcode = hold_info.barcodes.first %]
+                [% IF ( hold_info.itemnumber ) %]Only [% barcode | html %][% ELSE %][% barcode | html %] or any available.[% END %]
             [% END %]
         </td>
         <td>
-            [% IF ( reserve.value.callnumbers.size ) %]
+            [% IF ( hold_info.callnumbers.size ) %]
                 <ul>
-                    [% FOREACH callnumber IN reserve.value.callnumbers %]
+                    [% FOREACH callnumber IN hold_info.callnumbers %]
                         <li>
                             [% callnumber | html %]
                         </li>
@@ -117,9 +121,9 @@
             [% END %]
         </td>
         <td>
-            [% IF ( reserve.value.copynumbers.size ) %]
+            [% IF ( hold_info.copynumbers.size ) %]
                 <ul>
-                    [% FOREACH copyno IN reserve.value.copynumbers %]
+                    [% FOREACH copyno IN hold_info.copynumbers %]
                         <li>
                             [% copyno | html %]
                         </li>
@@ -128,9 +132,9 @@
             [% END %]
         </td>
         <td>
-            [% IF ( reserve.value.enumchrons.size ) %]
+            [% IF ( hold_info.enumchrons.size ) %]
                 <ul>
-                    [% FOREACH enumchron IN reserve.value.enumchrons %]
+                    [% FOREACH enumchron IN hold_info.enumchrons %]
                         <li>
                             [% enumchron | html %]
                         </li>
@@ -140,25 +144,25 @@
         </td>
         <td>
             <ul>
-            [% FOREACH type IN reserve.value.itemtypes %]
+            [% FOREACH type IN hold_info.itemtypes %]
                 <li>[% ItemTypes.GetDescription( type ) | html %]</li>
             [% END %]
             </ul>
         </td>
         <td>
             <ul>
-            [% FOREACH loc IN reserve.value.locations %]
+            [% FOREACH loc IN hold_info.locations %]
                 <li>[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => loc ) | html %]</li>
             [% END %]
             </ul>
         </td>
         <td>
-            <span title="[% reserve.value.reserve.reservedate | html %]">[% reserve.value.reserve.reservedate | $KohaDates %] in [% Branches.GetName ( reserve.value.reserve.branchcode ) | html %]</span>
+            <span title="[% hold.reservedate | html %]">[% hold.reservedate | $KohaDates %] in [% Branches.GetName ( hold.branchcode ) | html %]</span>
         </td>
         <td>
             <form name="cancelReserve" action="/cgi-bin/koha/circ/pendingreserves.pl" method="post">
                 <input type="hidden" name="op" value="cancel_reserve" />
-                <input type="hidden" name="reserve_id" value="[% reserve.value.reserve.reserve_id | html %]" />
+                <input type="hidden" name="reserve_id" value="[% hold.reserve_id | html %]" />
 
                 [% SET hold_cancellation = AuthorisedValues.GetAuthValueDropbox('HOLD_CANCELLATION') %]
                 [% IF hold_cancellation %]
@@ -173,17 +177,17 @@
                     </div>
                 [% END %]
 
-                [% IF reserve.value.item.holdingbranch != reserve.value.item.homebranch %]
-                    <input class="btn btn-default" type="submit" value="Cancel hold and return to : [% Branches.GetName( reserve.value.item.homebranch ) | html %]" />
+                [% IF item.holdingbranch != item.homebranch %]
+                    <input class="btn btn-default" type="submit" value="Cancel hold and return to : [% Branches.GetName( item.homebranch ) | html %]" />
                 [% ELSE %]
                     <input class="btn btn-default" type="submit" value="Cancel hold" />
                 [% END %]
             </form>
 
         [% IF Koha.Preference('CanMarkHoldsToPullAsLost') != 'do_not_allow' %]
-            [% IF reserve.value.reserve.itemnumber %]
+            [% IF hold.itemnumber %]
                 <form name="cancelReserve" action="/cgi-bin/koha/circ/pendingreserves.pl" method="post">
-                    <input type="hidden" name="reserve_id" value="[% reserve.value.reserve.reserve_id | html %]" />
+                    <input type="hidden" name="reserve_id" value="[% hold.reserve_id | html %]" />
                     [% IF Koha.Preference('CanMarkHoldsToPullAsLost') == 'allow' %]
                         <input type="hidden" name="op" value="mark_as_lost" />
                         <input type="submit" value="Mark item as lost" />
-- 
2.20.1