From 7f02e228a3c2699a7d165baba37be5a77bf41b1d Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 14 Jul 2021 13:02:01 +0000 Subject: [PATCH] Bug 28705: Renew should look at current article requests Content-Type: text/plain; charset=utf-8 This development is controlled by the new preference called CheckArticleRequestsOnRenewal. Test plan: [1] Run Circulation.t [2] Run the new test has_current_article_requests.t [3] Enable preference [4] Issue some item, allow it to be renewed now [5] Add an article request for that specific item [6] Verify that you cannot renew this item now [7] Cancel the former article request [8] Add an article request for same biblio on biblio level [9] If there are still available items, you should be able to renew. But if not, you should not ;) --- C4/Circulation.pm | 6 ++ Koha/Item.pm | 46 +++++++++++ t/db_dependent/Circulation.t | 24 +++++- .../Koha/Item/has_current_article_requests.t | 81 +++++++++++++++++++ 4 files changed, 156 insertions(+), 1 deletion(-) create mode 100755 t/db_dependent/Koha/Item/has_current_article_requests.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 10e17c045d..732a16525f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2931,6 +2931,12 @@ sub CanBookBeRenewed { return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed + # Check current article requests on item or biblio level + if( C4::Context->preference('CheckArticleRequestsOnRenewal') && $item->has_current_article_requests ) { + # TODO Can be improved. Currently we act like there is a reserve found. Same effect ;) + return ( 0, "on_reserve" ); + } + return ( 1, undef ); } diff --git a/Koha/Item.pm b/Koha/Item.pm index c653dc2583..9e96005c74 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -44,6 +44,7 @@ use Koha::Plugins; use Koha::Libraries; use Koha::StockRotationItem; use Koha::StockRotationRotas; +use Koha::ArticleRequests; use base qw(Koha::Object); @@ -1196,6 +1197,51 @@ sub _after_item_action_hooks { ); } +=head3 has_current_article_requests + + Checks if an item has current item or biblio level + requests. For biblio level requests the result depends + on another item being available. + Returns true or undef. + +=cut + +sub has_current_article_requests { + my ( $self, $params ) = @_; + return if !C4::Context->preference('ArticleRequests'); + + # First check item level requests + my $rs_1 = Koha::ArticleRequests->search({ + status => { -in => [ 'PENDING', 'PROCESSING' ] }, + itemnumber => $self->id, + }); + return 1 if $rs_1->count; + + # Now check biblio level requests + my $rs_2 = Koha::ArticleRequests->search({ + status => { -in => [ 'PENDING', 'PROCESSING' ] }, + itemnumber => undef, + biblionumber => $self->biblionumber, + }); + return if !$rs_2->count; + + # Is another item available for biblio level requests? + # Only one item should be enough. + # Note: NOT looking at notforloan (yet?) + my $rs_3 = Koha::Items->search({ + 'me.biblionumber' => $self->biblionumber, + itemlost => 0, + withdrawn => 0, + damaged => 0, + onloan => undef, + 'reserves.found' => undef, + }, { + join => 'reserves', + }); + return 1 if !$rs_3->count; + return; +} + =head3 _type =cut diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 8aa5d4c970..ef024ed168 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -51,6 +51,7 @@ use Koha::Subscriptions; use Koha::Account::Lines; use Koha::Account::Offsets; use Koha::ActionLogs; +use Koha::ArticleRequests; sub set_userenv { my ( $library ) = @_; @@ -415,7 +416,7 @@ subtest "GetIssuingCharges tests" => sub { my ( $reused_itemnumber_1, $reused_itemnumber_2 ); subtest "CanBookBeRenewed tests" => sub { - plan tests => 95; + plan tests => 99; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -1444,6 +1445,27 @@ subtest "CanBookBeRenewed tests" => sub { $item_3->itemcallnumber || '' ), "Account line description must not contain 'Lost Items ', but be title, barcode, itemcallnumber" ); + + # Optional test on pending article requests + t::lib::Mocks::mock_preference( 'OverduesBlockRenewing', 'allow' ); # allow again + t::lib::Mocks::mock_preference( 'CheckArticleRequestsOnRenewal', 0 ); + LostItem( $item_3->itemnumber, 'test', 1 ); # returned again + AddIssue( $renewing_borrower, $item_3->barcode ); + Koha::CirculationRules->set_rules({ + categorycode => undef, branchcode => undef, itemtype => undef, rules => { renewalsallowed => 10 }, + }); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_3->itemnumber ); + is( $renewokay, 1, 'Renewal possible again' ); + my $artreq1 = $builder->build_object({ class => 'Koha::ArticleRequests', + value => { borrowernumber => $renewing_borrowernumber, itemnumber => $item_3->itemnumber, biblionumber => $item_3->biblionumber, status => 'PENDING' }, + }); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_3->itemnumber ); + is( $renewokay, 1, 'Renewal still possible, still need to enable pref' ); + t::lib::Mocks::mock_preference( 'CheckArticleRequestsOnRenewal', 1 ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_3->itemnumber ); + is( $renewokay, 0, 'Renewal not possible, pref enabled' ); + is( $error, 'on_reserve', 'Error message currently resembles presence of holds' ); + }; subtest "GetUpcomingDueIssues" => sub { diff --git a/t/db_dependent/Koha/Item/has_current_article_requests.t b/t/db_dependent/Koha/Item/has_current_article_requests.t new file mode 100755 index 0000000000..05381d5fd3 --- /dev/null +++ b/t/db_dependent/Koha/Item/has_current_article_requests.t @@ -0,0 +1,81 @@ +#!/usr/bin/perl + +# Copyright 2021 Rijksmuseum +# +# 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 Data::Dumper qw/Dumper/; + +use Test::More tests => 1; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +use Koha::Database; +use Koha::ArticleRequests; +use Koha::DateUtils qw/dt_from_string/; +use Koha::Holds; +use Koha::Items; +use Koha::Patrons; +use C4::Circulation (); +use C4::Reserves (); + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'has_current_article_requests' => sub { + plan tests => 8; + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'ArticleRequests', 1 ); + + my $item1 = $builder->build_sample_item; + my $patron1 = $builder->build_object({ class => 'Koha::Patrons' }); + t::lib::Mocks::mock_userenv({ branchcode => $patron1->branchcode }); + + my $artreq1 = $builder->build_object({ class => 'Koha::ArticleRequests', + value => { borrowernumber => $patron1->borrowernumber, itemnumber => $item1->itemnumber, biblionumber => $item1->biblionumber, status => 'PENDING' }, + }); + C4::Circulation::AddIssue( $patron1->unblessed, $item1->barcode, dt_from_string() ); + is( $item1->has_current_article_requests, 1, 'Expect true for item level request' ); + + $artreq1->status('COMPLETED')->store; + is( $item1->has_current_article_requests, undef, 'Expect false for completed item level request' ); + + $artreq1->itemnumber(undef); + $artreq1->status('PROCESSING')->store; + is( $item1->has_current_article_requests, 1, 'Expect true for biblio level request with no other items' ); + + my $item2 = $builder->build_sample_item; + $item2->biblionumber( $item1->biblionumber )->store; + is( $item1->has_current_article_requests, undef, 'Expect false for biblio level request with another available item' ); + + $item2->damaged(1)->store; + is( $item1->has_current_article_requests, 1, 'Expect true for biblio level request with only another damaged item' ); + $item2->damaged(0)->store; + is( $item1->has_current_article_requests, undef, 'Expect false again when clearing damaged status' ); + + C4::Reserves::AddReserve({ + branchcode => $patron1->branchcode, borrowernumber => $patron1->borrowernumber, biblionumber => $item2->biblionumber, + priority => 1, itemnumber => $item2->itemnumber, + }); + is( $item1->has_current_article_requests, undef, 'Expect false for biblio level request with only another item and a reserve' ); + Koha::Holds->search({ itemnumber => $item2->itemnumber })->next->set_waiting; + is( $item1->has_current_article_requests, 1, 'Expect true for biblio level request with only another item on hold and waiting' ); + + $schema->storage->txn_rollback; +}; -- 2.20.1