From c98932dc77cf8bec838a2aea9afdaf6a94f5e20c Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Thu, 11 Apr 2024 14:20:15 +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 What this patch does : -- Move get_opacitemholds_policy check into Reserves.pm::CanBookBeReserved -- Update CanItemBeReserved to return recordHoldsOnly status if interface is OPAC -- Update warning labels in opac-reserve.tt accordingly. To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi Signed-off-by: Pedro Amorim Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Baptiste Wojtkowski --- C4/Reserves.pm | 16 +++++++- .../bootstrap/en/modules/opac-reserve.tt | 6 +-- opac/opac-reserve.pl | 37 ++++++++++++------- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index e96e7ee5c6..8c11527996 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -419,6 +419,16 @@ sub CanBookBeReserved{ my $patron = Koha::Patrons->find( $borrowernumber ); while ( my $item = $items->next ) { $canReserve = CanItemBeReserved( $patron, $item, $pickup_branchcode, $params ); + if ( C4::Context->interface eq 'opac' ) { + my $opacitemholds = Koha::CirculationRules->get_opacitemholds_policy( + { + item => $item, + patron => $patron + } + ) // 'Y'; + return { status => 'recordHoldNotAllowed' } if $opacitemholds eq 'F'; + return { status => 'OK' } if $canReserve->{status} eq 'recordHoldsOnly'; + } return { status => 'OK' } if $canReserve->{status} eq 'OK'; } return $canReserve; @@ -540,10 +550,11 @@ sub CanItemBeReserved { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, branchcode => $reserves_control_branch, - 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'; if ( defined $holds_per_record && $holds_per_record ne '' ){ if ( $holds_per_record == 0 ) { @@ -679,6 +690,9 @@ sub CanItemBeReserved { return _cache { status => 'pickupNotInHoldGroup' }; } } + if ( $opacitemholds eq "N" && C4::Context->interface eq 'opac' ) { + return _cache { status => "recordHoldsOnly" }; + } return _cache { status => 'OK' }; } 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 78db93ddbb..d87d2f3013 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -173,11 +173,9 @@ [% IF bibitemloo.forced_hold_level %]
[% IF bibitemloo.forced_hold_level == 'item' %] - You already have at least one item level hold on this title. - All further holds must be item level. + All further holds must be item level. [% ELSE %] - You already have at least one record level hold on this title. - All further holds must be record level. + All further holds must be record level. [% END %]
[% END %] diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 002a7b13bf..00f303f6b8 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -482,19 +482,23 @@ foreach my $biblioNum (@biblionumbers) { my $branch = Koha::Policy::Holds->holds_control_library( $item, $patron ); - my $policy_holdallowed = - CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status} eq 'OK' && - IsAvailableForItemLevelRequest($item, $patron, undef); + my $item_status = CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status}; + + if ( $item_status eq 'recordHoldsOnly' ) { + $biblioLoopIter{force_hold} = 1; + $biblioLoopIter{itemholdable} = 0; + $biblioLoopIter{forced_hold_level} = 'record'; + } + + my $policy_holdallowed = IsAvailableForItemLevelRequest( $item, $patron, undef ) + && ( $item_status eq 'OK' or $item_status eq 'recordHoldsOnly' ); if ($policy_holdallowed) { - my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } ); - if ( $opac_hold_policy ne 'N' ) { # If Y or F - $item_info->{available} = 1; + $numCopiesAvailable++; + if ( $item_status ne 'recordHoldsOnly' ) { $numCopiesOPACAvailable++; - $biblioLoopIter{force_hold} = 1 if $opac_hold_policy eq 'F'; + $item_info->{available} = 1; } - $numCopiesAvailable++; - unless ( $can_place_hold_if_available_at_pickup ) { my $items_in_this_library = Koha::Items->search({ biblionumber => $item->biblionumber, holdingbranch => $item->holdingbranch, notforloan => 0 }); my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count; @@ -544,9 +548,16 @@ foreach my $biblioNum (@biblionumbers) { } } - my $status = CanBookBeReserved( $borrowernumber, $biblioNum )->{status}; - $biblioLoopIter{holdable} &&= $status eq 'OK'; - $biblioLoopIter{$status} = 1; + my $record_status = CanBookBeReserved( $borrowernumber, $biblioNum )->{status}; + if ($record_status eq 'recordHoldNotAllowed') { + $biblioLoopIter{force_hold} = 1; + $biblioLoopIter{itemholdable} = 1; + $biblioLoopIter{forced_hold_level} = 'item'; + $biblioLoopIter{holdable} &&= 1; + } + else { + $biblioLoopIter{holdable} &&= $record_status eq 'OK'; + } if ( $biblioLoopIter{holdable} and C4::Context->preference('AllowHoldItemTypeSelection') ) { # build the allowed item types loop @@ -564,7 +575,7 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{allowed_item_types} = \@item_types; } - if ( $status eq 'recall' ) { + if ( $record_status eq 'recall' ) { $biblioLoopIter{recall} = 1; } -- 2.39.5