From 026993daf41534d16338eb37f2fe3fceec252efc Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 26 Mar 2020 06:30:09 +0000 Subject: [PATCH] Bug 24101: Enable renewals for on-site checkouts To test: 1. Enable system preference OnSiteCheckouts 2. Set onsite_renewalsallowed circulation rule (regardless the choice of GUI) by executing the following command on your Koha instance's shell RENEWS=0; perl -e 'use Koha::CirculationRules; '\ 'Koha::CirculationRules->set_rule({ branchcode => undef, categorycode => undef,'\ 'itemtype => undef, rule_name => "onsite_renewalsallowed", rule_value => '\ "$RENEWS"' });' 3. Perform an on-site checkout 4. Try to renew the loan 5. Observe a similar error dialog: "Cannot renew on-site checkout: test / ( test ) has been renewed the maximum number of times by ( admin )" 6. Execute the following shell command RENEWS=1; perl -e 'use Koha::CirculationRules; '\ 'Koha::CirculationRules->set_rule({ branchcode => undef, categorycode => undef,'\ 'itemtype => undef, rule_name => "onsite_renewalsallowed", rule_value => '\ "$RENEWS"' });' 7. Try to renew the loan 8. Observe success 9. Try to renew the loan 10. Observe the step 5 error dialog Run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/OnSiteCheckouts.t --- C4/Circulation.pm | 11 +- Koha/CirculationRules.pm | 3 + .../prog/en/modules/circ/renew.tt | 5 +- t/db_dependent/Circulation.t | 32 +--- t/db_dependent/OnSiteCheckouts.t | 145 ++++++++++++++++++ 5 files changed, 158 insertions(+), 38 deletions(-) create mode 100644 t/db_dependent/OnSiteCheckouts.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 3374ebef79..8661386234 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2713,7 +2713,6 @@ sub CanBookBeRenewed { my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' ); my $issue = $item->checkout or return ( 0, 'no_checkout' ); - return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout; return ( 0, 'item_denied_renewal') if _item_denied_renewal({ item => $item }); my $patron = $issue->patron or return; @@ -2728,6 +2727,7 @@ sub CanBookBeRenewed { branchcode => $branchcode, rules => [ 'renewalsallowed', + 'onsite_renewalsallowed', 'no_auto_renewal_after', 'no_auto_renewal_after_hard_limit', 'lengthunit', @@ -2736,8 +2736,11 @@ sub CanBookBeRenewed { } ); + my $renewals_allowed = $issue->onsite_checkout ? + $issuing_rule->{onsite_renewalsallowed} : $issuing_rule->{renewalsallowed}; + return ( 0, "too_many" ) - if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals; + if not $renewals_allowed or $renewals_allowed <= $issue->renewals; my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing'); @@ -3097,12 +3100,14 @@ sub GetRenewCount { # $item and $borrower should be calculated my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed); + my $rule_name = $data->{'onsite_checkout'} ? + 'onsite_renewalsallowed' : 'renewalsallowed'; my $rule = Koha::CirculationRules->get_effective_rule( { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, branchcode => $branchcode, - rule_name => 'renewalsallowed', + rule_name => $rule_name, } ); diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 5042e6f55d..9a4c310579 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -130,6 +130,9 @@ our $RULE_KINDS = { norenewalbefore => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, + onsite_renewalsallowed => { + scope => [ 'branchcode', 'categorycode', 'itemtype' ], + }, onshelfholds => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt index 608ed5c84d..4a502c929d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt @@ -30,7 +30,7 @@ [% IF error %]
-

Cannot renew:

+

Cannot renew[% IF issue.onsite_checkout %] on-site checkout[% END %]:

[% IF error == "no_item" %] @@ -134,9 +134,6 @@

Item is not allowed renewal.

- [% ELSIF error == "onsite_checkout" %] -

Item cannot be renewed because it's an onsite checkout

- [% ELSE %] [% error | html %] diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 104fc8fc85..f4b8add054 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 46; +use Test::More tests => 44; use Test::MockModule; use Test::Deep qw( cmp_deeply ); @@ -1514,36 +1514,6 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' ); }; -{ - # Don't allow renewing onsite checkout - my $branch = $library->{branchcode}; - - #Create another record - my $biblio = $builder->build_sample_biblio(); - - my $item = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - library => $branch, - itype => $itemtype, - } - ); - - my $borrowernumber = Koha::Patron->new({ - firstname => 'fn', - surname => 'dn', - categorycode => $patron_category->{categorycode}, - branchcode => $branch, - })->store->borrowernumber; - - my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; - - my $issue = AddIssue( $borrower, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); - my ( $renewed, $error ) = CanBookBeRenewed( $borrowernumber, $item->itemnumber ); - is( $renewed, 0, 'CanBookBeRenewed should not allow to renew on-site checkout' ); - is( $error, 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' ); -} - { my $library = $builder->build({ source => 'Branch' }); diff --git a/t/db_dependent/OnSiteCheckouts.t b/t/db_dependent/OnSiteCheckouts.t new file mode 100644 index 0000000000..2c85495562 --- /dev/null +++ b/t/db_dependent/OnSiteCheckouts.t @@ -0,0 +1,145 @@ +#!/usr/bin/perl + +# Copyright 2020 Hypernova Oy +# +# 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 => 1; + +use C4::Circulation; +use Koha::Database; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'test renewals' => sub { + plan tests => 7; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'OnSiteCheckouts', 1 ); + t::lib::Mocks::mock_preference( 'SwitchOnSiteCheckouts', 0 ); + + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rule( + { + categorycode => undef, + branchcode => undef, + itemtype => undef, + rule_name => 'onsite_renewalsallowed', + rule_value => '0', + } + ); + + my $librarian = mock_librarian_env(); + my $test_data = build_test_data(); + my $patron = $test_data->{patron}; + my $item = $test_data->{item}; + my $library = $test_data->{library}; + + my $checkout = C4::Circulation::AddIssue( + $patron->unblessed, $item->barcode, + undef, undef, undef, undef, { onsite_checkout => 1 } + ); + + is( $checkout->onsite_checkout, 1, 'Created an on-site checkout' ); + my ( $can_renew, $error ) = CanBookBeRenewed( + $patron->borrowernumber, $item->itemnumber + ); + is ( $can_renew, 0, 'Checkout cannot be renewed...' ); + is ( $error, 'too_many', '..because onsite_renewalsallowed = 0' ); + + Koha::CirculationRules->set_rule( + { + categorycode => undef, + branchcode => undef, + itemtype => undef, + rule_name => 'onsite_renewalsallowed', + rule_value => '1', + } + ); + + ( $can_renew, $error ) = CanBookBeRenewed( + $patron->borrowernumber, $item->itemnumber + ); + is ( $can_renew, 1, 'After onsite_renewalsallowed = 1, we can now renew' ); + + ok ( AddRenewal( $patron->id, $item->id, $library->id ), 'Renewal success' ); + + ( $can_renew, $error ) = CanBookBeRenewed( + $patron->borrowernumber, $item->itemnumber + ); + is ( $can_renew, 0, 'Now, checkout cannot be renewed...' ); + is ( $error, 'too_many', '..because we have exceeded max renewals' ); + + $schema->storage->txn_rollback; +}; + +sub build_test_data { + my $library = $builder->build_object({ + class => 'Koha::Libraries', + }); + + my $patron = $builder->build_object({ + class => 'Koha::Patrons' + }); + + my $itemtype = $builder->build_object({ + class => 'Koha::ItemTypes', + value => { + notforloan => undef, + rentalcharge => 0, + rentalcharge_daily => 0, + defaultreplacecost => undef, + processfee => undef + } + }); + + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item({ + biblionumber => $biblio->biblionumber, + library => $library->branchcode, + replacementprice => 12.00, + itype => $itemtype->itemtype + }); + + return { + biblio => $biblio, + item => $item, + itemtype => $itemtype, + library => $library, + patron => $patron, + }; +} + +sub mock_librarian_env { + my $librarian = $builder->build_object({ + class => 'Koha::Patrons', + value => { + flags => 1 + } + }); + t::lib::Mocks::mock_userenv({ + patron => $librarian, + branchcode => $librarian->branchcode + }); + return $librarian; +} -- 2.17.1