Bugzilla – Attachment 58673 Details for
Bug 17737
Move GetReservesFromItemnumber to Koha::Item->holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17737: Replace GetReservesFromItemnumber with Koha::Item->get_holds_placed_before_today
Bug-17737-Replace-GetReservesFromItemnumber-with-K.patch (text/plain), 15.07 KB, created by
Jonathan Druart
on 2017-01-09 08:13:33 UTC
(
hide
)
Description:
Bug 17737: Replace GetReservesFromItemnumber with Koha::Item->get_holds_placed_before_today
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-01-09 08:13:33 UTC
Size:
15.07 KB
patch
obsolete
>From 055b5873a5ecbdb0d42564b51655897e7aa887a7 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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
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 17737
:
58018
|
58019
|
58673
|
58674
|
58675
|
59872
|
59876
|
59877
|
59878
|
59879
|
62105
|
62106
|
62107
|
62108