Bug 16158: Move GetAuthorisedValues outside the loop

In catalogue/detail.pl and catalogue/moredetail.pl, the
GetAuthorisedValues subroutine is called once per item. It's not needed,
only 1 call is enough.

Test plan:
Edit item and view a catalogue record.
The withdrawn, lost and damaged values should be correctly
displayed/saved.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

diff --git a/catalogue/detail.pl b/catalogue/detail.pl
index 55fcab9..873d33e 100755 (executable)
--- a/catalogue/detail.pl
+++ b/catalogue/detail.pl
@@ -186,8 +186,13 @@ my $collections    = GetKohaAuthorisedValues('items.ccode'   , $fw);
 my $copynumbers    = GetKohaAuthorisedValues('items.copynumber', $fw);
 my (@itemloop, @otheritemloop, %itemfields);
 my $norequests = 1;
-my $authvalcode_items_itemlost = GetAuthValCode('items.itemlost',$fw);
-my $authvalcode_items_damaged  = GetAuthValCode('items.damaged', $fw);
+
+if ( my $lost_av = GetAuthValCode('items.itemlost', $fw) ) {
+    $template->param( itemlostloop => GetAuthorisedValues( $lost_av ) );
+}
+if ( my $damaged_av = GetAuthValCode('items.damaged', $fw) ) {
+    $template->param( itemdamagedloop => GetAuthorisedValues( $damaged_av ) );
+}
 
 my $materials_authvalcode = GetAuthValCode('items.materials', $fw);
 my %materials_map;
@@ -217,11 +222,7 @@ foreach my $item (@items) {
                                                : '';
 
     $item->{datedue} = format_sqldatetime($item->{datedue});
-    # item damaged, lost, withdrawn loops
-    $item->{itemlostloop} = GetAuthorisedValues($authvalcode_items_itemlost) if $authvalcode_items_itemlost;
-    if ($item->{damaged}) {
-        $item->{itemdamagedloop} = GetAuthorisedValues($authvalcode_items_damaged) if $authvalcode_items_damaged;
-    }
+
     #get shelf location and collection code description if they are authorised value.
     # same thing for copy number
     my $shelfcode = $item->{'location'};

diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl
index 82044ed..f94c1ff 100755 (executable)
--- a/catalogue/moredetail.pl
+++ b/catalogue/moredetail.pl
@@ -137,9 +137,6 @@ foreach ( keys %{$data} ) {
 ($itemnumber) and @items = (grep {$_->{'itemnumber'} == $itemnumber} @items);
 foreach my $item (@items){
     $item->{object} = Koha::Items->find( $item->{itemnumber} );
-    $item->{itemlostloop}= GetAuthorisedValues(GetAuthValCode('items.itemlost',$fw)) if GetAuthValCode('items.itemlost',$fw);
-    $item->{itemdamagedloop}= GetAuthorisedValues(GetAuthValCode('items.damaged',$fw)) if GetAuthValCode('items.damaged',$fw);
-    $item->{itemwithdrawnloop}= GetAuthorisedValues(GetAuthValCode('items.withdrawn',$fw)) if GetAuthValCode('items.withdrawn',$fw);
     $item->{'collection'}              = $ccodes->{ $item->{ccode} } if ($ccodes);
     $item->{'itype'}                   = $itemtypes->{ $item->{'itype'} }->{'translated_description'};
     $item->{'replacementprice'}        = sprintf( "%.2f", $item->{'replacementprice'} );
@@ -203,6 +200,17 @@ foreach my $item (@items){
     }
 
 }
+
+if ( my $lost_av = GetAuthValCode('items.itemlost', $fw) ) {
+    $template->param( itemlostloop => GetAuthorisedValues( $lost_av ) );
+}
+if ( my $damaged_av = GetAuthValCode('items.damaged', $fw) ) {
+    $template->param( itemdamagedloop => GetAuthorisedValues( $damaged_av ) );
+}
+if ( my $withdrawn_av = GetAuthValCode('items.withdrawn', $fw) ) {
+    $template->param( itemwithdrawnloop => GetAuthorisedValues( $withdrawn_av ) );
+}
+
 $template->param(count => $data->{'count'},
        subscriptionsnumber => $subscriptionsnumber,
     subscriptiontitle   => $data->{title},

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
index e314fa1..270fe05 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
@@ -648,8 +648,8 @@ function verify_images() {
                         [% END %]
 
                         [% IF ( item.itemlost ) %]
-                            [% IF ( item.itemlostloop ) %]
-                                [% FOREACH itemlostloo IN item.itemlostloop %]
+                            [% IF itemlostloop %]
+                                [% FOREACH itemlostloo IN itemlostloop %]
                                     [% IF itemlostloo.authorised_value == item.itemlost %]
                                         <span class="lost">[% itemlostloo.lib %]</span>
                                     [% END %]
@@ -664,8 +664,8 @@ function verify_images() {
                         [% END %]
 
                         [% IF ( item.damaged ) %]
-                            [% IF ( item.itemdamagedloop ) %]
-                                [% FOREACH itemdamagedloo IN item.itemdamagedloop %]
+                            [% IF itemdamagedloop %]
+                                [% FOREACH itemdamagedloo IN itemdamagedloop %]
                                     [% IF itemdamagedloo.authorised_value == item.damaged %]
                                         <span class="dmg">[% itemdamagedloo.lib %]</span>
                                     [% END %]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
index cea9cae..befa557 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
@@ -99,7 +99,7 @@
             [% END %]
             &nbsp;</li>
             <li><span class="label">Current renewals:</span> [% ITEM_DAT.renewals %]&nbsp;</li>
-            [% IF ( ITEM_DAT.itemlostloop ) %]
+            [% IF itemlostloop %]
                 <li><span class="label">Lost status:</span>
                     [% IF ( CAN_user_circulate ) %]
                         <form action="updateitem.pl" method="post">
@@ -108,7 +108,7 @@
                         <input type="hidden" name="itemnumber" value="[% ITEM_DAT.itemnumber %]" />
                         <select name="itemlost" >
                                     <option value="">Choose</option>
-                        [% FOREACH itemlostloo IN ITEM_DAT.itemlostloop %]
+                        [% FOREACH itemlostloo IN itemlostloop %]
                             [% IF itemlostloo.authorised_value == ITEM_DAT.itemlost %]
                                     <option value="[% itemlostloo.authorised_value %]" selected="selected">[% itemlostloo.lib %]</option>
                             [% ELSE %]
@@ -120,7 +120,7 @@
                         <input type="hidden" name="damaged" value="[% ITEM_DAT.damaged %]" />
                         <input type="submit" name="submit" class="submit" value="Set status" /></form>
                     [% ELSE %]
-                        [% FOREACH itemlostloo IN ITEM_DAT.itemlostloop %]
+                        [% FOREACH itemlostloo IN itemlostloop %]
                             [% IF ( itemlostloo.selected ) %]
                                 [% itemlostloo.lib %]
                             [% END %]
@@ -132,7 +132,7 @@
                     <li><span class="label">Lost on:</span>[% ITEM_DAT.itemlost_on | $KohaDates %] &nbsp;</li>
                 [% END %]
             [% END %]
-            [% IF ( ITEM_DAT.itemdamagedloop ) %]
+            [% IF itemdamagedloop %]
             <li><span class="label">Damaged status:</span>
                 [% IF ( CAN_user_circulate ) %]
                     <form action="updateitem.pl" method="post">
@@ -141,7 +141,7 @@
                     <input type="hidden" name="itemnumber" value="[% ITEM_DAT.itemnumber %]" />
                     <select name="damaged" >
                     <option value="">Choose</option>
-            [% FOREACH itemdamagedloo IN ITEM_DAT.itemdamagedloop %]
+            [% FOREACH itemdamagedloo IN itemdamagedloop %]
                 [% IF itemdamagedloo.authorised_value == ITEM_DAT.damaged %]
                     <option value="[% itemdamagedloo.authorised_value %]" selected="selected">[% itemdamagedloo.lib %]</option>
                 [% ELSE %]
@@ -153,7 +153,7 @@
                     <input type="hidden" name="itemlost" value="[% ITEM_DAT.itemlost %]" />
                     <input type="submit" name="submit" class="submit" value="Set status" /></form>
                 [% ELSE %]
-                    [% FOREACH itemdamagedloo IN ITEM_DAT.itemdamagedloop %]
+                    [% FOREACH itemdamagedloo IN itemdamagedloop %]
                         [% IF ( itemdamagedloo.selected ) %]
                             [% itemdamagedloo.lib %]
                         [% END %]
@@ -163,7 +163,7 @@
             </li>
             [% END %]
 
-            [% IF ( ITEM_DAT.itemwithdrawnloop ) %]
+            [% IF itemwithdrawnloop %]
                 <li><span class="label">Withdrawn status:</span>
                     [% IF ( CAN_user_circulate ) %]
                         <form action="updateitem.pl" method="post">
@@ -172,7 +172,7 @@
                             <input type="hidden" name="itemnumber" value="[% ITEM_DAT.itemnumber %]" />
                             <select name="withdrawn" >
                                 <option value="">Choose</option>
-                                [% FOREACH itemwithdrawn IN ITEM_DAT.itemwithdrawnloop %]
+                                [% FOREACH itemwithdrawn IN itemwithdrawnloop %]
                                     [% IF itemwithdrawn.authorised_value == ITEM_DAT.withdrawn %]
                                         <option value="[% itemwithdrawn.authorised_value %]" selected="selected">[% itemwithdrawn.lib %]</option>
                                     [% ELSE %]
@@ -184,7 +184,7 @@
                             <input type="hidden" name="damaged" value="[% ITEM_DAT.damaged %]" />
                             <input type="submit" name="submit" class="submit" value="Set status" /></form>
                     [% ELSE %]
-                        [% FOREACH itemwithdrawn IN ITEM_DAT.itemwithdrawnloop %]
+                        [% FOREACH itemwithdrawn IN itemwithdrawnloop %]
                             [% IF itemwithdrawn.authorised_value == ITEM_DAT.withdrawn %]
                                 [% itemwithdrawn.lib %]
                             [% END %]