Bug 7679: Followup: circulation statistics wizard improvements

[koha.git] / C4 / Circulation.pm

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index de4b299..40fdcfd 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -22,36 +22,38 @@ package C4::Circulation;
 use strict;
 #use warnings; FIXME - Bug 2505
 use DateTime;
 use strict;
 #use warnings; FIXME - Bug 2505
 use DateTime;

+use Koha::DateUtils;

 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
 use C4::Biblio;
 use C4::Items;
 use C4::Members;
 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
 use C4::Biblio;
 use C4::Items;
 use C4::Members;

-use C4::Dates;
-use C4::Dates qw(format_date);

 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Message;
 use C4::Debug;
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Message;
 use C4::Debug;

-use C4::Branch; # GetBranches

 use C4::Log; # logaction
 use C4::Log; # logaction

-use C4::Koha qw(
-    GetAuthorisedValueByCode
-    GetAuthValCode
-    GetKohaAuthorisedValueLib
-);

 use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
 use C4::RotatingCollections qw(GetCollectionItemBranches);
 use Algorithm::CheckDigits;
 
 use Data::Dumper;
 use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
 use C4::RotatingCollections qw(GetCollectionItemBranches);
 use Algorithm::CheckDigits;
 
 use Data::Dumper;

+use Koha::Account;
+use Koha::AuthorisedValues;

 use Koha::DateUtils;
 use Koha::Calendar;
 use Koha::DateUtils;
 use Koha::Calendar;

-use Koha::Borrower::Debarments;
+use Koha::Items;
+use Koha::Patrons;
+use Koha::Patron::Debarments;

 use Koha::Database;
 use Koha::Database;

+use Koha::Libraries;
+use Koha::Holds;
+use Koha::RefundLostItemFeeRule;
+use Koha::RefundLostItemFeeRules;

 use Carp;
 use List::MoreUtils qw( uniq );
 use Carp;
 use List::MoreUtils qw( uniq );

+use Scalar::Util qw( looks_like_number );

 use Date::Calc qw(
   Today
   Today_and_Now
 use Date::Calc qw(
   Today
   Today_and_Now


@@ -61,11 +63,10 @@ use Date::Calc qw(
   Day_of_Week
   Add_Delta_Days
 );
   Day_of_Week
   Add_Delta_Days
 );

-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
+use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);

 
 BEGIN {
        require Exporter;
 
 BEGIN {
        require Exporter;

-    $VERSION = 3.07.00.049;    # for version checking

        @ISA    = qw(Exporter);
 
        # FIXME subs that should probably be elsewhere
        @ISA    = qw(Exporter);
 
        # FIXME subs that should probably be elsewhere


@@ -95,6 +96,7 @@ BEGIN {
                &AnonymiseIssueHistory
         &CheckIfIssuedToPatron
         &IsItemIssued
                &AnonymiseIssueHistory
         &CheckIfIssuedToPatron
         &IsItemIssued

+        GetTopIssues

        );
 
        # subs to deal with returns
        );
 
        # subs to deal with returns


@@ -138,7 +140,7 @@ use C4::Circulation;
 
 The functions in this module deal with circulation, issues, and
 returns, as well as general information about the library.
 
 The functions in this module deal with circulation, issues, and
 returns, as well as general information about the library.

-Also deals with stocktaking.
+Also deals with inventory.

 
 =head1 FUNCTIONS
 
 
 =head1 FUNCTIONS
 


@@ -164,7 +166,7 @@ System Pref options.
 #
 sub barcodedecode {
     my ($barcode, $filter) = @_;
 #
 sub barcodedecode {
     my ($barcode, $filter) = @_;

-    my $branch = C4::Branch::mybranch();
+    my $branch = C4::Context::mybranch();

     $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
     $filter or return $barcode;     # ensure filter is defined, else return untouched barcode
        if ($filter eq 'whitespace') {
     $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
     $filter or return $barcode;     # ensure filter is defined, else return untouched barcode
        if ($filter eq 'whitespace') {


@@ -304,7 +306,6 @@ sub transferbook {
     my ( $tbr, $barcode, $ignoreRs ) = @_;
     my $messages;
     my $dotransfer      = 1;
     my ( $tbr, $barcode, $ignoreRs ) = @_;
     my $messages;
     my $dotransfer      = 1;

-    my $branches        = GetBranches();

     my $itemnumber = GetItemnumberFromBarcode( $barcode );
     my $issue      = GetItemIssue($itemnumber);
     my $biblio = GetBiblioFromItemNumber($itemnumber);
     my $itemnumber = GetItemnumberFromBarcode( $barcode );
     my $issue      = GetItemIssue($itemnumber);
     my $biblio = GetBiblioFromItemNumber($itemnumber);


@@ -333,7 +334,10 @@ sub transferbook {
     }
 
     # if is permanent...
     }
 
     # if is permanent...

-    if ( $hbr && $branches->{$hbr}->{'PE'} ) {
+    # FIXME Is this still used by someone?
+    # See other FIXME in AddReturn
+    my $library = Koha::Libraries->find($hbr);
+    if ( $library and $library->get_categories->search({'me.categorycode' => 'PE'})->count ) {

         $messages->{'IsPermanent'} = $hbr;
         $dotransfer = 0;
     }
         $messages->{'IsPermanent'} = $hbr;
         $dotransfer = 0;
     }


@@ -378,6 +382,9 @@ sub TooMany {
     my $borrower        = shift;
     my $biblionumber = shift;
        my $item                = shift;
     my $borrower        = shift;
     my $biblionumber = shift;
        my $item                = shift;

+    my $params = shift;
+    my $onsite_checkout = $params->{onsite_checkout} || 0;
+    my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0;

     my $cat_borrower    = $borrower->{'categorycode'};
     my $dbh             = C4::Context->dbh;
        my $branch;
     my $cat_borrower    = $borrower->{'categorycode'};
     my $dbh             = C4::Context->dbh;
        my $branch;


@@ -396,8 +403,11 @@ sub TooMany {
     # rule
     if (defined($issuing_rule) and defined($issuing_rule->{'maxissueqty'})) {
         my @bind_params;
     # rule
     if (defined($issuing_rule) and defined($issuing_rule->{'maxissueqty'})) {
         my @bind_params;

-        my $count_query = "SELECT COUNT(*) FROM issues
-                           JOIN items USING (itemnumber) ";
+        my $count_query = q|
+            SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
+            FROM issues
+            JOIN items USING (itemnumber)
+        |;

 
         my $rule_itemtype = $issuing_rule->{itemtype};
         if ($rule_itemtype eq "*") {
 
         my $rule_itemtype = $issuing_rule->{itemtype};
         if ($rule_itemtype eq "*") {


@@ -450,13 +460,37 @@ sub TooMany {
             }
         }
 
             }
         }
 

-        my $count_sth = $dbh->prepare($count_query);
-        $count_sth->execute(@bind_params);
-        my ($current_loan_count) = $count_sth->fetchrow_array;
+        my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params );

 
 

-        my $max_loans_allowed = $issuing_rule->{'maxissueqty'};
-        if ($current_loan_count >= $max_loans_allowed) {
-            return ($current_loan_count, $max_loans_allowed);
+        my $max_checkouts_allowed = $issuing_rule->{maxissueqty};
+        my $max_onsite_checkouts_allowed = $issuing_rule->{maxonsiteissueqty};
+
+        if ( $onsite_checkout ) {
+            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+                    count => $onsite_checkout_count,
+                    max_allowed => $max_onsite_checkouts_allowed,
+                }
+            }
+        }
+        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
+            my $delta = $switch_onsite_checkout ? 1 : 0;
+            if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }
+        } elsif ( not $onsite_checkout ) {
+            if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count - $onsite_checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }

         }
     }
 
         }
     }
 


@@ -464,9 +498,12 @@ sub TooMany {
     my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower);
     if (defined($branch_borrower_circ_rule->{maxissueqty})) {
         my @bind_params = ();
     my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower);
     if (defined($branch_borrower_circ_rule->{maxissueqty})) {
         my @bind_params = ();

-        my $branch_count_query = "SELECT COUNT(*) FROM issues
-                                  JOIN items USING (itemnumber)
-                                  WHERE borrowernumber = ? ";
+        my $branch_count_query = q|
+            SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts
+            FROM issues
+            JOIN items USING (itemnumber)
+            WHERE borrowernumber = ?
+        |;

         push @bind_params, $borrower->{borrowernumber};
 
         if (C4::Context->preference('CircControl') eq 'PickupLibrary') {
         push @bind_params, $borrower->{borrowernumber};
 
         if (C4::Context->preference('CircControl') eq 'PickupLibrary') {


@@ -478,13 +515,36 @@ sub TooMany {
             $branch_count_query .= " AND items.homebranch = ? ";
             push @bind_params, $branch;
         }
             $branch_count_query .= " AND items.homebranch = ? ";
             push @bind_params, $branch;
         }

-        my $branch_count_sth = $dbh->prepare($branch_count_query);
-        $branch_count_sth->execute(@bind_params);
-        my ($current_loan_count) = $branch_count_sth->fetchrow_array;
-
-        my $max_loans_allowed = $branch_borrower_circ_rule->{maxissueqty};
-        if ($current_loan_count >= $max_loans_allowed) {
-            return ($current_loan_count, $max_loans_allowed);
+        my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params );
+        my $max_checkouts_allowed = $branch_borrower_circ_rule->{maxissueqty};
+        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{maxonsiteissueqty};
+
+        if ( $onsite_checkout ) {
+            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+                    count => $onsite_checkout_count,
+                    max_allowed => $max_onsite_checkouts_allowed,
+                }
+            }
+        }
+        if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) {
+            my $delta = $switch_onsite_checkout ? 1 : 0;
+            if ( $checkout_count >= $max_checkouts_allowed + $delta ) {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }
+        } elsif ( not $onsite_checkout ) {
+            if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed )  {
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count - $onsite_checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
+            }

         }
     }
 
         }
     }
 


@@ -492,118 +552,10 @@ sub TooMany {
     return;
 }
 
     return;
 }
 

-=head2 itemissues
-
-  @issues = &itemissues($biblioitemnumber, $biblio);
-
-Looks up information about who has borrowed the bookZ<>(s) with the
-given biblioitemnumber.
-
-C<$biblio> is ignored.
-
-C<&itemissues> returns an array of references-to-hash. The keys
-include the fields from the C<items> table in the Koha database.
-Additional keys include:
-
-=over 4
-
-=item C<date_due>
-
-If the item is currently on loan, this gives the due date.
-
-If the item is not on loan, then this is either "Available" or
-"Cancelled", if the item has been withdrawn.
-
-=item C<card>
-
-If the item is currently on loan, this gives the card number of the
-patron who currently has the item.
-
-=item C<timestamp0>, C<timestamp1>, C<timestamp2>
-
-These give the timestamp for the last three times the item was
-borrowed.
-
-=item C<card0>, C<card1>, C<card2>
-
-The card number of the last three patrons who borrowed this item.
-
-=item C<borrower0>, C<borrower1>, C<borrower2>
-
-The borrower number of the last three patrons who borrowed this item.
-
-=back
-
-=cut
-
-#'
-sub itemissues {
-    my ( $bibitem, $biblio ) = @_;
-    my $dbh = C4::Context->dbh;
-    my $sth =
-      $dbh->prepare("Select * from items where items.biblioitemnumber = ?")
-      || die $dbh->errstr;
-    my $i = 0;
-    my @results;
-
-    $sth->execute($bibitem) || die $sth->errstr;
-
-    while ( my $data = $sth->fetchrow_hashref ) {
-
-        # Find out who currently has this item.
-        # FIXME - Wouldn't it be better to do this as a left join of
-        # some sort? Currently, this code assumes that if
-        # fetchrow_hashref() fails, then the book is on the shelf.
-        # fetchrow_hashref() can fail for any number of reasons (e.g.,
-        # database server crash), not just because no items match the
-        # search criteria.
-        my $sth2 = $dbh->prepare(
-            "SELECT * FROM issues
-                LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber
-                WHERE itemnumber = ?
-            "
-        );
-
-        $sth2->execute( $data->{'itemnumber'} );
-        if ( my $data2 = $sth2->fetchrow_hashref ) {
-            $data->{'date_due'} = $data2->{'date_due'};
-            $data->{'card'}     = $data2->{'cardnumber'};
-            $data->{'borrower'} = $data2->{'borrowernumber'};
-        }
-        else {
-            $data->{'date_due'} = ($data->{'withdrawn'} eq '1') ? 'Cancelled' : 'Available';
-        }
-
-
-        # Find the last 3 people who borrowed this item.
-        $sth2 = $dbh->prepare(
-            "SELECT * FROM old_issues
-                LEFT JOIN borrowers ON  issues.borrowernumber = borrowers.borrowernumber
-                WHERE itemnumber = ?
-                ORDER BY returndate DESC,timestamp DESC"
-        );
-
-        $sth2->execute( $data->{'itemnumber'} );
-        for ( my $i2 = 0 ; $i2 < 2 ; $i2++ )
-        {    # FIXME : error if there is less than 3 pple borrowing this item
-            if ( my $data2 = $sth2->fetchrow_hashref ) {
-                $data->{"timestamp$i2"} = $data2->{'timestamp'};
-                $data->{"card$i2"}      = $data2->{'cardnumber'};
-                $data->{"borrower$i2"}  = $data2->{'borrowernumber'};
-            }    # if
-        }    # for
-
-        $results[$i] = $data;
-        $i++;
-    }
-
-    return (@results);
-}
-

 =head2 CanBookBeIssued
 
   ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 
 =head2 CanBookBeIssued
 
   ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 

-                      $barcode, $duedatespec, $inprocess, $ignore_reserves );
+                      $barcode, $duedate, $inprocess, $ignore_reserves, $params );

 
 Check if a book can be issued.
 
 
 Check if a book can be issued.
 


@@ -615,11 +567,18 @@ C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 
 =item C<$barcode> is the bar code of the book being issued.
 
 
 =item C<$barcode> is the bar code of the book being issued.
 

-=item C<$duedatespec> is a C4::Dates object.
+=item C<$duedates> is a DateTime object.

 
 =item C<$inprocess> boolean switch
 
 =item C<$inprocess> boolean switch

+

 =item C<$ignore_reserves> boolean switch
 
 =item C<$ignore_reserves> boolean switch
 

+=item C<$params> Hashref of additional parameters
+
+Available keys:
+    override_high_holds - Ignore high holds
+    onsite_checkout     - Checkout is an onsite checkout that will not leave the library
+

 =back
 
 Returns :
 =back
 
 Returns :


@@ -695,10 +654,14 @@ if the borrower borrows to much things
 =cut
 
 sub CanBookBeIssued {
 =cut
 
 sub CanBookBeIssued {

-    my ( $borrower, $barcode, $duedate, $inprocess, $ignore_reserves ) = @_;
+    my ( $borrower, $barcode, $duedate, $inprocess, $ignore_reserves, $params ) = @_;

     my %needsconfirmation;    # filled with problems that needs confirmations
     my %issuingimpossible;    # filled with problems that causes the issue to be IMPOSSIBLE
     my %alerts;               # filled with messages that shouldn't stop issuing, but the librarian should be aware of.
     my %needsconfirmation;    # filled with problems that needs confirmations
     my %issuingimpossible;    # filled with problems that causes the issue to be IMPOSSIBLE
     my %alerts;               # filled with messages that shouldn't stop issuing, but the librarian should be aware of.

+    my %messages;             # filled with information messages that should be displayed.
+
+    my $onsite_checkout     = $params->{onsite_checkout}     || 0;
+    my $override_high_holds = $params->{override_high_holds} || 0;

 
     my $item = GetItem(GetItemnumberFromBarcode( $barcode ));
     my $issue = GetItemIssue($item->{itemnumber});
 
     my $item = GetItem(GetItemnumberFromBarcode( $barcode ));
     my $issue = GetItemIssue($item->{itemnumber});


@@ -748,43 +711,36 @@ sub CanBookBeIssued {
                      branch => C4::Context->userenv->{'branch'},
                      type => 'localuse',
                      itemnumber => $item->{'itemnumber'},
                      branch => C4::Context->userenv->{'branch'},
                      type => 'localuse',
                      itemnumber => $item->{'itemnumber'},

-                     itemtype => $item->{'itemtype'},
+                     itemtype => $item->{'itype'},

                      borrowernumber => $borrower->{'borrowernumber'},
                      ccode => $item->{'ccode'}}
                     );
         ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }
                      borrowernumber => $borrower->{'borrowernumber'},
                      ccode => $item->{'ccode'}}
                     );
         ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }

-    if ( $borrower->{flags}->{GNA} ) {
-        $issuingimpossible{GNA} = 1;
-    }
-    if ( $borrower->{flags}->{'LOST'} ) {
-        $issuingimpossible{CARD_LOST} = 1;
-    }
-    if ( $borrower->{flags}->{'DBARRED'} ) {
-        $issuingimpossible{DEBARRED} = 1;
+    if ( ref $borrower->{flags} ) {
+        if ( $borrower->{flags}->{GNA} ) {
+            $issuingimpossible{GNA} = 1;
+        }
+        if ( $borrower->{flags}->{'LOST'} ) {
+            $issuingimpossible{CARD_LOST} = 1;
+        }
+        if ( $borrower->{flags}->{'DBARRED'} ) {
+            $issuingimpossible{DEBARRED} = 1;
+        }

     }
     if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') {
         $issuingimpossible{EXPIRED} = 1;
     } else {
     }
     if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') {
         $issuingimpossible{EXPIRED} = 1;
     } else {

-        my ($y, $m, $d) =  split /-/,$borrower->{'dateexpiry'};
-        if ($y && $m && $d) { # are we really writing oinvalid dates to borrs
-            my $expiry_dt = DateTime->new(
-                year => $y,
-                month => $m,
-                day   => $d,
-                time_zone => C4::Context->tz,
-            );
-            $expiry_dt->truncate( to => 'day');
-            my $today = $now->clone()->truncate(to => 'day');
-            if (DateTime->compare($today, $expiry_dt) == 1) {
-                $issuingimpossible{EXPIRED} = 1;
-            }
-        } else {
-            carp("Invalid expity date in borr");
+        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'sql', 'floating' );
+        $expiry_dt->truncate( to => 'day');
+        my $today = $now->clone()->truncate(to => 'day');
+        $today->set_time_zone( 'floating' );
+        if ( DateTime->compare($today, $expiry_dt) == 1 ) {

             $issuingimpossible{EXPIRED} = 1;
         }
     }
             $issuingimpossible{EXPIRED} = 1;
         }
     }

+

     #
     # BORROWER STATUS
     #
     #
     # BORROWER STATUS
     #


@@ -792,9 +748,32 @@ sub CanBookBeIssued {
     # DEBTS
     my ($balance, $non_issue_charges, $other_charges) =
       C4::Members::GetMemberAccountBalance( $borrower->{'borrowernumber'} );
     # DEBTS
     my ($balance, $non_issue_charges, $other_charges) =
       C4::Members::GetMemberAccountBalance( $borrower->{'borrowernumber'} );

+

     my $amountlimit = C4::Context->preference("noissuescharge");
     my $allowfineoverride = C4::Context->preference("AllowFineOverride");
     my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");
     my $amountlimit = C4::Context->preference("noissuescharge");
     my $allowfineoverride = C4::Context->preference("AllowFineOverride");
     my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");

+
+    # Check the debt of this patrons guarantees
+    my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees");
+    $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );
+    if ( defined $no_issues_charge_guarantees ) {
+        my $p = Koha::Patrons->find( $borrower->{borrowernumber} );
+        my @guarantees = $p->guarantees();
+        my $guarantees_non_issues_charges;
+        foreach my $g ( @guarantees ) {
+            my ( $b, $n, $o ) = C4::Members::GetMemberAccountBalance( $g->id );
+            $guarantees_non_issues_charges += $n;
+        }
+
+        if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT_GUARANTEES} = $guarantees_non_issues_charges;
+        } elsif ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT_GUARANTEES} = $guarantees_non_issues_charges;
+        } elsif ( $allfinesneedoverride && $guarantees_non_issues_charges > 0 && $guarantees_non_issues_charges <= $no_issues_charge_guarantees && !$inprocess ) {
+            $needsconfirmation{DEBT_GUARANTEES} = $guarantees_non_issues_charges;
+        }
+    }
+

     if ( C4::Context->preference("IssuingInProcess") ) {
         if ( $non_issue_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
             $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );
     if ( C4::Context->preference("IssuingInProcess") ) {
         if ( $non_issue_charges > $amountlimit && !$inprocess && !$allowfineoverride) {
             $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_charges );


@@ -813,50 +792,62 @@ sub CanBookBeIssued {
             $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
         }
     }
             $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_charges );
         }
     }

+

     if ($balance > 0 && $other_charges > 0) {
         $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
     }
 
     if ($balance > 0 && $other_charges > 0) {
         $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
     }
 

-    my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
-    if ($blocktype == -1) {
-        ## patron has outstanding overdue loans
-           if ( C4::Context->preference("OverduesBlockCirc") eq 'block'){
-               $issuingimpossible{USERBLOCKEDOVERDUE} = $count;
-           }
-           elsif ( C4::Context->preference("OverduesBlockCirc") eq 'confirmation'){
-               $needsconfirmation{USERBLOCKEDOVERDUE} = $count;
-           }
-    } elsif($blocktype == 1) {
-        # patron has accrued fine days or has a restriction. $count is a date
-        if ($count eq '9999-12-31') {
-            $issuingimpossible{USERBLOCKEDNOENDDATE} = $count;
+    my $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') {
+            $issuingimpossible{USERBLOCKEDNOENDDATE} = $debarred_date;

         }
         else {
         }
         else {

-            $issuingimpossible{USERBLOCKEDWITHENDDATE} = $count;
+            $issuingimpossible{USERBLOCKEDWITHENDDATE} = $debarred_date;
+        }
+    } elsif ( my $num_overdues = $patron->has_overdues ) {
+        ## patron has outstanding overdue loans
+        if ( C4::Context->preference("OverduesBlockCirc") eq 'block'){
+            $issuingimpossible{USERBLOCKEDOVERDUE} = $num_overdues;
+        }
+        elsif ( C4::Context->preference("OverduesBlockCirc") eq 'confirmation'){
+            $needsconfirmation{USERBLOCKEDOVERDUE} = $num_overdues;

         }
     }
 
         }
     }
 

-#

     # JB34 CHECKS IF BORROWERS DON'T HAVE ISSUE TOO MANY BOOKS
     #
     # JB34 CHECKS IF BORROWERS DON'T HAVE ISSUE TOO MANY BOOKS
     #

-       my ($current_loan_count, $max_loans_allowed) = TooMany( $borrower, $item->{biblionumber}, $item );
-    # if TooMany max_loans_allowed returns 0 the user doesn't have permission to check out this book
-    if (defined $max_loans_allowed && $max_loans_allowed == 0) {
-        $needsconfirmation{PATRON_CANT} = 1;
-    } else {
-        if($max_loans_allowed){
-            if ( C4::Context->preference("AllowTooManyOverride") ) {
-                $needsconfirmation{TOO_MANY} = 1;
-                $needsconfirmation{current_loan_count} = $current_loan_count;
-                $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
-            } else {
-                $issuingimpossible{TOO_MANY} = 1;
-                $issuingimpossible{current_loan_count} = $current_loan_count;
-                $issuingimpossible{max_loans_allowed} = $max_loans_allowed;
-            }
+    my $switch_onsite_checkout =
+          C4::Context->preference('SwitchOnSiteCheckouts')
+      and $issue->{onsite_checkout}
+      and $issue
+      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->{max_allowed} == 0 ) {
+            $needsconfirmation{PATRON_CANT} = 1;
+        }
+        if ( C4::Context->preference("AllowTooManyOverride") ) {
+            $needsconfirmation{TOO_MANY} = $toomany->{reason};
+            $needsconfirmation{current_loan_count} = $toomany->{count};
+            $needsconfirmation{max_loans_allowed} = $toomany->{max_allowed};
+        } else {
+            $issuingimpossible{TOO_MANY} = $toomany->{reason};
+            $issuingimpossible{current_loan_count} = $toomany->{count};
+            $issuingimpossible{max_loans_allowed} = $toomany->{max_allowed};

         }
     }
 
         }
     }
 

+    #
+    # CHECKPREVCHECKOUT: CHECK IF ITEM HAS EVER BEEN LENT TO PATRON
+    #
+    $patron = Koha::Patrons->find($borrower->{borrowernumber});
+    my $wants_check = $patron->wants_check_for_previous_checkout;
+    $needsconfirmation{PREVISSUE} = 1
+        if ($wants_check and $patron->do_check_for_previous_checkout($item));
+

     #
     # ITEM CHECKING
     #
     #
     # ITEM CHECKING
     #


@@ -907,7 +898,8 @@ sub CanBookBeIssued {
         $issuingimpossible{RESTRICTED} = 1;
     }
     if ( $item->{'itemlost'} && C4::Context->preference("IssueLostItem") ne 'nothing' ) {
         $issuingimpossible{RESTRICTED} = 1;
     }
     if ( $item->{'itemlost'} && C4::Context->preference("IssueLostItem") ne 'nothing' ) {

-        my $code = GetAuthorisedValueByCode( 'LOST', $item->{'itemlost'} );
+        my $av = Koha::AuthorisedValues->search({ category => 'LOST', authorised_value => $item->{itemlost} });
+        my $code = $av->count ? $av->next->lib : '';

         $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' );
         $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' );
     }
         $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' );
         $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' );
     }


@@ -918,7 +910,7 @@ sub CanBookBeIssued {
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
                 $issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};
             }
                 $issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
                 $issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};
             }

-            $needsconfirmation{BORRNOTSAMEBRANCH} = GetBranchName( $borrower->{'branchcode'} )
+            $needsconfirmation{BORRNOTSAMEBRANCH} = $borrower->{'branchcode'}

               if ( $borrower->{'branchcode'} ne $userenv->{branch} );
         }
     }
               if ( $borrower->{'branchcode'} ne $userenv->{branch} );
         }
     }


@@ -940,17 +932,29 @@ sub CanBookBeIssued {
     #
     if ( $issue->{borrowernumber} && $issue->{borrowernumber} eq $borrower->{'borrowernumber'} ){
 
     #
     if ( $issue->{borrowernumber} && $issue->{borrowernumber} eq $borrower->{'borrowernumber'} ){
 

-        # Already issued to current borrower. Ask whether the loan should
-        # be renewed.
-        my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
-            $borrower->{'borrowernumber'},
-            $item->{'itemnumber'}
-        );
-        if ( $CanBookBeRenewed == 0 ) {    # no more renewals allowed
-            $issuingimpossible{NO_MORE_RENEWALS} = 1;
-        }
-        else {
-            $needsconfirmation{RENEW_ISSUE} = 1;
+        # 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->{borrowernumber}) {
         }
     }
     elsif ($issue->{borrowernumber}) {


@@ -958,12 +962,19 @@ sub CanBookBeIssued {
         # issued to someone else
         my $currborinfo =    C4::Members::GetMember( borrowernumber => $issue->{borrowernumber} );
 
         # issued to someone else
         my $currborinfo =    C4::Members::GetMember( borrowernumber => $issue->{borrowernumber} );
 

-#        warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
-        $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'};
+
+        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 ) {
     }
 
     unless ( $ignore_reserves ) {


@@ -973,7 +984,6 @@ sub CanBookBeIssued {
             my $resbor = $res->{'borrowernumber'};
             if ( $resbor ne $borrower->{'borrowernumber'} ) {
                 my ( $resborrower ) = C4::Members::GetMember( borrowernumber => $resbor );
             my $resbor = $res->{'borrowernumber'};
             if ( $resbor ne $borrower->{'borrowernumber'} ) {
                 my ( $resborrower ) = C4::Members::GetMember( borrowernumber => $resbor );

-                my $branchname = GetBranchName( $res->{'branchcode'} );

                 if ( $restype eq "Waiting" )
                 {
                     # The item is on reserve and waiting, but has been
                 if ( $restype eq "Waiting" )
                 {
                     # The item is on reserve and waiting, but has been


@@ -983,8 +993,8 @@ sub CanBookBeIssued {
                     $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
                     $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
                     $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
                     $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
                     $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
                     $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};

-                    $needsconfirmation{'resbranchname'} = $branchname;
-                    $needsconfirmation{'reswaitingdate'} = format_date($res->{'waitingdate'});
+                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
+                    $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'};

                 }
                 elsif ( $restype eq "Reserved" ) {
                     # The item is on reserve for someone else.
                 }
                 elsif ( $restype eq "Reserved" ) {
                     # The item is on reserve for someone else.


@@ -993,8 +1003,8 @@ sub CanBookBeIssued {
                     $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
                     $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
                     $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
                     $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
                     $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
                     $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};

-                    $needsconfirmation{'resbranchname'} = $branchname;
-                    $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'});
+                    $needsconfirmation{'resbranchcode'} = $res->{branchcode};
+                    $needsconfirmation{'resreservedate'} = $res->{'reservedate'};

                 }
             }
         }
                 }
             }
         }


@@ -1013,17 +1023,24 @@ sub CanBookBeIssued {
     }
 
     ## check for high holds decreasing loan period
     }
 
     ## check for high holds decreasing loan period

-    my $decrease_loan = C4::Context->preference('decreaseLoanHighHolds');
-    if ( $decrease_loan && $decrease_loan == 1 ) {
-        my ( $reserved, $num, $duration, $returndate ) =
-          checkHighHolds( $item, $borrower );
-
-        if ( $num >= C4::Context->preference('decreaseLoanHighHoldsValue') ) {
-            $needsconfirmation{HIGHHOLDS} = {
-                num_holds  => $num,
-                duration   => $duration,
-                returndate => output_pref($returndate),
-            };
+    if ( C4::Context->preference('decreaseLoanHighHolds') ) {
+        my $check = checkHighHolds( $item, $borrower );
+
+        if ( $check->{exceeded} ) {
+            if ($override_high_holds) {
+                $alerts{HIGHHOLDS} = {
+                    num_holds  => $check->{outstanding},
+                    duration   => $check->{duration},
+                    returndate => output_pref( $check->{due_date} ),
+                };
+            }
+            else {
+                $needsconfirmation{HIGHHOLDS} = {
+                    num_holds  => $check->{outstanding},
+                    duration   => $check->{duration},
+                    returndate => output_pref( $check->{due_date} ),
+                };
+            }

         }
     }
 
         }
     }
 


@@ -1053,7 +1070,7 @@ sub CanBookBeIssued {
         }
     }
 
         }
     }
 

-    return ( \%issuingimpossible, \%needsconfirmation, \%alerts );
+    return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages, );

 }
 
 =head2 CanBookBeReturned
 }
 
 =head2 CanBookBeReturned


@@ -1117,13 +1134,60 @@ sub checkHighHolds {
     my ( $item, $borrower ) = @_;
     my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} );
     my $branch = _GetCircControlBranch( $item, $borrower );
     my ( $item, $borrower ) = @_;
     my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} );
     my $branch = _GetCircControlBranch( $item, $borrower );

-    my $dbh    = C4::Context->dbh;
-    my $sth    = $dbh->prepare(
-'select count(borrowernumber) as num_holds from reserves where biblionumber=?'
-    );
-    $sth->execute( $item->{'biblionumber'} );
-    my ($holds) = $sth->fetchrow_array;
-    if ($holds) {
+
+    my $return_data = {
+        exceeded    => 0,
+        outstanding => 0,
+        duration    => 0,
+        due_date    => undef,
+    };
+
+    my $holds = Koha::Holds->search( { biblionumber => $item->{'biblionumber'} } );
+
+    if ( $holds->count() ) {
+        $return_data->{outstanding} = $holds->count();
+
+        my $decreaseLoanHighHoldsControl        = C4::Context->preference('decreaseLoanHighHoldsControl');
+        my $decreaseLoanHighHoldsValue          = C4::Context->preference('decreaseLoanHighHoldsValue');
+        my $decreaseLoanHighHoldsIgnoreStatuses = C4::Context->preference('decreaseLoanHighHoldsIgnoreStatuses');
+
+        my @decreaseLoanHighHoldsIgnoreStatuses = split( /,/, $decreaseLoanHighHoldsIgnoreStatuses );
+
+        if ( $decreaseLoanHighHoldsControl eq 'static' ) {
+
+            # static means just more than a given number of holds on the record
+
+            # If the number of holds is less than the threshold, we can stop here
+            if ( $holds->count() < $decreaseLoanHighHoldsValue ) {
+                return $return_data;
+            }
+        }
+        elsif ( $decreaseLoanHighHoldsControl eq 'dynamic' ) {
+
+            # dynamic means X more than the number of holdable items on the record
+
+            # let's get the items
+            my @items = $holds->next()->biblio()->items();
+
+            # Remove any items with status defined to be ignored even if the would not make item unholdable
+            foreach my $status (@decreaseLoanHighHoldsIgnoreStatuses) {
+                @items = grep { !$_->$status } @items;
+            }
+
+            # Remove any items that are not holdable for this patron
+            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber ) eq 'OK' } @items;
+
+            my $items_count = scalar @items;
+
+            my $threshold = $items_count + $decreaseLoanHighHoldsValue;
+
+            # If the number of holds is less than the count of items we have
+            # plus the number of holds allowed above that count, we can stop here
+            if ( $holds->count() <= $threshold ) {
+                return $return_data;
+            }
+        }
+

         my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
 
         my $calendar = Koha::Calendar->new( branchcode => $branch );
         my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
 
         my $calendar = Koha::Calendar->new( branchcode => $branch );


@@ -1132,21 +1196,21 @@ sub checkHighHolds {
           ( C4::Context->preference('item-level_itypes') )
           ? $biblio->{'itype'}
           : $biblio->{'itemtype'};
           ( C4::Context->preference('item-level_itypes') )
           ? $biblio->{'itype'}
           : $biblio->{'itemtype'};

-        my $orig_due =
-          C4::Circulation::CalcDateDue( $issuedate, $itype, $branch,
-            $borrower );

 
 

-        my $reduced_datedue =
-          $calendar->addDate( $issuedate,
-            C4::Context->preference('decreaseLoanHighHoldsDuration') );
+        my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branch, $borrower );
+
+        my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration');
+
+        my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration );

 
         if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {
 
         if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) {

-            return ( 1, $holds,
-                C4::Context->preference('decreaseLoanHighHoldsDuration'),
-                $reduced_datedue );
+            $return_data->{exceeded} = 1;
+            $return_data->{duration} = $decreaseLoanHighHoldsDuration;
+            $return_data->{due_date} = $reduced_datedue;

         }
     }
         }
     }

-    return ( 0, 0, 0, undef );
+
+    return $return_data;

 }
 
 =head2 AddIssue
 }
 
 =head2 AddIssue


@@ -1161,13 +1225,13 @@ Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this
 
 =item C<$barcode> is the barcode of the item being issued.
 
 
 =item C<$barcode> is the barcode of the item being issued.
 

-=item C<$datedue> is a C4::Dates object for the max date of return, i.e. the date due (optional).
+=item C<$datedue> is a DateTime object for the max date of return, i.e. the date due (optional).

 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).
 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 C4::Dates object, unfortunately.
+Defaults to today.  Unlike C<$datedue>, NOT a DateTime object, unfortunately.

 
 AddIssue does the following things :
 
 
 AddIssue does the following things :
 


@@ -1189,172 +1253,199 @@ AddIssue does the following things :
 
 sub AddIssue {
     my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode, $params ) = @_;
 
 sub AddIssue {
     my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode, $params ) = @_;

+

     my $onsite_checkout = $params && $params->{onsite_checkout} ? 1 : 0;
     my $onsite_checkout = $params && $params->{onsite_checkout} ? 1 : 0;

+    my $switch_onsite_checkout = $params && $params->{switch_onsite_checkout};

     my $auto_renew = $params && $params->{auto_renew};
     my $auto_renew = $params && $params->{auto_renew};

-    my $dbh = C4::Context->dbh;
-    my $barcodecheck=CheckValidBarcode($barcode);
+    my $dbh          = C4::Context->dbh;
+    my $barcodecheck = CheckValidBarcode($barcode);

 
     my $issue;
 
 
     my $issue;
 

-    if ($datedue && ref $datedue ne 'DateTime') {
+    if ( $datedue && ref $datedue ne 'DateTime' ) {

         $datedue = dt_from_string($datedue);
     }
         $datedue = dt_from_string($datedue);
     }

+

     # $issuedate defaults to today.
     # $issuedate defaults to today.

-    if ( ! defined $issuedate ) {
-        $issuedate = DateTime->now(time_zone => C4::Context->tz());
+    if ( !defined $issuedate ) {
+        $issuedate = DateTime->now( time_zone => C4::Context->tz() );

     }
     else {
     }
     else {

-        if ( ref $issuedate ne 'DateTime') {
+        if ( ref $issuedate ne 'DateTime' ) {

             $issuedate = dt_from_string($issuedate);
 
         }
     }
             $issuedate = dt_from_string($issuedate);
 
         }
     }

-       if ($borrower and $barcode and $barcodecheck ne '0'){#??? wtf
-               # find which item we issue
-               my $item = GetItem('', $barcode) or return;     # if we don't get an Item, abort.
-               my $branch = _GetCircControlBranch($item,$borrower);
-               
-               # get actual issuing if there is one
-               my $actualissue = GetItemIssue( $item->{itemnumber});
-               
-               # get biblioinformation for this item
-               my $biblio = GetBiblioFromItemNumber($item->{itemnumber});
-               
-               #
-               # check if we just renew the issue.
-               #
-               if ($actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}) {
-                   $datedue = AddRenewal(
-                       $borrower->{'borrowernumber'},
-                       $item->{'itemnumber'},
-                       $branch,
-                       $datedue,
-                       $issuedate, # here interpreted as the renewal date
-                       );
-               }
-               else {
-        # it's NOT a renewal
-                       if ( $actualissue->{borrowernumber}) {
-                               # This book is currently on loan, but not to the person
-                               # who wants to borrow it now. mark it returned before issuing to the new borrower
-                               AddReturn(
-                                       $item->{'barcode'},
-                                       C4::Context->userenv->{'branch'}
-                               );
-                       }
+
+    # Stop here if the patron or barcode doesn't exist
+    if ( $borrower && $barcode && $barcodecheck ) {
+        # find which item we issue
+        my $item = GetItem( '', $barcode )
+          or return;    # if we don't get an Item, abort.
+
+        my $branch = _GetCircControlBranch( $item, $borrower );
+
+        # get actual issuing if there is one
+        my $actualissue = GetItemIssue( $item->{itemnumber} );
+
+        # get biblioinformation for this item
+        my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} );
+
+        # check if we just renew the issue.
+        if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}
+                and not $switch_onsite_checkout ) {
+            $datedue = AddRenewal(
+                $borrower->{'borrowernumber'},
+                $item->{'itemnumber'},
+                $branch,
+                $datedue,
+                $issuedate,    # here interpreted as the renewal date
+            );
+        }
+        else {
+            # it's NOT a renewal
+            if ( $actualissue->{borrowernumber}
+                    and not $switch_onsite_checkout ) {
+                # This book is currently on loan, but not to the person
+                # who wants to borrow it now. mark it returned before issuing to the new borrower
+                my ( $allowed, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} );
+                return unless $allowed;
+                AddReturn( $item->{'barcode'}, C4::Context->userenv->{'branch'} );
+            }

 
             MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
 
             MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );

-                       # Starting process for transfer job (checking transfert and validate it if we have one)
-            my ($datesent) = GetTransfers($item->{'itemnumber'});
+
+            # Starting process for transfer job (checking transfert and validate it if we have one)
+            my ($datesent) = GetTransfers( $item->{'itemnumber'} );

             if ($datesent) {
             if ($datesent) {

-        #      updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for visibility of this case (maybe for stats ....)
-                my $sth =
-                    $dbh->prepare(
+                # updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for visibility of this case (maybe for stats ....)
+                my $sth = $dbh->prepare(

                     "UPDATE branchtransfers 
                         SET datearrived = now(),
                         tobranch = ?,
                         comments = 'Forced branchtransfer'
                     WHERE itemnumber= ? AND datearrived IS NULL"
                     "UPDATE branchtransfers 
                         SET datearrived = now(),
                         tobranch = ?,
                         comments = 'Forced branchtransfer'
                     WHERE itemnumber= ? AND datearrived IS NULL"

-                    );
-                $sth->execute(C4::Context->userenv->{'branch'},$item->{'itemnumber'});
+                );
+                $sth->execute( C4::Context->userenv->{'branch'},
+                    $item->{'itemnumber'} );

             }
 
             }
 

-        # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
-        unless ($auto_renew) {
-            my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branch);
-            $auto_renew = $issuingrule->{auto_renew};
-        }
+            # If automatic renewal wasn't selected while issuing, set the value according to the issuing rule.
+            unless ($auto_renew) {
+                my $issuingrule = GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branch );
+                $auto_renew = $issuingrule->{auto_renew};
+            }

 
 

-        # Record in the database the fact that the book was issued.
-        unless ($datedue) {
-            my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
-            $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );
+            # Record in the database the fact that the book was issued.
+            unless ($datedue) {
+                my $itype =
+                  ( C4::Context->preference('item-level_itypes') )
+                  ? $biblio->{'itype'}
+                  : $biblio->{'itemtype'};
+                $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower );

 
 

-        }
-        $datedue->truncate( to => 'minute');
-
-        $issue = Koha::Database->new()->schema()->resultset('Issue')->create(
-            {
-                borrowernumber  => $borrower->{'borrowernumber'},
-                itemnumber      => $item->{'itemnumber'},
-                issuedate       => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
-                date_due        => $datedue->strftime('%Y-%m-%d %H:%M:%S'),
-                branchcode      => C4::Context->userenv->{'branch'},
-                onsite_checkout => $onsite_checkout,
-                auto_renew      => $auto_renew ? 1 : 0

             }
             }

-        );
+            $datedue->truncate( to => 'minute' );

 
 

-        if ( C4::Context->preference('ReturnToShelvingCart') ) { ## ReturnToShelvingCart is on, anything issued should be taken off the cart.
-          CartToShelf( $item->{'itemnumber'} );
-        }
-        $item->{'issues'}++;
-        if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) {
-            UpdateTotalIssues($item->{'biblionumber'}, 1);
-        }
+            $issue = Koha::Database->new()->schema()->resultset('Issue')->update_or_create(
+                {
+                    borrowernumber => $borrower->{'borrowernumber'},
+                    itemnumber     => $item->{'itemnumber'},
+                    issuedate      => $issuedate->strftime('%Y-%m-%d %H:%M:%S'),
+                    date_due       => $datedue->strftime('%Y-%m-%d %H:%M:%S'),
+                    branchcode     => C4::Context->userenv->{'branch'},
+                    onsite_checkout => $onsite_checkout,
+                    auto_renew      => $auto_renew ? 1 : 0
+                }
+              );

 
 

-        ## If item was lost, it has now been found, reverse any list item charges if necessary.
-        if ( $item->{'itemlost'} ) {
-            if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
-                _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'} );
+            if ( C4::Context->preference('ReturnToShelvingCart') ) {
+                # ReturnToShelvingCart is on, anything issued should be taken off the cart.
+                CartToShelf( $item->{'itemnumber'} );
+            }
+            $item->{'issues'}++;
+            if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) {
+                UpdateTotalIssues( $item->{'biblionumber'}, 1 );

             }
             }

-        }

 
 

-        ModItem({ issues           => $item->{'issues'},
-                  holdingbranch    => C4::Context->userenv->{'branch'},
-                  itemlost         => 0,
-                  datelastborrowed => DateTime->now(time_zone => C4::Context->tz())->ymd(),
-                  onloan           => $datedue->ymd(),
-                }, $item->{'biblionumber'}, $item->{'itemnumber'});
-        ModDateLastSeen( $item->{'itemnumber'} );
+            ## If item was lost, it has now been found, reverse any list item charges if necessary.
+            if ( $item->{'itemlost'} ) {
+                if (
+                    Koha::RefundLostItemFeeRules->should_refund(
+                        {
+                            current_branch      => C4::Context->userenv->{branch},
+                            item_home_branch    => $item->{homebranch},
+                            item_holding_branch => $item->{holdingbranch}
+                        }
+                    )
+                  )
+                {
+                    _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef,
+                        $item->{'barcode'} );
+                }
+            }

 
 

-        # If it costs to borrow this book, charge it to the patron's account.
-        my ( $charge, $itemtype ) = GetIssuingCharges(
-            $item->{'itemnumber'},
-            $borrower->{'borrowernumber'}
-        );
-        if ( $charge > 0 ) {
-            AddIssuingCharge(
-                $item->{'itemnumber'},
-                $borrower->{'borrowernumber'}, $charge
+            ModItem(
+                {
+                    issues        => $item->{'issues'},
+                    holdingbranch => C4::Context->userenv->{'branch'},
+                    itemlost      => 0,
+                    onloan        => $datedue->ymd(),
+                    datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
+                },
+                $item->{'biblionumber'},
+                $item->{'itemnumber'}

             );
             );

-            $item->{'charge'} = $charge;
-        }
+            ModDateLastSeen( $item->{'itemnumber'} );

 
 

-        # Record the fact that this book was issued.
-        &UpdateStats({
-                      branch => C4::Context->userenv->{'branch'},
-                      type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
-                      amount => $charge,
-                      other => ($sipmode ? "SIP-$sipmode" : ''),
-                      itemnumber => $item->{'itemnumber'},
-                      itemtype => $item->{'itype'},
-                      borrowernumber => $borrower->{'borrowernumber'},
-                      ccode => $item->{'ccode'}}
-        );
+           # If it costs to borrow this book, charge it to the patron's account.
+            my ( $charge, $itemtype ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
+            if ( $charge > 0 ) {
+                AddIssuingCharge( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $charge );
+                $item->{'charge'} = $charge;
+            }

 
 

-        # Send a checkout slip.
-        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
-        my %conditions = (
-            branchcode   => $branch,
-            categorycode => $borrower->{categorycode},
-            item_type    => $item->{itype},
-            notification => 'CHECKOUT',
-        );
-        if ($circulation_alert->is_enabled_for(\%conditions)) {
-            SendCirculationAlert({
-                type     => 'CHECKOUT',
-                item     => $item,
-                borrower => $borrower,
-                branch   => $branch,
-            });
+            # Record the fact that this book was issued.
+            &UpdateStats(
+                {
+                    branch => C4::Context->userenv->{'branch'},
+                    type => ( $onsite_checkout ? 'onsite_checkout' : 'issue' ),
+                    amount         => $charge,
+                    other          => ( $sipmode ? "SIP-$sipmode" : '' ),
+                    itemnumber     => $item->{'itemnumber'},
+                    itemtype       => $item->{'itype'},
+                    borrowernumber => $borrower->{'borrowernumber'},
+                    ccode          => $item->{'ccode'}
+                }
+            );
+
+            # Send a checkout slip.
+            my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+            my %conditions        = (
+                branchcode   => $branch,
+                categorycode => $borrower->{categorycode},
+                item_type    => $item->{itype},
+                notification => 'CHECKOUT',
+            );
+            if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
+                SendCirculationAlert(
+                    {
+                        type     => 'CHECKOUT',
+                        item     => $item,
+                        borrower => $borrower,
+                        branch   => $branch,
+                    }
+                );
+            }

         }
         }

-    }

 
 

-    logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'itemnumber'})
-        if C4::Context->preference("IssueLog");
-  }
-  return $issue;
+        logaction(
+            "CIRCULATION", "ISSUE",
+            $borrower->{'borrowernumber'},
+            $biblio->{'itemnumber'}
+        ) if C4::Context->preference("IssueLog");
+    }
+    return $issue;

 }
 
 =head2 GetLoanLength
 }
 
 =head2 GetLoanLength


@@ -1383,47 +1474,47 @@ sub GetLoanLength {
     my $loanlength = $sth->fetchrow_hashref;
 
     return $loanlength
     my $loanlength = $sth->fetchrow_hashref;
 
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( $borrowertype, '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( $borrowertype, '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( '*', $itemtype, $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( '*', $itemtype, $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( '*', '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( '*', '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( $borrowertype, $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( $borrowertype, $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( $borrowertype, '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( $borrowertype, '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( '*', $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( '*', $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
     $sth->execute( '*', '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
 
     $sth->execute( '*', '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength

-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};

 
 

-    # if no rule is set => 21 days (hardcoded)
+    # if no rule is set => 0 day (hardcoded)

     return {
     return {

-        issuelength => 21,
-        renewalperiod => 21,
+        issuelength => 0,
+        renewalperiod => 0,

         lengthunit => 'days',
     };
 
         lengthunit => 'days',
     };
 


@@ -1519,6 +1610,10 @@ maxissueqty - maximum number of loans that a
 patron of the given category can have at the given
 branch.  If the value is undef, no limit.
 
 patron of the given category can have at the given
 branch.  If the value is undef, no limit.
 

+maxonsiteissueqty - maximum of on-site checkouts that a
+patron of the given category can have at the given
+branch.  If the value is undef, no limit.
+

 This will first check for a specific branch and
 category match from branch_borrower_circ_rules. 
 
 This will first check for a specific branch and
 category match from branch_borrower_circ_rules. 
 


@@ -1532,6 +1627,7 @@ If no rule has been found in the database, it will default to
 the buillt in rule:
 
 maxissueqty - undef
 the buillt in rule:
 
 maxissueqty - undef

+maxonsiteissueqty - undef

 
 C<$branchcode> and C<$categorycode> should contain the
 literal branch code and patron category code, respectively - no
 
 C<$branchcode> and C<$categorycode> should contain the
 literal branch code and patron category code, respectively - no


@@ -1540,53 +1636,45 @@ wildcards.
 =cut
 
 sub GetBranchBorrowerCircRule {
 =cut
 
 sub GetBranchBorrowerCircRule {

-    my $branchcode = shift;
-    my $categorycode = shift;
+    my ( $branchcode, $categorycode ) = @_;

 
 

-    my $branch_cat_query = "SELECT maxissueqty
-                            FROM branch_borrower_circ_rules
-                            WHERE branchcode = ?
-                            AND   categorycode = ?";
+    my $rules;

     my $dbh = C4::Context->dbh();
     my $dbh = C4::Context->dbh();

-    my $sth = $dbh->prepare($branch_cat_query);
-    $sth->execute($branchcode, $categorycode);
-    my $result;
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM branch_borrower_circ_rules
+        WHERE branchcode = ?
+        AND   categorycode = ?
+    |, {}, $branchcode, $categorycode ) ;
+    return $rules if $rules;

 
     # try same branch, default borrower category
 
     # try same branch, default borrower category

-    my $branch_query = "SELECT maxissueqty
-                        FROM default_branch_circ_rules
-                        WHERE branchcode = ?";
-    $sth = $dbh->prepare($branch_query);
-    $sth->execute($branchcode);
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM default_branch_circ_rules
+        WHERE branchcode = ?
+    |, {}, $branchcode ) ;
+    return $rules if $rules;

 
     # try default branch, same borrower category
 
     # try default branch, same borrower category

-    my $category_query = "SELECT maxissueqty
-                          FROM default_borrower_circ_rules
-                          WHERE categorycode = ?";
-    $sth = $dbh->prepare($category_query);
-    $sth->execute($categorycode);
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
-  
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM default_borrower_circ_rules
+        WHERE categorycode = ?
+    |, {}, $categorycode ) ;
+    return $rules if $rules;
+

     # try default branch, default borrower category
     # try default branch, default borrower category

-    my $default_query = "SELECT maxissueqty
-                          FROM default_circ_rules";
-    $sth = $dbh->prepare($default_query);
-    $sth->execute();
-    if ($result = $sth->fetchrow_hashref()) {
-        return $result;
-    }
-    
+    $rules = $dbh->selectrow_hashref( q|
+        SELECT maxissueqty, maxonsiteissueqty
+        FROM default_circ_rules
+    |, {} );
+    return $rules if $rules;
+

     # built-in default circulation rule
     return {
         maxissueqty => undef,
     # built-in default circulation rule
     return {
         maxissueqty => undef,

+        maxonsiteissueqty => undef,

     };
 }
 
     };
 }
 


@@ -1626,17 +1714,17 @@ sub GetBranchItemRule {
     my $result = {};
 
     my @attempts = (
     my $result = {};
 
     my @attempts = (

-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy

             FROM branch_item_rules
             WHERE branchcode = ?
               AND itemtype = ?', $branchcode, $itemtype],
             FROM branch_item_rules
             WHERE branchcode = ?
               AND itemtype = ?', $branchcode, $itemtype],

-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy

             FROM default_branch_circ_rules
             WHERE branchcode = ?', $branchcode],
             FROM default_branch_circ_rules
             WHERE branchcode = ?', $branchcode],

-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy

             FROM default_branch_item_rules
             WHERE itemtype = ?', $itemtype],
             FROM default_branch_item_rules
             WHERE itemtype = ?', $itemtype],

-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy

             FROM default_circ_rules'],
     );
 
             FROM default_circ_rules'],
     );
 


@@ -1649,11 +1737,13 @@ sub GetBranchItemRule {
         # defaults tables, we have to check that the key we want is set, not
         # just that a row was returned
         $result->{'holdallowed'}  = $search_result->{'holdallowed'}  unless ( defined $result->{'holdallowed'} );
         # defaults tables, we have to check that the key we want is set, not
         # just that a row was returned
         $result->{'holdallowed'}  = $search_result->{'holdallowed'}  unless ( defined $result->{'holdallowed'} );

+        $result->{'hold_fulfillment_policy'} = $search_result->{'hold_fulfillment_policy'} unless ( defined $result->{'hold_fulfillment_policy'} );

         $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} );
     }
     
     # built-in default circulation rule
     $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} );
         $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} );
     }
     
     # built-in default circulation rule
     $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} );

+    $result->{'hold_fulfillment_policy'} = 'any' unless ( defined $result->{'hold_fulfillment_policy'} );

     $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} );
 
     return $result;
     $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} );
 
     return $result;


@@ -1747,23 +1837,29 @@ patron who last borrowed the book.
 sub AddReturn {
     my ( $barcode, $branch, $exemptfine, $dropbox, $return_date, $dropboxdate ) = @_;
 
 sub AddReturn {
     my ( $barcode, $branch, $exemptfine, $dropbox, $return_date, $dropboxdate ) = @_;
 

-    if ($branch and not GetBranchDetail($branch)) {
+    if ($branch and not Koha::Libraries->find($branch)) {

         warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
         undef $branch;
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
     my $messages;
     my $borrower;
         warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
         undef $branch;
     }
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
     my $messages;
     my $borrower;

-    my $biblio;

     my $doreturn       = 1;
     my $validTransfert = 0;
     my $stat_type = 'return';
 
     # get information on item
     my $doreturn       = 1;
     my $validTransfert = 0;
     my $stat_type = 'return';
 
     # get information on item

-    my $itemnumber = GetItemnumberFromBarcode( $barcode );
-    unless ($itemnumber) {
-        return (0, { BadBarcode => $barcode }); # no barcode means no item or borrower.  bail out.
+    my $item = GetItem( undef, $barcode );
+    unless ($item) {
+        return ( 0, { BadBarcode => $barcode } );    # no barcode means no item or borrower.  bail out.

     }
     }

+
+    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 $issue  = GetItemIssue($itemnumber);
     if ($issue and $issue->{borrowernumber}) {
         $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber})
     my $issue  = GetItemIssue($itemnumber);
     if ($issue and $issue->{borrowernumber}) {
         $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber})


@@ -1781,8 +1877,6 @@ sub AddReturn {
         }
     }
 
         }
     }
 

-    my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed";
-

     if ( $item->{'location'} eq 'PROC' ) {
         if ( C4::Context->preference("InProcessingToShelvingCart") ) {
             $item->{'location'} = 'CART';
     if ( $item->{'location'} eq 'PROC' ) {
         if ( C4::Context->preference("InProcessingToShelvingCart") ) {
             $item->{'location'} = 'CART';


@@ -1826,8 +1920,10 @@ sub AddReturn {
     # check if the book is in a permanent collection....
     # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
     if ( $returnbranch ) {
     # check if the book is in a permanent collection....
     # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
     if ( $returnbranch ) {

-        my $branches = GetBranches();    # a potentially expensive call for a non-feature.
-        $branches->{$returnbranch}->{PE} and $messages->{'IsPermanent'} = $returnbranch;
+        my $library = Koha::Libraries->find($returnbranch);
+        if ( $library and $library->get_categories->search({'me.categorycode' => 'PE'})->count ) {
+            $messages->{'IsPermanent'} = $returnbranch;
+        }

     }
 
     # check if the return is allowed at this branch
     }
 
     # check if the return is allowed at this branch


@@ -1857,46 +1953,14 @@ sub AddReturn {
             # 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
             # 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) unless ( $item->{'issuedate'} eq C4::Dates->today('iso') );;
+

             $circControlBranch = _GetCircControlBranch($item,$borrower);
             $issue->{'overdue'} = DateTime->compare($issue->{'date_due'}, $dropboxdate ) == -1 ? 1 : 0;
         }
 
         if ($borrowernumber) {
             if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
             $circControlBranch = _GetCircControlBranch($item,$borrower);
             $issue->{'overdue'} = DateTime->compare($issue->{'date_due'}, $dropboxdate ) == -1 ? 1 : 0;
         }
 
         if ($borrowernumber) {
             if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {

-                # we only need to calculate and change the fines if we want to do that on return
-                # Should be on for hourly loans
-                my $control = C4::Context->preference('CircControl');
-                my $control_branchcode =
-                    ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
-                  : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
-                  :                                     $issue->{branchcode};
-
-                my $date_returned =
-                  $return_date ? dt_from_string($return_date) : $today;
-
-                my ( $amount, $type, $unitcounttotal ) =
-                  C4::Overdues::CalcFine( $item, $borrower->{categorycode},
-                    $control_branchcode, $datedue, $date_returned );
-
-                $type ||= q{};
-
-                if ( C4::Context->preference('finesMode') eq 'production' ) {
-                    if ( $amount > 0 ) {
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            $amount, $type, output_pref($datedue) );
-                    }
-                    elsif ($return_date) {
-
-                       # Backdated returns may have fines that shouldn't exist,
-                       # so in this case, we need to drop those fines to 0
-
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            0, $type, output_pref($datedue) );
-                    }
-                }
+                _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );

             }
 
             eval {
             }
 
             eval {


@@ -1922,6 +1986,7 @@ sub AddReturn {
 
     # the holdingbranch is updated if the document is returned to another location.
     # this is always done regardless of whether the item was on loan or not
 
     # the holdingbranch is updated if the document is returned to another location.
     # this is always done regardless of whether the item was on loan or not

+    my $item_holding_branch = $item->{ holdingbranch };

     if ($item->{'holdingbranch'} ne $branch) {
         UpdateHoldingbranch($branch, $item->{'itemnumber'});
         $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
     if ($item->{'holdingbranch'} ne $branch) {
         UpdateHoldingbranch($branch, $item->{'itemnumber'});
         $item->{'holdingbranch'} = $branch; # update item data holdingbranch too


@@ -1955,9 +2020,20 @@ sub AddReturn {
     if ( $item->{'itemlost'} ) {
         $messages->{'WasLost'} = 1;
 
     if ( $item->{'itemlost'} ) {
         $messages->{'WasLost'} = 1;
 

-        if ( C4::Context->preference('RefundLostItemFeeOnReturn' ) ) {
-            _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $barcode);    # can tolerate undef $borrowernumber
-            $messages->{'LostItemFeeRefunded'} = 1;
+        if ( $item->{'itemlost'} ) {
+            if (
+                Koha::RefundLostItemFeeRules->should_refund(
+                    {
+                        current_branch      => C4::Context->userenv->{branch},
+                        item_home_branch    => $item->{homebranch},
+                        item_holding_branch => $item_holding_branch
+                    }
+                )
+              )
+            {
+                _FixAccountForLostAndReturned( $item->{'itemnumber'}, $borrowernumber, $barcode );
+                $messages->{'LostItemFeeRefunded'} = 1;
+            }

         }
     }
 
         }
     }
 


@@ -2001,15 +2077,14 @@ sub AddReturn {
     }
 
     # Record the fact that this book was returned.
     }
 
     # Record the fact that this book was returned.

-    # FIXME itemtype should record item level type, not bibliolevel type

     UpdateStats({
     UpdateStats({

-                branch => $branch,
-                type => $stat_type,
-                itemnumber => $item->{'itemnumber'},
-                itemtype => $biblio->{'itemtype'},
-                borrowernumber => $borrowernumber,
-                ccode => $item->{'ccode'}}
-    );
+        branch         => $branch,
+        type           => $stat_type,
+        itemnumber     => $itemnumber,
+        itemtype       => $itemtype,
+        borrowernumber => $borrowernumber,
+        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';
 
     # Send a check-in slip. # NOTE: borrower may be undef.  probably shouldn't try to send messages then.
     my $circulation_alert = 'C4::ItemCirculationAlertPreference';


@@ -2035,7 +2110,7 @@ sub AddReturn {
     if ( $borrowernumber
       && $borrower->{'debarred'}
       && C4::Context->preference('AutoRemoveOverduesRestrictions')
     if ( $borrowernumber
       && $borrower->{'debarred'}
       && C4::Context->preference('AutoRemoveOverduesRestrictions')

-      && !C4::Members::HasOverdues( $borrowernumber )
+      && !Koha::Patrons->find( $borrowernumber )->has_overdues

       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });


@@ -2092,7 +2167,7 @@ sub MarkIssueReturned {
         # We need to check if the anonymous patron exist, Koha will fail loudly if it does not
         # Note that a warning should appear on the about page (System information tab).
         $anonymouspatron = C4::Context->preference('AnonymousPatron');
         # We need to check if the anonymous patron exist, Koha will fail loudly if it does not
         # 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 a privacy on returning item but the AnonymousPatron pref is not set correctly"
+        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 $dbh   = C4::Context->dbh;
             unless C4::Members::GetMember( borrowernumber => $anonymouspatron );
     }
     my $dbh   = C4::Context->dbh;


@@ -2131,6 +2206,12 @@ sub MarkIssueReturned {
     $sth_del->execute($borrowernumber, $itemnumber);
 
     ModItem( { 'onloan' => undef }, undef, $itemnumber );
     $sth_del->execute($borrowernumber, $itemnumber);
 
     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
 }
 
 =head2 _debar_user_on_return


@@ -2192,14 +2273,15 @@ sub _debar_user_on_return {
             my $new_debar_dt =
               $dt_today->clone()->add_duration( $suspension_days );
 
             my $new_debar_dt =
               $dt_today->clone()->add_duration( $suspension_days );
 

-            Koha::Borrower::Debarments::AddUniqueDebarment({
+            Koha::Patron::Debarments::AddUniqueDebarment({

                 borrowernumber => $borrower->{borrowernumber},
                 expiration     => $new_debar_dt->ymd(),
                 type           => 'SUSPENSION',
             });
             # if borrower was already debarred but does not get an extra debarment
                 borrowernumber => $borrower->{borrowernumber},
                 expiration     => $new_debar_dt->ymd(),
                 type           => 'SUSPENSION',
             });
             # if borrower was already debarred but does not get an extra debarment

-            if ( $borrower->{debarred} eq Koha::Borrower::Debarments::IsDebarred($borrower->{borrowernumber}) ) {
-                    return ($borrower->{debarred},1);
+            my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
+            if ( $borrower->{debarred} eq $patron->is_debarred ) {
+                return ($borrower->{debarred},1);

             }
             return $new_debar_dt->ymd();
         }
             }
             return $new_debar_dt->ymd();
         }


@@ -2676,6 +2758,7 @@ sub CanBookBeRenewed {
 
     my $item      = GetItem($itemnumber)      or return ( 0, 'no_item' );
     my $itemissue = GetItemIssue($itemnumber) or return ( 0, 'no_checkout' );
 
     my $item      = GetItem($itemnumber)      or return ( 0, 'no_item' );
     my $itemissue = GetItemIssue($itemnumber) or return ( 0, 'no_checkout' );

+    return ( 0, 'onsite_checkout' ) if $itemissue->{onsite_checkout};

 
     $borrowernumber ||= $itemissue->{borrowernumber};
     my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )
 
     $borrowernumber ||= $itemissue->{borrowernumber};
     my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber )


@@ -2702,6 +2785,7 @@ sub CanBookBeRenewed {
                 {
                     biblionumber => $resrec->{biblionumber},
                     onloan       => undef,
                 {
                     biblionumber => $resrec->{biblionumber},
                     onloan       => undef,

+                    notforloan   => 0,

                     -not         => { itemnumber => $itemnumber }
                 },
                 { columns => 'itemnumber' }
                     -not         => { itemnumber => $itemnumber }
                 },
                 { columns => 'itemnumber' }


@@ -2746,7 +2830,6 @@ sub CanBookBeRenewed {
             }
         }
     }
             }
         }
     }

-

     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
 
     return ( 1, undef ) if $override_limit;
     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
 
     return ( 1, undef ) if $override_limit;


@@ -2758,22 +2841,54 @@ sub CanBookBeRenewed {
     return ( 0, "too_many" )
       if $issuingrule->{renewalsallowed} <= $itemissue->{renewals};
 
     return ( 0, "too_many" )
       if $issuingrule->{renewalsallowed} <= $itemissue->{renewals};
 

-    if ( $issuingrule->{norenewalbefore} ) {
+    my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
+    my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
+    my $patron      = Koha::Patrons->find($borrowernumber);
+    my $restricted  = $patron->is_debarred;
+    my $hasoverdues = $patron->has_overdues;
+
+    if ( $restricted and $restrictionblockrenewing ) {
+        return ( 0, 'restriction');
+    } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) {
+        return ( 0, 'overdue');
+    }
+
+    if ( defined $issuingrule->{norenewalbefore}
+        and $issuingrule->{norenewalbefore} ne "" )
+    {
+
+        # Calculate soonest renewal by subtracting 'No renewal before' from due date
+        my $soonestrenewal =
+          $itemissue->{date_due}->clone()
+          ->subtract(
+            $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore} );
+
+        # Depending on syspref reset the exact time, only check the date
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuingrule->{lengthunit} eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }

 
 

-        # Get current time and add norenewalbefore.
-        # If this is smaller than date_due, it's too soon for renewal.
-        if (
-            DateTime->now( time_zone => C4::Context->tz() )->add(
-                $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore}
-            ) < $itemissue->{date_due}
-          )
+        if ( $soonestrenewal > DateTime->now( time_zone => C4::Context->tz() ) )

         {
             return ( 0, "auto_too_soon" ) if $itemissue->{auto_renew};
             return ( 0, "too_soon" );
         }
         {
             return ( 0, "auto_too_soon" ) if $itemissue->{auto_renew};
             return ( 0, "too_soon" );
         }

+        elsif ( $itemissue->{auto_renew} ) {
+            return ( 0, "auto_renew" );
+        }
+    }
+
+    # Fallback for automatic renewals:
+    # If norenewalbefore is undef, don't renew before due date.
+    elsif ( $itemissue->{auto_renew} ) {
+        my $now = dt_from_string;
+        return ( 0, "auto_renew" )
+          if $now >= $itemissue->{date_due};
+        return ( 0, "auto_too_soon" );

     }
 
     }
 

-    return ( 0, "auto_renew" ) if $itemissue->{auto_renew};

     return ( 1, undef );
 }
 
     return ( 1, undef );
 }
 


@@ -2791,7 +2906,7 @@ C<$itemnumber> is the number of the item to renew.
 C<$branch> is the library where the renewal took place (if any).
            The library that controls the circ policies for the renewal is retrieved from the issues record.
 
 C<$branch> is the library where the renewal took place (if any).
            The library that controls the circ policies for the renewal is retrieved from the issues record.
 

-C<$datedue> can be a C4::Dates object used to set the due date.
+C<$datedue> can be a DateTime object used to set the due date.

 
 C<$lastreneweddate> is an optional ISO-formatted date used to set issues.lastreneweddate.  If
 this parameter is not supplied, lastreneweddate is set to the current date.
 
 C<$lastreneweddate> is an optional ISO-formatted date used to set issues.lastreneweddate.  If
 this parameter is not supplied, lastreneweddate is set to the current date.


@@ -2814,10 +2929,7 @@ sub AddRenewal {
     my $dbh = C4::Context->dbh;
 
     # Find the issues record for this book
     my $dbh = C4::Context->dbh;
 
     # Find the issues record for this book

-    my $sth =
-      $dbh->prepare("SELECT * FROM issues WHERE itemnumber = ?");
-    $sth->execute( $itemnumber );
-    my $issuedata = $sth->fetchrow_hashref;
+    my $issuedata  = GetItemIssue($itemnumber);

 
     return unless ( $issuedata );
 
 
     return unless ( $issuedata );
 


@@ -2828,12 +2940,18 @@ sub AddRenewal {
         return;
     }
 
         return;
     }
 

+    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return;
+
+    if ( C4::Context->preference('CalculateFinesOnReturn') && $issuedata->{overdue} ) {
+        _CalculateAndUpdateFine( { issue => $issuedata, item => $item, borrower => $borrower } );
+    }
+    _FixOverduesOnReturn( $borrowernumber, $itemnumber );
+

     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date or the current due date
     # based on the value of the RenewalPeriodBase syspref.
     unless ($datedue) {
 
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date or the current due date
     # based on the value of the RenewalPeriodBase syspref.
     unless ($datedue) {
 

-        my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return;

         my $itemtype = (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'};
 
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
         my $itemtype = (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'};
 
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?


@@ -2845,7 +2963,7 @@ sub AddRenewal {
     # Update the issues record to have the new due date, and a new count
     # of how many times it has been renewed.
     my $renews = $issuedata->{'renewals'} + 1;
     # Update the issues record to have the new due date, and a new count
     # of how many times it has been renewed.
     my $renews = $issuedata->{'renewals'} + 1;

-    $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
+    my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?

                             WHERE borrowernumber=? 
                             AND itemnumber=?"
     );
                             WHERE borrowernumber=? 
                             AND itemnumber=?"
     );


@@ -2875,30 +2993,32 @@ sub AddRenewal {
     }
 
     # Send a renewal slip according to checkout alert preferencei
     }
 
     # Send a renewal slip according to checkout alert preferencei

-    if ( C4::Context->preference('RenewalSendNotice') eq '1') {
-       my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
-       my $circulation_alert = 'C4::ItemCirculationAlertPreference';
-       my %conditions = (
-               branchcode   => $branch,
-               categorycode => $borrower->{categorycode},
-               item_type    => $item->{itype},
-               notification => 'CHECKOUT',
-       );
-       if ($circulation_alert->is_enabled_for(\%conditions)) {
-               SendCirculationAlert({
-                       type     => 'RENEWAL',
-                       item     => $item,
-               borrower => $borrower,
-               branch   => $branch,
-               });
-       }
+    if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
+        $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
+        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+        my %conditions        = (
+            branchcode   => $branch,
+            categorycode => $borrower->{categorycode},
+            item_type    => $item->{itype},
+            notification => 'CHECKOUT',
+        );
+        if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
+            SendCirculationAlert(
+                {
+                    type     => 'RENEWAL',
+                    item     => $item,
+                    borrower => $borrower,
+                    branch   => $branch,
+                }
+            );
+        }

     }
 
     # Remove any OVERDUES related debarment if the borrower has no overdues
     }
 
     # Remove any OVERDUES related debarment if the borrower has no overdues

-    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );
+    $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber );

     if ( $borrowernumber
       && $borrower->{'debarred'}
     if ( $borrowernumber
       && $borrower->{'debarred'}

-      && !C4::Members::HasOverdues( $borrowernumber )
+      && !Koha::Patrons->find( $borrowernumber )->has_overdues

       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
       && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
     ) {
         DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });


@@ -2985,15 +3105,22 @@ sub GetSoonestRenewDate {
     my $issuingrule =
       GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
 
     my $issuingrule =
       GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode );
 

-    my $now = DateTime->now( time_zone => C4::Context->tz() );
+    my $now = dt_from_string;

 
 

-    if ( $issuingrule->{norenewalbefore} ) {
+    if ( defined $issuingrule->{norenewalbefore}
+        and $issuingrule->{norenewalbefore} ne "" )
+    {

         my $soonestrenewal =
         my $soonestrenewal =

-          $itemissue->{date_due}->subtract(
+          $itemissue->{date_due}->clone()
+          ->subtract(

             $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore} );
 
             $issuingrule->{lengthunit} => $issuingrule->{norenewalbefore} );
 

-        $soonestrenewal = $now > $soonestrenewal ? $now : $soonestrenewal;
-        return $soonestrenewal;
+        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
+            and $issuingrule->{lengthunit} eq 'days' )
+        {
+            $soonestrenewal->truncate( to => 'day' );
+        }
+        return $soonestrenewal if $now < $soonestrenewal;

     }
     return $now;
 }
     }
     return $now;
 }


@@ -3037,7 +3164,7 @@ sub GetIssuingCharges {
     if ( my $item_data = $sth->fetchrow_hashref ) {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
     if ( my $item_data = $sth->fetchrow_hashref ) {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};

-        my $branch = C4::Branch::mybranch();
+        my $branch = C4::Context::mybranch();

         my $discount_query = q|SELECT rentaldiscount,
             issuingrules.itemtype, issuingrules.branchcode
             FROM borrowers
         my $discount_query = q|SELECT rentaldiscount,
             issuingrules.itemtype, issuingrules.branchcode
             FROM borrowers


@@ -3216,8 +3343,9 @@ sub AnonymiseIssueHistory {
     ";
 
     # The default of 0 does not work due to foreign key constraints
     ";
 
     # The default of 0 does not work due to foreign key constraints

-    # The anonymisation will fail quietly if AnonymousPatron is not a valid entry
-    my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0;
+    # 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 = ?";
     my @bind_params = ($anonymouspatron, $date);
     if (defined $borrowernumber) {
        $query .= " AND borrowernumber = ?";


@@ -3374,7 +3502,7 @@ $newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower);
 
 this function calculates the due date given the start date and configured circulation rules,
 checking against the holidays calendar as per the 'useDaysMode' syspref.
 
 this function calculates the due date given the start date and configured circulation rules,
 checking against the holidays calendar as per the 'useDaysMode' syspref.

-C<$startdate>   = C4::Dates object representing start date of loan period (assumed to be today)
+C<$startdate>   = DateTime object representing start date of loan period (assumed to be today)

 C<$itemtype>  = itemtype code of item in question
 C<$branch>  = location whose calendar to use
 C<$borrower> = Borrower object
 C<$itemtype>  = itemtype code of item in question
 C<$branch>  = location whose calendar to use
 C<$borrower> = Borrower object


@@ -3457,10 +3585,13 @@ sub CalcDateDue {
 
     # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
     if ( C4::Context->preference('ReturnBeforeExpiry') ) {
 
     # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
     if ( C4::Context->preference('ReturnBeforeExpiry') ) {

-        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'iso' );
-        $expiry_dt->set( hour => 23, minute => 59);
-        if ( DateTime->compare( $datedue, $expiry_dt ) == 1 ) {
-            $datedue = $expiry_dt->clone;
+        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'iso', 'floating');
+        if( $expiry_dt ) { #skip empty expiry date..
+            $expiry_dt->set( hour => 23, minute => 59);
+            my $d1= $datedue->clone->set_time_zone('floating');
+            if ( DateTime->compare( $d1, $expiry_dt ) == 1 ) {
+                $datedue = $expiry_dt->clone->set_time_zone( C4::Context->tz );
+            }

         }
     }
 
         }
     }
 


@@ -3468,92 +3599,6 @@ sub CalcDateDue {
 }
 
 
 }
 
 

-=head2 CheckRepeatableHolidays
-
-  $countrepeatable = CheckRepeatableHoliday($itemnumber,$week_day,$branchcode);
-
-This function checks if the date due is a repeatable holiday
-
-C<$date_due>   = returndate calculate with no day check
-C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
-
-=cut
-
-sub CheckRepeatableHolidays{
-my($itemnumber,$week_day,$branchcode)=@_;
-my $dbh = C4::Context->dbh;
-my $query = qq|SELECT count(*)  
-       FROM repeatable_holidays 
-       WHERE branchcode=?
-       AND weekday=?|;
-my $sth = $dbh->prepare($query);
-$sth->execute($branchcode,$week_day);
-my $result=$sth->fetchrow;
-return $result;
-}
-
-
-=head2 CheckSpecialHolidays
-
-  $countspecial = CheckSpecialHolidays($years,$month,$day,$itemnumber,$branchcode);
-
-This function check if the date is a special holiday
-
-C<$years>   = the years of datedue
-C<$month>   = the month of datedue
-C<$day>     = the day of datedue
-C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
-
-=cut
-
-sub CheckSpecialHolidays{
-my ($years,$month,$day,$itemnumber,$branchcode) = @_;
-my $dbh = C4::Context->dbh;
-my $query=qq|SELECT count(*) 
-            FROM `special_holidays`
-            WHERE year=?
-            AND month=?
-            AND day=?
-             AND branchcode=?
-           |;
-my $sth = $dbh->prepare($query);
-$sth->execute($years,$month,$day,$branchcode);
-my $countspecial=$sth->fetchrow ;
-return $countspecial;
-}
-
-=head2 CheckRepeatableSpecialHolidays
-
-  $countspecial = CheckRepeatableSpecialHolidays($month,$day,$itemnumber,$branchcode);
-
-This function check if the date is a repeatble special holidays
-
-C<$month>   = the month of datedue
-C<$day>     = the day of datedue
-C<$itemnumber>  = itemnumber
-C<$branchcode>  = localisation of issue 
-
-=cut
-
-sub CheckRepeatableSpecialHolidays{
-my ($month,$day,$itemnumber,$branchcode) = @_;
-my $dbh = C4::Context->dbh;
-my $query=qq|SELECT count(*) 
-            FROM `repeatable_holidays`
-            WHERE month=?
-            AND day=?
-             AND branchcode=?
-           |;
-my $sth = $dbh->prepare($query);
-$sth->execute($month,$day,$branchcode);
-my $countspecial=$sth->fetchrow ;
-return $countspecial;
-}
-
-
-

 sub CheckValidBarcode{
 my ($barcode) = @_;
 my $dbh = C4::Context->dbh;
 sub CheckValidBarcode{
 my ($barcode) = @_;
 my $dbh = C4::Context->dbh;


@@ -3790,10 +3835,10 @@ sub ProcessOfflineIssue {
 sub ProcessOfflinePayment {
     my $operation = shift;
 
 sub ProcessOfflinePayment {
     my $operation = shift;
 

-    my $borrower = C4::Members::GetMemberDetails( undef, $operation->{cardnumber} ); # Get borrower from operation cardnumber
+    my $patron = Koha::Patrons->find( { cardnumber => $operation->{cardnumber} });

     my $amount = $operation->{amount};
 
     my $amount = $operation->{amount};
 

-    recordpayment( $borrower->{borrowernumber}, $amount );
+    Koha::Account->new( { patron_id => $patron->id } )->pay( { amount => $amount } );

 
     return "Success."
 }
 
     return "Success."
 }


@@ -3813,8 +3858,6 @@ sub TransferSlip {
     my $item =  GetItem( $itemnumber, $barcode )
       or return;
 
     my $item =  GetItem( $itemnumber, $barcode )
       or return;
 

-    my $pulldate = C4::Dates->new();
-

     return C4::Letters::GetPreparedLetter (
         module => 'circulation',
         letter_code => 'TRANSFERSLIP',
     return C4::Letters::GetPreparedLetter (
         module => 'circulation',
         letter_code => 'TRANSFERSLIP',


@@ -3932,7 +3975,8 @@ sub GetAgeRestriction {
             }
 
             #Get how many days the borrower has to reach the age restriction
             }
 
             #Get how many days the borrower has to reach the age restriction

-            my $daysToAgeRestriction = Date_to_Days(@alloweddate) - Date_to_Days(Today);
+            my @Today = split /-/, DateTime->today->ymd();
+            my $daysToAgeRestriction = Date_to_Days(@alloweddate) - Date_to_Days(@Today);

             #Negative days means the borrower went past the age restriction age
             return ($restriction_year, $daysToAgeRestriction);
         }
             #Negative days means the borrower went past the age restriction age
             return ($restriction_year, $daysToAgeRestriction);
         }


@@ -3941,7 +3985,6 @@ sub GetAgeRestriction {
     return ($restriction_year);
 }
 
     return ($restriction_year);
 }
 

-1;

 
 =head2 GetPendingOnSiteCheckouts
 
 
 =head2 GetPendingOnSiteCheckouts
 


@@ -3974,6 +4017,125 @@ sub GetPendingOnSiteCheckouts {
     |, { Slice => {} } );
 }
 
     |, { Slice => {} } );
 }
 

+sub GetTopIssues {
+    my ($params) = @_;
+
+    my ($count, $branch, $itemtype, $ccode, $newness)
+        = @$params{qw(count branch itemtype ccode newness)};
+
+    my $dbh = C4::Context->dbh;
+    my $query = q{
+        SELECT b.biblionumber, b.title, b.author, bi.itemtype, bi.publishercode,
+          bi.place, bi.publicationyear, b.copyrightdate, bi.pages, bi.size,
+          i.ccode, SUM(i.issues) AS count
+        FROM biblio b
+        LEFT JOIN items i ON (i.biblionumber = b.biblionumber)
+        LEFT JOIN biblioitems bi ON (bi.biblionumber = b.biblionumber)
+    };
+
+    my (@where_strs, @where_args);
+
+    if ($branch) {
+        push @where_strs, 'i.homebranch = ?';
+        push @where_args, $branch;
+    }
+    if ($itemtype) {
+        if (C4::Context->preference('item-level_itypes')){
+            push @where_strs, 'i.itype = ?';
+            push @where_args, $itemtype;
+        } else {
+            push @where_strs, 'bi.itemtype = ?';
+            push @where_args, $itemtype;
+        }
+    }
+    if ($ccode) {
+        push @where_strs, 'i.ccode = ?';
+        push @where_args, $ccode;
+    }
+    if ($newness) {
+        push @where_strs, 'TO_DAYS(NOW()) - TO_DAYS(b.datecreated) <= ?';
+        push @where_args, $newness;
+    }
+
+    if (@where_strs) {
+        $query .= 'WHERE ' . join(' AND ', @where_strs);
+    }
+
+    $query .= q{
+        GROUP BY b.biblionumber
+        HAVING count > 0
+        ORDER BY count DESC
+    };
+
+    $count = int($count);
+    if ($count > 0) {
+        $query .= "LIMIT $count";
+    }
+
+    my $rows = $dbh->selectall_arrayref($query, { Slice => {} }, @where_args);
+
+    return @$rows;
+}
+
+sub _CalculateAndUpdateFine {
+    my ($params) = @_;
+
+    my $borrower    = $params->{borrower};
+    my $item        = $params->{item};
+    my $issue       = $params->{issue};
+    my $return_date = $params->{return_date};
+
+    unless ($borrower) { carp "No borrower passed in!" && return; }
+    unless ($item)     { carp "No item passed in!"     && return; }
+    unless ($issue)    { carp "No issue passed in!"    && return; }
+
+    my $datedue = $issue->{date_due};
+
+    # we only need to calculate and change the fines if we want to do that on return
+    # Should be on for hourly loans
+    my $control = C4::Context->preference('CircControl');
+    my $control_branchcode =
+        ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
+      : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
+      :                                     $issue->{branchcode};
+
+    my $date_returned = $return_date ? dt_from_string($return_date) : dt_from_string();
+
+    my ( $amount, $type, $unitcounttotal ) =
+      C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned );
+
+    $type ||= q{};
+
+    if ( C4::Context->preference('finesMode') eq 'production' ) {
+        if ( $amount > 0 ) {
+            C4::Overdues::UpdateFine({
+                issue_id       => $issue->{issue_id},
+                itemnumber     => $issue->{itemnumber},
+                borrowernumber => $issue->{borrowernumber},
+                amount         => $amount,
+                type           => $type,
+                due            => output_pref($datedue),
+            });
+        }
+        elsif ($return_date) {
+
+            # Backdated returns may have fines that shouldn't exist,
+            # so in this case, we need to drop those fines to 0
+
+            C4::Overdues::UpdateFine({
+                issue_id       => $issue->{issue_id},
+                itemnumber     => $issue->{itemnumber},
+                borrowernumber => $issue->{borrowernumber},
+                amount         => 0,
+                type           => $type,
+                due            => output_pref($datedue),
+            });
+        }
+    }
+}
+
+1;
+

 __END__
 
 =head1 AUTHOR
 __END__
 
 =head1 AUTHOR


@@ -3981,4 +4143,3 @@ __END__
 Koha Development Team <http://koha-community.org/>
 
 =cut
 Koha Development Team <http://koha-community.org/>
 
 =cut

-