From b608380e146c15fbe25396999a7d3d02347731d7 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 26 May 2022 00:59:10 +0000 Subject: [PATCH] Bug 30851: Return rights info from CanItemBeReserved Rather than fath holds rules outside of the routine, let's return the ones we already calculated. There is precedent to return some rules in some instances, this just expands that --- C4/Reserves.pm | 34 ++++++++++++++++++---------------- reserve/request.pl | 14 +++++--------- t/db_dependent/Holds.t | 1 + 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index ea2bd47762..b03c7bc635 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -483,13 +483,18 @@ sub CanItemBeReserved { branchcode => $reserves_control_branch, rules => ['holds_per_record','holds_per_day','opacitemholds'] }); + my $branchitemrule = + C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype ); + + $rights->{holdallowed} = $branchitemrule->{holdallowed}; + 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 ) { - return { status => "noReservesAllowed" }; + return { status => "noReservesAllowed", rights => $rights }; } if ( !$params->{ignore_hold_counts} ) { my $search_params = { @@ -498,7 +503,7 @@ sub CanItemBeReserved { }; $search_params->{found} = undef if $params->{ignore_found_holds}; my $holds = Koha::Holds->search($search_params); - return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record } if $holds->count() >= $holds_per_record; + return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record, rights => $rights } if $holds->count() >= $holds_per_record; } } @@ -508,7 +513,7 @@ sub CanItemBeReserved { borrowernumber => $patron->borrowernumber, reservedate => dt_from_string->date }); - return { status => 'tooManyReservesToday', limit => $holds_per_day } if $today_holds->count() >= $holds_per_day; + return { status => 'tooManyReservesToday', limit => $holds_per_day, rights => $rights } if $today_holds->count() >= $holds_per_day; } # we check if it's ok or not @@ -550,7 +555,7 @@ sub CanItemBeReserved { $reservecount = $rowcount->{count}; } - return { status => 'tooManyReserves', limit => $allowedreserves } if $reservecount >= $allowedreserves; + return { status => 'tooManyReserves', limit => $allowedreserves, rights => $rights } if $reservecount >= $allowedreserves; } } @@ -569,26 +574,23 @@ sub CanItemBeReserved { } )->count(); - return { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value; + return { status => 'tooManyReserves', limit => $rule->rule_value, rights => $rights } if $total_holds_count >= $rule->rule_value; } - my $branchitemrule = - C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype ); - if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) { - return { status => 'notReservable' }; + return { status => 'notReservable', rights => $rights }; } if ( $branchitemrule->{holdallowed} eq 'from_home_library' && $borrower->{branchcode} ne $item->homebranch ) { - return { status => 'cannotReserveFromOtherBranches' }; + return { status => 'cannotReserveFromOtherBranches', rights => $rights }; } my $item_library = Koha::Libraries->find( {branchcode => $item->homebranch} ); if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group') { if($patron->branchcode ne $item->homebranch && !$item_library->validate_hold_sibling( {branchcode => $patron->branchcode} )) { - return { status => 'branchNotInHoldGroup' }; + return { status => 'branchNotInHoldGroup', rights => $rights }; } } @@ -598,19 +600,19 @@ sub CanItemBeReserved { }); unless ($destination) { - return { status => 'libraryNotFound' }; + return { status => 'libraryNotFound', rights => $rights }; } unless ($destination->pickup_location) { - return { status => 'libraryNotPickupLocation' }; + return { status => 'libraryNotPickupLocation', rigths => $rights }; } unless ($item->can_be_transferred({ to => $destination })) { - return { status => 'cannotBeTransferred' }; + return { status => 'cannotBeTransferred', rights => $rights }; } if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) { - return { status => 'pickupNotInHoldGroup' }; + return { status => 'pickupNotInHoldGroup', rights => $rights }; } if ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' && !Koha::Libraries->find({branchcode => $borrower->{branchcode}})->validate_hold_sibling({branchcode => $pickup_branchcode})) { - return { status => 'pickupNotInHoldGroup' }; + return { status => 'pickupNotInHoldGroup', rights => $rights }; } } diff --git a/reserve/request.pl b/reserve/request.pl index c89a89ee90..f5e028172c 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -477,22 +477,18 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) } if ( $patron ) { - my $patron_unblessed = $patron->unblessed; - my $branch = C4::Circulation::_GetCircControlBranch($item, $patron_unblessed); - my $branchitemrule = GetBranchItemRule( $branch, $item->{'itype'} ); + my $can_item_be_reserved = CanItemBeReserved( $patron, $item_object ); - $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; - - my $can_item_be_reserved = CanItemBeReserved( $patron, $item_object )->{status}; - $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' ); + $item->{'holdallowed'} = $can_item_be_reserved->{rights}->{holdallowed}; + $item->{not_holdable} = $can_item_be_reserved->{status} unless ( $can_item_be_reserved->{status} eq 'OK' ); $item->{item_level_holds} = Koha::CirculationRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } ); if ( !$item->{cantreserve} && !$exceeded_maxreserves - && $can_item_be_reserved eq 'OK' + && $can_item_be_reserved->{status} eq 'OK' # items_any_available defined outside of the current loop, # so we avoiding loop inside IsAvailableForItemLevelRequest: && IsAvailableForItemLevelRequest($item_object, $patron, undef, $items_any_available) @@ -518,7 +514,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) { # If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules # with the exception of itemAlreadyOnHold because, you know, the item is already on hold - if ( $can_item_be_reserved ne 'itemAlreadyOnHold' ) { + if ( $can_item_be_reserved->{status} ne 'itemAlreadyOnHold' ) { # Send the pickup locations count to the UI, the pickup locations will be pulled using the API my @pickup_locations = $item_object->pickup_locations({ patron => $patron })->as_list; $item->{pickup_locations_count} = scalar @pickup_locations; diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 8a19f019bb..53e59b551c 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -881,6 +881,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { } ); + warn Data::Dumper::Dumper( CanItemBeReserved( $patron, $item_1 )->{rights} ); is_deeply( CanItemBeReserved( $patron, $item_1 ), { status => 'OK' }, -- 2.30.2