From ef85487387fa33798bcee96e36e07b0bf91b2d9e Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Thu, 7 May 2020 12:33:46 +0200 Subject: [PATCH] Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo --- C4/Reserves.pm | 14 ++++++++++++-- koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt | 6 ++++-- reserve/placerequest.pl | 2 +- reserve/request.pl | 4 ++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8941d4f72c..83b63545b3 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -335,9 +335,14 @@ sub CanBookBeReserved{ } my $canReserve = { status => '' }; + my ($rights,$item); + foreach my $itemnumber (@itemnumbers) { $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $pickup_branchcode, $params ); + my $opacitemholds = $canReserve->{rights}->{opacitemholds} // 'Y'; + return { status => 'recordHoldNotAllowed' } if $opacitemholds eq 'F'; return { status => 'OK' } if $canReserve->{status} eq 'OK'; + return { status => 'OK' } if $canReserve->{status} eq 'recordHoldsOnly'; } return $canReserve; } @@ -445,10 +450,11 @@ sub CanItemBeReserved { categorycode => $borrower->{'categorycode'}, itemtype => $item->effective_itemtype, branchcode => $branchcode, - rules => ['holds_per_record','holds_per_day'] + rules => ['holds_per_record','holds_per_day','opacitemholds'] }); my $holds_per_record = $rights->{holds_per_record} // 1; my $holds_per_day = $rights->{holds_per_day}; + my $opacitemholds = $rights->{opacitemholds} // 'Y'; my $search_params = { borrowernumber => $borrowernumber, @@ -585,7 +591,11 @@ sub CanItemBeReserved { } } - return { status => 'OK' }; + if ( $opacitemholds eq "N" && C4::Context->interface eq 'opac') { + return { status => "recordHoldsOnly", right => $rights }; + } + + return { status => 'OK', rights => $rights }; } =head2 CanReserveBeCanceledFromOpac diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 023049f011..6888717cc4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -333,7 +333,7 @@ [% END %] - [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %] + [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted || recordHoldNotAllowed ) %]
[% UNLESS ( multi_hold ) %] @@ -355,6 +355,8 @@
  • No items are available to be placed on hold.
  • [% ELSIF ( maxreserves ) %]
  • Too many holds: [% patron.firstname | html %] [% patron.surname | html %] has too many holds.
  • + [% ELSIF ( recordHoldNotAllowed ) %] +
  • Record holds not allowed : [% patron.firstname | html %] [% patron.surname | html %] cannot put a record-level hold on this ressource.
  • [% END # /IF exceeded_maxreserves %] [% ELSE # UNLESS multi_hold %] @@ -531,7 +533,7 @@ [% IF ( patron.borrowernumber ) %] [% IF ( override_required ) %] - [% ELSIF ( none_available ) %] + [% ELSIF ( none_available || recordHoldNotAllowed ) %] [% ELSE %] diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 05b5c7f3c9..f8581c9417 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -145,7 +145,7 @@ if ( $type eq 'str8' && $borrower ) { } else { # place a request on 1st available for ( my $i = 0 ; $i < $holds_to_place_count ; $i++ ) { - if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) { + if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber, undef, { context => 'staff' })->{status} eq 'OK' ) { AddReserve( { branchcode => $branch, diff --git a/reserve/request.pl b/reserve/request.pl index 7637692c99..d9ec0c9ca0 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -337,6 +337,10 @@ foreach my $biblionumber (@biblionumbers) { $template->param( $canReserve->{status} => 1); $biblioloopiter{ $canReserve->{status} } = 1; } + elsif ( $canReserve->{status} eq 'recordHoldNotAllowed' ) { + $template->param( $canReserve->{status} => 1 ); + $biblioloopiter{ $canReserve->{status} } = 1; + } else { $biblioloopiter{ $canReserve->{status} } = 1; } -- 2.11.0