From 7e36c51751ba55ecbd5c45a0e3ef1e8121eccdf8 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 21 Apr 2016 11:00:20 +0100
Subject: [PATCH] Bug 16272: Automatically switch an on-site checkout to a
 regular checkout when checked out

Use case: A patron checks some items out on-site and want to take it home.
To facilitate the librarian work the checkout is directly switched from
on-site to regular when checked out if the new pref SwitchOnSiteCheckouts is on.

Test plan:
0/ Let the new pref SwitchOnSiteCheckouts off
1/ Checkout one items to a patron and tick the "on-site checkout"
checkbox
2/ Check the same item out without ticking the "on-site checkout"
checkbox
=> You should get "This item can not be renewed, it's an on-site checkout"
3/ Switch the pref on
4/ Repeat 2
=> The on-site checkout should be automatically switched to a regular checkout

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
With small changes to apply to master.
---
 C4/Circulation.pm                                  |  45 +++++----
 circ/circulation.pl                                |   6 +-
 installer/data/mysql/atomicupdate/bug_bulac1.sql   |   1 +
 installer/data/mysql/sysprefs.sql                  |   1 +
 .../en/modules/admin/preferences/circulation.pref  |   6 ++
 t/db_dependent/Circulation/SwitchOnSiteCheckouts.t | 110 +++++++++++++++++++++
 6 files changed, 150 insertions(+), 19 deletions(-)
 create mode 100644 installer/data/mysql/atomicupdate/bug_bulac1.sql
 create mode 100644 t/db_dependent/Circulation/SwitchOnSiteCheckouts.t

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 8d585a2..19bc39b 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -763,6 +763,7 @@ sub CanBookBeIssued {
     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;
@@ -1023,23 +1024,30 @@ sub CanBookBeIssued {
     #
     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
-            if ( $renewerror eq 'onsite_checkout' ) {
-                $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 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 {
-                $issuingimpossible{NO_MORE_RENEWALS} = 1;
+                $needsconfirmation{RENEW_ISSUE} = 1;
             }
         }
-        else {
-            $needsconfirmation{RENEW_ISSUE} = 1;
-        }
     }
     elsif ($issue->{borrowernumber}) {
 
@@ -1148,7 +1156,7 @@ sub CanBookBeIssued {
         }
     }
 
-    return ( \%issuingimpossible, \%needsconfirmation, \%alerts );
+    return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages, );
 }
 
 =head2 CanBookBeReturned
@@ -1332,6 +1340,7 @@ AddIssue does the following things :
 sub AddIssue {
     my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode, $params ) = @_;
     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 $dbh = C4::Context->dbh;
     my $barcodecheck=CheckValidBarcode($barcode);
@@ -1365,7 +1374,8 @@ sub AddIssue {
 		#
 		# check if we just renew the issue.
 		#
-		if ($actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}) {
+        if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}
+                and not $switch_onsite_checkout ) {
 		    $datedue = AddRenewal(
 			$borrower->{'borrowernumber'},
 			$item->{'itemnumber'},
@@ -1376,7 +1386,8 @@ sub AddIssue {
 		}
 		else {
         # it's NOT a renewal
-			if ( $actualissue->{borrowernumber}) {
+            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
 				AddReturn(
@@ -1415,7 +1426,7 @@ sub AddIssue {
         }
         $datedue->truncate( to => 'minute');
 
-        $issue = Koha::Database->new()->schema()->resultset('Issue')->create(
+        $issue = Koha::Database->new()->schema()->resultset('Issue')->update_or_create(
             {
                 borrowernumber  => $borrower->{'borrowernumber'},
                 itemnumber      => $item->{'itemnumber'},
diff --git a/circ/circulation.pl b/circ/circulation.pl
index e31c7c5..a220180 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -321,7 +321,7 @@ if (@$barcodes) {
   for my $barcode ( @$barcodes ) {
     my $template_params = { barcode => $barcode };
     # always check for blockers on issuing
-    my ( $error, $question, $alerts ) = CanBookBeIssued(
+    my ( $error, $question, $alerts, $messages ) = CanBookBeIssued(
         $borrower,
         $barcode, $datedue,
         $inprocess,
@@ -335,6 +335,7 @@ if (@$barcodes) {
     my $blocker = $invalidduedate ? 1 : 0;
 
     $template_params->{alert} = $alerts;
+    $template_params->{messages} = $messages;
 
     #  Get the item title for more information
     my $getmessageiteminfo = GetBiblioFromItemNumber(undef,$barcode);
@@ -406,7 +407,8 @@ if (@$barcodes) {
             }
         }
         unless($confirm_required) {
-            my $issue = AddIssue( $borrower, $barcode, $datedue, $cancelreserve, undef, undef, { onsite_checkout => $onsite_checkout, auto_renew => $session->param('auto_renew') } );
+	    my $switch_onsite_checkout = exists $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED};
+            my $issue = AddIssue( $borrower, $barcode, $datedue, $cancelreserve, undef, undef, { onsite_checkout => $onsite_checkout, auto_renew => $session->param('auto_renew'), switch_onsite_checkout => $switch_onsite_checkout, } );
             $template_params->{issue} = $issue;
             $session->clear('auto_renew');
             $inprocess = 1;
diff --git a/installer/data/mysql/atomicupdate/bug_bulac1.sql b/installer/data/mysql/atomicupdate/bug_bulac1.sql
new file mode 100644
index 0000000..275f6e5
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_bulac1.sql
@@ -0,0 +1 @@
+INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('SwitchOnSiteCheckouts', '0', 'Automatically switch an on-site checkout to a normal checkout', NULL, 'YesNo');
diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
index d95754d..65a6d32 100644
--- a/installer/data/mysql/sysprefs.sql
+++ b/installer/data/mysql/sysprefs.sql
@@ -468,6 +468,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('SuspendHoldsIntranet','1','Allow holds to be suspended from the intranet.',NULL,'YesNo'),
 ('SuspendHoldsOpac','1','Allow holds to be suspended from the OPAC.',NULL,'YesNo'),
 ('SvcMaxReportRows','10',NULL,'Maximum number of rows to return via the report web service.','Integer'),
+('SwitchOnSiteCheckouts','0',NULL,'Automatically switch an on-site checkout to a normal checkout','YesNo');
 ('SyndeticsAuthorNotes','0','','Display Notes about the Author on OPAC from Syndetics','YesNo'),
 ('SyndeticsAwards','0','','Display Awards on OPAC from Syndetics','YesNo'),
 ('SyndeticsClientCode','0','','Client Code for using Syndetics Solutions content','free'),
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
index 9a37608..c81607c 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -412,6 +412,12 @@ Circulation:
             - If enabled, the number of checkouts allowed will be normal checkouts + on-site checkouts.
             - If disabled, both values will be checked separately.
         -
+            - pref: SwitchOnSiteCheckouts
+              choices:
+                  yes: Switch
+                  no: "Don't switch"
+            - on-site checkouts to normal checkouts when checked out.
+        -
             - When a patron's checked out item is overdue,
             - pref: OverduesBlockRenewing
               type: choice
diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
new file mode 100644
index 0000000..dd98993
--- /dev/null
+++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
@@ -0,0 +1,110 @@
+#!/usr/bin/perl
+
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 3 of the License, or (at your option) any later
+# version.
+#
+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+use Test::More tests => 4;
+use C4::Context;
+
+use C4::Biblio;
+use C4::Members;
+use C4::Branch;
+use C4::Circulation;
+use C4::Items;
+use C4::Context;
+
+use Koha::DateUtils qw( dt_from_string );
+use Koha::Database;
+
+use t::lib::TestBuilder;
+use t::lib::Mocks;
+
+my $schema = Koha::Database->new->schema;
+$schema->storage->txn_begin;
+
+our $dbh = C4::Context->dbh;
+
+$dbh->do(q|DELETE FROM branch_item_rules|);
+$dbh->do(q|DELETE FROM issues|);
+$dbh->do(q|DELETE FROM branch_borrower_circ_rules|);
+$dbh->do(q|DELETE FROM default_branch_circ_rules|);
+$dbh->do(q|DELETE FROM default_circ_rules|);
+$dbh->do(q|DELETE FROM default_branch_item_rules|);
+$dbh->do(q|DELETE FROM issuingrules|);
+
+my $builder = t::lib::TestBuilder->new();
+
+my $branch = $builder->build({
+    source => 'Branch',
+});
+
+my $patron = $builder->build({
+    source => 'Borrower',
+    value => {
+        branchcode => $branch->{branchcode},
+    },
+});
+
+my $biblio = $builder->build({
+    source => 'Biblio',
+    value => {
+        branchcode => $branch->{branchcode},
+    },
+});
+my $item = $builder->build({
+    source => 'Item',
+    value => {
+        biblionumber => $biblio->{biblionumber},
+        homebranch => $branch->{branchcode},
+        holdingbranch => $branch->{branchcode},
+    },
+});
+
+my $issuingrule = $builder->build({
+    source => 'Issuingrule',
+    value => {
+        branchcode         => $branch->{branchcode},
+        categorycode       => '*',
+        itemtype           => '*',
+        maxissueqty        => 2,
+        maxonsiteissueqty  => 1,
+        lengthunit         => 'days',
+        issuelength        => 5,
+    },
+});
+
+C4::Context->_new_userenv ('DUMMY_SESSION_ID');
+C4::Context->set_userenv($patron->{borrowernumber}, $patron->{userid}, 'usercnum', 'First name', 'Surname', $branch->{branchcode}, 'My Library', 0);
+
+# Add onsite checkout
+C4::Circulation::AddIssue( $patron, $item->{barcode}, dt_from_string, undef, dt_from_string, undef, { onsite_checkout => 1 } );
+
+my ( $impossible, $messages );
+t::lib::Mocks::mock_preference('SwitchOnSiteCheckouts', 0);
+( $impossible, undef, undef, $messages ) = C4::Circulation::CanBookBeIssued( $patron, $item->{barcode} );
+is( $impossible->{NO_RENEWAL_FOR_ONSITE_CHECKOUTS}, 1, '' );
+
+t::lib::Mocks::mock_preference('SwitchOnSiteCheckouts', 1);
+( undef, undef, undef, $messages ) = C4::Circulation::CanBookBeIssued( $patron, $item->{barcode} );
+is( $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED}, 1, '' );
+C4::Circulation::AddIssue( $patron, $item->{barcode}, undef, undef, undef, undef, { switch_onsite_checkout => 1 } );
+my $issue = C4::Circulation::GetItemIssue( $item->{itemnumber} );
+is( $issue->{onsite_checkout}, 0, '' );
+my $five_days_after = dt_from_string->add( days => 5 )->set( hour => 23, minute => 59, second => 0 );
+is( $issue->{date_due}, $five_days_after );
+
+$schema->storage->txn_rollback;
+
+1;
-- 
2.1.4