From 0e93351f999199354fa4369d5a2b3a2b7902259c 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 --- C4/Reserves.pm | 19 ++++++++- .../bootstrap/en/modules/opac-reserve.tt | 6 +-- opac/opac-reserve.pl | 39 ++++++++++++------- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index c95a3d7ad4..2ce817eef4 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -425,6 +425,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; @@ -551,10 +561,11 @@ sub CanItemBeReserved { categorycode => $borrower->{'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 ) { @@ -691,6 +702,10 @@ sub CanItemBeReserved { } } + if ( $opacitemholds eq "N" && C4::Context->interface eq 'opac' ) { + return _cache { status => "recordHoldsOnly" }; + } + return _cache { status => 'OK' }; } @@ -2021,7 +2036,7 @@ sub _ShiftPriority { $sth = $dbh->prepare( $query ); $sth->execute( $new_priority, $biblio ); while ( my $row = $sth->fetchrow_hashref ) { - $sth_update->execute( $biblio, $row->{borrowernumber}, $row->{reservedate} ); + $sth_update->execute( $biblio, $row->{borrowernumber}, $row->{reservedate} ); } return $new_priority; # so the caller knows what priority they wind up receiving 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 bacd8b77cb..f45e7ad5fb 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -157,11 +157,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 783b95f027..877aa4f49a 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -481,19 +481,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; @@ -543,9 +547,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 @@ -563,7 +574,7 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{allowed_item_types} = \@item_types; } - if ( $status eq 'recall' ) { + if ( $record_status eq 'recall' ) { $biblioLoopIter{recall} = 1; } @@ -623,7 +634,7 @@ if ( C4::Context->preference( 'OPACAllowHoldDateInFuture' ) ) { $template->param( - reserve_in_future => 1, + reserve_in_future => 1, ); } -- 2.44.0