Bug 15494: (QA follow-up) Additional polishing
[koha.git] / C4 / Items.pm
index a3ecd87..ec2c0a4 100644 (file)
@@ -21,81 +21,66 @@ package C4::Items;
 use strict;
 #use warnings; FIXME - Bug 2505
 
-use Carp;
-use C4::Context;
-use C4::Koha;
-use C4::Biblio;
-use Koha::DateUtils;
-use MARC::Record;
-use C4::ClassSource;
-use C4::Log;
-use List::MoreUtils qw/any/;
-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;
-
-use Koha::Biblioitems;
-use Koha::Items;
-use Koha::ItemTypes;
-use Koha::SearchEngine;
-use Koha::SearchEngine::Search;
-use Koha::Libraries;
-
 use vars qw(@ISA @EXPORT);
-
 BEGIN {
+    require Exporter;
+    @ISA = qw(Exporter);
 
-       require Exporter;
-    @ISA = qw( Exporter );
-
-    # function exports
     @EXPORT = qw(
         GetItem
         AddItemFromMarc
         AddItem
         AddItemBatchFromMarc
         ModItemFromMarc
-    Item2Marc
+        Item2Marc
         ModItem
         ModDateLastSeen
         ModItemTransfer
         DelItem
-    
         CheckItemPreSave
-    
         GetItemsForInventory
-        GetItemsByBiblioitemnumber
         GetItemsInfo
-       GetItemsLocationInfo
-       GetHostItemsInfo
-        GetItemnumbersForBiblio
-       get_hostitemnumbers_of
-        GetItemnumberFromBarcode
-        GetBarcodeFromItemnumber
+        GetItemsLocationInfo
+        GetHostItemsInfo
+        get_hostitemnumbers_of
         GetHiddenItemnumbers
         ItemSafeToDelete
         DelItemCheck
-    MoveItemFromBiblio
-    GetLatestAcquisitions
-
+        MoveItemFromBiblio
         CartToShelf
         ShelfToCart
-
-       GetAnalyticsCount
-
+        GetAnalyticsCount
         SearchItemsByField
         SearchItems
-
         PrepareItemrecordDisplay
-
     );
 }
 
+use Carp;
+use C4::Context;
+use C4::Koha;
+use C4::Biblio;
+use Koha::DateUtils;
+use MARC::Record;
+use C4::ClassSource;
+use C4::Log;
+use List::MoreUtils qw(any);
+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;
+
+use Koha::Biblioitems;
+use Koha::Items;
+use Koha::ItemTypes;
+use Koha::SearchEngine;
+use Koha::SearchEngine::Search;
+use Koha::Libraries;
+
 =head1 NAME
 
 C4::Items - item management functions
@@ -237,10 +222,10 @@ sub AddItemFromMarc {
     # parse item hash from MARC
     my $frameworkcode = C4::Biblio::GetFrameworkCode( $biblionumber );
     my ($itemtag,$itemsubfield)=C4::Biblio::GetMarcFromKohaField("items.itemnumber",$frameworkcode);
-       
-       my $localitemmarc=MARC::Record->new;
-       $localitemmarc->append_fields($source_item_marc->field($itemtag));
-    my $item = &TransformMarcToKoha( $localitemmarc, $frameworkcode ,'items');
+
+    my $localitemmarc=MARC::Record->new;
+    $localitemmarc->append_fields($source_item_marc->field($itemtag));
+    my $item = TransformMarcToKoha( $localitemmarc, $frameworkcode ,'items');
     my $unlinked_item_subfields = _get_unlinked_item_subfields($localitemmarc, $frameworkcode);
     return AddItem($item, $biblionumber, $dbh, $frameworkcode, $unlinked_item_subfields);
 }
@@ -497,13 +482,12 @@ sub ModItemFromMarc {
     my $biblionumber = shift;
     my $itemnumber = shift;
 
-    my $dbh           = C4::Context->dbh;
     my $frameworkcode = C4::Biblio::GetFrameworkCode($biblionumber);
     my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber", $frameworkcode );
 
     my $localitemmarc = MARC::Record->new;
     $localitemmarc->append_fields( $item_marc->field($itemtag) );
-    my $item = &TransformMarcToKoha( $localitemmarc, $frameworkcode, 'items' );
+    my $item = TransformMarcToKoha( $localitemmarc, $frameworkcode, 'items' );
     my $default_values = _build_default_values_for_mod_marc();
     foreach my $item_field ( keys %$default_values ) {
         $item->{$item_field} = $default_values->{$item_field}
@@ -511,13 +495,21 @@ sub ModItemFromMarc {
     }
     my $unlinked_item_subfields = _get_unlinked_item_subfields( $localitemmarc, $frameworkcode );
 
-    ModItem($item, $biblionumber, $itemnumber, $dbh, $frameworkcode, $unlinked_item_subfields); 
+    ModItem( $item, $biblionumber, $itemnumber, { unlinked_item_subfields => $unlinked_item_subfields } );
     return $item;
 }
 
 =head2 ModItem
 
-  ModItem({ column => $newvalue }, $biblionumber, $itemnumber);
+ModItem(
+    { column => $newvalue },
+    $biblionumber,
+    $itemnumber,
+    {
+        [ unlinked_item_subfields => $unlinked_item_subfields, ]
+        [ log_action => 1, ]
+    }
+);
 
 Change one or more columns in an item record and update
 the MARC representation of the item.
@@ -525,9 +517,11 @@ the MARC representation of the item.
 The first argument is a hashref mapping from item column
 names to the new values.  The second and third arguments
 are the biblionumber and itemnumber, respectively.
+The fourth, optional parameter (additional_params) may contain the keys
+unlinked_item_subfields and log_action.
 
-The fourth, optional parameter, C<$unlinked_item_subfields>, contains
-an arrayref containing subfields present in the original MARC
+C<$unlinked_item_subfields> contains an arrayref containing
+subfields present in the original MARC
 representation of the item (e.g., from the item editor) that are
 not mapped to C<items> columns directly but should instead
 be stored in C<items.more_subfields_xml> and included in 
@@ -538,29 +532,28 @@ the derived value of a column such as C<items.cn_sort>,
 this routine will perform the necessary calculation
 and set the value.
 
+If log_action is set to false, the action will not be logged.
+If log_action is true or undefined, the action will be logged.
+
 =cut
 
 sub ModItem {
-    my $item = shift;
-    my $biblionumber = shift;
-    my $itemnumber = shift;
+    my ( $item, $biblionumber, $itemnumber, $additional_params ) = @_;
+    my $log_action = $additional_params->{log_action} // 1;
+    my $unlinked_item_subfields = $additional_params->{unlinked_item_subfields};
+
+    return unless %$item;
+    $item->{'itemnumber'} = $itemnumber or return;
 
     # if $biblionumber is undefined, get it from the current item
     unless (defined $biblionumber) {
         $biblionumber = _get_single_item_column('biblionumber', $itemnumber);
     }
 
-    my $dbh           = @_ ? shift : C4::Context->dbh;
-    my $frameworkcode = @_ ? shift : C4::Biblio::GetFrameworkCode( $biblionumber );
-    
-    my $unlinked_item_subfields;  
-    if (@_) {
-        $unlinked_item_subfields = shift;
+    if ($unlinked_item_subfields) {
         $item->{'more_subfields_xml'} = _get_unlinked_subfields_xml($unlinked_item_subfields);
     };
 
-    $item->{'itemnumber'} = $itemnumber or return;
-
     my @fields = qw( itemlost withdrawn damaged );
 
     # Only call GetItem if we need to set an "on" date field
@@ -602,7 +595,8 @@ sub ModItem {
     # item status is possible
     ModZebra( $biblionumber, "specialUpdate", "biblioserver" );
 
-    logaction("CATALOGUING", "MODIFY", $itemnumber, "item ".Dumper($item)) if C4::Context->preference("CataloguingLog");
+    logaction( "CATALOGUING", "MODIFY", $itemnumber, "item " . Dumper($item) )
+      if $log_action && C4::Context->preference("CataloguingLog");
 }
 
 =head2 ModItemTransfer
@@ -637,18 +631,24 @@ sub ModItemTransfer {
 
 =head2 ModDateLastSeen
 
-  ModDateLastSeen($itemnum);
+ModDateLastSeen( $itemnumber, $leave_item_lost );
 
 Mark item as seen. Is called when an item is issued, returned or manually marked during inventory/stocktaking.
-C<$itemnum> is the item number
+C<$itemnumber> is the item number
+C<$leave_item_lost> determines if a lost item will be found or remain lost
 
 =cut
 
 sub ModDateLastSeen {
-    my ($itemnumber) = @_;
-    
+    my ( $itemnumber, $leave_item_lost ) = @_;
+
     my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 });
-    ModItem({ itemlost => 0, datelastseen => $today }, undef, $itemnumber);
+
+    my $params;
+    $params->{datelastseen} = $today;
+    $params->{itemlost} = 0 unless $leave_item_lost;
+
+    ModItem( $params, undef, $itemnumber, { log_action => 0 } );
 }
 
 =head2 DelItem
@@ -732,10 +732,10 @@ sub CheckItemPreSave {
 
     # check for duplicate barcode
     if (exists $item_ref->{'barcode'} and defined $item_ref->{'barcode'}) {
-        my $existing_itemnumber = GetItemnumberFromBarcode($item_ref->{'barcode'});
-        if ($existing_itemnumber) {
+        my $existing_item= Koha::Items->find({barcode => $item_ref->{'barcode'}});
+        if ($existing_item) {
             if (!exists $item_ref->{'itemnumber'}                       # new item
-                or $item_ref->{'itemnumber'} != $existing_itemnumber) { # existing item
+                or $item_ref->{'itemnumber'} != $existing_item->itemnumber) { # existing item
                 $errors{'duplicate_barcode'} = $item_ref->{'barcode'};
             }
         }
@@ -767,11 +767,6 @@ The following functions provide various ways of
 getting an item record, a set of item records, or
 lists of authorized values for certain item fields.
 
-Some of the functions in this group are candidates
-for refactoring -- for example, some of the code
-in C<GetItemsByBiblioitemnumber> and C<GetItemsInfo>
-has copy-and-paste work.
-
 =cut
 
 =head2 GetItemsForInventory
@@ -924,58 +919,6 @@ sub GetItemsForInventory {
     return (\@results, $iTotalRecords);
 }
 
-=head2 GetItemsByBiblioitemnumber
-
-  GetItemsByBiblioitemnumber($biblioitemnumber);
-
-Returns an arrayref of hashrefs suitable for use in a TMPL_LOOP
-Called by C<C4::XISBN>
-
-=cut
-
-sub GetItemsByBiblioitemnumber {
-    my ( $bibitem ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare("SELECT * FROM items WHERE items.biblioitemnumber = ?") || die $dbh->errstr;
-    # Get all items attached to a biblioitem
-    my $i = 0;
-    my @results; 
-    $sth->execute($bibitem) || die $sth->errstr;
-    while ( my $data = $sth->fetchrow_hashref ) {  
-        # Foreach item, get circulation information
-        my $sth2 = $dbh->prepare( "SELECT * FROM issues,borrowers
-                                   WHERE itemnumber = ?
-                                   AND issues.borrowernumber = borrowers.borrowernumber"
-        );
-        $sth2->execute( $data->{'itemnumber'} );
-        if ( my $data2 = $sth2->fetchrow_hashref ) {
-            # if item is out, set the due date and who it is out too
-            $data->{'date_due'}   = $data2->{'date_due'};
-            $data->{'cardnumber'} = $data2->{'cardnumber'};
-            $data->{'borrowernumber'}   = $data2->{'borrowernumber'};
-        }
-        else {
-            # set date_due to blank, so in the template we check itemlost, and withdrawn
-            $data->{'date_due'} = '';                                                                                                         
-        }    # else         
-        # Find the last 3 people who borrowed this item.                  
-        my $query2 = "SELECT * FROM old_issues, borrowers WHERE itemnumber = ?
-                      AND old_issues.borrowernumber = borrowers.borrowernumber
-                      ORDER BY returndate desc,timestamp desc LIMIT 3";
-        $sth2 = $dbh->prepare($query2) || die $dbh->errstr;
-        $sth2->execute( $data->{'itemnumber'} ) || die $sth2->errstr;
-        my $i2 = 0;
-        while ( my $data2 = $sth2->fetchrow_hashref ) {
-            $data->{"timestamp$i2"} = $data2->{'timestamp'};
-            $data->{"card$i2"}      = $data2->{'cardnumber'};
-            $data->{"borrower$i2"}  = $data2->{'borrowernumber'};
-            $i2++;
-        }
-        push(@results,$data);
-    } 
-    return (\@results); 
-}
-
 =head2 GetItemsInfo
 
   @results = GetItemsInfo($biblionumber);
@@ -1198,114 +1141,39 @@ sub GetItemsLocationInfo {
 
 =head2 GetHostItemsInfo
 
-       $hostiteminfo = GetHostItemsInfo($hostfield);
-       Returns the iteminfo for items linked to records via a host field
+    $hostiteminfo = GetHostItemsInfo($hostfield);
+    Returns the iteminfo for items linked to records via a host field
 
 =cut
 
 sub GetHostItemsInfo {
-       my ($record) = @_;
-       my @returnitemsInfo;
-
-       if (C4::Context->preference('marcflavour') eq 'MARC21' ||
-        C4::Context->preference('marcflavour') eq 'NORMARC'){
-           foreach my $hostfield ( $record->field('773') ) {
-               my $hostbiblionumber = $hostfield->subfield("0");
-               my $linkeditemnumber = $hostfield->subfield("9");
-               my @hostitemInfos = GetItemsInfo($hostbiblionumber);
-               foreach my $hostitemInfo (@hostitemInfos){
-                       if ($hostitemInfo->{itemnumber} eq $linkeditemnumber){
-                               push (@returnitemsInfo,$hostitemInfo);
-                               last;
-                       }
-               }
-           }
-       } elsif ( C4::Context->preference('marcflavour') eq 'UNIMARC'){
-           foreach my $hostfield ( $record->field('461') ) {
-               my $hostbiblionumber = $hostfield->subfield("0");
-               my $linkeditemnumber = $hostfield->subfield("9");
-               my @hostitemInfos = GetItemsInfo($hostbiblionumber);
-               foreach my $hostitemInfo (@hostitemInfos){
-                       if ($hostitemInfo->{itemnumber} eq $linkeditemnumber){
-                               push (@returnitemsInfo,$hostitemInfo);
-                               last;
-                       }
-               }
-           }
-       }
-       return @returnitemsInfo;
-}
-
-
-=head2 GetLastAcquisitions
-
-  my $lastacq = GetLastAcquisitions({'branches' => ('branch1','branch2'), 
-                                    'itemtypes' => ('BK','BD')}, 10);
-
-=cut
-
-sub  GetLastAcquisitions {
-       my ($data,$max) = @_;
-
-       my $itemtype = C4::Context->preference('item-level_itypes') ? 'itype' : 'itemtype';
-       
-       my $number_of_branches = @{$data->{branches}};
-       my $number_of_itemtypes   = @{$data->{itemtypes}};
-       
-       
-       my @where = ('WHERE 1 '); 
-       $number_of_branches and push @where
-          , 'AND holdingbranch IN (' 
-          , join(',', ('?') x $number_of_branches )
-          , ')'
-        ;
-       
-       $number_of_itemtypes and push @where
-          , "AND $itemtype IN (" 
-          , join(',', ('?') x $number_of_itemtypes )
-          , ')'
-        ;
-
-       my $query = "SELECT biblio.biblionumber as biblionumber, title, dateaccessioned
-                                FROM items RIGHT JOIN biblio ON (items.biblionumber=biblio.biblionumber) 
-                                   RIGHT JOIN biblioitems ON (items.biblioitemnumber=biblioitems.biblioitemnumber)
-                                   @where
-                                   GROUP BY biblio.biblionumber 
-                                   ORDER BY dateaccessioned DESC LIMIT $max";
-
-       my $dbh = C4::Context->dbh;
-       my $sth = $dbh->prepare($query);
-    
-    $sth->execute((@{$data->{branches}}, @{$data->{itemtypes}}));
-       
-       my @results;
-       while( my $row = $sth->fetchrow_hashref){
-               push @results, {date => $row->{dateaccessioned} 
-                                               , biblionumber => $row->{biblionumber}
-                                               , title => $row->{title}};
-       }
-       
-       return @results;
-}
-
-=head2 GetItemnumbersForBiblio
+    my ($record) = @_;
+    my @returnitemsInfo;
 
-  my $itemnumbers = GetItemnumbersForBiblio($biblionumber);
-
-Given a single biblionumber, return an arrayref of all the corresponding itemnumbers
+    if( !C4::Context->preference('EasyAnalyticalRecords') ) {
+        return @returnitemsInfo;
+    }
 
-=cut
+    my @fields;
+    if( C4::Context->preference('marcflavour') eq 'MARC21' ||
+      C4::Context->preference('marcflavour') eq 'NORMARC') {
+        @fields = $record->field('773');
+    } elsif( C4::Context->preference('marcflavour') eq 'UNIMARC') {
+        @fields = $record->field('461');
+    }
 
-sub GetItemnumbersForBiblio {
-    my $biblionumber = shift;
-    my @items;
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber = ?");
-    $sth->execute($biblionumber);
-    while (my $result = $sth->fetchrow_hashref) {
-        push @items, $result->{'itemnumber'};
+    foreach my $hostfield ( @fields ) {
+        my $hostbiblionumber = $hostfield->subfield("0");
+        my $linkeditemnumber = $hostfield->subfield("9");
+        my @hostitemInfos = GetItemsInfo($hostbiblionumber);
+        foreach my $hostitemInfo (@hostitemInfos) {
+            if( $hostitemInfo->{itemnumber} eq $linkeditemnumber ) {
+                push @returnitemsInfo, $hostitemInfo;
+                last;
+            }
+        }
     }
-    return \@items;
+    return @returnitemsInfo;
 }
 
 =head2 get_hostitemnumbers_of
@@ -1356,53 +1224,27 @@ sub get_hostitemnumbers_of {
     return @returnhostitemnumbers;
 }
 
-
-=head2 GetItemnumberFromBarcode
-
-  $result = GetItemnumberFromBarcode($barcode);
-
-=cut
-
-sub GetItemnumberFromBarcode {
-    my ($barcode) = @_;
-    my $dbh = C4::Context->dbh;
-
-    my $rq =
-      $dbh->prepare("SELECT itemnumber FROM items WHERE items.barcode=?");
-    $rq->execute($barcode);
-    my ($result) = $rq->fetchrow;
-    return ($result);
-}
-
-=head2 GetBarcodeFromItemnumber
-
-  $result = GetBarcodeFromItemnumber($itemnumber);
-
-=cut
-
-sub GetBarcodeFromItemnumber {
-    my ($itemnumber) = @_;
-    my $dbh = C4::Context->dbh;
-
-    my $rq =
-      $dbh->prepare("SELECT barcode FROM items WHERE items.itemnumber=?");
-    $rq->execute($itemnumber);
-    my ($result) = $rq->fetchrow;
-    return ($result);
-}
-
 =head2 GetHiddenItemnumbers
 
-    my @itemnumbers_to_hide = GetHiddenItemnumbers(@items);
+    my @itemnumbers_to_hide = GetHiddenItemnumbers({ items => \@items, borcat => $category });
 
 Given a list of items it checks which should be hidden from the OPAC given
 the current configuration. Returns a list of itemnumbers corresponding to
-those that should be hidden.
+those that should be hidden. Optionally takes a borcat parameter for certain borrower types
+to be excluded
 
 =cut
 
 sub GetHiddenItemnumbers {
-    my (@items) = @_;
+    my $params = shift;
+    my $items = $params->{items};
+    if (my $exceptions = C4::Context->preference('OpacHiddenItemsExceptions') and $params->{'borcat'}){
+        foreach my $except (split(/\|/, $exceptions)){
+            if ($params->{'borcat'} eq $except){
+                return; # we don't hide anything for this borrower category
+            }
+        }
+    }
     my @resultitems;
 
     my $yaml = C4::Context->preference('OpacHiddenItems');
@@ -1419,7 +1261,7 @@ sub GetHiddenItemnumbers {
     my $dbh = C4::Context->dbh;
 
     # For each item
-    foreach my $item (@items) {
+    foreach my $item (@$items) {
 
         # We check each rule
         foreach my $field (keys %$hidingrules) {
@@ -2558,7 +2400,7 @@ sub PrepareItemrecordDisplay {
     # Note: $tagslib obtained from GetMarcStructure() in 'unsafe' mode is
     # a shared data structure. No plugin (including custom ones) should change
     # its contents. See also GetMarcStructure.
-    my $tagslib = &GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } );
+    my $tagslib = GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } );
 
     # return nothing if we don't have found an existing framework.
     return q{} unless $tagslib;