Bug 6875 de-nesting C4::Items

C4::Branch is used only in CheckItemPresave, moving from a use to a require in the sub
C4::Reserve:
    This package is loaded just for C4::Reserves::CheckReserves called in C4::Items::GetItemsInfo
    The GetItemsInfo stores the result of CheckReserves in a hash entry, count_reserve, that is used only in opac_detail to display the status of a hold. We could remove the reserve_count hash entry and inline C4::Reserves::CheckReserves directly from opac-detail.pl page
    in opac-detail.pl, instead of
    if( $itm->{'count_reserves'} eq "Waiting"){ $itm->{'waiting'} = 1; }
    write :
    if ( C4::Reserves::CheckReserves(<<parameters>>) eq "Waiting"){ $itm->{'waiting'} = 1; }
C4::Acquisition is used only in MoveItemFromBiblio, a sub that is rarely called. Moving from a use to a require in the sub
C4::Charset is used only in _parse_unlinked_item_subfields_from_xml. Moving from a use to require in the sub

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Checked opac-detail and cataloging. Code looks good.

diff --git a/C4/Items.pm b/C4/Items.pm
index 9c85338..1107db9 100644 (file)
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -29,10 +29,6 @@ use C4::Dates qw/format_date format_date_in_iso/;
 use MARC::Record;
 use C4::ClassSource;
 use C4::Log;
-use C4::Branch;
-require C4::Reserves;
-use C4::Charset;
-use C4::Acquisition;
 use List::MoreUtils qw/any/;
 use Data::Dumper; # used as part of logging item record changes, not just for
                   # debugging; so please don't remove this
@@ -640,6 +636,7 @@ item that has a given branch code.
 
 sub CheckItemPreSave {
     my $item_ref = shift;
+    require C4::Branch;
 
     my %errors = ();
 
@@ -1210,7 +1207,6 @@ sub GetItemsInfo {
        my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=? "); 
        while ( my $data = $sth->fetchrow_hashref ) {
         my $datedue = '';
-        my $count_reserves;
         $isth->execute( $data->{'itemnumber'} );
         if ( my $idata = $isth->fetchrow_hashref ) {
             $data->{borrowernumber} = $idata->{borrowernumber};
@@ -1230,14 +1226,6 @@ sub GetItemsInfo {
                        $ssth->execute($data->{'itemnumber'}) ;
                        ($data->{'serialseq'} , $data->{'publisheddate'}) = $ssth->fetchrow_array();
                        $serial = 1;
-        }
-               if ( $datedue eq '' ) {
-            my ( $restype, $reserves, undef ) =
-              C4::Reserves::CheckReserves( $data->{'itemnumber'} );
-# Previous conditional check with if ($restype) is not needed because a true
-# result for one item will result in subsequent items defaulting to this true
-# value.
-            $count_reserves = $restype;
         }
         #get branch information.....
         my $bsth = $dbh->prepare(
@@ -1249,7 +1237,6 @@ sub GetItemsInfo {
             $data->{'branchname'} = $bdata->{'branchname'};
         }
         $data->{'datedue'}        = $datedue;
-        $data->{'count_reserves'} = $count_reserves;
 
         # get notforloan complete status if applicable
         my $sthnflstatus = $dbh->prepare(
@@ -2182,11 +2169,12 @@ sub MoveItemFromBiblio {
         ModZebra( $tobiblio, "specialUpdate", "biblioserver", undef, undef );
         ModZebra( $frombiblio, "specialUpdate", "biblioserver", undef, undef );
            # Checking if the item we want to move is in an order 
-        my $order = GetOrderFromItemnumber($itemnumber);
+        require C4::Acquisition;
+        my $order = C4::Acquisition::GetOrderFromItemnumber($itemnumber);
            if ($order) {
                    # Replacing the biblionumber within the order if necessary
                    $order->{'biblionumber'} = $tobiblio;
-               ModOrder($order);
+               C4::Acquisition::ModOrder($order);
            }
         return $tobiblio;
        }
@@ -2441,9 +2429,9 @@ sub _get_unlinked_subfields_xml {
 
 sub  _parse_unlinked_item_subfields_from_xml {
     my $xml = shift;
-
+    require C4::Charset;
     return unless defined $xml and $xml ne "";
-    my $marc = MARC::Record->new_from_xml(StripNonXmlChars($xml),'UTF-8');
+    my $marc = MARC::Record->new_from_xml(C4::Charset::StripNonXmlChars($xml),'UTF-8');
     my $unlinked_subfields = [];
     my @fields = $marc->fields();
     if ($#fields > -1) {

diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index dd41bbb..b5a25a8 100755 (executable)
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -41,6 +41,7 @@ use C4::Members;
 use C4::VirtualShelves;
 use C4::XSLT;
 use C4::ShelfBrowser;
+use C4::Reserves;
 use C4::Charset;
 use MARC::Record;
 use MARC::Field;
@@ -509,11 +510,9 @@ for my $itm (@items) {
          $itm->{'lostimageurl'}   = $lostimageinfo->{ 'imageurl' };
          $itm->{'lostimagelabel'} = $lostimageinfo->{ 'label' };
      }
-
-     if( $itm->{'count_reserves'}){
-          if( $itm->{'count_reserves'} eq "Waiting"){ $itm->{'waiting'} = 1; }
-          if( $itm->{'count_reserves'} eq "Reserved"){ $itm->{'onhold'} = 1; }
-     }
+     my ($reserve_status) = C4::Reserves::CheckReserves($itm->{itemnumber});
+      if( $reserve_status eq "Waiting"){ $itm->{'waiting'} = 1; }
+      if( $reserve_status eq "Reserved"){ $itm->{'onhold'} = 1; }
     
      my ( $transfertwhen, $transfertfrom, $transfertto ) = GetTransfers($itm->{itemnumber});
      if ( defined( $transfertwhen ) && $transfertwhen ne '' ) {