From 055b5873a5ecbdb0d42564b51655897e7aa887a7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 7 Dec 2016 13:50:46 +0100 Subject: [PATCH] Bug 17737: Replace GetReservesFromItemnumber with Koha::Item->get_holds_placed_before_today On the same way of Koha::Biblio->get_holds, Koha::Biblio->get_holds_placed_before_today and Koha::Patron->get_holds, this new subroutin will permit to retrieve the holds placed on a specific item. Note that at the moment we do not need a Koha::Item->get_holds method: we do not want to display future holds placed in the future. Test plan: I would suggest to test this patch with patches from bug 17736 and bug 17738, to place different kind of holds (biblio and item level, future and past). Then do a whole workflow to detect bug, view a record, delete record, order, place a hold on an item which has been ordered, etc. The hold's informations should always be the same without or without these patches. --- Koha/Item.pm | 21 +++++++++++++++++++++ catalogue/detail.pl | 22 ++++++++++++---------- circ/transferstoreceive.pl | 8 +++++--- opac/opac-reserve.pl | 19 ++++++++++--------- reserve/request.pl | 15 ++++++++------- t/db_dependent/Holds.t | 22 ++++++++++++++++------ t/db_dependent/Reserves.t | 25 ++++++++++++++----------- 7 files changed, 86 insertions(+), 46 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 52a7700..0bb8757 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -22,6 +22,7 @@ use Modern::Perl; use Carp; use Koha::Database; +use Koha::DateUtils qw( dt_from_string ); use C4::Context; use Koha::IssuingRules; @@ -170,6 +171,26 @@ sub article_request_type { return $issuing_rule->article_requests || q{} } +=head3 holds_placed_before_today + +=cut + +sub holds_placed_before_today { + my ( $self ) = @_; + my $attributes = { order_by => 'priority' }; + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + my $params = { + itemnumber => $self->itemnumber, + suspend => 0, + -or => [ + reservedate => { '<=' => $dtf->format_date(dt_from_string) }, + waitingdate => { '!=' => undef }, + ], + }; + my $hold_rs = $self->_result->reserves->search( $params, $attributes ); + return Koha::Holds->_new_from_dbic($hold_rs); +} + =head3 type =cut diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 3043661..25a28c5 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -43,6 +43,7 @@ use C4::CourseReserves qw(GetItemCourseReservesInfo); use C4::Acquisition qw(GetOrdersByBiblionumber); use Koha::AuthorisedValues; use Koha::Biblios; +use Koha::Items; use Koha::Patrons; use Koha::Virtualshelves; @@ -251,24 +252,25 @@ foreach my $item (@items) { $itemfields{$_} = 1 if ( $item->{$_} ); } - # checking for holds - my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($item->{itemnumber}); - my $ItemBorrowerReserveInfo = C4::Members::GetMember( borrowernumber => $reservedfor); - if (C4::Context->preference('HidePatronName')){ - $item->{'hidepatronname'} = 1; + $item->{'hidepatronname'} = 1; } - if ( defined $reservedate ) { + + # checking for holds + my $item_object = Koha::Items->find( $item->{itemnumber} ); + my $holds = $item_object->holds_placed_before_today; + if ( my $first_hold = $holds->next ) { + my $ItemBorrowerReserveInfo = C4::Members::GetMember( borrowernumber => $first_hold->borrowernumber); # FIXME could be improved $item->{backgroundcolor} = 'reserved'; - $item->{reservedate} = $reservedate; - $item->{ReservedForBorrowernumber} = $reservedfor; + $item->{reservedate} = $first_hold->reservedate; + $item->{ReservedForBorrowernumber} = $first_hold->borrowernumber; $item->{ReservedForSurname} = $ItemBorrowerReserveInfo->{'surname'}; $item->{ReservedForFirstname} = $ItemBorrowerReserveInfo->{'firstname'}; - $item->{ExpectedAtLibrary} = $expectedAt; + $item->{ExpectedAtLibrary} = $first_hold->branchcode; $item->{Reservedcardnumber} = $ItemBorrowerReserveInfo->{'cardnumber'}; # Check waiting status - $item->{waitingdate} = $wait; + $item->{waitingdate} = $first_hold->waitingdate; } diff --git a/circ/transferstoreceive.pl b/circ/transferstoreceive.pl index a67f799..8be5efc 100755 --- a/circ/transferstoreceive.pl +++ b/circ/transferstoreceive.pl @@ -36,6 +36,7 @@ use Date::Calc qw( use C4::Koha; use C4::Reserves; +use Koha::Items; use Koha::Libraries; my $input = new CGI; @@ -99,9 +100,10 @@ while ( my $library = $libraries->next ) { $getransf{'subtitle'} = GetRecordValue('subtitle', $record, GetFrameworkCode($gettitle->{'biblionumber'})); # we check if we have a reserv for this transfer - my @checkreserv = GetReservesFromItemnumber($num->{'itemnumber'}); - if ( $checkreserv[0] ) { - my $getborrower = C4::Members::GetMember( borrowernumber => $checkreserv[1] ); + my $item = Koha::Items->find( $num->{itemnumber} ); + my $holds = $item->holds_placed_before_today; + if ( my $first_hold = $holds->next ) { + my $getborrower = C4::Members::GetMember( borrowernumber => $first_hold->borrowernumber ); $getransf{'borrowernum'} = $getborrower->{'borrowernumber'}; $getransf{'borrowername'} = $getborrower->{'surname'}; $getransf{'borrowerfirstname'} = $getborrower->{'firstname'}; diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 0b92e27..1b0d2a8 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -33,6 +33,7 @@ use C4::Members; use C4::Overdues; use C4::Debug; use Koha::DateUtils; +use Koha::Items; use Koha::Libraries; use Koha::Patrons; use Date::Calc qw/Today Date_to_Days/; @@ -448,21 +449,21 @@ foreach my $biblioNum (@biblionumbers) { } # checking reserve - my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($itemNum); - my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); + my $item = Koha::Items->find( $itemNum ); + my $holds = $item->holds_placed_before_today; # the item could be reserved for this borrower vi a host record, flag this - $reservedfor //= ''; + my $reservedfor = q||; - if ( defined $reservedate ) { + if ( my $first_hold = $holds->next ) { + my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $first_hold->borrowernumber ); $itemLoopIter->{backgroundcolor} = 'reserved'; - $itemLoopIter->{reservedate} = output_pref({ dt => dt_from_string($reservedate), dateonly => 1 }); - $itemLoopIter->{ReservedForBorrowernumber} = $reservedfor; + $itemLoopIter->{reservedate} = output_pref({ dt => dt_from_string($first_hold->reservedate), dateonly => 1 }); # FIXME Should be formatted in the template + $itemLoopIter->{ReservedForBorrowernumber} = $first_hold->borrowernumber; $itemLoopIter->{ReservedForSurname} = $ItemBorrowerReserveInfo->{'surname'}; $itemLoopIter->{ReservedForFirstname} = $ItemBorrowerReserveInfo->{'firstname'}; - $itemLoopIter->{ExpectedAtLibrary} = $expectedAt; - #waiting status - $itemLoopIter->{waitingdate} = $wait; + $itemLoopIter->{ExpectedAtLibrary} = $first_hold->branchcode; + $itemLoopIter->{waitingdate} = $first_hold->waitingdate; } $itemLoopIter->{notforloan} = $itemInfo->{notforloan}; diff --git a/reserve/request.pl b/reserve/request.pl index 3f4dba3..c155e50 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -44,6 +44,7 @@ use C4::Members; use C4::Search; # enabled_staff_search_views use Koha::DateUtils; use Koha::Holds; +use Koha::Items; use Koha::Libraries; use Koha::Patrons; @@ -386,17 +387,17 @@ foreach my $biblionumber (@biblionumbers) { } # checking reserve - my ($reservedate,$reservedfor,$expectedAt,$reserve_id,$wait) = GetReservesFromItemnumber($itemnumber); - if ( defined $reservedate ) { - my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); + my $holds = Koha::Items->find( $itemnumber )->holds_placed_before_today; + if ( my $first_hold = $holds->next ) { + my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $first_hold->borrowernumber ); $item->{backgroundcolor} = 'reserved'; - $item->{reservedate} = output_pref({ dt => dt_from_string( $reservedate ), dateonly => 1 }); - $item->{ReservedForBorrowernumber} = $reservedfor; + $item->{reservedate} = output_pref({ dt => dt_from_string( $first_hold->reservedate ), dateonly => 1 }); # FIXME Should be formatted in the template + $item->{ReservedForBorrowernumber} = $first_hold->borrowernumber; $item->{ReservedForSurname} = $ItemBorrowerReserveInfo->{'surname'}; $item->{ReservedForFirstname} = $ItemBorrowerReserveInfo->{'firstname'}; - $item->{ExpectedAtLibrary} = $expectedAt; - $item->{waitingdate} = $wait; + $item->{ExpectedAtLibrary} = $first_hold->branchcode; + $item->{waitingdate} = $first_hold->waitingdate; } # Management of the notforloan document diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 5825f7c..d49deea 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -94,11 +94,17 @@ is( $holds->next->priority, 3, "Reserve 3 has a priority of 3" ); is( $holds->next->priority, 4, "Reserve 4 has a priority of 4" ); is( $holds->next->priority, 5, "Reserve 5 has a priority of 5" ); -my ( $reservedate, $borrowernumber, $branch_1code, $reserve_id ) = GetReservesFromItemnumber($itemnumber); -is( $reservedate, output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), "GetReservesFromItemnumber should return a valid reserve date"); -is( $borrowernumber, $borrowernumbers[0], "GetReservesFromItemnumber should return a valid borrowernumber"); -is( $branch_1code, $branch_1, "GetReservesFromItemnumber should return a valid branchcode"); -ok($reserve_id, "Test GetReservesFromItemnumber()"); +my $item = Koha::Items->find( $itemnumber ); +$holds = $item->holds_placed_before_today; +my $first_hold = $holds->next; +my $reservedate = $first_hold->reservedate; +my $borrowernumber = $first_hold->borrowernumber; +my $branch_1code = $first_hold->branchcode; +my $reserve_id = $first_hold->reserve_id; +is( $reservedate, output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), "holds_placed_today should return a valid reserve date"); +is( $borrowernumber, $borrowernumbers[0], "holds_placed_today should return a valid borrowernumber"); +is( $branch_1code, $branch_1, "holds_placed_today should return a valid branchcode"); +ok($reserve_id, "Test holds_placed_today()"); my $hold = Koha::Holds->find( $reserve_id ); ok( $hold, "Koha::Holds found the hold" ); @@ -126,8 +132,12 @@ CancelReserve({ 'reserve_id' => $reserve_id }); $holds = $biblio->holds; is( $holds->count, $borrowers_count - 1, "Test CancelReserve()" ); +$holds = $item->holds_placed_before_today; +$first_hold = $holds->next; +$borrowernumber = $first_hold->borrowernumber; +$branch_1code = $first_hold->branchcode; +$reserve_id = $first_hold->reserve_id; -( $reservedate, $borrowernumber, $branch_1code, $reserve_id ) = GetReservesFromItemnumber($itemnumber); ModReserve({ reserve_id => $reserve_id, rank => '4', diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index c5d2c06..430c26d 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -396,8 +396,8 @@ is( my $letter = ReserveSlip($branch_1, $requesters{$branch_1}, $bibnum); ok(defined($letter), 'can successfully generate hold slip (bug 10949)'); -# Tests for bug 9788: Does GetReservesFromItemnumber return a future wait? -# 9788a: GetReservesFromItemnumber does not return future next available hold +# Tests for bug 9788: Does Koha::Item->holds_placed_before_today return a future wait? +# 9788a: holds_placed_before_today does not return future next available hold $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); t::lib::Mocks::mock_preference('ConfirmFutureHolds', 2); t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); @@ -407,19 +407,22 @@ $resdate=output_pref($resdate); AddReserve($branch_1, $requesters{$branch_1}, $bibnum, $bibitems, 1, $resdate, $expdate, $notes, $title, $checkitem, $found); -my @results= GetReservesFromItemnumber($itemnumber); -is(defined $results[3]?1:0, 0, 'GetReservesFromItemnumber does not return a future next available hold'); -# 9788b: GetReservesFromItemnumber does not return future item level hold +my $item = Koha::Items->find( $itemnumber ); +$holds = $item->holds_placed_before_today; +my $dtf = Koha::Database->new->schema->storage->datetime_parser; +my $future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +is( $future_holds->count, 0, 'holds_placed_before_today does not return a future next available hold'); +# 9788b: holds_placed_before_today does not return future item level hold $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); AddReserve($branch_1, $requesters{$branch_1}, $bibnum, $bibitems, 1, $resdate, $expdate, $notes, $title, $itemnumber, $found); #item level hold -@results= GetReservesFromItemnumber($itemnumber); -is(defined $results[3]?1:0, 0, 'GetReservesFromItemnumber does not return a future item level hold'); -# 9788c: GetReservesFromItemnumber returns future wait (confirmed future hold) +$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +is( $future_holds->count, 0, 'holds_placed_before_today does not return a future item level hold' ); +# 9788c: holds_placed_before_today returns future wait (confirmed future hold) ModReserveAffect( $itemnumber, $requesters{$branch_1} , 0); #confirm hold -@results= GetReservesFromItemnumber($itemnumber); -is(defined $results[3]?1:0, 1, 'GetReservesFromItemnumber returns a future wait (confirmed future hold)'); +$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +is( $future_holds->count, 1, 'holds_placed_before_today returns a future wait (confirmed future hold)' ); # End of tests for bug 9788 $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); @@ -539,7 +542,7 @@ is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Res ####### EO Bug 13113 <<< #### -my $item = GetItem($itemnumber); +$item = GetItem($itemnumber); ok( C4::Reserves::IsAvailableForItemLevelRequest($item, $borrower), "Reserving a book on item level" ); -- 2.9.3