Bugzilla – Attachment 62188 Details for
Bug 17738
Move GetReservesFromBorrowernumber to Koha::Patron->holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17738: Replace GetReservesFromBorrowernumber with Koha::Patron->get_holds
Bug-17738-Replace-GetReservesFromBorrowernumber-wi.patch (text/plain), 14.00 KB, created by
Marc Véron
on 2017-04-15 07:02:33 UTC
(
hide
)
Description:
Bug 17738: Replace GetReservesFromBorrowernumber with Koha::Patron->get_holds
Filename:
MIME Type:
Creator:
Marc Véron
Created:
2017-04-15 07:02:33 UTC
Size:
14.00 KB
patch
obsolete
>From 856a6ea6a099a0a9a384d0d0978b521333169ab6 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 7 Dec 2016 14:42:48 +0100 >Subject: [PATCH] Bug 17738: Replace GetReservesFromBorrowernumber with > Koha::Patron->get_holds >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This patch replace the different calls to GetReservesFromBorrowernumber >with a calls to Koha::Patron->get_holds. >In some places we need to get a restricted set of holds, that's why we >process a search on this holds returned by ->get_holds (on the found >status for instance). > >The changes are quite trivial and reading the diff should be enough to >catch bugs. > >Test plan: >I would suggest to test this patch with patches from bug 17736 and bug 17737, >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. > >Tested both patches together, works as expected. >Signed-off-by: Marc Véron <veron@veron.ch> >--- > C4/ILSDI/Services.pm | 30 ++++++++++++++++++------------ > C4/Members.pm | 8 +++++--- > C4/SIP/ILS/Patron.pm | 20 +++++++++++++------- > Koha/Patron/Discharge.pm | 9 +++++---- > circ/returns.pl | 12 ++++-------- > members/discharge.pl | 6 ++++-- > opac/opac-reserve.pl | 14 ++++++++------ > t/db_dependent/Holds.t | 17 ++++++++++------- > 8 files changed, 67 insertions(+), 49 deletions(-) > >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index 287ac02..c1632d5 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -25,7 +25,7 @@ use C4::Items; > use C4::Circulation; > use C4::Accounts; > use C4::Biblio; >-use C4::Reserves qw(AddReserve GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved IsAvailableForItemLevelRequest); >+use C4::Reserves qw(AddReserve CanBookBeReserved CanItemBeReserved IsAvailableForItemLevelRequest); > use C4::Context; > use C4::AuthoritiesMarc; > use XML::Simple; >@@ -37,6 +37,7 @@ use C4::Members::Attributes qw(GetBorrowerAttributes); > > use Koha::Biblios; > use Koha::Libraries; >+use Koha::Patrons; > > =head1 NAME > >@@ -375,6 +376,7 @@ sub GetPatronInfo { > my $borrowernumber = $cgi->param('patron_id'); > my $borrower = GetMember( borrowernumber => $borrowernumber ); > return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; >+ my $patron = Koha::Patrons->find( $borrowernumber ); > > # Cleaning the borrower hashref > my $flags = C4::Members::patronflags( $borrower ); >@@ -414,23 +416,25 @@ sub GetPatronInfo { > if ( $cgi->param('show_holds') && $cgi->param('show_holds') eq "1" ) { > > # Get borrower's reserves >- my @reserves = GetReservesFromBorrowernumber( $borrowernumber, undef ); >- foreach my $reserve (@reserves) { >+ my $holds = $patron->holds; >+ while ( my $hold = $holds->next ) { > >+ my $unblessed_hold = $hold->unblessed; > # Get additional informations >- my $item = GetBiblioFromItemNumber( $reserve->{'itemnumber'}, undef ); >- my $library = Koha::Libraries->find( $reserve->{branchcode} ); >+ my $item = GetBiblioFromItemNumber( $hold->itemnumber, undef ); >+ my $library = Koha::Libraries->find( $hold->branchcode ); # Should $hold->get_library > my $branchname = $library ? $library->branchname : ''; > > # Remove unwanted fields > delete $item->{'more_subfields_xml'}; > > # Add additional fields >- $reserve->{'item'} = $item; >- $reserve->{'branchname'} = $branchname; >- $reserve->{'title'} = GetBiblio( $reserve->{'biblionumber'} )->{'title'}; >+ $unblessed_hold->{item} = $item; >+ $unblessed_hold->{branchname} = $branchname; >+ $unblessed_hold->{title} = GetBiblio( $hold->biblionumber )->{'title'}; # Should be $hold->get_biblio >+ >+ push @{ $borrower->{holds}{hold} }, $unblessed_hold; > } >- $borrower->{'holds'}->{'hold'} = \@reserves; > } > > # Issues management >@@ -500,6 +504,8 @@ sub GetServices { > my $borrower = GetMember( borrowernumber => $borrowernumber ); > return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; > >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ > # Get the item, or return an error code if not found > my $itemnumber = $cgi->param('item_id'); > my $item = GetItem( $itemnumber ); >@@ -519,10 +525,10 @@ sub GetServices { > } > > # Reserve cancellation management >- my @reserves = GetReservesFromBorrowernumber( $borrowernumber, undef ); >+ my $holds = $patron->holds; > my @reserveditems; >- foreach my $reserve (@reserves) { >- push @reserveditems, $reserve->{'itemnumber'}; >+ while ( my $hold = $holds->next ) { # FIXME This could be improved >+ push @reserveditems, $hold->itemnumber; > } > if ( grep { $itemnumber eq $_ } @reserveditems ) { > push @availablefor, 'hold cancellation'; >diff --git a/C4/Members.pm b/C4/Members.pm >index ee10cc7..acb4d99 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -267,12 +267,14 @@ sub patronflags { > } > $flags{'ODUES'} = \%flaginfo; > } >- my @itemswaiting = C4::Reserves::GetReservesFromBorrowernumber( $patroninformation->{'borrowernumber'},'W' ); >- my $nowaiting = scalar @itemswaiting; >+ >+ my $patron = Koha::Patrons->find( $patroninformation->{borrowernumber} ); >+ my $waiting_holds = $patron->holds->search({ found => 'W' }); >+ my $nowaiting = $waiting_holds->count; > if ( $nowaiting > 0 ) { > my %flaginfo; > $flaginfo{'message'} = "Reserved items available"; >- $flaginfo{'itemlist'} = \@itemswaiting; >+ $flaginfo{'itemlist'} = $waiting_holds->unblessed; > $flags{'WAITING'} = \%flaginfo; > } > return ( \%flags ); >diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm >index 0012776..64ef4f6 100644 >--- a/C4/SIP/ILS/Patron.pm >+++ b/C4/SIP/ILS/Patron.pm >@@ -24,6 +24,7 @@ use C4::Items qw( GetBarcodeFromItemnumber GetItemnumbersForBiblio); > use C4::Auth qw(checkpw); > > use Koha::Libraries; >+use Koha::Patrons; > > our $kp; # koha patron > >@@ -431,20 +432,25 @@ sub _get_address { > > sub _get_outstanding_holds { > my $borrowernumber = shift; >- my @hold_array = grep { !defined $_->{found} || $_->{found} ne 'W'} GetReservesFromBorrowernumber($borrowernumber); >- foreach my $h (@hold_array) { >+ >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $holds = $patron->holds->search( { -or => [ { found => undef }, { found => { '!=' => 'W' } } ] } ); >+ my @holds; >+ while ( my $hold = $holds->next ) { > my $item; >- if ($h->{itemnumber}) { >- $item = $h->{itemnumber}; >+ if ($hold->itemnumber) { >+ $item = $hold->itemnumber; > } > else { > # We need to return a barcode for the biblio so the client > # can request the biblio info >- $item = ( GetItemnumbersForBiblio($h->{biblionumber}) )->[0]; >+ $item = ( GetItemnumbersForBiblio($hold->biblionumber) )->[0]; > } >- $h->{barcode} = GetBarcodeFromItemnumber($item); >+ my $unblessed_hold = $hold->unblessed; >+ $unblessed_hold->{barcode} = GetBarcodeFromItemnumber($item); >+ push @holds, $unblessed_hold; > } >- return \@hold_array; >+ return \@holds; > } > > 1; >diff --git a/Koha/Patron/Discharge.pm b/Koha/Patron/Discharge.pm >index d519d40..7452811 100644 >--- a/Koha/Patron/Discharge.pm >+++ b/Koha/Patron/Discharge.pm >@@ -7,7 +7,7 @@ use Carp; > > use C4::Templates qw ( gettemplate ); > use C4::Members qw( GetPendingIssues ); >-use C4::Reserves qw( GetReservesFromBorrowernumber CancelReserve ); >+use C4::Reserves qw( CancelReserve ); > > use Koha::Database; > use Koha::DateUtils qw( dt_from_string output_pref ); >@@ -79,9 +79,10 @@ sub discharge { > return unless $borrowernumber and can_be_discharged( { borrowernumber => $borrowernumber } ); > > # Cancel reserves >- my @reserves = GetReservesFromBorrowernumber($borrowernumber); >- for my $reserve (@reserves) { >- CancelReserve( { reserve_id => $reserve->{reserve_id} } ); >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $holds = $patron->holds; >+ while ( my $hold = $holds->next ) { >+ CancelReserve( { reserve_id => $hold->reserve_id } ); > } > > # Debar the member >diff --git a/circ/returns.pl b/circ/returns.pl >index 6e27645..0ba88e8 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -50,8 +50,9 @@ use C4::RotatingCollections; > use Koha::AuthorisedValues; > use Koha::DateUtils; > use Koha::Calendar; >-use Koha::Checkouts; > use Koha::BiblioFrameworks; >+use Koha::Checkouts; >+use Koha::Patrons; > > my $query = new CGI; > >@@ -336,13 +337,8 @@ if ($barcode) { > > if (C4::Context->preference("WaitingNotifyAtCheckin") ) { > #Check for waiting holds >- my @reserves = GetReservesFromBorrowernumber($borrower->{'borrowernumber'}); >- my $waiting_holds = 0; >- foreach my $num_res (@reserves) { >- if ( $num_res->{'found'} eq 'W' && $num_res->{'branchcode'} eq $userenv_branch) { >- $waiting_holds++; >- } >- } >+ my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); >+ my $waiting_holds = $patron->holds->search({ found => 'W', branchcode => $userenv_branch })->count; > if ($waiting_holds > 0) { > $template->param( > waiting_holds => $waiting_holds, >diff --git a/members/discharge.pl b/members/discharge.pl >index ededd23..23b4378 100755 >--- a/members/discharge.pl >+++ b/members/discharge.pl >@@ -38,6 +38,7 @@ use C4::Reserves; > use C4::Letters; > use Koha::Patron::Discharge; > use Koha::Patron::Images; >+use Koha::Patrons; > > use Koha::DateUtils; > >@@ -70,8 +71,9 @@ if ( $input->param('borrowernumber') ) { > }); > > # Getting reserves >- my @reserves = GetReservesFromBorrowernumber($borrowernumber); >- my $has_reserves = scalar(@reserves); >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $holds = $patron->holds; >+ my $has_reserves = $holds->count; > > # Generating discharge if needed > if ( $input->param('discharge') and $can_be_discharged ) { >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 7de7b97..2cf8e59 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -199,7 +199,8 @@ foreach my $biblioNumber (@biblionumbers) { > if ( $query->param('place_reserve') ) { > my $reserve_cnt = 0; > if ($maxreserves) { >- $reserve_cnt = GetReservesFromBorrowernumber( $borrowernumber ); >+ my $patron = Koha::Patrons->find( $borrowernumber ); >+ $reserve_cnt = $patron->holds->count; > } > > # List is composed of alternating biblio/item/branch >@@ -350,7 +351,8 @@ if ( $borr->{lost} && ($borr->{lost} == 1) ) { > ); > } > >-if ( Koha::Patrons->find( $borrowernumber )->is_debarred ) { >+my $patron = Koha::Patrons->find( $borrowernumber ); >+if ( $patron->is_debarred ) { > $noreserves = 1; > $template->param( > message => 1, >@@ -360,13 +362,13 @@ if ( Koha::Patrons->find( $borrowernumber )->is_debarred ) { > ); > } > >-my @reserves = GetReservesFromBorrowernumber( $borrowernumber ); >-my $reserves_count = scalar(@reserves); >-$template->param( RESERVES => \@reserves ); >+my $holds = $patron->holds; >+my $reserves_count = $holds->count; >+$template->param( RESERVES => $holds->unblessed ); > if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) { > $template->param( message => 1 ); > $noreserves = 1; >- $template->param( too_many_reserves => scalar(@reserves)); >+ $template->param( too_many_reserves => $holds->count ); > } > > unless ( $noreserves ) { >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index 1b4f1df..77df2e1 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -17,6 +17,7 @@ use Koha::Database; > use Koha::DateUtils qw( dt_from_string output_pref ); > use Koha::Biblios; > use Koha::Holds; >+use Koha::Patrons; > > BEGIN { > use FindBin; >@@ -121,8 +122,9 @@ my $hold_found = $hold->found(); > $hold->set({ found => 'W'})->store(); > is( Koha::Holds->waiting()->count(), 1, "Koha::Holds->waiting returns waiting holds" ); > >-my ( $reserve ) = GetReservesFromBorrowernumber($borrowernumbers[0]); >-ok( $reserve->{'borrowernumber'} eq $borrowernumbers[0], "Test GetReservesFromBorrowernumber()"); >+my $patron = Koha::Patrons->find( $borrowernumbers[0] ); >+$holds = $patron->holds; >+is( $holds->next->borrowernumber, $borrowernumbers[0], "Test Koha::Patron->holds"); > > > ok( GetReserveCount( $borrowernumbers[0] ), "Test GetReserveCount()" ); >@@ -146,7 +148,7 @@ ModReserve({ > suspend_until => output_pref( { dt => dt_from_string( "2013-01-01", "iso" ), dateonly => 1 } ), > }); > >-$reserve = GetReserve( $reserve_id ); >+my $reserve = GetReserve( $reserve_id ); > ok( $reserve->{'priority'} eq '4', "Test GetReserve(), priority changed correctly" ); > ok( $reserve->{'suspend'}, "Test GetReserve(), suspend hold" ); > is( $reserve->{'suspend_until'}, '2013-01-01 00:00:00', "Test GetReserve(), suspend until date" ); >@@ -196,17 +198,18 @@ AddReserve( > my $checkitem, > my $found, > ); >-( $reserve ) = GetReservesFromBorrowernumber($borrowernumber); >+$patron = Koha::Patrons->find( $borrowernumber ); >+$holds = $patron->holds; > my $reserveid = C4::Reserves::GetReserveId( > { > biblionumber => $biblionumber, > borrowernumber => $borrowernumber > } > ); >-is( $reserveid, $reserve->{reserve_id}, "Test GetReserveId" ); >+is( $reserveid, $holds->next->reserve_id, "Test GetReserveId" ); > ModReserveMinusPriority( $itemnumber, $reserve->{'reserve_id'} ); >-( $reserve ) = GetReservesFromBorrowernumber($borrowernumber); >-ok( $reserve->{'itemnumber'} eq $itemnumber, "Test ModReserveMinusPriority()" ); >+$holds = $patron->holds; >+is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" ); > > > my $reserve2 = GetReserveInfo( $reserve->{'reserve_id'} ); >-- >2.1.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=62188">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17738
:
58020
|
58021
|
62109
|
62110
|
62188
|
62189
|
62208
|
62219
|
64330
|
64331
|
64332
|
64749
|
64750
|
64751
|
64752