From 7ec14bcafb233bc5c1b542f7c523f1be660b02c0 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 10 Aug 2016 13:04:48 +0100
Subject: [PATCH] Bug 17248 - Koha::AuthorisedValues - Remove
 GetKohaAuthorisedValueLib

The subroutine C4::Koha::GetKohaAuthorisedValueLib just retrieves a description
(lib) for a given authorised value.

We can easily replace it using:
  Koha::AuthorisedValues->search({ category => $cat, authorised_value => $av })->lib
or
  Koha::AuthorisedValues->search({ category => $cat, authorised_value => $av })->opac_description

Test plan:
- On the detail page of a bibliographic record, the description for notforloan,
restricted and stack (?) should be correctly displayed
- View a shelf, the location (LOC) description should be displayed
- On the search result page, the location description should be displayed in the
  facets
- Set AcqCreateItem=ordering and receiving items.
The description for notforloan, restricted, location, ccode, etc. field
should be displayed.
- When creating item in the acquisition module, the dropdown list for
  field linked to AV should display the AV' descriptions
- On the transfers page, the description of the location should be
  displayed.
- On the checkout list from the circulation.pl and returns.pl pages, the
  description for "materials" should be displayed
- Fill some OPAC_SUG AV and create a suggestion, the reason dropdown
  list should display the description of OPAC_SUG
---
 C4/Circulation.pm              |  1 -
 C4/Items.pm                    | 23 ++++++++++++++++-------
 C4/Koha.pm                     | 22 ----------------------
 C4/Search.pm                   |  7 ++++---
 acqui/orderreceive.pl          | 15 ++++++++++-----
 catalogue/getitem-ajax.pl      | 17 ++++++++++++-----
 circ/branchtransfers.pl        |  8 ++++++--
 circ/circulation.pl            |  4 +++-
 circ/returns.pl                |  4 +++-
 opac/opac-suggestions.pl       |  5 ++++-
 serials/subscription-detail.pl |  6 ++++--
 11 files changed, 62 insertions(+), 50 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 0964050..f1c1260 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -38,7 +38,6 @@ use C4::Log; # logaction
 use C4::Koha qw(
     GetAuthorisedValueByCode
     GetAuthValCode
-    GetKohaAuthorisedValueLib
 );
 use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
 use C4::RotatingCollections qw(GetCollectionItemBranches);
diff --git a/C4/Items.pm b/C4/Items.pm
index 2f31897..2e89b66 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -34,6 +34,8 @@ use YAML qw/Load/;
 use DateTime::Format::MySQL;
 use Data::Dumper; # used as part of logging item record changes, not just for
                   # debugging; so please don't remove this
+
+use Koha::AuthorisedValues;
 use Koha::DateUtils qw/dt_from_string/;
 use Koha::Database;
 
@@ -1380,19 +1382,24 @@ sub GetItemsInfo {
 
         # get notforloan complete status if applicable
         if ( my $code = C4::Koha::GetAuthValCode( 'items.notforloan', $data->{frameworkcode} ) ) {
-            $data->{notforloanvalue}     = C4::Koha::GetKohaAuthorisedValueLib( $code, $data->{itemnotforloan} );
-            $data->{notforloanvalueopac} = C4::Koha::GetKohaAuthorisedValueLib( $code, $data->{itemnotforloan}, 1 );
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $data->{itemnotforloan} });
+            $av = $av->count ? $av->next : undef;
+            $data->{notforloanvalue}     = $av ? $av->lib : '';
+            $data->{notforloanvalueopac} = $av ? $av->opac_description : '';
         }
 
         # get restricted status and description if applicable
         if ( my $code = C4::Koha::GetAuthValCode( 'items.restricted', $data->{frameworkcode} ) ) {
-            $data->{restrictedopac} = C4::Koha::GetKohaAuthorisedValueLib( $code, $data->{restricted}, 1 );
-            $data->{restricted}     = C4::Koha::GetKohaAuthorisedValueLib( $code, $data->{restricted} );
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $data->{restricted} });
+            $av = $av->count ? $av->next : undef;
+            $data->{restricted}     = $av ? $av->lib : '';
+            $data->{restrictedopac} = $av ? $av->opac_description : '';
         }
 
         # my stack procedures
         if ( my $code = C4::Koha::GetAuthValCode( 'items.stack', $data->{frameworkcode} ) ) {
-            $data->{stack}          = C4::Koha::GetKohaAuthorisedValueLib( $code, $data->{stack} );
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $data->{stack} });
+            $data->{stack}          = $av->count ? $av->next->lib : '';
         }
 
         # Find the last 3 people who borrowed this item.
@@ -1477,8 +1484,10 @@ sub GetItemsLocationInfo {
         $sth->execute($biblionumber);
 
         while ( my $data = $sth->fetchrow_hashref ) {
-             $data->{location_intranet} = GetKohaAuthorisedValueLib('LOC', $data->{location});
-             $data->{location_opac}= GetKohaAuthorisedValueLib('LOC', $data->{location}, 1);
+             my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $data->{location} });
+             $av = $av->count ? $av->next : undef;
+             $data->{location_intranet} = $av ? $av->lib : '';
+             $data->{location_opac}     = $av ? $av->opac_description : '';
 	     push @results, $data;
 	}
 	return @results;
diff --git a/C4/Koha.pm b/C4/Koha.pm
index bb35811..8a00ca3 100644
--- a/C4/Koha.pm
+++ b/C4/Koha.pm
@@ -57,7 +57,6 @@ BEGIN {
 		&GetKohaAuthorisedValues
 		&GetKohaAuthorisedValuesFromField
     &GetKohaAuthorisedValuesMapping
-    &GetKohaAuthorisedValueLib
     &GetAuthorisedValueByCode
 		&GetAuthValCode
 		&GetNormalizedUPC
@@ -1228,27 +1227,6 @@ sub xml_escape {
     return $str;
 }
 
-=head2 GetKohaAuthorisedValueLib
-
-Takes $category, $authorised_value as parameters.
-
-If $opac parameter is set to a true value, displays OPAC descriptions rather than normal ones when they exist.
-
-Returns authorised value description
-
-=cut
-
-sub GetKohaAuthorisedValueLib {
-  my ($category,$authorised_value,$opac) = @_;
-  my $value;
-  my $dbh = C4::Context->dbh;
-  my $sth = $dbh->prepare("select lib, lib_opac from authorised_values where category=? and authorised_value=?");
-  $sth->execute($category,$authorised_value);
-  my $data = $sth->fetchrow_hashref;
-  $value = ($opac && $$data{'lib_opac'}) ? $$data{'lib_opac'} : $$data{'lib'};
-  return $value;
-}
-
 =head2 display_marc_indicators
 
   my $display_form = C4::Koha::display_marc_indicators($field);
diff --git a/C4/Search.pm b/C4/Search.pm
index d9a193c..4a1e85a 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -31,6 +31,7 @@ use C4::Branch;
 use C4::Reserves;    # GetReserveStatus
 use C4::Debug;
 use C4::Charset;
+use Koha::AuthorisedValues;
 use Koha::Libraries;
 use YAML;
 use URI::Escape;
@@ -575,9 +576,9 @@ sub getRecords {
 
                # also, if it's a location code, use the name instead of the code
                                 if ( $link_value =~ /location/ ) {
-                                    $facet_label_value =
-                                      GetKohaAuthorisedValueLib( 'LOC',
-                                        $one_facet, $opac );
+                                    # TODO Retrieve all authorised values at once, instead of 1 query per entry
+                                    my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $one_facet });
+                                    $facet_label_value = $av->count ? $av->next->opac_description : '';
                                 }
 
                 # but we're down with the whole label being in the link's title.
diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl
index a75a624..db615de 100755
--- a/acqui/orderreceive.pl
+++ b/acqui/orderreceive.pl
@@ -136,19 +136,24 @@ if ($AcqCreateItem eq 'receiving') {
             $item->{holdingbranchname} = GetBranchName($item->{holdingbranch});
         }
         if(my $code = GetAuthValCode("items.notforloan", $fw)) {
-            $item->{notforloan} = GetKohaAuthorisedValueLib($code, $item->{notforloan});
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $item->{notforloan} });
+            $item->{notforloan} = $av->count ? $av->next->lib : '';
         }
         if(my $code = GetAuthValCode("items.restricted", $fw)) {
-            $item->{restricted} = GetKohaAuthorisedValueLib($code, $item->{restricted});
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $item->{restricted} });
+            $item->{restricted} = $av->count ? $av->next->lib : '';
         }
         if(my $code = GetAuthValCode("items.location", $fw)) {
-            $item->{location} = GetKohaAuthorisedValueLib($code, $item->{location});
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $item->{location} });
+            $item->{location} = $av->count ? $av->next->lib : '';
         }
         if(my $code = GetAuthValCode("items.ccode", $fw)) {
-            $item->{collection} = GetKohaAuthorisedValueLib($code, $item->{ccode});
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $item->{collection} });
+            $item->{collection} = $av->count ? $av->next->lib : '';
         }
         if(my $code = GetAuthValCode("items.materials", $fw)) {
-            $item->{materials} = GetKohaAuthorisedValueLib($code, $item->{materials});
+            my $av = Koha::AuthorisedValues->search({ category => $code, authorised_value => $item->{materials} });
+            $item->{materials} = $av->count ? $av->next->lib : '';
         }
         my $itemtype = getitemtypeinfo($item->{itype});
         $item->{itemtype} = $itemtype->{description};
diff --git a/catalogue/getitem-ajax.pl b/catalogue/getitem-ajax.pl
index fd69a74..3274a46 100755
--- a/catalogue/getitem-ajax.pl
+++ b/catalogue/getitem-ajax.pl
@@ -28,6 +28,8 @@ use C4::Items;
 use C4::Koha;
 use C4::Output;
 
+use Koha::AuthorisedValues;
+
 my $cgi = new CGI;
 
 my ( $status, $cookie, $sessionID ) = C4::Auth::check_api_auth( $cgi, { acquisition => 'order_receive' } );
@@ -54,23 +56,28 @@ if($itemnumber) {
     }
 
     if(my $code = GetAuthValCode("items.notforloan", $fw)) {
-        $item->{notforloan} = GetKohaAuthorisedValueLib($code, $item->{notforloan});
+        my $av = Koha::AuthorisedValues->search({ category => $code, authorised_values => $item->{notforloan} });
+        $item->{notforloan} = $av->count ? $av->next->lib : '';
     }
 
     if(my $code = GetAuthValCode("items.restricted", $fw)) {
-        $item->{restricted} = GetKohaAuthorisedValueLib($code, $item->{restricted});
+        my $av = Koha::AuthorisedValues->search({ category => $code, authorised_values => $item->{restricted} });
+        $item->{restricted} = $av->count ? $av->next->lib : '';
     }
 
     if(my $code = GetAuthValCode("items.location", $fw)) {
-        $item->{location} = GetKohaAuthorisedValueLib($code, $item->{location});
+        my $av = Koha::AuthorisedValues->search({ category => $code, authorised_values => $item->{location} });
+        $item->{location} = $av->count ? $av->next->lib : '';
     }
 
     if(my $code = GetAuthValCode("items.ccode", $fw)) {
-        $item->{collection} = GetKohaAuthorisedValueLib($code, $item->{ccode});
+        my $av = Koha::AuthorisedValues->search({ category => $code, authorised_values => $item->{collection} });
+        $item->{collection} = $av->count ? $av->next->lib : '';
     }
 
     if(my $code = GetAuthValCode("items.materials", $fw)) {
-        $item->{materials} = GetKohaAuthorisedValueLib($code, $item->{materials});
+        my $av = Koha::AuthorisedValues->search({ category => $code, authorised_values => $item->{materials} });
+        $item->{materials} = $av->count ? $av->next->lib : '';
     }
 
     my $itemtype = getitemtypeinfo($item->{itype});
diff --git a/circ/branchtransfers.pl b/circ/branchtransfers.pl
index a826289..0601da1 100755
--- a/circ/branchtransfers.pl
+++ b/circ/branchtransfers.pl
@@ -33,6 +33,8 @@ use C4::Branch; # GetBranches
 use C4::Koha;
 use C4::Members;
 
+use Koha::AuthorisedValues;
+
 ###############################################
 #  Getting state
 
@@ -130,7 +132,8 @@ if ($barcode) {
         $item{'itemtype'}              = $iteminformation->{'itemtype'};
         $item{'ccode'}                 = $iteminformation->{'ccode'};
         $item{'itemcallnumber'}        = $iteminformation->{'itemcallnumber'};
-        $item{'location'}              = GetKohaAuthorisedValueLib("LOC",$iteminformation->{'location'});
+        my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $iteminformation->{location} });
+        $item{'location'}              = $av->count ? $av->next->lib : '';
         $item{'frbrname'}              = $branches->{$frbranchcd}->{'branchname'};
         $item{'tobrname'}              = $branches->{$tobranchcd}->{'branchname'};
 #         }
@@ -163,7 +166,8 @@ foreach ( $query->param ) {
     $item{'itemtype'}              = $iteminformation->{'itemtype'};
     $item{'ccode'}                 = $iteminformation->{'ccode'};
     $item{'itemcallnumber'}        = $iteminformation->{'itemcallnumber'};
-    $item{'location'}              = GetKohaAuthorisedValueLib("LOC",$iteminformation->{'location'});
+    my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $iteminformation->{location} });
+    $item{'location'}              = $av->count ? $av->next->lib : '';
     $item{'frbrname'}              = $branches->{$frbcd}->{'branchname'};
     $item{'tobrname'}              = $branches->{$tobcd}->{'branchname'};
     push( @trsfitemloop, \%item );
diff --git a/circ/circulation.pl b/circ/circulation.pl
index 867466e..3fc6829 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -43,6 +43,7 @@ use Koha::Holds;
 use C4::Context;
 use CGI::Session;
 use C4::Members::Attributes qw(GetBorrowerAttributes);
+use Koha::AuthorisedValues;
 use Koha::Patron;
 use Koha::Patron::Debarments qw(GetDebarments);
 use Koha::DateUtils;
@@ -389,7 +390,8 @@ if (@$barcodes) {
             my $materials = $iteminfo->{'materials'};
             my $avcode = GetAuthValCode('items.materials');
             if ($avcode) {
-                $materials = GetKohaAuthorisedValueLib($avcode, $materials);
+                my $av = Koha::AuthorisedValues->search({ category => $avcode, authorised_value => $materials });
+                $materials = $av->count ? $av->next->lib : '';
             }
             $template_params->{additional_materials} = $materials;
             $template_params->{itemhomebranch} = $iteminfo->{'homebranch'};
diff --git a/circ/returns.pl b/circ/returns.pl
index 418f94e..4bfa3d2 100755
--- a/circ/returns.pl
+++ b/circ/returns.pl
@@ -48,6 +48,7 @@ use C4::Members::Messaging;
 use C4::Branch; # GetBranches GetBranchName
 use C4::Koha;   # FIXME : is it still useful ?
 use C4::RotatingCollections;
+use Koha::AuthorisedValues;
 use Koha::DateUtils;
 use Koha::Calendar;
 
@@ -282,7 +283,8 @@ if ($barcode) {
     my $materials = $biblio->{'materials'};
     my $avcode = GetAuthValCode('items.materials');
     if ($avcode) {
-        $materials = GetKohaAuthorisedValueLib($avcode, $materials);
+        my $av = Koha::AuthorisedValues->search({ category => $avcode, authorised_value => $materials });
+        $materials = $av->count ? $av->next->lib : '';
     }
 
     $template->param(
diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl
index b183364..368c2fc 100755
--- a/opac/opac-suggestions.pl
+++ b/opac/opac-suggestions.pl
@@ -28,6 +28,8 @@ use C4::Output;
 use C4::Suggestions;
 use C4::Koha;
 use C4::Scrubber;
+
+use Koha::AuthorisedValues;
 use Koha::Libraries;
 
 use Koha::DateUtils qw( dt_from_string );
@@ -185,7 +187,8 @@ foreach my $suggestion(@$suggestions_loop) {
         $suggestion->{'showcheckbox'} = 0;
     }
     if($suggestion->{'patronreason'}){
-        $suggestion->{'patronreason'} = GetKohaAuthorisedValueLib("OPAC_SUG",$suggestion->{'patronreason'},1);
+        my $av = Koha::AuthorisedValues->search({ category => 'OPAC_SUG', authorised_value => $suggestion->{patronreason} });
+        $suggestion->{'patronreason'} = $av->count ? $av->next->opac_description : '';
     }
 }
 
diff --git a/serials/subscription-detail.pl b/serials/subscription-detail.pl
index 824ffc2..6a2fdbd 100755
--- a/serials/subscription-detail.pl
+++ b/serials/subscription-detail.pl
@@ -25,8 +25,9 @@ use C4::Serials;
 use C4::Output;
 use C4::Context;
 use C4::Search qw/enabled_staff_search_views/;
-use Koha::DateUtils;
 
+use Koha::AuthorisedValues;
+use Koha::DateUtils;
 use Koha::Acquisition::Bookseller;
 
 use Date::Calc qw/Today Day_of_Year Week_of_Year Add_Delta_Days/;
@@ -103,7 +104,8 @@ for my $date ( qw(startdate enddate firstacquidate histstartdate histenddate) )
     $subs->{$date} = output_pref( { str => $subs->{$date}, dateonly => 1 } )
         if $subs->{$date};
 }
-$subs->{location} = GetKohaAuthorisedValueLib("LOC",$subs->{location});
+my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $subs->{location} });
+$subs->{location} = $av->count ? $av->next->lib : '';
 $subs->{abouttoexpire}  = abouttoexpire($subs->{subscriptionid});
 $template->param(%{ $subs });
 $template->param(biblionumber_for_new_subscription => $subs->{bibnum});
-- 
2.8.1