From 9128f721fd5fba5cad7d6562fe36250c6251c24e Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Wed, 30 Sep 2020 10:33:12 +0200 Subject: [PATCH] Bug 22806: (QA follow-up) --- C4/Reserves.pm | 4 ++-- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 12 ++++++------ opac/opac-reserve.pl | 9 +++------ reserve/request.pl | 14 ++++---------- t/db_dependent/ILSDI_Services.t | 2 +- t/db_dependent/Reserves.t | 6 +++--- 6 files changed, 19 insertions(+), 28 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8e40db5296..69acf486b6 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -321,7 +321,7 @@ sub CanBookBeReserved{ # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') && C4::Circulation::CheckIfIssuedToPatron( $borrowernumber, $biblionumber ) ) { - return { status =>'itemAlreadyOnLoan' }; + return { status =>'alreadypossession' }; } my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); @@ -395,7 +395,7 @@ sub CanItemBeReserved { # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') && C4::Circulation::CheckIfIssuedToPatron( $patron->borrowernumber, $biblio->biblionumber ) ) { - return { status =>'itemAlreadyOnLoan' }; + return { status =>'alreadypossession' }; } my $controlbranch = C4::Context->preference('ReservesControlBranch'); diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt index feee22d125..05a1d4044f 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -200,15 +200,15 @@ [% IF ( bibitemloo.already_reserved ) %]
You have already requested this title.
[% ELSE %] - [% UNLESS ( bibitemloo.bib_available ) %] -
There are no items that can be placed on hold.
+ [% IF ( bibitemloo.already_patron_possession ) %] +
This title cannot be requested because it's already in your possession.
[% ELSE %] - [% IF ( bibitemloo.already_patron_possession ) %] -
This title cannot be requested because it's already in your possession.
+ [% UNLESS ( bibitemloo.bib_available ) %] +
There are no items that can be placed on hold.
[% ELSE %] -
This title cannot be requested.
+
This title cannot be requested.
[% END %] - [% END # / UNLESS bibitemloo.bib_available %] + [% END # / UNLESS bibitemloo.already_patron_possession %] [% END # / IF bibitemloo.already_reserved %] [% END # / UNLESS bibitemloo.holdable %] diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index a930ddbf76..7fa8539f44 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -601,11 +601,6 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{holdable} = undef; $biblioLoopIter{itemholdable} = undef; } - if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) { - $biblioLoopIter{holdable} = undef; - $biblioLoopIter{already_patron_possession} = 1; - } - if ( $biblioLoopIter{holdable} ) { @not_available_at = uniq @not_available_at; $biblioLoopIter{not_available_at} = \@not_available_at ; @@ -620,7 +615,9 @@ foreach my $biblioNum (@biblionumbers) { } } - $biblioLoopIter{holdable} &&= CanBookBeReserved( $borrowernumber, $biblioNum )->{status} eq 'OK'; + my $status = CanBookBeReserved( $borrowernumber, $biblioNum )->{status}; + $biblioLoopIter{holdable} &&= $status eq 'OK'; + $biblioLoopIter{already_patron_possession} = $status eq 'alreadypossession'; # For multiple holds per record, if a patron has previously placed a hold, # the patron can only place more holds of the same type. That is, if the diff --git a/reserve/request.pl b/reserve/request.pl index 688cb82d97..a4a166973f 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -314,6 +314,10 @@ foreach my $biblionumber (@biblionumbers) { $template->param( $canReserve->{status} => 1 ); $biblioloopiter{ $canReserve->{status} } = 1; } + elsif ( $canReserve->{status} eq 'alreadypossession' ) { + $template->param( $canReserve->{status} => 1); + $biblioloopiter{ $canReserve->{status} } = 1; + } else { $biblioloopiter{ $canReserve->{status} } = 1; } @@ -342,16 +346,6 @@ foreach my $biblionumber (@biblionumbers) { $biblioloopiter{remaining_holds_for_record} = $max_holds_for_record; $template->param( max_holds_for_record => $max_holds_for_record ); $template->param( remaining_holds_for_record => $remaining_holds_for_record ); - - { # alreadypossession - # Check to see if patron is allowed to place holds on records where the - # patron already has an item from that record checked out - if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') - && CheckIfIssuedToPatron( $patron->borrowernumber, $biblionumber ) ) - { - $template->param( alreadypossession => 1, ); - } - } } diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index f8ab8921ca..e600779c6a 100755 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -424,7 +424,7 @@ subtest 'Holds test' => sub { $query->param( 'pickup_location', $origin_branch->{branchcode}); $reply = C4::ILSDI::Services::HoldItem( $query ); - is( $reply->{code}, 'itemAlreadyOnLoan', "Patron has issued same book" ); + is( $reply->{code}, 'alreadypossession', "Patron has issued same book" ); is( $reply->{pickup_location}, undef, "No reserve placed"); $schema->storage->txn_rollback; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 52c779510b..6ddb3f4f56 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 64; +use Test::More tests => 65; use Test::MockModule; use Test::Warn; @@ -1169,12 +1169,12 @@ subtest 'AllowHoldOnPatronPossession test' => sub { is(C4::Reserves::CanBookBeReserved($patron->borrowernumber, $item->biblionumber)->{status}, - 'itemAlreadyOnLoan', + 'alreadypossession', 'Patron cannot place hold on a book loaned to itself'); is(C4::Reserves::CanItemBeReserved($patron->borrowernumber, $item->itemnumber)->{status}, - 'itemAlreadyOnLoan', + 'alreadypossession', 'Patron cannot place hold on an item loaned to itself'); t::lib::Mocks::mock_preference('AllowHoldsOnPatronsPossessions', 1); -- 2.11.0