From a9779b67d652351c32486ffc005a01b465ec463e Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Tue, 3 Oct 2017 14:50:36 +0000 Subject: [PATCH] Bug 4319: [OPAC] Allow holds on waiting/transit items Test plan: - Checkout an item - Place hold on this item, - Return the item - Make sure the hold is waiting (found W) and AllowOnShelfHolds is not to 'Allow' - Check that the button "Place hold" appears in opac detail page of the biblio - do the samewith items/reserves in transit Changes on C4::Reserves::IsAvailableForItemLevelRequest Make sure this tests pass: - t/db_dependent/Reserves.t - t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Rebased - 2017-12-12 - Alex Arnaud Bug 4319 - [QA fix] Create Koha::Biblio->hasItemswaitingOrInTransit Signed-off-by: Jon Knight Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart --- C4/Reserves.pm | 12 ++++- Koha/Biblio.pm | 18 +++++++ .../en/includes/opac-detail-sidebar.inc | 2 +- opac/opac-detail.pl | 13 +++-- .../Holds/DisallowHoldIfItemsAvailable.t | 49 ++++++++++++++++++- t/db_dependent/Koha/Biblios.t | 42 +++++++++++++++- 6 files changed, 127 insertions(+), 9 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 216ececdba..f39dc3b57a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1078,6 +1078,7 @@ 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, @@ -1142,7 +1143,16 @@ sub IsAvailableForItemLevelRequest { return $any_available ? 0 : 1; } - return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting"; + if ($item->{onloan}) { + return 1; + } + + if ( Koha::Holds->search({itemnumber => $item->{itemnumber}, + found => ['W', 'T']})->count ) { + return 1; + } + + return 0; } =head2 OnShelfHoldsAllowed diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index f4e5e2f9e2..9a4937da49 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -318,6 +318,24 @@ sub subscriptions { return $self->{_subscriptions}; } +=head3 hasItemswaitingOrInTransit + +=cut + +sub hasItemswaitingOrInTransit { + my ( $self ) = @_; + + if ( Koha::Holds->search({ biblionumber => $self->id, + found => ['W', 'T'] })->count ) { + return 1; + } + + foreach my $item ( $self->items ) { + return 1 if $item->get_transfer; + } + + return 0; +} =head3 type diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc index 7ceaa9590e..e408d0af9e 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc @@ -3,7 +3,7 @@ [% UNLESS ( norequests ) %] [% IF Koha.Preference( 'opacuserlogin' ) == 1 %] [% IF Koha.Preference( 'RequestOnOpac' ) == 1 %] - [% IF ( AllowOnShelfHolds OR ItemsIssued ) %] + [% IF ( ReservableItems ) %]
  • Place hold
    • [% END %] [% END %] diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index bdef7d8b5e..d6754bf4f9 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -460,9 +460,9 @@ if ($session->param('busc')) { } } - -$template->param( 'ItemsIssued' => CountItemsIssued( $biblionumber ) ); -$template->param('OPACShowCheckoutName' => C4::Context->preference("OPACShowCheckoutName") ); +$template->param( + OPACShowCheckoutName => C4::Context->preference("OPACShowCheckoutName"), +); # adding items linked via host biblios @@ -652,13 +652,13 @@ if ( C4::Context->preference('OPACAcquisitionDetails' ) ) { }; } +my $allow_onshelf_holds; if ( not $viewallitems and @items > $max_items_to_display ) { $template->param( too_many_items => 1, items_count => scalar( @items ), ); } else { - my $allow_onshelf_holds; my $patron = Koha::Patrons->find( $borrowernumber ); for my $itm (@items) { $itm->{holds_count} = $item_reserves{ $itm->{itemnumber} }; @@ -717,9 +717,12 @@ if ( not $viewallitems and @items > $max_items_to_display ) { push @itemloop, $itm; } } - $template->param( 'AllowOnShelfHolds' => $allow_onshelf_holds ); } +my $itemsWaitingOrInTransit = Koha::Biblios->find($biblionumber)->hasItemswaitingOrInTransit || 0; +my $itemsIssued = CountItemsIssued( $biblionumber ); +$template->param( 'ReservableItems' => $itemsWaitingOrInTransit || $itemsIssued || $allow_onshelf_holds ); + # Display only one tab if one items list is empty if (scalar(@itemloop) == 0 || scalar(@otheritemloop) == 0) { $template->param(SeparateHoldings => 0); diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t index 3c5fd72708..3abba349c1 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -7,7 +7,7 @@ use C4::Items; use C4::Circulation; use Koha::IssuingRule; -use Test::More tests => 5; +use Test::More tests => 7; use t::lib::TestBuilder; use t::lib::Mocks; @@ -213,5 +213,52 @@ AddReturn( $item1->{barcode} ); }; } +my $biblio = $builder->build({ + source => 'Biblio', +}); + +my $item3 = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblio->{biblionumber}, + itemlost => 0, + notforloan => 0, + withdrawn => 0, + damaged => 0, + onloan => 0 + } +}); + +my $hold = $builder->build({ + source => 'Reserve', + value =>{ + itemnumber => $item3->{itemnumber}, + found => 'T' + } +}); + +$dbh->do("DELETE FROM issuingrules"); +$rule = Koha::IssuingRule->new( + { + categorycode => '*', + itemtype => '*', + branchcode => '*', + maxissueqty => 99, + issuelength => 7, + lengthunit => 8, + reservesallowed => 99, + onshelfholds => 0, + } +); +$rule->store(); + +$is = IsAvailableForItemLevelRequest( $item3, $borrower1); +is( $is, 1, "Item can be held, items in transit are not available" ); + +Koha::Holds->find($hold->{reserve_id})->found('F')->store; + +$is = IsAvailableForItemLevelRequest( $item3, $borrower1); +is( $is, 0, "Item is neither waiting nor in transit." ); + # Cleanup $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index 9f6a14a872..9d4153138b 100644 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 2; +use Test::More tests => 3; use C4::Reserves; @@ -80,5 +80,45 @@ subtest 'subscriptions' => sub { is( $subscriptions->count, 2, 'Koha::Biblio->subscriptions should return the correct number of subscriptions'); }; +subtest 'waiting_or_in_transit' => sub { + plan tests => 4; + my $biblio = $builder->build( { source => 'Biblio' } ); + my $item = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblio->{biblionumber} + } + }); + my $reserve = $builder->build({ + source => 'Reserve', + value => { + biblionumber => $biblio->{biblionumber}, + found => undef + } + }); + + $reserve = Koha::Holds->find($reserve->{reserve_id}); + $biblio = Koha::Biblios->find($biblio->{biblionumber}); + + is($biblio->hasItemswaitingOrInTransit, 0, 'Item is neither waiting nor in transit'); + + $reserve->found('W')->store; + is($biblio->hasItemswaitingOrInTransit, 1, 'Item is waiting'); + + $reserve->found('T')->store; + is($biblio->hasItemswaitingOrInTransit, 1, 'Item is in transit'); + + my $transfer = $builder->build({ + source => 'Branchtransfer', + value => { + itemnumber => $item->{itemnumber}, + datearrived => undef + } + }); + my $t = Koha::Database->new()->schema()->resultset( 'Branchtransfer' )->find($transfer->{branchtransfer_id}); + $reserve->found(undef)->store; + is($biblio->hasItemswaitingOrInTransit, 1, 'Item has transfer'); +}; + $schema->storage->txn_rollback; -- 2.39.5