From dec1a5a937f10e8fdd24cabc339a1c8e92b2cbac Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Fri, 3 Jul 2020 11:58:49 +0200 Subject: [PATCH] Bug 11999: (QA follow-up) --- C4/Reserves.pm | 33 ++++++++++++++------------------- t/db_dependent/Reserves.t | 14 ++++++-------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 094fb38a09..23b07d07fc 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -324,7 +324,7 @@ sub CanBookBeReserved{ # Check if borrower has reached the maximum number of holds allowed my $maxreserves = C4::Context->preference('maxreserves'); if ($maxreserves && $holds->count >= $maxreserves) { - return { status => 'tooManyReserves' }; + return { status => 'tooManyReserves', limit => $maxreserves }; } my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); @@ -376,6 +376,9 @@ sub CanItemBeReserved { my $holds_per_record = 1; # Total number of holds allowed for this one given record my $holds_per_day; # Default to unlimited + # Get calling context + my $caller = (caller(1))[3]; + # we retrieve borrowers and items informations # # item->{itype} will come for biblioitems if necessery my $item = Koha::Items->find($itemnumber); @@ -397,13 +400,6 @@ sub CanItemBeReserved { return { status =>'itemAlreadyOnHold' } if Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count(); - # Check if borrower has reached the maximum number of holds allowed - my $maxreserves = C4::Context->preference('maxreserves'); - my $holds = Koha::Holds->search( { borrowernumber => $borrowernumber } ); - if ($maxreserves && $holds->count >= $maxreserves) { - return { status => 'tooManyReserves', limit => $maxreserves }; - } - my $controlbranch = C4::Context->preference('ReservesControlBranch'); my $querycount = q{ @@ -427,6 +423,15 @@ sub CanItemBeReserved { $branchcode = $borrower->{branchcode}; } + # Check if borrower has reached the maximum number of holds allowed + my $holds = $patron->holds; + my $maxreserves = C4::Context->preference('maxreserves'); + if ( index($caller,"CanBookBeReserved") eq -1 + && $maxreserves + && $holds->count >= $maxreserves) { + return { status => 'tooManyReserves', limit => $maxreserves }; + } + # we retrieve rights if ( my $rights = GetHoldRule( $borrower->{'categorycode'}, $item->effective_itemtype, $branchcode ) ) { $ruleitemtype = $rights->{itemtype}; @@ -438,25 +443,15 @@ sub CanItemBeReserved { $ruleitemtype = undef; } -<<<<<<< HEAD my $search_params = { borrowernumber => $borrowernumber, biblionumber => $item->biblionumber, }; $search_params->{found} = undef if $params->{ignore_found_holds}; - my $holds = Koha::Holds->search($search_params); + $holds = Koha::Holds->search($search_params); if ( defined $holds_per_record && $holds_per_record ne '' && $holds->count() >= $holds_per_record ) { -======= - $holds = Koha::Holds->search( - { - borrowernumber => $borrowernumber, - biblionumber => $item->biblionumber, - } - ); - if ( $holds->count() >= $holds_per_record ) { ->>>>>>> Bug 11999: Add maxreserves check in CanBookBeReserved and CanItemBeReserved return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record }; } diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 11cfa9d9c6..413fa24b05 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,11 +17,7 @@ use Modern::Perl; -<<<<<<< HEAD -use Test::More tests => 64; -======= -use Test::More tests => 63; ->>>>>>> Bug 11999: Test improvements + check maxreserves +use Test::More tests => 65; use Test::MockModule; use Test::Warn; @@ -1171,7 +1167,8 @@ subtest 'check maxreserve test' => sub { my $itype = $builder->build_object({ class => "Koha::ItemTypes", value => { notforloan => 0 } }); my $item_1 = $builder->build_sample_item({ biblionumber => $biblio_1->biblionumber,notforloan => 0, itype => $itype->itemtype }); my $item_2 = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber, notforloan => 0, itype => $itype->itemtype }); - my $patron = $builder->build_object({ class => "Koha::Patrons" }); + my $patron = $builder->build_object({ class => "Koha::Patrons", + value => { branchcode => $item_2->homebranch }}); # Place a hold on the title for both patrons @@ -1187,8 +1184,9 @@ subtest 'check maxreserve test' => sub { my $borrowernumber = $patron->borrowernumber; - is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblio_2->biblionumber)->{status} , 'tooManyReserves', "Reserving more than maxreserves is forbidden"); - is( C4::Reserves::CanItemBeReserved($borrowernumber, $item_2->itemnumber, $item_2->homebranch)->{status} , 'tooManyReserves', "Reserving more than maxreserves is forbidden"); + is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblio_2->biblionumber)->{status} , 'tooManyReserves', "CanBookBeReserved: Reserving more than maxreserves is forbidden"); + is( C4::Reserves::CanItemBeReserved($borrowernumber, $item_2->itemnumber, $item_2->homebranch)->{status} , 'tooManyReserves', "CanItemBeReserved: Reserving more than maxreserves is forbidden"); + }; sub count_hold_print_messages { -- 2.11.0