From c5eb19fe18e041e264f0069ab1b7100cc3bb1c25 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 | 24 ++++++++++++++++++++-- .../prog/en/modules/reserve/request.tt | 7 +++++-- reserve/placerequest.pl | 4 ++-- reserve/request.pl | 4 ++++ 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8941d4f72c..fedb146ac3 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; } @@ -381,6 +386,7 @@ sub CanItemBeReserved { my $biblio = $item->biblio; my $patron = Koha::Patrons->find( $borrowernumber ); my $borrower = $patron->unblessed; + my $context = $params->{context} // ''; # If an item is damaged and we don't allow holds on damaged items, we can stop right here return { status =>'damaged' } @@ -445,10 +451,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, @@ -466,6 +473,15 @@ sub CanItemBeReserved { } } + $item = Koha::Items->find( $itemnumber ); + + $holds = Koha::Holds->search( + { + borrowernumber => $borrowernumber, + biblionumber => $item->biblionumber, + } + ); + my $today_holds = Koha::Holds->search({ borrowernumber => $borrowernumber, reservedate => dt_from_string->date @@ -585,7 +601,11 @@ sub CanItemBeReserved { } } - return { status => 'OK' }; + if ( $opacitemholds eq "N" && $context ne 'staff') { + 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..702de2489d 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,7 +355,10 @@
  • 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 %]

    Cannot place hold on some items

    @@ -531,7 +534,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..a6d1f715b3 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -100,7 +100,7 @@ if ( $type eq 'str8' && $borrower ) { $biblionumber = $item->biblionumber; } - my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $branch)->{status}; + my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $branch, { context => 'staff' })->{status}; if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) { AddReserve( @@ -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