Bug 15685: Send the basket to _cancel_items_receipt
[koha-ffzg.git] / C4 / Circulation.pm
index 69ac19a..b915b34 100644 (file)
@@ -41,6 +41,7 @@ use Algorithm::CheckDigits;
 use Data::Dumper;
 use Koha::Account;
 use Koha::AuthorisedValues;
+use Koha::Biblioitems;
 use Koha::DateUtils;
 use Koha::Calendar;
 use Koha::Checkouts;
@@ -564,16 +565,20 @@ sub TooMany {
 
 =head2 CanBookBeIssued
 
-  ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 
+  ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) =  CanBookBeIssued( $borrower,
                       $barcode, $duedate, $inprocess, $ignore_reserves, $params );
 
 Check if a book can be issued.
 
-C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
+C<$issuingimpossible> and C<$needsconfirmation> are hashrefs.
+
+IMPORTANT: The assumption by users of this routine is that causes blocking
+the issue are keyed by uppercase labels and other returned
+data is keyed in lower case!
 
 =over 4
 
-=item C<$borrower> hash with borrower informations (from GetMember)
+=item C<$borrower> hash with borrower informations (from Koha::Patron->unblessed)
 
 =item C<$barcode> is the bar code of the book being issued.
 
@@ -715,7 +720,8 @@ sub CanBookBeIssued {
     #
     # BORROWER STATUS
     #
-    if ( $borrower->{'category_type'} eq 'X' && (  $item->{barcode}  )) { 
+    my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+    if ( $patron->category->category_type eq 'X' && (  $item->{barcode}  )) {
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
         &UpdateStats({
                      branch => C4::Context->userenv->{'branch'},
@@ -809,7 +815,7 @@ sub CanBookBeIssued {
         $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
     }
 
-    my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+    $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
     if ( my $debarred_date = $patron->is_debarred ) {
          # patron has accrued fine days or has a restriction. $count is a date
         if ($debarred_date eq '9999-12-31') {
@@ -828,6 +834,56 @@ sub CanBookBeIssued {
         }
     }
 
+    #
+    # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER
+    #
+    if ( $issue && $issue->borrowernumber eq $borrower->{'borrowernumber'} ){
+
+        # Already issued to current borrower.
+        # If it is an on-site checkout if it can be switched to a normal checkout
+        # or ask whether the loan should be renewed
+
+        if ( $issue->onsite_checkout
+                and C4::Context->preference('SwitchOnSiteCheckouts') ) {
+            $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1;
+        } else {
+            my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
+                $borrower->{'borrowernumber'},
+                $item->{'itemnumber'},
+            );
+            if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
+                if ( $renewerror eq 'onsite_checkout' ) {
+                    $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 1;
+                }
+                else {
+                    $issuingimpossible{NO_MORE_RENEWALS} = 1;
+                }
+            }
+            else {
+                $needsconfirmation{RENEW_ISSUE} = 1;
+            }
+        }
+    }
+    elsif ( $issue ) {
+
+        # issued to someone else
+
+        my $patron = Koha::Patrons->find( $issue->borrowernumber );
+
+        my ( $can_be_returned, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
+
+        unless ( $can_be_returned ) {
+            $issuingimpossible{RETURN_IMPOSSIBLE} = 1;
+            $issuingimpossible{branch_to_return} = $message;
+        } else {
+            $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
+            $needsconfirmation{issued_firstname} = $patron->firstname;
+            $needsconfirmation{issued_surname} = $patron->surname;
+            $needsconfirmation{issued_cardnumber} = $patron->cardnumber;
+            $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber;
+        }
+    }
+
     # JB34 CHECKS IF BORROWERS DON'T HAVE ISSUE TOO MANY BOOKS
     #
     my $switch_onsite_checkout = (
@@ -837,7 +893,7 @@ sub CanBookBeIssued {
       and $issue->borrowernumber == $borrower->{'borrowernumber'} ? 1 : 0 );
     my $toomany = TooMany( $borrower, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } );
     # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book
-    if ( $toomany ) {
+    if ( $toomany && not exists $needsconfirmation{RENEW_ISSUE} ) {
         if ( $toomany->{max_allowed} == 0 ) {
             $needsconfirmation{PATRON_CANT} = 1;
         }
@@ -938,84 +994,34 @@ sub CanBookBeIssued {
         }
     }
 
-    #
-    # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER
-    #
-    if ( $issue && $issue->borrowernumber eq $borrower->{'borrowernumber'} ){
-
-        # Already issued to current borrower.
-        # If it is an on-site checkout if it can be switched to a normal checkout
-        # or ask whether the loan should be renewed
-
-        if ( $issue->onsite_checkout
-                and C4::Context->preference('SwitchOnSiteCheckouts') ) {
-            $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1;
-        } else {
-            my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
-                $borrower->{'borrowernumber'},
-                $item->{'itemnumber'},
-            );
-            if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
-                if ( $renewerror eq 'onsite_checkout' ) {
-                    $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 1;
-                }
-                else {
-                    $issuingimpossible{NO_MORE_RENEWALS} = 1;
-                }
-            }
-            else {
-                $needsconfirmation{RENEW_ISSUE} = 1;
-            }
-        }
-    }
-    elsif ( $issue ) {
-
-        # issued to someone else
-        my $currborinfo =    C4::Members::GetMember( borrowernumber => $issue->borrowernumber );
-
-
-        my ( $can_be_returned, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
-
-        unless ( $can_be_returned ) {
-            $issuingimpossible{RETURN_IMPOSSIBLE} = 1;
-            $issuingimpossible{branch_to_return} = $message;
-        } else {
-            $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
-            $needsconfirmation{issued_firstname} = $currborinfo->{'firstname'};
-            $needsconfirmation{issued_surname} = $currborinfo->{'surname'};
-            $needsconfirmation{issued_cardnumber} = $currborinfo->{'cardnumber'};
-            $needsconfirmation{issued_borrowernumber} = $currborinfo->{'borrowernumber'};
-        }
-    }
-
     unless ( $ignore_reserves ) {
         # See if the item is on reserve.
         my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
         if ($restype) {
             my $resbor = $res->{'borrowernumber'};
             if ( $resbor ne $borrower->{'borrowernumber'} ) {
-                my ( $resborrower ) = C4::Members::GetMember( borrowernumber => $resbor );
+                my $patron = Koha::Patrons->find( $resbor );
                 if ( $restype eq "Waiting" )
                 {
                     # The item is on reserve and waiting, but has been
                     # reserved by some other patron.
                     $needsconfirmation{RESERVE_WAITING} = 1;
-                    $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
-                    $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
-                    $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
-                    $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
                     $needsconfirmation{'resbranchcode'} = $res->{branchcode};
                     $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'};
                 }
                 elsif ( $restype eq "Reserved" ) {
                     # The item is on reserve for someone else.
                     $needsconfirmation{RESERVED} = 1;
-                    $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
-                    $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
-                    $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
-                    $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
-                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
-                    $needsconfirmation{'resreservedate'} = $res->{'reservedate'};
+                    $needsconfirmation{'resfirstname'} = $patron->firstname;
+                    $needsconfirmation{'ressurname'} = $patron->surname;
+                    $needsconfirmation{'rescardnumber'} = $patron->cardnumber;
+                    $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
+                    $needsconfirmation{'resbranchcode'} = $patron->branchcode;
+                    $needsconfirmation{'resreservedate'} = $res->{reservedate};
                 }
             }
         }
@@ -1235,7 +1241,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).
+=item C<$borrower> is a hash with borrower informations (from Koha::Patron->unblessed).
 
 =item C<$barcode> is the barcode of the item being issued.
 
@@ -1451,13 +1457,12 @@ sub AddIssue {
                     }
                 );
             }
+            logaction(
+                "CIRCULATION", "ISSUE",
+                $borrower->{'borrowernumber'},
+                $item->{'itemnumber'}
+            ) if C4::Context->preference("IssueLog");
         }
-
-        logaction(
-            "CIRCULATION", "ISSUE",
-            $borrower->{'borrowernumber'},
-            $item->{'itemnumber'}
-        ) if C4::Context->preference("IssueLog");
     }
     return $issue;
 }
@@ -1808,7 +1813,7 @@ sub AddReturn {
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
     my $messages;
-    my $borrower;
+    my $patron;
     my $doreturn       = 1;
     my $validTransfert = 0;
     my $stat_type = 'return';
@@ -1820,14 +1825,11 @@ sub AddReturn {
     }
 
     my $itemnumber = $item->{ itemnumber };
-
-    my $item_level_itypes = C4::Context->preference("item-level_itypes");
-    my $biblio   = $item_level_itypes ? undef : GetBiblioData( $item->{ biblionumber } ); # don't get bib data unless we need it
-    my $itemtype = $item_level_itypes ? $item->{itype} : $biblio->{itemtype};
+    my $itemtype = $item->{itype}; # GetItem called effective_itemtype
 
     my $issue  = Koha::Checkouts->find( { itemnumber => $itemnumber } );
     if ( $issue ) {
-        $borrower = C4::Members::GetMember( borrowernumber => $issue->borrowernumber )
+        $patron = Koha::Patrons->find( $issue->borrowernumber )
             or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existent borrowernumber '" . $issue->borrowernumber . "'\n"
                 . Dumper($issue->unblessed) . "\n";
     } else {
@@ -1859,7 +1861,8 @@ sub AddReturn {
     my $returnbranch = $item->{$hbr} || $branch ;
         # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
 
-    my $borrowernumber = $borrower->{'borrowernumber'} || undef;    # we don't know if we had a borrower or not
+    my $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
+    my $patron_unblessed = $patron ? $patron->unblessed : {};
 
     my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin');
     if ($yaml) {
@@ -1898,7 +1901,7 @@ sub AddReturn {
             Rightbranch => $message
         };
         $doreturn = 0;
-        return ( $doreturn, $messages, $issue, $borrower );
+        return ( $doreturn, $messages, $issue, $patron_unblessed);
     }
 
     if ( $item->{'withdrawn'} ) { # book has been cancelled
@@ -1912,36 +1915,32 @@ sub AddReturn {
     if ($doreturn) {
         my $is_overdue;
         die "The item is not issed and cannot be returned" unless $issue; # Just in case...
-        $borrower or warn "AddReturn without current borrower";
+        $patron or warn "AddReturn without current borrower";
                my $circControlBranch;
         if ($dropbox) {
             # define circControlBranch only if dropbox mode is set
             # don't allow dropbox mode to create an invalid entry in issues (issuedate > today)
             # FIXME: check issuedate > returndate, factoring in holidays
 
-            $circControlBranch = _GetCircControlBranch($item,$borrower);
+            $circControlBranch = _GetCircControlBranch($item,$patron_unblessed);
             $is_overdue = $issue->is_overdue( $dropboxdate );
         } else {
             $is_overdue = $issue->is_overdue;
         }
 
-        if ($borrowernumber) {
+        if ($patron) {
             eval {
-                my $issue_id = MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
-                    $circControlBranch, $return_date, $borrower->{'privacy'} );
-                $issue->issue_id($issue_id);
+                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
+                    $circControlBranch, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
                 if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
-                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );
+                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
-                $messages->{'Wrongbranch'} = {
-                    Wrongbranch => $branch,
-                    Rightbranch => $message
-                };
-                carp $@;
-                return ( 0, { WasReturned => 0 }, $issue, $borrower );
+                carp "The checkin for the following issue failed, Please go to the about page, section 'data corrupted' to know how to fix this problem ($@)" . Dumper( $issue->unblessed );
+
+                return ( 0, { WasReturned => 0, DataCorrupted => 1 }, $issue, $patron_unblessed );
             }
 
             # FIXME is the "= 1" right?  This could be the borrower hash.
@@ -2013,22 +2012,22 @@ sub AddReturn {
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
             $today = $dropboxdate if $dropbox;
-            my ($debardate,$reminder) = _debar_user_on_return( $borrower, $item, dt_from_string($issue->date_due), $today );
+            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item, dt_from_string($issue->date_due), $today );
             if ($reminder){
                 $messages->{'PrevDebarred'} = $debardate;
             } else {
                 $messages->{'Debarred'} = $debardate if $debardate;
             }
         # there's no overdue on the item but borrower had been previously debarred
-        } elsif ( $issue->date_due and $borrower->{'debarred'} ) {
-             if ( $borrower->{debarred} eq "9999-12-31") {
-                $messages->{'ForeverDebarred'} = $borrower->{'debarred'};
+        } elsif ( $issue->date_due and $patron->debarred ) {
+             if ( $patron->debarred eq "9999-12-31") {
+                $messages->{'ForeverDebarred'} = $patron->debarred;
              } else {
-                  my $borrower_debar_dt = dt_from_string( $borrower->{debarred} );
+                  my $borrower_debar_dt = dt_from_string( $patron->debarred );
                   $borrower_debar_dt->truncate(to => 'day');
                   my $today_dt = $today->clone()->truncate(to => 'day');
                   if ( DateTime->compare( $borrower_debar_dt, $today_dt ) != -1 ) {
-                      $messages->{'PrevDebarred'} = $borrower->{'debarred'};
+                      $messages->{'PrevDebarred'} = $patron->debarred;
                   }
              }
         }
@@ -2054,29 +2053,31 @@ sub AddReturn {
         ccode          => $item->{ ccode }
     });
 
-    # Send a check-in slip. # NOTE: borrower may be undef.  probably shouldn't try to send messages then.
-    my $circulation_alert = 'C4::ItemCirculationAlertPreference';
-    my %conditions = (
-        branchcode   => $branch,
-        categorycode => $borrower->{categorycode},
-        item_type    => $item->{itype},
-        notification => 'CHECKIN',
-    );
-    if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
-        SendCirculationAlert({
-            type     => 'CHECKIN',
-            item     => $item,
-            borrower => $borrower,
-            branch   => $branch,
-        });
-    }
-    
-    logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'})
-        if C4::Context->preference("ReturnLog");
-    
+    # Send a check-in slip. # NOTE: borrower may be undef. Do not try to send messages then.
+    if ( $patron ) {
+        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+        my %conditions = (
+            branchcode   => $branch,
+            categorycode => $patron->categorycode,
+            item_type    => $item->{itype},
+            notification => 'CHECKIN',
+        );
+        if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
+            SendCirculationAlert({
+                type     => 'CHECKIN',
+                item     => $item,
+                borrower => $patron->unblessed,
+                branch   => $branch,
+            });
+        }
+
+        logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'})
+            if C4::Context->preference("ReturnLog");
+        }
+
     # Remove any OVERDUES related debarment if the borrower has no overdues
     if ( $borrowernumber
-      && $borrower->{'debarred'}
+      && $patron->debarred
       && C4::Context->preference('AutoRemoveOverduesRestrictions')
       && !Koha::Patrons->find( $borrowernumber )->has_overdues
       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
@@ -2099,7 +2100,7 @@ sub AddReturn {
         }
     }
 
-    return ( $doreturn, $messages, $issue, $borrower );
+    return ( $doreturn, $messages, $issue, ( $patron ? $patron->unblessed : {} ));
 }
 
 =head2 MarkIssueReturned
@@ -2136,7 +2137,7 @@ sub MarkIssueReturned {
         # Note that a warning should appear on the about page (System information tab).
         $anonymouspatron = C4::Context->preference('AnonymousPatron');
         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 );
+            unless Koha::Patrons->find( $anonymouspatron );
     }
     my $database = Koha::Database->new();
     my $schema   = $database->schema;
@@ -2166,33 +2167,21 @@ sub MarkIssueReturned {
     # FIXME Improve the return value and handle it from callers
     $schema->txn_do(sub {
 
+        # Update the returndate
         $dbh->do( $query, undef, @bind );
 
+        # Retrieve the issue
         my $issue = Koha::Checkouts->find( $issue_id ); # FIXME should be fetched earlier
 
         # Create the old_issues entry
-        my $old_checkout_data = $issue->unblessed;
-
-        if ( Koha::Old::Checkouts->find( $issue_id ) ) {
-            my $new_issue_id = ( Koha::Old::Checkouts->search(
-                {},
-                { columns => [ { max_issue_id => { max => 'issue_id' } } ] }
-            )->get_column('max_issue_id') )[0];
-            $new_issue_id++;
-            $issue_id = $new_issue_id;
-        }
-        $old_checkout_data->{issue_id} = $issue_id;
-        my $old_checkout = Koha::Old::Checkout->new($old_checkout_data)->store;
-
-        # Update the fines
-        $dbh->do(q|UPDATE accountlines SET issue_id = ? WHERE issue_id = ?|, undef, $old_checkout->issue_id, $issue->issue_id);
+        my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
 
         # 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, $old_checkout->issue_id);
         }
 
-        # Delete the issue
+        # And finally delete the issue
         $issue->delete;
 
         ModItem( { 'onloan' => undef }, undef, $itemnumber );
@@ -2619,7 +2608,7 @@ sub CanBookBeRenewed {
     return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout;
 
     $borrowernumber ||= $issue->borrowernumber;
-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
+    my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
     my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
@@ -2673,7 +2662,7 @@ sub CanBookBeRenewed {
                 my $item = GetItem($i);
                 next if IsItemOnHoldAndFound($i);
                 for my $b (@borrowernumbers) {
-                    my $borr = $borrowers{$b}//= C4::Members::GetMember(borrowernumber => $b);
+                    my $borr = $borrowers{$b} //= Koha::Patrons->find( $b )->unblessed;
                     next unless IsAvailableForItemLevelRequest($item, $borr);
                     next unless CanItemBeReserved($b,$i);
 
@@ -2691,9 +2680,9 @@ sub CanBookBeRenewed {
 
     return ( 1, undef ) if $override_limit;
 
-    my $branchcode = _GetCircControlBranch( $item, $borrower );
+    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $borrower->{categorycode},
+        {   categorycode => $patron->categorycode,
             itemtype     => $item->{itype},
             branchcode   => $branchcode
         }
@@ -2704,7 +2693,7 @@ sub CanBookBeRenewed {
 
     my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
     my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
-    my $patron      = Koha::Patrons->find($borrowernumber);
+    $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
     my $restricted  = $patron->is_debarred;
     my $hasoverdues = $patron->has_overdues;
 
@@ -2738,7 +2727,7 @@ sub CanBookBeRenewed {
 
         if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
             my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
-            my ( $amountoutstanding ) = C4::Members::GetMemberAccountRecords($borrower->{borrowernumber});
+            my ( $amountoutstanding ) = C4::Members::GetMemberAccountRecords($patron->borrowernumber);
             if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
                 return ( 0, "auto_too_much_oweing" );
             }
@@ -2831,10 +2820,11 @@ sub AddRenewal {
         return;
     }
 
-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return;
+    my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return
+    my $patron_unblessed = $patron->unblessed;
 
     if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) {
-        _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower } );
+        _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed } );
     }
     _FixOverduesOnReturn( $borrowernumber, $itemnumber );
 
@@ -2847,7 +2837,7 @@ sub AddRenewal {
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
                                         dt_from_string( $issue->date_due, 'sql' ) :
                                         DateTime->now( time_zone => C4::Context->tz());
-        $datedue =  CalcDateDue($datedue, $itemtype, _GetCircControlBranch($item, $borrower), $borrower, 'is a renewal');
+        $datedue =  CalcDateDue($datedue, $itemtype, _GetCircControlBranch($item, $patron_unblessed), $patron_unblessed, 'is a renewal');
     }
 
     # Update the issues record to have the new due date, and a new count
@@ -2877,17 +2867,16 @@ sub AddRenewal {
                     VALUES (now(),?,?,?,?,?,?,?,?)"
         );
         $sth->execute( $borrowernumber, $accountno, $charge, $manager_id,
-            "Renewal of Rental Item " . $item_object->title . " $item->{'barcode'}",
+            "Renewal of Rental Item " . $biblio->title . " $item->{'barcode'}",
             'Rent', $charge, $itemnumber );
     }
 
     # Send a renewal slip according to checkout alert preferencei
     if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
-        $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );
         my $circulation_alert = 'C4::ItemCirculationAlertPreference';
         my %conditions        = (
             branchcode   => $branch,
-            categorycode => $borrower->{categorycode},
+            categorycode => $patron->categorycode,
             item_type    => $item->{itype},
             notification => 'CHECKOUT',
         );
@@ -2896,7 +2885,7 @@ sub AddRenewal {
                 {
                     type     => 'RENEWAL',
                     item     => $item,
-                    borrower => $borrower,
+                    borrower => $patron->unblessed,
                     branch   => $branch,
                 }
             );
@@ -2904,19 +2893,22 @@ sub AddRenewal {
     }
 
     # Remove any OVERDUES related debarment if the borrower has no overdues
-    $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );
-    if ( $borrowernumber
-      && $borrower->{'debarred'}
-      && !Koha::Patrons->find( $borrowernumber )->has_overdues
+    if ( $patron
+      && $patron->is_debarred
+      && ! $patron->has_overdues
       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
     }
 
+    unless ( C4::Context->interface eq 'opac' ) { #if from opac we are obeying OpacRenewalBranch as calculated in opac-renew.pl
+        $branch = C4::Context->userenv ? C4::Context->userenv->{branch} : $branch;
+    }
+
     # Add the renewal to stats
     UpdateStats(
         {
-            branch => C4::Context->userenv ? C4::Context->userenv->{branch} : $branch,
+            branch         => $branch,
             type           => 'renew',
             amount         => $charge,
             itemnumber     => $itemnumber,
@@ -2939,8 +2931,10 @@ sub GetRenewCount {
     my $renewsallowed = 0;
     my $renewsleft    = 0;
 
-    my $borrower = C4::Members::GetMember( borrowernumber => $bornum);
-    my $item     = GetItem($itemno); 
+    my $patron = Koha::Patrons->find( $bornum );
+    my $item     = GetItem($itemno);
+
+    return (0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
 
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
@@ -2955,16 +2949,16 @@ sub GetRenewCount {
     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
     # $item and $borrower should be calculated
-    my $branchcode = _GetCircControlBranch($item, $borrower);
+    my $branchcode = _GetCircControlBranch($item, $patron->unblessed);
 
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $borrower->{categorycode},
+        {   categorycode => $patron->categorycode,
             itemtype     => $item->{itype},
             branchcode   => $branchcode
         }
     );
 
-    $renewsallowed = $issuing_rule ? $issuing_rule->renewalsallowed : undef; # FIXME Just replace undef with 0 to get what we expected. But what about the side-effects? TODO LATER
+    $renewsallowed = $issuing_rule ? $issuing_rule->renewalsallowed : 0;
     $renewsleft    = $renewsallowed - $renewcount;
     if($renewsleft < 0){ $renewsleft = 0; }
     return ( $renewcount, $renewsallowed, $renewsleft );
@@ -2998,12 +2992,12 @@ sub GetSoonestRenewDate {
     my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
 
     $borrowernumber ||= $itemissue->borrowernumber;
-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
+    my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
-    my $branchcode = _GetCircControlBranch( $item, $borrower );
+    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $borrower->{categorycode},
+        {   categorycode => $patron->categorycode,
             itemtype     => $item->{itype},
             branchcode   => $branchcode
         }
@@ -3057,12 +3051,12 @@ sub GetLatestAutoRenewDate {
     my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
 
     $borrowernumber ||= $itemissue->borrowernumber;
-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
+    my $patron = Koha::Patrons->find( $borrowernumber )
       or return;
 
-    my $branchcode = _GetCircControlBranch( $item, $borrower );
+    my $branchcode = _GetCircControlBranch( $item, $patron->unblessed );
     my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
-        {   categorycode => $borrower->{categorycode},
+        {   categorycode => $patron->categorycode,
             itemtype     => $item->{itype},
             branchcode   => $branchcode
         }
@@ -3518,6 +3512,13 @@ sub CalcDateDue {
                 $datedue = $expiry_dt->clone->set_time_zone( C4::Context->tz );
             }
         }
+        if ( C4::Context->preference('useDaysMode') ne 'Days' ) {
+          my $calendar = Koha::Calendar->new( branchcode => $branch );
+          if ( $calendar->is_holiday($datedue) ) {
+              # Don't return on a closed day
+              $datedue = $calendar->prev_open_day( $datedue );
+          }
+        }
     }
 
     return $datedue;
@@ -3606,12 +3607,12 @@ sub ReturnLostItem{
     my ( $borrowernumber, $itemnum ) = @_;
 
     MarkIssueReturned( $borrowernumber, $itemnum );
-    my $borrower = C4::Members::GetMember( 'borrowernumber'=>$borrowernumber );
+    my $patron = Koha::Patrons->find( $borrowernumber );
     my $item = C4::Items::GetItem( $itemnum );
     my $old_note = ($item->{'paidfor'} && ($item->{'paidfor'} ne q{})) ? $item->{'paidfor'}.' / ' : q{};
     my @datearr = localtime(time);
     my $date = ( 1900 + $datearr[5] ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
-    my $bor = "$borrower->{'firstname'} $borrower->{'surname'} $borrower->{'cardnumber'}";
+    my $bor = $patron->firstname . ' ' . $patron->surname . ' ' . $patron->cardnumber;
     ModItem({ paidfor =>  $old_note."Paid for by $bor $date" }, undef, $itemnum);
 }
 
@@ -3630,7 +3631,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::GetMember( borrowernumber => $borrowernumber );
+        my $patron = Koha::Patrons->find( $borrowernumber );
 
         if (C4::Context->preference('WhenLostForgiveFine')){
             my $fix = _FixOverduesOnReturn($borrowernumber, $itemnumber, 1, 0); # 1, 0 = exemptfine, no-dropbox
@@ -3642,7 +3643,7 @@ sub LostItem{
             #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
         }
 
-        MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$borrower->{'privacy'}) if $mark_returned;
+        MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned;
     }
 }
 
@@ -3727,9 +3728,9 @@ sub ProcessOfflineReturn {
 sub ProcessOfflineIssue {
     my $operation = shift;
 
-    my $borrower = C4::Members::GetMember( cardnumber => $operation->{cardnumber} );
+    my $patron = Koha::Patrons->find( { cardnumber => $operation->{cardnumber} } );
 
-    if ( $borrower->{borrowernumber} ) {
+    if ( $patron ) {
         my $item = Koha::Items->find({ barcode => $operation->{barcode} });
         unless ($item) {
             return "Barcode not found.";
@@ -3737,7 +3738,7 @@ sub ProcessOfflineIssue {
         my $itemnumber = $item->itemnumber;
         my $issue = GetOpenIssue( $itemnumber );
 
-        if ( $issue and ( $issue->{borrowernumber} ne $borrower->{borrowernumber} ) ) { # Item already issued to another borrower, mark it returned
+        if ( $issue and ( $issue->{borrowernumber} ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned
             MarkIssueReturned(
                 $issue->{borrowernumber},
                 $itemnumber,
@@ -3746,7 +3747,7 @@ sub ProcessOfflineIssue {
             );
         }
         AddIssue(
-            $borrower,
+            $patron->unblessed,
             $operation->{'barcode'},
             undef,
             1,