Bug 28290: Don't send subfields to 'as_string' if none to send
[koha-ffzg.git] / C4 / Circulation.pm
index 9cb56f7..58ceeae 100644 (file)
@@ -24,7 +24,6 @@ use POSIX qw( floor );
 use YAML::XS;
 use Encode;
 
-use Koha::DateUtils qw( dt_from_string output_pref );
 use C4::Context;
 use C4::Stats qw( UpdateStats );
 use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest ItemsAnyAvailableAndNotRestricted );
@@ -43,7 +42,7 @@ use Koha::Account;
 use Koha::AuthorisedValues;
 use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue;
 use Koha::Biblioitems;
-use Koha::DateUtils qw( dt_from_string output_pref );
+use Koha::DateUtils qw( dt_from_string );
 use Koha::Calendar;
 use Koha::Checkouts;
 use Koha::Illrequests;
@@ -96,7 +95,6 @@ BEGIN {
       GetBranchBorrowerCircRule
       GetBranchItemRule
       GetBiblioIssues
-      GetOpenIssue
       GetUpcomingDueIssues
       CheckIfIssuedToPatron
       IsItemIssued
@@ -108,7 +106,6 @@ BEGIN {
 
       transferbook
       TooMany
-      GetTransfers
       GetTransfersFromTo
       updateWrongTransfer
       CalcDateDue
@@ -124,6 +121,7 @@ BEGIN {
       DeleteOfflineOperation
       ProcessOfflineOperation
       ProcessOfflinePayment
+      ProcessOfflineIssue
     );
     push @EXPORT_OK, '_GetCircControlBranch';    # This is wrong!
 }
@@ -798,7 +796,7 @@ sub CanBookBeIssued {
     my $now = dt_from_string();
     $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
     if (DateTime->compare($duedate,$now) == -1 ) { # duedate cannot be before now
-         $needsconfirmation{INVALID_DATE} = output_pref($duedate);
+         $needsconfirmation{INVALID_DATE} = $duedate;
     }
 
     my $fees = Koha::Charges::Fees->new(
@@ -1235,19 +1233,16 @@ sub CanBookBeIssued {
         my $check = checkHighHolds( $item_object, $patron );
 
         if ( $check->{exceeded} ) {
+            my $highholds = {
+                num_holds  => $check->{outstanding},
+                duration   => $check->{duration},
+                returndate => $check->{due_date},
+            };
             if ($override_high_holds) {
-                $alerts{HIGHHOLDS} = {
-                    num_holds  => $check->{outstanding},
-                    duration   => $check->{duration},
-                    returndate => output_pref( { dt => dt_from_string($check->{due_date}), dateformat => 'iso', timeformat => '24hr' }),
-                };
+                $alerts{HIGHHOLDS} = $highholds;
             }
             else {
-                $needsconfirmation{HIGHHOLDS} = {
-                    num_holds  => $check->{outstanding},
-                    duration   => $check->{duration},
-                    returndate => output_pref( { dt => dt_from_string($check->{due_date}), dateformat => 'iso', timeformat => '24hr' }),
-                };
+                $needsconfirmation{HIGHHOLDS} = $highholds;
             }
         }
     }
@@ -1462,8 +1457,8 @@ Calculated if empty.
 
 =item C<$cancelreserve> is 1 to override and cancel any pending reserves for the item (optional).
 
-=item C<$issuedate> is the date to issue the item in iso (YYYY-MM-DD) format (optional).
-Defaults to today.  Unlike C<$datedue>, NOT a DateTime object, unfortunately.
+=item C<$issuedate> is a DateTime object for the date to issue the item (optional).
+Defaults to today.
 
 AddIssue does the following things :
 
@@ -1635,8 +1630,8 @@ sub AddIssue {
 
             my $issue_attributes = {
                 borrowernumber  => $borrower->{'borrowernumber'},
-                issuedate       => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
-                date_due        => $datedue->strftime('%Y-%m-%d %H:%M:%S'),
+                issuedate       => $issuedate,
+                date_due        => $datedue,
                 branchcode      => C4::Context->userenv->{'branch'},
                 onsite_checkout => $onsite_checkout,
                 auto_renew      => $auto_renew ? 1 : 0,
@@ -2794,28 +2789,6 @@ sub _GetCircControlBranch {
     return $branch;
 }
 
-=head2 GetOpenIssue
-
-  $issue = GetOpenIssue( $itemnumber );
-
-Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued
-
-C<$itemnumber> is the item's itemnumber
-
-Returns a hashref
-
-=cut
-
-sub GetOpenIssue {
-  my ( $itemnumber ) = @_;
-  return unless $itemnumber;
-  my $dbh = C4::Context->dbh;  
-  my $sth = $dbh->prepare( "SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL" );
-  $sth->execute( $itemnumber );
-  return $sth->fetchrow_hashref();
-
-}
-
 =head2 GetUpcomingDueIssues
 
   my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
@@ -2949,87 +2922,59 @@ sub CanBookBeRenewed {
         }
     }
 
-    # Note: possible_reserves will contain all title level holds on this bib and item level
-    # holds on the checked out item
-    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
-
-    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
-    if ( $resfound && $resrec->{non_priority} ) {
-        $resfound = Koha::Holds->search(
-            { biblionumber => $resrec->{biblionumber}, non_priority => 0 } )
-          ->count > 0;
-    }
-
-
-
-    # This item can fill one or more unfilled reserve, can those unfilled reserves
-    # all be filled by other available items?
-    if ( $resfound
-        && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') )
-    {
-        my $item_holds = Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count();
-        if ($item_holds) {
-            # There is an item level hold on this item, no other item can fill the hold
-            $resfound = 1;
-        }
-        else {
+    my $fillable_holds = Koha::Holds->search(
+        {
+            biblionumber => $item->biblionumber,
+            non_priority => 0,
+            found        => undef,
+            reservedate  => { '<=' => \'NOW()' },
+            suspend      => 0,
+        },
+        { prefetch => 'patron' }
+    );
+    if ( $fillable_holds->count ) {
+        if ( C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) {
+            my @possible_holds = $fillable_holds->as_list;
 
             # Get all other items that could possibly fill reserves
             # FIXME We could join reserves (or more tables) here to eliminate some checks later
-            my $items = Koha::Items->search({
-                biblionumber => $resrec->{biblionumber},
+            my @other_items = Koha::Items->search({
+                biblionumber => $item->biblionumber,
                 onloan       => undef,
                 notforloan   => 0,
-                -not         => { itemnumber => $itemnumber }
-            });
-            my $item_count = $items->count();
-
-            # Get all other reserves that could have been filled by this item
-            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
-            # Note: fetching the patrons in this manner means that a patron with 2 holds will
-            # not block renewal if one reserve can be satisfied i.e. each patron is checked once
-            my $patrons = Koha::Patrons->search({
-                borrowernumber => { -in => \@borrowernumbers }
-            });
-            my $patron_count = $patrons->count();
+                -not         => { itemnumber => $itemnumber } })->as_list;
 
-            return ( 0, "on_reserve" ) if ($patron_count > $item_count);
-            # We cannot possibly fill all reserves if we don't have enough items
+            return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds);
 
-            # If we can fill each hold that has been found with the available items on the record
-            # then the patron can renew. If we cannot, they cannot renew.
-            # FIXME This code does not check whether the item we are renewing can fill
-            # any of the existing reserves.
-            my $reservable = 0;
             my %matched_items;
-            my $seen = 0;
-            PATRON: while ( my $patron = $patrons->next ) {
-                # If there is a reserve that cannot be filled we are done
-                return ( 0, "on_reserve" ) if ( $seen > $reservable );
-                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
-                while ( my $other_item = $items->next ) {
-                    next if defined $matched_items{$other_item->itemnumber};
-                    next if IsItemOnHoldAndFound( $other_item->itemnumber );
-                    next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available);
-                    next unless CanItemBeReserved($patron,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
-                    # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
-                    # CanItemBeReserved checks 'rules' and 'policies' which means
-                    # items will fill holds at checkin that are rejected here
-                    $reservable++;
-                    if ($reservable >= $patron_count) {
-                        $resfound = 0;
-                        last PATRON;
-                    }
-                    $matched_items{$other_item->itemnumber} = 1;
-                    last;
+            foreach my $possible_hold (@possible_holds) {
+                my $fillable = 0;
+                my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);
+                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron_with_reserve });
+
+                # FIXME: We are not checking whether the item we are renewing can fill the hold
+
+                foreach my $other_item (@other_items) {
+                  next if defined $matched_items{$other_item->itemnumber};
+                  next if IsItemOnHoldAndFound( $other_item->itemnumber );
+                  next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef, $items_any_available);
+                  next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
+                  # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
+                  # CanItemBeReserved checks 'rules' and 'policies' which means
+                  # items will fill holds at checkin that are rejected here
+                  $fillable = 1;
+                  $matched_items{$other_item->itemnumber} = 1;
+                  last;
                 }
-                $items->reset;
-                $seen++;
+                return ( 0, "on_reserve" ) unless $fillable;
             }
+
+        } else {
+            my ($status, $matched_reserve, $possible_reserves) = CheckReserves($itemnumber);
+            return ( 0, "on_reserve" ) if $status;
         }
     }
 
-    return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
     return ( 0, $auto_renew, { soonest_renew_date => $soonest } ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
     $soonest = GetSoonestRenewDate($borrowernumber, $itemnumber);
     if ( $soonest > dt_from_string() ){
@@ -3098,8 +3043,7 @@ sub AddRenewal {
     $borrowernumber ||= $issue->borrowernumber;
 
     if ( defined $datedue && ref $datedue ne 'DateTime' ) {
-        carp 'Invalid date passed to AddRenewal.';
-        return;
+        $datedue = dt_from_string($datedue, 'sql');
     }
 
     my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
@@ -3543,35 +3487,6 @@ sub AddIssuingCharge {
     );
 }
 
-=head2 GetTransfers
-
-  GetTransfers($itemnumber);
-
-=cut
-
-sub GetTransfers {
-    my ($itemnumber) = @_;
-
-    my $dbh = C4::Context->dbh;
-
-    my $query = '
-        SELECT datesent,
-               frombranch,
-               tobranch,
-               branchtransfer_id,
-               daterequested,
-               reason
-        FROM branchtransfers
-        WHERE itemnumber = ?
-          AND datearrived IS NULL
-          AND datecancelled IS NULL
-        ';
-    my $sth = $dbh->prepare($query);
-    $sth->execute($itemnumber);
-    my @row = $sth->fetchrow_array();
-    return @row;
-}
-
 =head2 GetTransfersFromTo
 
   @results = GetTransfersFromTo($frombranch,$tobranch);
@@ -4073,12 +3988,12 @@ sub ProcessOfflineReturn {
 
     if ( $item ) {
         my $itemnumber = $item->itemnumber;
-        my $issue = GetOpenIssue( $itemnumber );
+        my $issue = $item->checkout;
         if ( $issue ) {
             my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
             ModDateLastSeen( $itemnumber, $leave_item_lost );
             MarkIssueReturned(
-                $issue->{borrowernumber},
+                $issue->borrowernumber,
                 $itemnumber,
                 $operation->{timestamp},
             );
@@ -4105,11 +4020,11 @@ sub ProcessOfflineIssue {
             return "Barcode not found.";
         }
         my $itemnumber = $item->itemnumber;
-        my $issue = GetOpenIssue( $itemnumber );
+        my $issue = $item->checkout;
 
-        if ( $issue and ( $issue->{borrowernumber} ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned
+        if ( $issue and ( $issue->borrowernumber ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned
             MarkIssueReturned(
-                $issue->{borrowernumber},
+                $issue->borrowernumber,
                 $itemnumber,
                 $operation->{timestamp},
             );
@@ -4423,7 +4338,7 @@ sub _CalculateAndUpdateFine {
                 itemnumber     => $issue->itemnumber,
                 borrowernumber => $issue->borrowernumber,
                 amount         => $amount,
-                due            => output_pref($datedue),
+                due            => $datedue,
             });
         }
         elsif ($return_date) {
@@ -4436,7 +4351,7 @@ sub _CalculateAndUpdateFine {
                 itemnumber     => $issue->itemnumber,
                 borrowernumber => $issue->borrowernumber,
                 amount         => 0,
-                due            => output_pref($datedue),
+                due            => $datedue,
             });
         }
     }