Bugzilla – Attachment 186122 Details for
Bug 32702
Item statuses that block holds should be checked in CanItemBeReserved
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32702: (QA follow-up) remove IsAvailableForItemLevelRequest and move checks to CanItemBeReserved
Bug-32702-QA-follow-up-remove-IsAvailableForItemLe.patch (text/plain), 32.81 KB, created by
Arthur Suzuki
on 2025-09-04 09:01:28 UTC
(
hide
)
Description:
Bug 32702: (QA follow-up) remove IsAvailableForItemLevelRequest and move checks to CanItemBeReserved
Filename:
MIME Type:
Creator:
Arthur Suzuki
Created:
2025-09-04 09:01:28 UTC
Size:
32.81 KB
patch
obsolete
>From ceb3ba1fd62ebd2437aff27edf0881118daa80ba Mon Sep 17 00:00:00 2001 >From: Arthur Suzuki <arthur.suzuki@biblibre.com> >Date: Thu, 4 Sep 2025 08:49:38 +0000 >Subject: [PATCH] Bug 32702: (QA follow-up) remove > IsAvailableForItemLevelRequest and move checks to CanItemBeReserved > >--- > C4/Circulation.pm | 10 +- > C4/ILSDI/Services.pm | 10 +- > C4/Reserves.pm | 51 -------- > Koha/Item.pm | 5 +- > opac/opac-ISBDdetail.pl | 9 +- > opac/opac-MARCdetail.pl | 9 +- > opac/opac-detail.pl | 6 +- > opac/opac-reserve.pl | 6 +- > reserve/request.pl | 13 +- > t/db_dependent/Holds.t | 2 +- > .../Holds/DisallowHoldIfItemsAvailable.t | 118 ++++++++++-------- > t/db_dependent/Reserves.t | 68 +++------- > 12 files changed, 102 insertions(+), 205 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 59e12c72cd0..271a93baaa2 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -25,11 +25,10 @@ use Encode; > use Try::Tiny; > > use C4::Context; >-use C4::Stats qw( UpdateStats ); >-use C4::Reserves >- qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority IsAvailableForItemLevelRequest ); >-use C4::Biblio qw( UpdateTotalIssues ); >-use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf ); >+use C4::Stats qw( UpdateStats ); >+use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority ); >+use C4::Biblio qw( UpdateTotalIssues ); >+use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf ); > use C4::Accounts; > use C4::ItemCirculationAlertPreference; > use C4::Message; >@@ -3267,7 +3266,6 @@ sub CanBookBeRenewed { > foreach my $other_item (@other_items) { > next if defined $matched_items{ $other_item->itemnumber }; > next if $other_item->holds->filter_by_found->count; >- next unless IsAvailableForItemLevelRequest( $other_item, $patron_with_reserve, undef ); > next > unless CanItemBeReserved( > $patron_with_reserve, $other_item, undef, >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index 4aa7a3d9324..41efaaa91ef 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -24,7 +24,7 @@ use C4::Members; > use C4::Items qw( get_hostitemnumbers_of ); > use C4::Circulation qw( CanBookBeRenewed barcodedecode CanBookBeIssued AddRenewal ); > use C4::Accounts; >-use C4::Reserves qw( CanBookBeReserved IsAvailableForItemLevelRequest CalculatePriority AddReserve CanItemBeReserved ); >+use C4::Reserves qw( CanBookBeReserved CalculatePriority AddReserve CanItemBeReserved ); > use C4::Context; > use C4::Auth; > use CGI qw ( -utf8 ); >@@ -642,12 +642,10 @@ sub GetServices { > my @availablefor; > > # Reserve level management >- my $biblionumber = $item->biblionumber; >- my $canbookbereserved = CanBookBeReserved( $borrower, $biblionumber ); >- if ( $canbookbereserved->{status} eq 'OK' ) { >+ my $biblionumber = $item->biblionumber; >+ if ( CanBookBeReserved( $borrower, $biblionumber )->{status} eq 'OK' ) { > push @availablefor, 'title level hold'; >- my $canitembereserved = IsAvailableForItemLevelRequest( $item, $patron ); >- if ($canitembereserved) { >+ if ( CanItemBeReserved( $patron, $item )->{status} eq 'OK' ) { > push @availablefor, 'item level hold'; > } > } >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 8ea531d04a7..982d59c05dd 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -123,7 +123,6 @@ BEGIN { > > AutoUnsuspendReserves > >- IsAvailableForItemLevelRequest > ItemsAnyAvailableAndNotRestricted > > AlterPriority >@@ -1366,56 +1365,6 @@ sub ModReserveMinusPriority { > _FixPriority( { reserve_id => $reserve_id, rank => '0' } ); > } > >-=head2 IsAvailableForItemLevelRequest >- >- my $is_available = IsAvailableForItemLevelRequest( $item_record, $borrower_record, $pickup_branchcode ); >- >-Checks whether a given item record is available for an >-item-level hold request. An item is available if >- >-* it is not lost AND >-* it is not damaged AND >-* it is not withdrawn AND >-* a waiting or in transit reserve is placed on >-* does not have a not for loan value > 0 >- >-Need to check the issuingrules onshelfholds column, >-if this is set items on the shelf can be placed on hold >- >-Note that IsAvailableForItemLevelRequest() does not >-check if the staff operator is authorized to place >-a request on the item - in particular, >-this routine does not check IndependentBranches >-and canreservefromotherbranches. >- >-Note also that this subroutine does not checks smart >-rules limits for item by reservesallowed/holds_per_record >-values, this complemented in calling code with calls and >-checks with CanItemBeReserved or CanBookBeReserved. >- >-=cut >- >-sub IsAvailableForItemLevelRequest { >- my $item = shift; >- my $patron = shift; >- my $pickup_branchcode = shift; >- >- if ($pickup_branchcode) { >- my $destination = Koha::Libraries->find($pickup_branchcode); >- return 0 unless $destination; >- return 0 unless $destination->pickup_location; >- return 0 unless $item->can_be_transferred( { to => $destination } ); >- my $reserves_control_branch = Koha::Policy::Holds->holds_control_library( $item, $patron ); >- my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); >- my $home_library = Koha::Libraries->find( { branchcode => $item->homebranch } ); >- return 0 >- unless $branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' >- || $home_library->validate_hold_sibling( { branchcode => $pickup_branchcode } ); >- } >- >- return IsOnShelfHoldsPolicySatisfied( { item => $item, patron => $patron } ); >-} >- > =head2 ItemsAnyAvailableAndNotRestricted > > ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron }); >diff --git a/Koha/Item.pm b/Koha/Item.pm >index df6a4a52d9d..205d5742b50 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -411,7 +411,10 @@ Returns the itemtype for the item based on whether item level itemtypes are set > > sub effective_itemtype { > my ($self) = @_; >- >+ if ( C4::Context->preference('item-level_itypes') ) { >+ return undef >+ unless defined $self->itype; >+ } > return $self->_result()->effective_itemtype(); > } > >diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl >index 4bf0bc08cb0..7eb05963b42 100755 >--- a/opac/opac-ISBDdetail.pl >+++ b/opac/opac-ISBDdetail.pl >@@ -51,7 +51,7 @@ use C4::Biblio qw( > TransformMarcToKoha > ); > use C4::Record qw( marc2cites ); >-use C4::Reserves qw( IsAvailableForItemLevelRequest ); >+use C4::Reserves qw( CanBookBeReserved ); > use C4::Serials qw( CountSubscriptionFromBiblionumber SearchSubscriptions GetLatestSerials ); > use C4::Koha qw( > GetNormalizedEAN >@@ -214,12 +214,7 @@ my $holdable_items = $biblio->items->filter_by_for_hold->count; > # If we don't have a patron, then holdable items determines holdability > my $can_holds_be_placed = $patron ? 0 : $holdable_items; > if ($patron) { >- my $items = $biblio->items; >- while ( my $item = $items->next ) { >- $can_holds_be_placed = $can_holds_be_placed >- || ( IsAvailableForItemLevelRequest( $item, $patron, undef ) >- && CanItemBeReserved( $patron, $item, undef ) ); >- } >+ $can_holds_be_placed = CanBookBeReserved( $patron->id, $biblio->id ); > } > > $template->param( >diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl >index 9c9d0f59c73..43ef286aeeb 100755 >--- a/opac/opac-MARCdetail.pl >+++ b/opac/opac-MARCdetail.pl >@@ -57,7 +57,7 @@ use C4::Biblio qw( > TransformMarcToKoha > ); > use C4::Record qw( marc2cites ); >-use C4::Reserves qw( IsAvailableForItemLevelRequest ); >+use C4::Reserves qw( CanBookBeReserved ); > use C4::Members; > use C4::Koha qw( GetNormalizedISBN ); > use List::MoreUtils qw( uniq ); >@@ -175,12 +175,7 @@ my $holdable_items = $biblio->items->filter_by_for_hold->count; > my $can_holds_be_placed = $patron ? 0 : $holdable_items; > > if ($patron) { >- $items->reset; >- while ( my $item = $items->next ) { >- $can_holds_be_placed = $can_holds_be_placed >- || ( IsAvailableForItemLevelRequest( $item, $patron, undef ) >- && CanItemBeReserved( $patron, $item, undef ) ); >- } >+ $can_holds_be_placed = CanBookBeReserved( $patron->id, $biblio->id ); > } > > $template->param( ReservableItems => $can_holds_be_placed ); >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index 851c9c101b2..05620202e12 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -59,7 +59,7 @@ use C4::External::Syndetics qw( > use C4::Members; > use C4::XSLT qw( XSLTParse4Display ); > use C4::ShelfBrowser qw( GetNearbyItems ); >-use C4::Reserves qw( GetReserveStatus IsAvailableForItemLevelRequest ); >+use C4::Reserves qw( GetReserveStatus CanItemBeReserved ); > use C4::Charset qw( SetUTF8Flag ); > use MARC::Field; > use List::MoreUtils qw( any ); >@@ -747,9 +747,7 @@ if ( not $viewallitems and $items->count > $max_items_to_display ) { > # We only need to check if we haven't determined holds can be placed > # and if we don't have patron, we have already decided > $can_holds_be_placed = $can_holds_be_placed >- || ( $patron >- && IsAvailableForItemLevelRequest( $item, $patron, undef ) >- && CanItemBeReserved( $patron, $item, undef ) ); >+ || ( $patron && CanItemBeReserved( $patron, $item, undef ) ); > > # get collection code description, too > my $ccode = $item->ccode; >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 6db213f2985..38161bb58c0 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -24,7 +24,7 @@ use CGI qw ( -utf8 ); > use C4::Auth qw( get_template_and_user ); > use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); > use C4::Circulation qw( GetBranchItemRule ); >-use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve IsAvailableForItemLevelRequest GetReserveFee ); >+use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReserveFee ); > use C4::Biblio qw( GetBiblioData GetFrameworkCode ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Context; >@@ -418,7 +418,6 @@ foreach my $biblioNum (@biblionumbers) { > my $numCopiesOPACAvailable = 0; > > # iterating through all items first to check if any of them available >- # to pass this value further inside down to IsAvailableForItemLevelRequest to > # it's complicated logic to analyse. > # (before this loop was inside that sub loop so it was O(n^2) ) > foreach my $item ( @{ $biblioData->{items} } ) { >@@ -461,8 +460,7 @@ 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 $policy_holdallowed = CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status} eq 'OK'; > > if ($policy_holdallowed) { > my $opac_hold_policy = >diff --git a/reserve/request.pl b/reserve/request.pl >index a553070e4b3..24806dee42d 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -33,7 +33,7 @@ use Date::Calc qw( Date_to_Days ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Auth qw( get_template_and_user ); > use C4::Reserves >- qw( AlterPriority ToggleLowestPriority CanBookBeReserved GetMaxPatronHoldsForRecord CanItemBeReserved IsAvailableForItemLevelRequest ); >+ qw( AlterPriority ToggleLowestPriority CanBookBeReserved GetMaxPatronHoldsForRecord CanItemBeReserved ); > use C4::Items qw( get_hostitemnumbers_of ); > use C4::Koha qw( getitemtypeimagelocation ); > use C4::Serials qw( CountSubscriptionFromBiblionumber ); >@@ -395,7 +395,6 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) > my $valid_items = 0; > > # iterating through all items first to check if any of them available >- # to pass this value further inside down to IsAvailableForItemLevelRequest to > # it's complicated logic to analyse. > # (before this loop was inside that sub loop so it was O(n^2) ) > >@@ -507,15 +506,9 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) > $default_pickup_branch = C4::Context->userenv->{branch}; > } > >- if ( >- !$item->{cantreserve} >+ if ( !$item->{cantreserve} > && !$exceeded_maxreserves >- && $can_item_be_reserved eq 'OK' >- >- # items_any_available defined outside of the current loop, >- # so we avoiding loop inside IsAvailableForItemLevelRequest: >- && IsAvailableForItemLevelRequest( $item_object, $patron, undef ) >- ) >+ && $can_item_be_reserved eq 'OK' ) > { > $valid_items = 1; > } >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index 483f0f443c7..9ffe9064e8d 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -743,7 +743,7 @@ subtest 'CanItemBeReserved' => sub { > > is( > CanItemBeReserved( $patrons[0], $item )->{status}, 'notforloan', >- "Item with no notforloan itemtype cannot be reserved" >+ "Item with notforloan status cannot be reserved" > ); > > $itemtype->notforloan(0)->store(); >diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >index 5617e38b93c..3f032d24f0c 100755 >--- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >+++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >@@ -16,7 +16,7 @@ use t::lib::TestBuilder; > use t::lib::Mocks; > > BEGIN { >- use_ok( 'C4::Reserves', qw( ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest ) ); >+ use_ok( 'C4::Reserves', qw( ItemsAnyAvailableAndNotRestricted CanItemBeReserved ) ); > } > > my $schema = Koha::Database->schema; >@@ -35,13 +35,16 @@ my $itemtype = $builder->build( > } > )->{itemtype}; > >-t::lib::Mocks::mock_userenv( { branchcode => $library1->{branchcode} } ); >+my $library_A = $library1->{branchcode}; >+my $library_B = $library2->{branchcode}; >+ >+t::lib::Mocks::mock_userenv( { branchcode => $library_A } ); > > my $patron1 = $builder->build_object( > { > class => 'Koha::Patrons', > value => { >- branchcode => $library1->{branchcode}, >+ branchcode => $library_A, > dateexpiry => '3000-01-01', > } > } >@@ -52,7 +55,7 @@ my $patron2 = $builder->build_object( > { > class => 'Koha::Patrons', > value => { >- branchcode => $library1->{branchcode}, >+ branchcode => $library_A, > dateexpiry => '3000-01-01', > } > } >@@ -62,15 +65,12 @@ my $patron3 = $builder->build_object( > { > class => 'Koha::Patrons', > value => { >- branchcode => $library2->{branchcode}, >+ branchcode => $library_B, > dateexpiry => '3000-01-01', > } > } > ); > >-my $library_A = $library1->{branchcode}; >-my $library_B = $library2->{branchcode}; >- > my $biblio = $builder->build_sample_biblio( { itemtype => $itemtype } ); > my $biblionumber = $biblio->biblionumber; > my $item1 = $builder->build_sample_item( >@@ -111,16 +111,16 @@ my $is; > $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron1 } ); > is( $is, 1, "Items availability: both of 2 items are available" ); > >-$is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >-is( $is, 0, "Item cannot be held, 2 items available" ); >+$is = CanItemBeReserved( $patron1, $item1 ); >+is( $is->{status}, "onShelfHoldsNotAllowed", "Item cannot be held, 2 items available" ); > > my $issue1 = AddIssue( $patron2, $item1->barcode ); > > $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron1 } ); > is( $is, 1, "Items availability: one item is available" ); > >-$is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >-is( $is, 0, "Item cannot be held, 1 item available" ); >+$is = CanItemBeReserved( $patron1, $item1 ); >+is( $is->{status}, "onShelfHoldsNotAllowed", "Item cannot be held, 1 item available" ); > > AddIssue( $patron2, $item2->barcode ); > $cache->flush(); >@@ -128,13 +128,13 @@ $cache->flush(); > $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron1 } ); > is( $is, 0, "Items availability: none of items are available" ); > >-$is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >-is( $is, 1, "Item can be held, no items available" ); >+$is = CanItemBeReserved( $patron1, $item1 ); >+is( $is->{status}, "OK", "Item can be held, no items available" ); > > AddReturn( $item1->barcode ); > > { # Remove the issue for the first patron, and modify the branch for item1 >- subtest 'IsAvailableForItemLevelRequest behaviours depending on ReservesControlBranch + holdallowed' => sub { >+ subtest 'CanItemBeReserved behaviours depending on ReservesControlBranch + holdallowed' => sub { > plan tests => 2; > > my $hold_allowed_from_home_library = 'from_home_library'; >@@ -163,15 +163,15 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from home + ReservesControlBranch=ItemHomeLibrary + one item is available at different library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 1, >+ $is->{status}, "cannotReserveFromOtherBranches", > "Hold allowed from home library + ReservesControlBranch=ItemHomeLibrary, " >- . "One item is available at different library, not holdable = none available => the hold is allowed at item level" >+ . "One item is available at different library, not holdable = none available => the hold is not allowed at item level" > ); >- $is = IsAvailableForItemLevelRequest( $item1, $patron2 ); >+ $is = CanItemBeReserved( $patron2, $item1 ); > is( >- $is, 1, >+ $is->{status}, "cannotReserveFromOtherBranches", > "Hold allowed from home library + ReservesControlBranch=ItemHomeLibrary, " > . "One item is available at home library, holdable = one available => the hold is not allowed at item level" > ); >@@ -187,9 +187,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library for library B + ReservesControlBranch=ItemHomeLibrary + one item is available at different library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from home library + ReservesControlBranch=ItemHomeLibrary, " > . "One item is available at different library, holdable = one available => the hold is not allowed at item level" > ); >@@ -204,11 +204,11 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library for library B + ReservesControlBranch=PatronLibrary + one item is available at different library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 1, >+ $is->{status}, "cannotReserveFromOtherBranches", > "Hold allowed from home library + ReservesControlBranch=PatronLibrary, " >- . "One item is available at different library, not holdable = none available => the hold is allowed at item level" >+ . "One item is available at different library, not holdable = none available => the hold is not allowed at item level" > ); > > #Adding a rule for the patron's home library affects the availability for an item from another library because ReservesControlBranch is set to PatronLibrary >@@ -222,9 +222,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library for library A + ReservesControlBranch=PatronLibrary + one item is available at different library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from home library + ReservesControlBranch=PatronLibrary, " > . "One item is available at different library, holdable = one available => the hold is not allowed at item level" > ); >@@ -243,9 +243,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library + ReservesControlBranch=ItemHomeLibrary + one item is available at different library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from any library + ReservesControlBranch=ItemHomeLibrary, " > . "One item is available at the diff library, holdable = 1 available => the hold is not allowed at item level" > ); >@@ -260,9 +260,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library + ReservesControlBranch=PatronLibrary + one item is available at different library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from any library + ReservesControlBranch=PatronLibrary, " > . "One item is available at the diff library, holdable = 1 available => the hold is not allowed at item level" > ); >@@ -294,9 +294,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from home library + ReservesControlBranch=ItemHomeLibrary + one item is available at home library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from home library + ReservesControlBranch=ItemHomeLibrary, " > . "One item is available at the same library, holdable = 1 available => the hold is not allowed at item level" > ); >@@ -310,9 +310,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from home library + ReservesControlBranch=PatronLibrary + one item is available at home library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from home library + ReservesControlBranch=PatronLibrary, " > . "One item is available at the same library, holdable = 1 available => the hold is not allowed at item level" > ); >@@ -330,9 +330,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library + ReservesControlBranch=ItemHomeLibrary + one item is available at home library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from any library + ReservesControlBranch=ItemHomeLibrary, " > . "One item is available at the same library, holdable = 1 available => the hold is not allowed at item level" > ); >@@ -346,9 +346,9 @@ AddReturn( $item1->barcode ); > "Items availability: hold allowed from any library + ReservesControlBranch=PatronLibrary + one item is available at home library" > ); > >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >+ $is = CanItemBeReserved( $patron1, $item1 ); > is( >- $is, 0, >+ $is->{status}, "onShelfHoldsNotAllowed", > "Hold allowed from any library + ReservesControlBranch=PatronLibrary, " > . "One item is available at the same library, holdable = 1 available => the hold is not allowed at item level" > ); >@@ -381,8 +381,9 @@ Koha::CirculationRules->set_rules( > itemtype => $itemtype2, > branchcode => undef, > rules => { >- maxissueqty => 99, >- onshelfholds => 0, >+ reservesallowed => 99, >+ maxissueqty => 99, >+ onshelfholds => 0, > } > } > ); >@@ -391,8 +392,8 @@ $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron > ; # patron1 in library A, library A 1 item > is( $is, 1, "Items availability: 1 item is available, 1 item held in T" ); > >-$is = IsAvailableForItemLevelRequest( $item3, $patron1 ); >-is( $is, 1, "Item can be held, items in transit are not available" ); >+$is = CanItemBeReserved( $patron1, $item3 ); >+is( $is->{status}, "OK", "Item can be held, items in transit are not available" ); > > subtest 'Check holds availability with different item types' => sub { > plan tests => 6; >@@ -459,11 +460,17 @@ subtest 'Check holds availability with different item types' => sub { > "Items availability: 2 items, one allowed by smart rule but not checked out, another one not allowed to be held by smart rule" > ); > >- $is = IsAvailableForItemLevelRequest( $item4, $patron1 ); >- is( $is, 0, "Item4 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" ); >+ $is = CanItemBeReserved( $patron1, $item4 ); >+ is( >+ $is->{status}, "onShelfHoldsNotAllowed", >+ "Item4 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" >+ ); > >- $is = IsAvailableForItemLevelRequest( $item5, $patron1 ); >- is( $is, 0, "Item5 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" ); >+ $is = CanItemBeReserved( $patron1, $item5 ); >+ is( >+ $is->{status}, "noReservesAllowed", >+ "Item5 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" >+ ); > > AddIssue( $patron2, $item4->barcode ); > $cache->flush(); >@@ -474,13 +481,14 @@ subtest 'Check holds availability with different item types' => sub { > "Items availability: 2 items, one allowed by smart rule and checked out, another one not allowed to be held by smart rule" > ); > >- $is = IsAvailableForItemLevelRequest( $item4, $patron1 ); >- is( $is, 1, "Item4 can be requested to hold, 2 items, Item4 checked out, Item5 restricted" ); >+ $is = CanItemBeReserved( $patron1, $item4 ); >+ is( $is->{status}, "OK", "Item4 can be requested to hold, 2 items, Item4 checked out, Item5 restricted" ); > >- $is = IsAvailableForItemLevelRequest( $item5, $patron1 ); >- >- # Note: read IsAvailableForItemLevelRequest sub description about CanItemBeReserved/CanBookBeReserved: >- is( $is, 1, "Item5 can be requested to hold, 2 items, Item4 checked out, Item5 restricted" ); >+ $is = CanItemBeReserved( $patron1, $item5 ); >+ is( >+ $is->{status}, "noReservesAllowed", >+ "Item5 can be requested to hold, 2 items, Item4 checked out, Item5 restricted" >+ ); > }; > > subtest 'Check item checkout availability with ordered item' => sub { >@@ -549,8 +557,8 @@ subtest 'Check item availability for hold with ordered item' => sub { > ); > > $cache->flush(); >- $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); >- is( $is, 1, "Ordered items are available for hold" ); >+ $is = CanItemBeReserved( $patron1, $item1 ); >+ is( $is->{status}, "OK", "Ordered items are available for hold" ); > }; > > # Cleanup >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 52409d9246d..403f03a3739 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 71; >+use Test::More tests => 70; > use Test::NoWarnings; > use Test::MockModule; > use Test::Warn; >@@ -34,7 +34,7 @@ use C4::Biblio qw( GetMarcFromKohaField ModBiblio ); > use C4::HoldsQueue; > use C4::Members; > use C4::Reserves >- qw( AddReserve AlterPriority CheckReserves ModReserve ModReserveAffect ReserveSlip CalculatePriority CanBookBeReserved IsAvailableForItemLevelRequest MoveReserve ChargeReserveFee CanItemBeReserved MergeHolds ); >+ qw( AddReserve AlterPriority CheckReserves ModReserve ModReserveAffect ReserveSlip CalculatePriority CanBookBeReserved MoveReserve ChargeReserveFee CanItemBeReserved MergeHolds ); > use Koha::ActionLogs; > use Koha::Biblios; > use Koha::Caches; >@@ -659,9 +659,18 @@ is( > ####### EO Bug 13113 <<< > #### > >-ok( C4::Reserves::IsAvailableForItemLevelRequest( $item, $patron ), "Reserving a book on item level" ); >- >-my $pickup_branch = $builder->build( { source => 'Branch' } )->{branchcode}; >+is( C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, "OK", "Reserving a book on item level" ); >+my $branch = $builder->build_object( >+ { >+ class => 'Koha::Libraries', >+ value => { >+ branchemail => 'branch@e.mail', >+ branchreplyto => 'branch@reply.to', >+ pickup_location => 1 >+ } >+ } >+); >+my $pickup_branch = $branch->branchcode; > t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); > my $limit = Koha::Item::Transfer::Limit->new( >@@ -672,7 +681,7 @@ my $limit = Koha::Item::Transfer::Limit->new( > } > )->store(); > is( >- C4::Reserves::IsAvailableForItemLevelRequest( $item, $patron, $pickup_branch ), 0, >+ C4::Reserves::CanItemBeReserved( $patron, $item, $pickup_branch )->{status}, "cannotBeTransferred", > "Item level request not available due to transfer limit" > ); > t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); >@@ -1534,53 +1543,6 @@ sub place_item_hold { > # we reached the finish > $schema->storage->txn_rollback(); > >-subtest 'IsAvailableForItemLevelRequest() tests' => sub { >- >- plan tests => 2; >- >- $schema->storage->txn_begin; >- >- my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >- >- my $item_type = undef; >- >- my $item_mock = Test::MockModule->new('Koha::Item'); >- $item_mock->mock( 'effective_itemtype', sub { return $item_type; } ); >- >- my $item = $builder->build_sample_item; >- >- # Weird use case to highlight issue >- $item_type = '0'; >- Koha::ItemTypes->search( { itemtype => $item_type } )->delete; >- my $itemtype = $builder->build_object( >- { >- class => 'Koha::ItemTypes', >- value => { itemtype => $item_type } >- } >- ); >- ok( >- C4::Reserves::IsAvailableForItemLevelRequest( $item, $patron ), >- "Item not available for item-level hold because no effective item type" >- ); >- >- Koha::CirculationRules->set_rules( >- { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >- rules => { >- onshelfholds => 0, >- } >- } >- ); >- my $item_1 = $builder->build_sample_item( { notforloan => -1 } ); >- ok( >- C4::Reserves::IsAvailableForItemLevelRequest( $item_1, $patron ), >- "We can placing hold on item with negative not for loan values when 'On shelf holds allowed' is set to 'If any unavailable'" >- ); >- $schema->storage->txn_rollback; >-}; >- > subtest 'AddReserve() tests' => sub { > > plan tests => 2; >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32702
:
145590
|
149762
|
149763
|
157133
|
157134
|
161725
|
161726
|
170156
|
170157
|
170158
|
179474
|
179475
|
179476
|
179477
| 186122