From e018b69dca6e702c4abd2fd58e6e99868eada9aa Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 With small changes to apply to master. Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall --- C4/Circulation.pm | 45 ++++--- circ/circulation.pl | 6 +- .../data/mysql/atomicupdate/bug_bulac1.sql | 1 + installer/data/mysql/sysprefs.sql | 1 + .../admin/preferences/circulation.pref | 6 + .../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 19f9a9a1a1..4d8793e7e8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -658,6 +658,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; @@ -926,23 +927,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}) { @@ -1057,7 +1065,7 @@ sub CanBookBeIssued { } } - return ( \%issuingimpossible, \%needsconfirmation, \%alerts ); + return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages, ); } =head2 CanBookBeReturned @@ -1242,6 +1250,7 @@ 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); @@ -1278,7 +1287,8 @@ sub AddIssue { my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} ); # 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'}, @@ -1289,7 +1299,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 my ( $allowed, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} ); @@ -1331,7 +1342,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 ff32c0cc0d..b5442e4ff5 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -319,7 +319,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, @@ -333,6 +333,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); @@ -404,7 +405,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 0000000000..275f6e58db --- /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 ac6f852131..264d1a381d 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -477,6 +477,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 a3ce0b3d16..fc9706b059 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 @@ -411,6 +411,12 @@ Circulation: - on-site checkouts as normal checkouts. - 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 diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t new file mode 100644 index 0000000000..dd98993dfc --- /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 . + +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.39.5