Bug 18242: [SOLUTION 2]Handle correctly move to old_issues
[koha_ffzg] / C4 / Circulation.pm
index 9e085ef..2dcb1e9 100644 (file)
@@ -43,6 +43,7 @@ use Koha::Account;
 use Koha::AuthorisedValues;
 use Koha::DateUtils;
 use Koha::Calendar;
+use Koha::Checkouts;
 use Koha::IssuingRules;
 use Koha::Items;
 use Koha::Patrons;
@@ -88,13 +89,11 @@ BEGIN {
         &GetSoonestRenewDate
         &GetLatestAutoRenewDate
                &GetItemIssue
-               &GetItemIssues
                &GetIssuingCharges
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
                &GetBiblioIssues
                &GetOpenIssue
-               &AnonymiseIssueHistory
         &CheckIfIssuedToPatron
         &IsItemIssued
         GetTopIssues
@@ -555,6 +554,10 @@ sub TooMany {
         }
     }
 
+    if ( not defined( $issuing_rule ) and not defined($branch_borrower_circ_rule->{maxissueqty}) ) {
+        return { reason => 'NO_RULE_DEFINED', max_allowed => 0 };
+    }
+
     # OK, the patron can issue !!!
     return;
 }
@@ -570,7 +573,7 @@ C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 
 =over 4
 
-=item C<$borrower> hash with borrower informations (from GetMember or GetMemberDetails)
+=item C<$borrower> hash with borrower informations (from GetMember)
 
 =item C<$barcode> is the bar code of the book being issued.
 
@@ -725,14 +728,16 @@ sub CanBookBeIssued {
         ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }
-    if ( ref $borrower->{flags} ) {
-        if ( $borrower->{flags}->{GNA} ) {
+
+    my $flags = C4::Members::patronflags( $borrower );
+    if ( ref $flags ) {
+        if ( $flags->{GNA} ) {
             $issuingimpossible{GNA} = 1;
         }
-        if ( $borrower->{flags}->{'LOST'} ) {
+        if ( $flags->{'LOST'} ) {
             $issuingimpossible{CARD_LOST} = 1;
         }
-        if ( $borrower->{flags}->{'DBARRED'} ) {
+        if ( $flags->{'DBARRED'} ) {
             $issuingimpossible{DEBARRED} = 1;
         }
     }
@@ -929,7 +934,6 @@ sub CanBookBeIssued {
     if ( $rentalConfirmation ){
         my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
         if ( $rentalCharge > 0 ){
-            $rentalCharge = sprintf("%.02f", $rentalCharge);
             $needsconfirmation{RENTALCHARGE} = $rentalCharge;
         }
     }
@@ -1064,14 +1068,18 @@ sub CanBookBeIssued {
         require C4::Serials;
         my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber);
         unless ($is_a_subscription) {
-            my $issues = GetIssues( {
-                borrowernumber => $borrower->{borrowernumber},
-                biblionumber   => $biblionumber,
-            } );
-            my @issues = $issues ? @$issues : ();
+            my $checkouts = Koha::Checkouts->search(
+                {
+                    borrowernumber => $borrower->{borrowernumber},
+                    biblionumber   => $biblionumber,
+                },
+                {
+                    join => 'item',
+                }
+            );
             # if we get here, we don't already have a loan on this item,
             # so if there are any loans on this bib, ask for confirmation
-            if (scalar @issues > 0) {
+            if ( $checkouts->count ) {
                 $needsconfirmation{BIBLIO_ALREADY_ISSUED} = 1;
             }
         }
@@ -1209,6 +1217,9 @@ sub checkHighHolds {
         my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
 
         my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration );
+        $reduced_datedue->set_hour($orig_due->hour);
+        $reduced_datedue->set_minute($orig_due->minute);
+        $reduced_datedue->truncate( to => 'minute' );
 
         if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {
             $return_data->{exceeded} = 1;
@@ -1228,7 +1239,7 @@ Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this
 
 =over 4
 
-=item C<$borrower> is a hash with borrower informations (from GetMember or GetMemberDetails).
+=item C<$borrower> is a hash with borrower informations (from GetMember).
 
 =item C<$barcode> is the barcode of the item being issued.
 
@@ -1826,7 +1837,7 @@ sub AddReturn {
 
     my $issue  = GetItemIssue($itemnumber);
     if ($issue and $issue->{borrowernumber}) {
-        $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber})
+        $borrower = C4::Members::GetMember( borrowernumber => $issue->{borrowernumber} )
             or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existent borrowernumber '$issue->{borrowernumber}'\n"
                 . Dumper($issue) . "\n";
     } else {
@@ -2134,7 +2145,15 @@ sub MarkIssueReturned {
         die "Fatal error: the patron ($borrowernumber) has requested their circulation history be anonymized on check-in, but the AnonymousPatron system preference is empty or not set correctly."
             unless C4::Members::GetMember( borrowernumber => $anonymouspatron );
     }
+    my $database = Koha::Database->new();
+    my $schema   = $database->schema;
     my $dbh   = C4::Context->dbh;
+
+    my $issue_id = $dbh->selectrow_array(
+        q|SELECT issue_id FROM issues WHERE itemnumber = ?|,
+        undef, $itemnumber
+    );
+
     my $query = 'UPDATE issues SET returndate=';
     my @bind;
     if ($dropbox_branch) {
@@ -2148,34 +2167,44 @@ sub MarkIssueReturned {
     } else {
         $query .= ' now() ';
     }
-    $query .= ' WHERE  borrowernumber = ?  AND itemnumber = ?';
-    push @bind, $borrowernumber, $itemnumber;
-    # FIXME transaction
-    my $sth_upd  = $dbh->prepare($query);
-    $sth_upd->execute(@bind);
-    my $sth_copy = $dbh->prepare('INSERT INTO old_issues SELECT * FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?');
-    $sth_copy->execute($borrowernumber, $itemnumber);
-    # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
-    if ( $privacy == 2) {
-        my $sth_ano = $dbh->prepare("UPDATE old_issues SET borrowernumber=?
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-       $sth_ano->execute($anonymouspatron, $borrowernumber, $itemnumber);
-    }
-    my $sth_del  = $dbh->prepare("DELETE FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-    $sth_del->execute($borrowernumber, $itemnumber);
+    $query .= ' WHERE issue_id = ?';
+    push @bind, $issue_id;
 
-    ModItem( { 'onloan' => undef }, undef, $itemnumber );
+    # FIXME Improve the return value and handle it from callers
+    $schema->txn_do(sub {
+        $dbh->do( $query, undef, @bind );
 
-    if ( C4::Context->preference('StoreLastBorrower') ) {
-        my $item = Koha::Items->find( $itemnumber );
-        my $patron = Koha::Patrons->find( $borrowernumber );
-        $item->last_returned_by( $patron );
-    }
+        my $id_already_exists = $dbh->selectrow_array(
+            q|SELECT COUNT(*) FROM old_issues WHERE issue_id = ?|,
+            undef, $issue_id
+        );
+
+        if ( $id_already_exists ) {
+            my $new_issue_id = $dbh->selectrow_array(q|SELECT MAX(issue_id)+1 FROM old_issues|);
+            $dbh->do(
+                q|UPDATE issues SET issue_id = ? WHERE issue_id = ?|,
+                undef, $new_issue_id, $issue_id
+            );
+            $issue_id = $new_issue_id;
+        }
+
+        $dbh->do(q|INSERT INTO old_issues SELECT * FROM issues WHERE issue_id = ?|, undef, $issue_id);
+
+        # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
+        if ( $privacy == 2) {
+            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $issue_id);
+        }
+
+        $dbh->do(q|DELETE FROM issues WHERE issue_id = ?|, undef, $issue_id);
+
+        ModItem( { 'onloan' => undef }, undef, $itemnumber );
+
+        if ( C4::Context->preference('StoreLastBorrower') ) {
+            my $item = Koha::Items->find( $itemnumber );
+            my $patron = Koha::Patrons->find( $borrowernumber );
+            $item->last_returned_by( $patron );
+        }
+    });
 }
 
 =head2 _debar_user_on_return
@@ -2506,120 +2535,6 @@ sub GetOpenIssue {
 
 }
 
-=head2 GetIssues
-
-    $issues = GetIssues({});    # return all issues!
-    $issues = GetIssues({ borrowernumber => $borrowernumber, biblionumber => $biblionumber });
-
-Returns all pending issues that match given criteria.
-Returns a arrayref or undef if an error occurs.
-
-Allowed criteria are:
-
-=over 2
-
-=item * borrowernumber
-
-=item * biblionumber
-
-=item * itemnumber
-
-=back
-
-=cut
-
-sub GetIssues {
-    my ($criteria) = @_;
-
-    # Build filters
-    my @filters;
-    my @allowed = qw(borrowernumber biblionumber itemnumber);
-    foreach (@allowed) {
-        if (defined $criteria->{$_}) {
-            push @filters, {
-                field => $_,
-                value => $criteria->{$_},
-            };
-        }
-    }
-
-    # Do we need to join other tables ?
-    my %join;
-    if (defined $criteria->{biblionumber}) {
-        $join{items} = 1;
-    }
-
-    # Build SQL query
-    my $where = '';
-    if (@filters) {
-        $where = "WHERE " . join(' AND ', map { "$_->{field} = ?" } @filters);
-    }
-    my $query = q{
-        SELECT issues.*
-        FROM issues
-    };
-    if (defined $join{items}) {
-        $query .= q{
-            LEFT JOIN items ON (issues.itemnumber = items.itemnumber)
-        };
-    }
-    $query .= $where;
-
-    # Execute SQL query
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare($query);
-    my $rv = $sth->execute(map { $_->{value} } @filters);
-
-    return $rv ? $sth->fetchall_arrayref({}) : undef;
-}
-
-=head2 GetItemIssues
-
-  $issues = &GetItemIssues($itemnumber, $history);
-
-Returns patrons that have issued a book
-
-C<$itemnumber> is the itemnumber
-C<$history> is false if you just want the current "issuer" (if any)
-and true if you want issues history from old_issues also.
-
-Returns reference to an array of hashes
-
-=cut
-
-sub GetItemIssues {
-    my ( $itemnumber, $history ) = @_;
-    
-    my $today = DateTime->now( time_zome => C4::Context->tz);  # get today date
-    $today->truncate( to => 'minute' );
-    my $sql = "SELECT * FROM issues
-              JOIN borrowers USING (borrowernumber)
-              JOIN items     USING (itemnumber)
-              WHERE issues.itemnumber = ? ";
-    if ($history) {
-        $sql .= "UNION ALL
-                 SELECT * FROM old_issues
-                 LEFT JOIN borrowers USING (borrowernumber)
-                 JOIN items USING (itemnumber)
-                 WHERE old_issues.itemnumber = ? ";
-    }
-    $sql .= "ORDER BY date_due DESC";
-    my $sth = C4::Context->dbh->prepare($sql);
-    if ($history) {
-        $sth->execute($itemnumber, $itemnumber);
-    } else {
-        $sth->execute($itemnumber);
-    }
-    my $results = $sth->fetchall_arrayref({});
-    foreach (@$results) {
-        my $date_due = dt_from_string($_->{date_due},'sql');
-        $date_due->truncate( to => 'minute' );
-
-        $_->{overdue} = (DateTime->compare($date_due, $today) == -1) ? 1 : 0;
-    }
-    return $results;
-}
-
 =head2 GetBiblioIssues
 
   $issues = GetBiblioIssues($biblionumber);
@@ -2778,24 +2693,23 @@ sub CanBookBeRenewed {
             # can be filled with available items. We can get the union of the sets simply
             # by pushing all the elements onto an array and removing the duplicates.
             my @reservable;
-            foreach my $b (@borrowernumbers) {
-                my ($borr) = C4::Members::GetMemberDetails($b);
-                foreach my $i (@itemnumbers) {
-                    my $item = GetItem($i);
-                    if (   IsAvailableForItemLevelRequest( $item, $borr )
-                        && CanItemBeReserved( $b, $i )
-                        && !IsItemOnHoldAndFound($i) )
-                    {
-                        push( @reservable, $i );
+            my %borrowers;
+            ITEM: foreach my $i (@itemnumbers) {
+                my $item = GetItem($i);
+                next if IsItemOnHoldAndFound($i);
+                for my $b (@borrowernumbers) {
+                    my $borr = $borrowers{$b}//= C4::Members::GetMember(borrowernumber => $b);
+                    next unless IsAvailableForItemLevelRequest($item, $borr);
+                    next unless CanItemBeReserved($b,$i);
+
+                    push @reservable, $i;
+                    if (@reservable >= @borrowernumbers) {
+                        $resfound = 0;
+                        last ITEM;
                     }
+                    last;
                 }
             }
-
-            @reservable = uniq(@reservable);
-
-            if ( @reservable >= @borrowernumbers ) {
-                $resfound = 0;
-            }
         }
     }
     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
@@ -2982,7 +2896,7 @@ sub AddRenewal {
 
     # Send a renewal slip according to checkout alert preferencei
     if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
-        $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
+        $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );
         my $circulation_alert = 'C4::ItemCirculationAlertPreference';
         my %conditions        = (
             branchcode   => $branch,
@@ -3013,15 +2927,19 @@ sub AddRenewal {
     }
 
     # Log the renewal
-    UpdateStats({branch => $branch,
-                type => 'renew',
-                amount => $charge,
-                itemnumber => $itemnumber,
-                itemtype => $item->{itype},
-                borrowernumber => $borrowernumber,
-                ccode => $item->{'ccode'}}
-                );
-       return $datedue;
+    UpdateStats(
+        {
+            branch => C4::Context->userenv ? C4::Context->userenv->{branch} : $branch,
+            type           => 'renew',
+            amount         => $charge,
+            itemnumber     => $itemnumber,
+            itemtype       => $item->{itype},
+            borrowernumber => $borrowernumber,
+            ccode          => $item->{'ccode'}
+        }
+    );
+
+    return $datedue;
 }
 
 sub GetRenewCount {
@@ -3091,7 +3009,7 @@ sub GetSoonestRenewDate {
     my $itemissue = GetItemIssue($itemnumber) or return;
 
     $borrowernumber ||= $itemissue->{borrowernumber};
-    my $borrower = C4::Members::GetMemberDetails($borrowernumber)
+    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
       or return;
 
     my $branchcode = _GetCircControlBranch( $item, $borrower );
@@ -3230,6 +3148,9 @@ sub GetIssuingCharges {
             my $discount = _get_discount_from_rule($discount_rules, $branch, $item_type);
             $charge = ( $charge * ( 100 - $discount ) ) / 100;
         }
+        if ($charge) {
+            $charge = sprintf '%.2f', $charge; # ensure no fractions of a penny returned
+        }
     }
 
     return ( $charge, $item_type );
@@ -3309,7 +3230,8 @@ sub GetTransfers {
     my $query = '
         SELECT datesent,
                frombranch,
-               tobranch
+               tobranch,
+               branchtransfer_id
         FROM branchtransfers
         WHERE itemnumber = ?
           AND datearrived IS NULL
@@ -3333,7 +3255,7 @@ sub GetTransfersFromTo {
     return unless ( $frombranch && $tobranch );
     my $dbh   = C4::Context->dbh;
     my $query = "
-        SELECT itemnumber,datesent,frombranch
+        SELECT branchtransfer_id,itemnumber,datesent,frombranch
         FROM   branchtransfers
         WHERE  frombranch=?
           AND  tobranch=?
@@ -3367,49 +3289,6 @@ sub DeleteTransfer {
     return $sth->execute($itemnumber);
 }
 
-=head2 AnonymiseIssueHistory
-
-  ($rows,$err_history_not_deleted) = AnonymiseIssueHistory($date,$borrowernumber)
-
-This function write NULL instead of C<$borrowernumber> given on input arg into the table issues.
-if C<$borrowernumber> is not set, it will delete the issue history for all borrower older than C<$date>.
-
-If c<$borrowernumber> is set, it will delete issue history for only that borrower, regardless of their opac privacy
-setting (force delete).
-
-return the number of affected rows and a value that evaluates to true if an error occurred deleting the history.
-
-=cut
-
-sub AnonymiseIssueHistory {
-    my $date           = shift;
-    my $borrowernumber = shift;
-    my $dbh            = C4::Context->dbh;
-    my $query          = "
-        UPDATE old_issues
-        SET    borrowernumber = ?
-        WHERE  returndate < ?
-          AND borrowernumber IS NOT NULL
-    ";
-
-    # The default of 0 does not work due to foreign key constraints
-    # The anonymisation should not fail quietly if AnonymousPatron is not a valid entry
-    # Set it to undef (NULL)
-    my $anonymouspatron = C4::Context->preference('AnonymousPatron') || undef;
-    my @bind_params = ($anonymouspatron, $date);
-    if (defined $borrowernumber) {
-       $query .= " AND borrowernumber = ?";
-       push @bind_params, $borrowernumber;
-    } else {
-       $query .= " AND (SELECT privacy FROM borrowers WHERE borrowers.borrowernumber=old_issues.borrowernumber) <> 0";
-    }
-    my $sth = $dbh->prepare($query);
-    $sth->execute(@bind_params);
-    my $anonymisation_err = $dbh->err;
-    my $rows_affected = $sth->rows;  ### doublecheck row count return function
-    return ($rows_affected, $anonymisation_err);
-}
-
 =head2 SendCirculationAlert
 
 Send out a C<check-in> or C<checkout> alert using the messaging system.
@@ -3755,7 +3634,7 @@ sub LostItem{
 
     # If a borrower lost the item, add a replacement cost to the their record
     if ( my $borrowernumber = $issues->{borrowernumber} ){
-        my $borrower = C4::Members::GetMemberDetails( $borrowernumber );
+        my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );
 
         if (C4::Context->preference('WhenLostForgiveFine')){
             my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, 1, 0); # 1, 0 = exemptfine, no-dropbox
@@ -3851,7 +3730,7 @@ sub ProcessOfflineReturn {
 sub ProcessOfflineIssue {
     my $operation = shift;
 
-    my $borrower = C4::Members::GetMemberDetails( undef, $operation->{cardnumber} ); # Get borrower from operation cardnumber
+    my $borrower = C4::Members::GetMember( cardnumber => $operation->{cardnumber} );
 
     if ( $borrower->{borrowernumber} ) {
         my $itemnumber = C4::Items::GetItemnumberFromBarcode( $operation->{barcode} );