Bugzilla – Attachment 65636 Details for
Bug 19059
Move C4::Reserves::CancelReserve to the Koha::Hold->cancel
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED-OFF] Bug 19059: Move C4::Reserves::CancelReserve to Koha::Hold->cancel
SIGNED-OFF-Bug-19059-Move-C4ReservesCancelReserve-.patch (text/plain), 32.46 KB, created by
Owen Leonard
on 2017-08-08 14:00:38 UTC
(
hide
)
Description:
[SIGNED-OFF] Bug 19059: Move C4::Reserves::CancelReserve to Koha::Hold->cancel
Filename:
MIME Type:
Creator:
Owen Leonard
Created:
2017-08-08 14:00:38 UTC
Size:
32.46 KB
patch
obsolete
>From 267ba800f8adc55eed1f6043bf6183ec3aba20c2 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 2 Aug 2017 14:00:12 -0300 >Subject: [PATCH] [SIGNED-OFF] Bug 19059: Move C4::Reserves::CancelReserve to > Koha::Hold->cancel > >This patch adds a new Koha::Hold->cancel method and replaces the calls >to C4::Reserves::CancelReserve with it. > >Test plan: >- Add and cancel holds >- Change priority of holds > >Signed-off-by: Owen Leonard <oleonard@myacpl.org> >--- > C4/Biblio.pm | 3 +- > C4/ILSDI/Services.pm | 2 +- > C4/Reserves.pm | 51 +++++++++++++++--------- > C4/SIP/ILS/Transaction/Hold.pm | 14 +++++-- > Koha/Hold.pm | 58 +++++++++++++++++++++++++++- > Koha/Patron/Discharge.pm | 3 +- > Koha/REST/V1/Hold.pm | 2 +- > circ/branchtransfers.pl | 23 +++++++---- > circ/returns.pl | 6 ++- > circ/waitingreserves.pl | 2 +- > opac/opac-modrequest.pl | 6 ++- > reserve/request.pl | 3 +- > t/db_dependent/Circulation.t | 4 +- > t/db_dependent/Holds.t | 14 +++---- > t/db_dependent/Holds/HoldFulfillmentPolicy.t | 19 ++++----- > t/db_dependent/Holds/HoldItemtypeLimit.t | 8 ++-- > t/db_dependent/HoldsQueue.t | 25 ++++++------ > t/db_dependent/Reserves.t | 18 +++------ > t/db_dependent/UsageStats.t | 2 +- > tools/batch_delete_records.pl | 2 +- > 20 files changed, 176 insertions(+), 89 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 0ee689d..cee8fe8 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -406,9 +406,8 @@ sub DelBiblio { > # We delete any existing holds > my $biblio = Koha::Biblios->find( $biblionumber ); > my $holds = $biblio->holds; >- require C4::Reserves; > while ( my $hold = $holds->next ) { >- C4::Reserves::CancelReserve({ reserve_id => $hold->reserve_id }); # TODO Replace with $hold->cancel >+ $hold->cancel; > } > > # Delete in Zebra. Be careful NOT to move this line after _koha_delete_biblio >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index dd52619..54ca13d 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -773,7 +773,7 @@ sub CancelHold { > return { code => 'RecordNotFound' } unless $hold; > return { code => 'RecordNotFound' } unless ($hold->borrowernumber == $borrowernumber); > >- C4::Reserves::CancelReserve({reserve_id => $reserve_id}); >+ $hold->cancel; > > return { code => 'Canceled' }; > } >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 3be9df9..20ad806 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -121,7 +121,6 @@ BEGIN { > &CanBookBeReserved > &CanItemBeReserved > &CanReserveBeCanceledFromOpac >- &CancelReserve > &CancelExpiredReserves > > &AutoUnsuspendReserves >@@ -798,23 +797,27 @@ sub CancelExpiredReserves { > my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); > > my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare( " >- SELECT * FROM reserves WHERE DATE(expirationdate) < DATE( CURDATE() ) >- AND expirationdate IS NOT NULL >- " ); >- $sth->execute(); > >- while ( my $res = $sth->fetchrow_hashref() ) { >- my $calendar = Koha::Calendar->new( branchcode => $res->{'branchcode'} ); >- my $cancel_params = { reserve_id => $res->{'reserve_id'} }; >+ my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+ my $today = dt_from_string; >+ # FIXME To move to Koha::Holds->search_expired (?) >+ my $holds = Koha::Holds->search( >+ { >+ expirationdate => { '<', $dtf->format_date($today) } >+ } >+ ); >+ >+ while ( my $hold = $holds->next ) { >+ my $calendar = Koha::Calendar->new( branchcode => $hold->branchcode ); > > next if !$cancel_on_holidays && $calendar->is_holiday( $today ); > >- if ( $res->{found} eq 'W' ) { >+ my $cancel_params = {}; >+ if ( $holds->found eq 'W' ) { > $cancel_params->{charge_cancel_fee} = 1; > } >+ $hold->cancel( $cancel_params ); > >- CancelReserve($cancel_params); > } > } > >@@ -954,11 +957,12 @@ sub ModReserve { > $reserve_id = $hold->reserve_id; > } > >+ $hold ||= Koha::Holds->find($reserve_id); >+ > if ( $rank eq "del" ) { >- CancelReserve({ reserve_id => $reserve_id }); >+ $hold->cancel; > } > elsif ($rank =~ /^\d+/ and $rank > 0) { >- $hold ||= Koha::Holds->find($reserve_id); > logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) ) > if C4::Context->preference('HoldsLog'); > >@@ -1016,6 +1020,7 @@ sub ModReserveFill { > } > ); > >+ # FIXME Must call Koha::Hold->cancel ? > Koha::Old::Hold->new( $hold->unblessed() )->store(); > > $hold->delete(); >@@ -1128,7 +1133,9 @@ sub ModReserveCancelAll { > my ( $itemnumber, $borrowernumber ) = @_; > > #step 1 : cancel the reservation >- my $CancelReserve = CancelReserve({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); >+ my $holds = Koha::Holds->search({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); >+ return unless $holds->count; >+ $holds->next->cancel; > > #step 2 launch the subroutine of the others reserves > ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber); >@@ -1452,13 +1459,18 @@ sub _FixPriority { > > my $dbh = C4::Context->dbh; > >- unless ( $biblionumber ) { >- my $hold = Koha::Holds->find( $reserve_id ); >+ my $hold; >+ if ( $reserve_id ) { >+ $hold = Koha::Holds->find( $reserve_id ); >+ return unless $hold; >+ } >+ >+ unless ( $biblionumber ) { # FIXME This is a very weird API > $biblionumber = $hold->biblionumber; > } > >- if ( $rank eq "del" ) { >- CancelReserve({ reserve_id => $reserve_id }); >+ if ( $rank eq "del" ) { # FIXME will crash if called without $hold >+ $hold->cancel; > } > elsif ( $rank eq "W" || $rank eq "0" ) { > >@@ -1894,7 +1906,8 @@ sub MoveReserve { > RevertWaitingStatus({ itemnumber => $itemnumber }); > } > elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item >- CancelReserve( { reserve_id => $res->{'reserve_id'} } ); >+ my $hold = Koha::Holds->find( $res->{reserve_id} ); >+ $hold->cancel; > } > } > } >diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm >index 3d38bc4..d834516 100644 >--- a/C4/SIP/ILS/Transaction/Hold.pm >+++ b/C4/SIP/ILS/Transaction/Hold.pm >@@ -8,6 +8,7 @@ use Modern::Perl; > use C4::SIP::ILS::Transaction; > > use C4::Reserves; # AddReserve >+use Koha::Holds; > use Koha::Patrons; > use parent qw(C4::SIP::ILS::Transaction); > >@@ -81,11 +82,16 @@ sub drop_hold { > } > my $item = Koha::Items->find({ barcode => $self->{item}->id }); > >- CancelReserve({ >+ my $holds = Koha::Holds->search( >+ { > biblionumber => $item->biblionumber, >- itemnumber => $self->{item}->id, >- borrowernumber => $patron->borrowernumber >- }); >+ itemnumber => $self->{item}->id, >+ borrowernumber => $patron->borrowernumber >+ } >+ ); >+ return $self unless $holds->count; >+ >+ $holds->next->cancel; > > $self->ok(1); > return $self; >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index d708054..5c6d000 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -1,6 +1,7 @@ > package Koha::Hold; > > # Copyright ByWater Solutions 2014 >+# Copyright 2017 Koha Development team > # > # This file is part of Koha. > # >@@ -30,6 +31,7 @@ use Koha::Patrons; > use Koha::Biblios; > use Koha::Items; > use Koha::Libraries; >+use Koha::Old::Holds; > > use base qw(Koha::Object); > >@@ -294,6 +296,58 @@ sub is_suspended { > return $self->suspend(); > } > >+ >+=head3 cancel >+ >+my $cancel_hold = $hold->cancel(); >+ >+Cancel a hold: >+- The hold will be moved to the old_reserves table with a priority=0 >+- The priority of other holds will be updated >+- The patron will be charge (see ExpireReservesMaxPickUpDelayCharge) if the charge_cancel_fee parameter is set >+- a CANCEL HOLDS log will be done if the pref HoldsLog is on >+ >+=cut >+ >+sub cancel { >+ my ( $self, $params ) = @_; >+ $self->_result->result_source->schema->txn_do( >+ sub { >+ $self->cancellationdate(dt_from_string); >+ $self->priority(0); >+ $self->_move_to_old; >+ $self->delete; >+ >+ # now fix the priority on the others.... >+ C4::Reserves::_FixPriority({ biblionumber => $self->biblionumber }); >+ >+ # and, if desired, charge a cancel fee >+ my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); >+ if ( $charge && $params->{'charge_cancel_fee'} ) { >+ C4::Accounts::manualinvoice($self->borrowernumber, $self->itemnumber, '', 'HE', $charge); >+ } >+ >+ C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, Dumper($self->unblessed) ) >+ if C4::Context->preference('HoldsLog'); >+ } >+ ); >+ return $self; >+} >+ >+=head3 _move_to_old >+ >+my $is_moved = $hold->_move_to_old; >+ >+Move a hold to the old_reserve table following the same pattern as Koha::Patron->move_to_deleted >+ >+=cut >+ >+sub _move_to_old { >+ my ($self) = @_; >+ my $hold_infos = $self->unblessed; >+ return Koha::Old::Hold->new( $hold_infos )->store; >+} >+ > =head3 type > > =cut >@@ -302,10 +356,12 @@ sub _type { > return 'Reserve'; > } > >-=head1 AUTHOR >+=head1 AUTHORS > > Kyle M Hall <kyle@bywatersolutions.com> > >+Jonathan Druart <jonathan.druart@bugs.koha-community.org> >+ > =cut > > 1; >diff --git a/Koha/Patron/Discharge.pm b/Koha/Patron/Discharge.pm >index 06e3101..a9a6b8d 100644 >--- a/Koha/Patron/Discharge.pm >+++ b/Koha/Patron/Discharge.pm >@@ -7,7 +7,6 @@ use Carp; > > use C4::Templates qw ( gettemplate ); > use C4::Members qw( GetPendingIssues ); >-use C4::Reserves qw( CancelReserve ); > > use Koha::Database; > use Koha::DateUtils qw( dt_from_string output_pref ); >@@ -82,7 +81,7 @@ sub discharge { > my $patron = Koha::Patrons->find( $borrowernumber ); > my $holds = $patron->holds; > while ( my $hold = $holds->next ) { >- CancelReserve( { reserve_id => $hold->reserve_id } ); >+ $hold->cancel; > } > > # Debar the member >diff --git a/Koha/REST/V1/Hold.pm b/Koha/REST/V1/Hold.pm >index a31a9e6..ed4f1dd 100644 >--- a/Koha/REST/V1/Hold.pm >+++ b/Koha/REST/V1/Hold.pm >@@ -155,7 +155,7 @@ sub delete { > return $c->$cb({error => "Reserve not found"}, 404) > unless $hold; > >- C4::Reserves::CancelReserve({ reserve_id => $reserve_id }); >+ $hold->cancel; > > return $c->$cb({}, 200); > } >diff --git a/circ/branchtransfers.pl b/circ/branchtransfers.pl >index 32e77f2..bdb007f 100755 >--- a/circ/branchtransfers.pl >+++ b/circ/branchtransfers.pl >@@ -33,6 +33,7 @@ use C4::Koha; > use C4::Members; > use Koha::BiblioFrameworks; > use Koha::AuthorisedValues; >+use Koha::Holds; > use Koha::Items; > use Koha::Patrons; > >@@ -81,12 +82,15 @@ my $ignoreRs = 0; > # Deal with the requests.... > if ( $request eq "KillWaiting" ) { > my $item = $query->param('itemnumber'); >- CancelReserve({ >+ my $holds = Koha::Holds->search( > itemnumber => $item, > borrowernumber => $borrowernumber > }); >- $cancelled = 1; >- $reqmessage = 1; >+ if ( $holds->count ) { >+ $holds->next->cancel; >+ $cancelled = 1; >+ $reqmessage = 1; >+ } # FIXME else? > } > elsif ( $request eq "SetWaiting" ) { > my $item = $query->param('itemnumber'); >@@ -96,13 +100,16 @@ elsif ( $request eq "SetWaiting" ) { > $reqmessage = 1; > } > elsif ( $request eq 'KillReserved' ) { >- my $biblio = $query->param('biblionumber'); >- CancelReserve({ >- biblionumber => $biblio, >+ my $biblionumber = $query->param('biblionumber'); >+ my $holds = Koha::Holds->search( >+ biblionumber => $biblionumber, > borrowernumber => $borrowernumber > }); >- $cancelled = 1; >- $reqmessage = 1; >+ if ( $holds->count ) { >+ $holds->next->cancel; >+ $cancelled = 1; >+ $reqmessage = 1; >+ } # FIXME else? > } > > # collect the stack of books already transfered so they can printed... >diff --git a/circ/returns.pl b/circ/returns.pl >index 5fe3c70..c42e750 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -54,6 +54,7 @@ use Koha::DateUtils; > use Koha::Calendar; > use Koha::BiblioFrameworks; > use Koha::Checkouts; >+use Koha::Holds; > use Koha::Items; > use Koha::Patrons; > >@@ -159,7 +160,10 @@ if ( $query->param('reserve_id') ) { > my $biblio = $item->biblio; > > if ( $cancel_reserve ) { >- CancelReserve({ reserve_id => $reserve_id, charge_cancel_fee => !$forgivemanualholdsexpire }); >+ my $hold = Koha::Holds->find( $reserve_id ); >+ if ( $hold ) { >+ $hold->cancel( { charge_cancel_fee => !$forgivemanualholdsexpire } ); >+ } # FIXME else? > } else { > my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef; > # diffBranchSend tells ModReserveAffect whether document is expected in this library or not, >diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl >index 555d04e..bab2387 100755 >--- a/circ/waitingreserves.pl >+++ b/circ/waitingreserves.pl >@@ -68,7 +68,7 @@ my $transfer_when_cancel_all = C4::Context->preference('TransferWhenCancelAllWai > $template->param( TransferWhenCancelAllWaitingHolds => 1 ) if $transfer_when_cancel_all; > > my @cancel_result; >-# if we have a return from the form we launch the subroutine CancelReserve >+# if we have a return from the form we cancel the holds > if ($item) { > my $res = cancel( $item, $borrowernumber, $fbr, $tbr ); > push @cancel_result, $res if $res; >diff --git a/opac/opac-modrequest.pl b/opac/opac-modrequest.pl >index 8a2e041..1c7a235 100755 >--- a/opac/opac-modrequest.pl >+++ b/opac/opac-modrequest.pl >@@ -29,6 +29,7 @@ use CGI qw ( -utf8 ); > use C4::Output; > use C4::Reserves; > use C4::Auth; >+use Koha::Holds; > my $query = new CGI; > > my ( $template, $borrowernumber, $cookie ) = get_template_and_user( >@@ -44,7 +45,10 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > my $reserve_id = $query->param('reserve_id'); > > if ($reserve_id && $borrowernumber) { >- CancelReserve({ reserve_id => $reserve_id }) if CanReserveBeCanceledFromOpac($reserve_id, $borrowernumber); >+ if ( CanReserveBeCanceledFromOpac($reserve_id, $borrowernumber) ) { >+ my $hold = Koha::Holds->find( $reserve_id ); >+ $hold->cancel if $hold; >+ } > } > > print $query->redirect("/cgi-bin/koha/opac-user.pl#opac-user-holds"); >diff --git a/reserve/request.pl b/reserve/request.pl >index 2728170..3930e27 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -91,7 +91,8 @@ if ( $action eq 'move' ) { > AlterPriority( $where, $reserve_id ); > } elsif ( $action eq 'cancel' ) { > my $reserve_id = $input->param('reserve_id'); >- CancelReserve({ reserve_id => $reserve_id }); >+ my $hold = Koha::Holds->find( $reserve_id ); >+ $hold->cancel if $hold; > } elsif ( $action eq 'setLowestPriority' ) { > my $reserve_id = $input->param('reserve_id'); > ToggleLowestPriority( $reserve_id ); >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 781f496..040fe16 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -503,8 +503,8 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); > > >- $reserveid = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $reserving_borrowernumber })->next->reserve_id; >- CancelReserve({ reserve_id => $reserveid }); >+ my $hold = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $reserving_borrowernumber })->next; >+ $hold->cancel; > > # Bug 14101 > # Test automatic renewal before value for "norenewalbefore" in policy is set >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index f9083d4..241d024 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -127,9 +127,10 @@ $holds = $patron->holds; > is( $holds->next->borrowernumber, $borrowernumbers[0], "Test Koha::Patron->holds"); > > >-CancelReserve({ 'reserve_id' => $reserve_id }); >+Koha::Holds->find( $reserve_id )->cancel; >+ > $holds = $biblio->holds; >-is( $holds->count, $borrowers_count - 1, "Test CancelReserve()" ); >+is( $holds->count, $borrowers_count - 1, "Koha::Hold->cancel" ); > > $holds = $item->current_holds; > $first_hold = $holds->next; >@@ -288,29 +289,28 @@ AddReserve( > my $reserveid1 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[0] })->next->reserve_id; > > ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum); >-AddReserve( >+my $reserveid2 = AddReserve( > $branch_1, > $borrowernumbers[1], > $bibnum, > '', > 2, > ); >-my $reserveid2 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[1] })->next->reserve_id; > >-CancelReserve({ reserve_id => $reserveid1 }); >+my $hold1 = Koha::Holds->find( $reserveid1 ); >+$hold1->cancel; > > my $hold2 = Koha::Holds->find( $reserveid2 ); > is( $hold2->priority, 1, "After cancelreserve, the 2nd reserve becomes the first on the waiting list" ); > > ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum); >-AddReserve( >+my $reserveid3 = AddReserve( > $branch_1, > $borrowernumbers[0], > $bibnum, > '', > 2, > ); >-my $reserveid3 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[0] })->next->reserve_id; > > my $hold3 = Koha::Holds->find( $reserveid3 ); > is( $hold3->priority, 2, "New reserve for patron 0, the reserve has a priority = 2" ); >diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >index 33c0b1f..34bb3ed 100755 >--- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t >+++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >@@ -7,6 +7,7 @@ use C4::Context; > use Test::More tests => 10; > > use t::lib::TestBuilder; >+use Koha::Holds; > > BEGIN { > use_ok('C4::Reserves'); >@@ -79,19 +80,19 @@ $dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy > my $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 ); > my ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Hold where pickup branch matches home branch targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is($status, q{}, "Hold where pickup ne home, pickup eq home not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch > $dbh->do("DELETE FROM default_circ_rules"); >@@ -101,19 +102,19 @@ $dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy > $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, q{}, "Hold where pickup eq home, pickup ne holding not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch > $dbh->do("DELETE FROM default_circ_rules"); >@@ -123,16 +124,16 @@ $dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy > $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Hold where pickup eq home, pickup ne holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); > ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; >diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t >index 8fc8b3e..ba3e9c7 100644 >--- a/t/db_dependent/Holds/HoldItemtypeLimit.t >+++ b/t/db_dependent/Holds/HoldItemtypeLimit.t >@@ -8,6 +8,8 @@ use Test::More tests => 4; > > use t::lib::TestBuilder; > >+use Koha::Holds; >+ > BEGIN { > use FindBin; > use lib $FindBin::Bin; >@@ -94,19 +96,19 @@ $dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy > my $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype ); > my ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Hold where itemtype matches item's itemtype targed" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Itemtypes don't match > $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype ); > ( $status ) = CheckReserves($itemnumber); > is($status, q{}, "Hold where itemtype does not match item's itemtype not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # No itemtype set > $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef ); > ( $status ) = CheckReserves($itemnumber); > is( $status, 'Reserved', "Item targeted with no hold itemtype set" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Cleanup > $schema->storage->txn_rollback; >diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t >index 288432e..e2f749a 100755 >--- a/t/db_dependent/HoldsQueue.t >+++ b/t/db_dependent/HoldsQueue.t >@@ -17,6 +17,7 @@ use C4::Members; > use Koha::Database; > use Koha::DateUtils; > use Koha::Items; >+use Koha::Holds; > > use t::lib::TestBuilder; > use t::lib::Mocks; >@@ -561,21 +562,21 @@ $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Hold where pickup branch matches home branch targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 0, "Hold where pickup ne home, pickup eq home not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch > $dbh->do("DELETE FROM default_circ_rules"); >@@ -586,21 +587,21 @@ $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 0, "Hold where pickup eq home, pickup ne holding not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch > $dbh->do("DELETE FROM default_circ_rules"); >@@ -611,21 +612,21 @@ $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Hold where pickup eq home, pickup ne holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Hold where pickup ne home, pickup ne holding targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # End testing hold_fulfillment_policy > >@@ -673,21 +674,21 @@ $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, und > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 0, "Item with incorrect itemtype not targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Holding branch matches pickup branch > $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Item with matching itemtype is targeted" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # Neither branch matches pickup branch > $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef ); > C4::HoldsQueue::CreateQueue(); > $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } ); > is( @$holds_queue, 1, "Item targeted when hold itemtype is not set" ); >-CancelReserve( { reserve_id => $reserve_id } ); >+Koha::Holds->find( $reserve_id )->cancel; > > # End testing hold itemtype limit > >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 94c3c94..5b6656d 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 70; >+use Test::More tests => 67; > use Test::MockModule; > use Test::Warn; > >@@ -321,16 +321,10 @@ $holds = $biblio->holds; > is($holds->count, 1, "Only one reserves for this biblio"); > my $reserve_id = $holds->next->reserve_id; > >-$reserve = CancelReserve({reserve_id => $reserve_id}); >-isa_ok($reserve, 'HASH', "CancelReserve return"); >-is($reserve->{biblionumber}, $biblionumber); >+Koha::Holds->find( $reserve_id )->cancel; > > my $hold = Koha::Holds->find( $reserve_id ); >-is($hold, undef, "CancelReserve should have cancel the reserve"); >- >-$reserve = CancelReserve({reserve_id => $reserve_id}); >-is($reserve, undef, "CancelReserve return undef if reserve does not exist"); >- >+is($hold, undef, "Koha::Holds->cancel should have cancel the reserve"); > > # Tests for bug 9761 (ConfirmFutureHolds): new CheckReserves lookahead parameter, and corresponding change in AddReturn > # Note that CheckReserve uses its lookahead parameter and does not check ConfirmFutureHolds pref (it should be passed if needed like AddReturn does) >@@ -602,7 +596,7 @@ my $bz14464_reserve = AddReserve( > > ok( $bz14464_reserve, 'Bug 14464 - 1st reserve correctly created' ); > >-CancelReserve({ reserve_id => $bz14464_reserve, charge_cancel_fee => 1 }); >+Koha::Holds->find( $bz14464_reserve )->cancel( { charge_cancel_fee => 1 } ); > > my $old_reserve = Koha::Database->new()->schema()->resultset('OldReserve')->find( $bz14464_reserve ); > is($old_reserve->get_column('found'), 'W', 'Bug 14968 - Keep found column from reserve'); >@@ -629,7 +623,7 @@ $bz14464_reserve = AddReserve( > > ok( $bz14464_reserve, 'Bug 14464 - 2nd reserve correctly created' ); > >-CancelReserve({ reserve_id => $bz14464_reserve }); >+Koha::Holds->find( $bz14464_reserve )->cancel(); > > $bz14464_fines = $patron->account->balance; > is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge desired' ); >@@ -651,7 +645,7 @@ $bz14464_reserve = AddReserve( > > ok( $bz14464_reserve, 'Bug 14464 - 1st reserve correctly created' ); > >-CancelReserve({ reserve_id => $bz14464_reserve, charge_cancel_fee => 1 }); >+Koha::Holds->find( $bz14464_reserve )->cancel( { charge_cancel_fee => 1 } ); > > $bz14464_fines = $patron->account->balance; > is( int( $bz14464_fines ), 42, 'Bug 14464 - Fine applied after cancelling reserve with charge desired and configured' ); >diff --git a/t/db_dependent/UsageStats.t b/t/db_dependent/UsageStats.t >index 5f4dfad..f6b5c5a 100644 >--- a/t/db_dependent/UsageStats.t >+++ b/t/db_dependent/UsageStats.t >@@ -304,7 +304,7 @@ sub construct_objects_needed { > AddReserve( $branchcode, $borrowernumber1, $biblionumber1, '', 1, undef, undef, '', 'Title', undef, undef ); > my $biblio = Koha::Biblios->find( $biblionumber1 ); > my $holds = $biblio->holds; >- CancelReserve( { reserve_id => $holds->next->reserve_id } ); >+ $holds->next->cancel if $holds->count; > > # ---------- Add 1 aqbudgets > $query = ' >diff --git a/tools/batch_delete_records.pl b/tools/batch_delete_records.pl >index 4be2427..715347c 100755 >--- a/tools/batch_delete_records.pl >+++ b/tools/batch_delete_records.pl >@@ -147,7 +147,7 @@ if ( $op eq 'form' ) { > my $holds = $biblio->holds; > while ( my $hold = $holds->next ) { > eval{ >- C4::Reserves::CancelReserve( { reserve_id => $hold->reserve_id } ); >+ $hold->cancel; > }; > if ( $@ ) { > push @messages, { >-- >2.1.4
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 19059
:
65607
|
65608
|
65609
|
65610
|
65611
|
65630
|
65631
|
65632
|
65633
|
65634
|
65636
|
65637
|
65638
|
65639
|
65640
|
66783
|
66930
|
66975
|
66976
|
66977
|
66978
|
66979
|
66980
|
66981