From bd46adb41622b91523e7f9978b768577ef02eccb Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 23 Apr 2020 11:03:28 -0300 Subject: [PATCH] Bug 25260: Adapt all the things This patch adapts many pm and test files to work with unified holds table. To test: 1. Remove all previous holds in reserves and old_reserves tables. 2. Create 6 holds. Cancel 2 of them, fulfill one, set one in waiting and another in transit. CHECK => "select reserve_id, found from reserves" query in mysql should return 3 rows, where in found column should find 'W', 'T' and another with null values. => "select reserve_id, found from old_reserves" query in mysql should return 3 rows, where in found column you should find 2 with 'C', and one with 'F' values. 3. in mysql change reserve_id to the value of a hold in reserve table. 4. apply patches 5. updatedatabase SUCCESS => you should get the following warning "There were 1 old reserves that could not be moved, please check '_old_reserves'" => "select id, status, completed from holds" query in mysql should return 5 rows. => The hold that had null value in found column, now should have 'placed' status and 0 in completed. => The one that had 'W' in found column, now should have 'waiting' status and 0 in completed. => The one that had 'T' in found column, now should have 'in_transit' status and 0 in completed. => The one that had 'F' in found column, now should have 'fulfilled' status and 1 in completed. => The unmodified cancelled hold, now should have 'cancelled' status and 1 in completed. => "select * from _old_reserves" should give you one row with the modified hold. 6. in staff interface create, cancel, modify priorities, etc and everything should work exactly as before. # Run every test that has reserve and hold word wihtin # check that you are in main koha directory # check you have correctly setted KOHA_INTRANET_URL and KOHA_OPAC_URL environment variables # check plack is up and running.. tests will use selenium # prepare yourself a mug of coffe/te or take a nap.. this will take a while 7. prove $(grep -ilr -e reserve -e hold t|grep "\.t$") --- C4/Circulation.pm | 24 +- C4/Letters.pm | 10 +- C4/Members.pm | 2 +- C4/Reserves.pm | 496 +++++++++--------- C4/SIP/ILS/Patron.pm | 4 +- C4/SIP/ILS/Transaction/Hold.pm | 2 +- C4/XSLT.pm | 2 +- Koha/Acquisition/Order.pm | 2 +- Koha/Biblio.pm | 9 +- Koha/Item.pm | 16 +- Koha/Patron.pm | 12 +- Koha/REST/V1/Auth.pm | 20 +- Koha/REST/V1/Holds.pm | 28 +- Koha/Schema/Result/Hold.pm | 8 +- Koha/Schema/Result/OldReserve.pm | 10 +- Koha/Schema/Result/Reserve.pm | 10 +- api/v1/swagger/definitions/hold.json | 14 +- api/v1/swagger/paths/holds.json | 4 +- reserve/request.pl | 2 +- t/Koha/REST/Plugin/Query.t | 4 +- t/db_dependent/Circulation.t | 56 +- t/db_dependent/Circulation/issue.t | 5 +- t/db_dependent/DecreaseLoanHighHolds.t | 12 +- t/db_dependent/Hold.t | 103 ++-- t/db_dependent/Holds.t | 68 +-- .../Holds/DisallowHoldIfItemsAvailable.t | 9 +- t/db_dependent/Holds/LocalHoldsPriority.t | 10 +- t/db_dependent/Holds/RevertWaitingStatus.t | 3 +- t/db_dependent/Holds/WaitingReserves.t | 96 ++-- t/db_dependent/HoldsQueue.t | 36 +- t/db_dependent/ILSDI_Services.t | 25 +- t/db_dependent/Items/GetItemsForInventory.t | 2 +- t/db_dependent/Items/MoveItemFromBiblio.t | 24 +- t/db_dependent/Koha/Biblios.t | 18 +- t/db_dependent/Koha/Club/Hold.t | 2 +- t/db_dependent/Koha/Holds.t | 75 +-- t/db_dependent/Koha/Items.t | 12 +- t/db_dependent/Koha/Object.t | 4 +- t/db_dependent/Koha/Objects.t | 6 +- t/db_dependent/Koha/Patrons.t | 16 +- t/db_dependent/Koha/REST/Plugin/Objects.t | 2 +- t/db_dependent/Koha/Z3950Responder/Session.t | 2 +- t/db_dependent/Koha/Z3950Responder/Session2.t | 2 +- t/db_dependent/Letters/TemplateToolkit.t | 19 +- t/db_dependent/Reserves.t | 82 +-- .../Reserves/AutoUnsuspendReserves.t | 32 +- .../Reserves/CancelExpiredReserves.t | 197 +++---- t/db_dependent/Reserves/GetReserveFee.t | 14 +- t/db_dependent/Reserves/MultiplePerRecord.t | 18 +- t/db_dependent/Reserves/ReserveSlip.t | 23 +- t/db_dependent/TestBuilder.t | 11 +- t/db_dependent/XSLT.t | 9 +- t/db_dependent/api/v1/holds.t | 25 +- 53 files changed, 863 insertions(+), 834 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index faaa2692e8..4a35c36ed3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1024,7 +1024,7 @@ sub CanBookBeIssued { # See if the item is on reserve. my ( $restype, $res ) = C4::Reserves::CheckReserves( $item_object->itemnumber ); if ($restype) { - my $resbor = $res->{'borrowernumber'}; + my $resbor = $res->{'patron_id'}; if ( $resbor ne $patron->borrowernumber ) { my $patron = Koha::Patrons->find( $resbor ); if ( $restype eq "Waiting" ) @@ -1036,8 +1036,8 @@ sub CanBookBeIssued { $needsconfirmation{'ressurname'} = $patron->surname; $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $res->{branchcode}; - $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'}; + $needsconfirmation{'resbranchcode'} = $res->{pickup_library_id}; + $needsconfirmation{'reswaitingdate'} = $res->{'waiting_date'}; } elsif ( $restype eq "Reserved" ) { # The item is on reserve for someone else. @@ -1046,8 +1046,8 @@ sub CanBookBeIssued { $needsconfirmation{'ressurname'} = $patron->surname; $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{reservedate}; + $needsconfirmation{'resbranchcode'} = $patron->pickup_library_id; + $needsconfirmation{'resreservedate'} = $res->{placing_date}; } } } @@ -1190,7 +1190,7 @@ sub checkHighHolds { due_date => undef, }; - my $holds = Koha::Holds->search( { biblionumber => $item->{'biblionumber'} } ); + my $holds = Koha::Holds->search( { biblio_id => $item->{'biblionumber'} } ); if ( $holds->count() ) { $return_data->{outstanding} = $holds->count(); @@ -2018,8 +2018,8 @@ sub AddReturn { "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL" ); $sth->execute( $item->itemnumber ); - # if we have a reservation with valid transfer, we can set it's status to 'W' - C4::Reserves::ModReserveStatus($item->itemnumber, 'W'); + # if we have a reservation with valid transfer, we can set it's status to 'waiting' + C4::Reserves::ModReserveStatus($item->itemnumber, 'waiting'); } else { $messages->{'WrongTransfer'} = $tobranch; $messages->{'WrongTransferItem'} = $item->itemnumber; @@ -2082,7 +2082,7 @@ sub AddReturn { my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn ); # if a hold is found and is waiting at another branch, change the priority back to 1 and trigger the hold (this will trigger a transfer and update the hold status properly) - if ( $resfound eq "Waiting" and $branch ne $resrec->{branchcode} ) { + if ( $resfound && $resfound eq "Waiting" and $branch ne $resrec->{pickup_library_id} ) { my $hold = C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } ); $resfound = 'Reserved'; $resrec = $hold->unblessed; @@ -2844,7 +2844,7 @@ sub CanBookBeRenewed { { my $schema = Koha::Database->new()->schema(); - my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count(); + my $item_holds = Koha::Holds->search( { item_id => $itemnumber, status => 'placed', completed => 0 } )->count(); if ($item_holds) { # There is an item level hold on this item, no other item can fill the hold $resfound = 1; @@ -2854,7 +2854,7 @@ sub CanBookBeRenewed { # Get all other items that could possibly fill reserves my @itemnumbers = $schema->resultset('Item')->search( { - biblionumber => $resrec->{biblionumber}, + biblionumber => $resrec->{biblio_id}, onloan => undef, notforloan => 0, -not => { itemnumber => $itemnumber } @@ -2869,7 +2869,7 @@ sub CanBookBeRenewed { C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers ); if ($reserve_found) { - push( @borrowernumbers, $reserve->{borrowernumber} ); + push( @borrowernumbers, $reserve->{patron_id} ); } else { last; diff --git a/C4/Letters.pm b/C4/Letters.pm index 1036c461f3..418b63caee 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -745,7 +745,7 @@ sub _parseletter_sth { ($table eq 'items' ) ? "SELECT * FROM $table WHERE itemnumber = ?" : ($table eq 'issues' ) ? "SELECT * FROM $table WHERE itemnumber = ?" : ($table eq 'old_issues' ) ? "SELECT * FROM $table WHERE itemnumber = ? ORDER BY timestamp DESC LIMIT 1" : - ($table eq 'reserves' ) ? "SELECT * FROM $table WHERE borrowernumber = ? and biblionumber = ?" : + ($table eq 'holds' ) ? "SELECT * FROM $table WHERE patron_id = ? and biblio_id = ?" : ($table eq 'borrowers' ) ? "SELECT * FROM $table WHERE borrowernumber = ?" : ($table eq 'branches' ) ? "SELECT * FROM $table WHERE branchcode = ?" : ($table eq 'suggestions' ) ? "SELECT * FROM $table WHERE suggestionid = ?" : @@ -791,8 +791,8 @@ sub _parseletter { $values->{'dateexpiry'} = output_pref({ dt => dt_from_string( $values->{'dateexpiry'} ), dateonly => 1 }); } - if ( $table eq 'reserves' && $values->{'waitingdate'} ) { - $values->{'waitingdate'} = output_pref({ dt => dt_from_string( $values->{'waitingdate'} ), dateonly => 1 }); + if ( $table eq 'holds' && $values->{'waiting_date'} ) { + $values->{'waiting_date'} = output_pref({ dt => dt_from_string( $values->{'waiting_date'} ), dateonly => 1 }); } if ($letter->{content} && $letter->{content} =~ /<>/) { @@ -1515,11 +1515,11 @@ sub _get_tt_params { plural => 'orders', pk => 'ordernumber', }, - reserves => { + holds => { module => 'Koha::Holds', singular => 'hold', plural => 'holds', - fk => [ 'borrowernumber', 'biblionumber' ], + fk => [ 'patron_id', 'biblio_id' ], }, serial => { module => 'Koha::Serials', diff --git a/C4/Members.pm b/C4/Members.pm index 6c10eeeb2c..423f989a78 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -234,7 +234,7 @@ sub patronflags { $flags{'ODUES'} = \%flaginfo; } - my $waiting_holds = $patron->holds->search({ found => 'W' }); + my $waiting_holds = $patron->holds->search({ status => 'waiting' }); my $nowaiting = $waiting_holds->count; if ( $nowaiting > 0 ) { my %flaginfo; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 9c040bdc12..37f92628ae 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -38,12 +38,10 @@ use Koha::Calendar; use Koha::CirculationRules; use Koha::Database; use Koha::DateUtils; -use Koha::Hold; use Koha::Holds; use Koha::ItemTypes; use Koha::Items; use Koha::Libraries; -use Koha::Old::Hold; use Koha::Patrons; use Carp; @@ -170,7 +168,7 @@ The following tables are available witin the HOLDPLACED message: reserves =cut - +use Carp::Always; sub AddReserve { my ($params) = @_; my $branch = $params->{branchcode}; @@ -212,7 +210,7 @@ sub AddReserve { && !$item->current_holds->count ) { $priority = 0; - $found = 'W'; + $found = 'waiting'; } } @@ -225,7 +223,7 @@ sub AddReserve { my $waitingdate; # If the reserv had the waiting status, we had the value of the resdate - if ( $found && $found eq 'W' ) { + if ( $found && $found eq 'waiting' ) { $waitingdate = $resdate; } @@ -235,21 +233,21 @@ sub AddReserve { # updates take place here my $hold = Koha::Hold->new( { - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, - reservedate => $resdate, - branchcode => $branch, + patron_id => $borrowernumber, + biblio_id => $biblionumber, + placing_date => $resdate, + pickup_library_id => $branch, priority => $priority, - reservenotes => $notes, - itemnumber => $checkitem, - found => $found, - waitingdate => $waitingdate, - expirationdate => $expdate, - itemtype => $itemtype, - item_level_hold => $checkitem ? 1 : 0, + notes => $notes, + item_id => $checkitem, + status => $found, + waiting_date => $waitingdate, + expiration_date => $expdate, + item_type => $itemtype, + item_level => $checkitem ? 1 : 0, } )->store(); - $hold->set_waiting() if $found && $found eq 'W'; + $hold->set_waiting() if $found && $found eq 'waiting'; logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); @@ -373,21 +371,21 @@ sub CanItemBeReserved { # Check that the patron doesn't have an item level hold on this item already return { status =>'itemAlreadyOnHold' } - if Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count(); + if Koha::Holds->search( { patron_id => $borrowernumber, item_id => $itemnumber, completed=>0 } )->count(); my $controlbranch = C4::Context->preference('ReservesControlBranch'); my $querycount = q{ SELECT count(*) AS count - FROM reserves - LEFT JOIN items USING (itemnumber) - LEFT JOIN biblioitems ON (reserves.biblionumber=biblioitems.biblionumber) - LEFT JOIN borrowers USING (borrowernumber) - WHERE borrowernumber = ? + FROM holds + LEFT JOIN items ON (holds.item_id=items.itemnumber) + LEFT JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber) + LEFT JOIN borrowers ON (holds.patron_id=borrowers.borrowernumber) + WHERE completed = 0 and borrowernumber = ? }; my $branchcode = ""; - my $branchfield = "reserves.branchcode"; + my $branchfield = "holds.pickup_library_id"; if ( $controlbranch eq "ItemHomeLibrary" ) { $branchfield = "items.homebranch"; @@ -411,8 +409,8 @@ sub CanItemBeReserved { my $holds = Koha::Holds->search( { - borrowernumber => $borrowernumber, - biblionumber => $item->biblionumber, + patron_id => $borrowernumber, + biblio_id => $item->biblionumber, } ); if ( defined $holds_per_record && $holds_per_record ne '' @@ -421,8 +419,8 @@ sub CanItemBeReserved { } my $today_holds = Koha::Holds->search({ - borrowernumber => $borrowernumber, - reservedate => dt_from_string->date + patron_id => $borrowernumber, + placing_date => dt_from_string->date }); if ( defined $holds_per_day && $holds_per_day ne '' @@ -474,7 +472,7 @@ sub CanItemBeReserved { if ( $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) { my $total_holds_count = Koha::Holds->search( { - borrowernumber => $borrower->{borrowernumber} + patron_id => $borrower->{borrowernumber} } )->count(); @@ -554,8 +552,9 @@ sub CanReserveBeCanceledFromOpac { return unless $reserve_id and $borrowernumber; my $reserve = Koha::Holds->find($reserve_id); - return 0 unless $reserve->borrowernumber == $borrowernumber; - return 0 if ( $reserve->found eq 'W' ) or ( $reserve->found eq 'T' ); + return 0 if $reserve->completed; + return 0 unless $reserve->patron_id == $borrowernumber; + return 0 if ( $reserve->status eq 'waiting' ) or ( $reserve->status eq 'in_transit' ); return 1; @@ -576,19 +575,19 @@ sub GetOtherReserves { my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber); if ($checkreserves) { my $item = Koha::Items->find($itemnumber); - if ( $item->holdingbranch ne $checkreserves->{'branchcode'} ) { - $messages->{'transfert'} = $checkreserves->{'branchcode'}; + if ( $item->holdingbranch ne $checkreserves->{pickup_library_id} ) { + $messages->{'transfert'} = $checkreserves->{pickup_library_id}; #minus priorities of others reservs ModReserveMinusPriority( $itemnumber, - $checkreserves->{'reserve_id'}, + $checkreserves->{id}, ); #launch the subroutine dotransfer C4::Items::ModItemTransfer( $itemnumber, $item->holdingbranch, - $checkreserves->{'branchcode'}, + $checkreserves->{pickup_library_id}, 'Reserve' ), ; @@ -599,12 +598,12 @@ sub GetOtherReserves { $messages->{'waiting'} = 1; ModReserveMinusPriority( $itemnumber, - $checkreserves->{'reserve_id'}, + $checkreserves->{id}, ); - ModReserveStatus($itemnumber,'W'); + ModReserveStatus($itemnumber,'waiting'); } - $nextreservinfo = $checkreserves->{'borrowernumber'}; + $nextreservinfo = $checkreserves->{patron_id}; } return ( $messages, $nextreservinfo ); @@ -696,18 +695,18 @@ sub GetReserveStatus { my $dbh = C4::Context->dbh; - my ($sth, $found, $priority); + my ($sth, $status, $priority); if ( $itemnumber ) { - $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE itemnumber = ? order by priority LIMIT 1"); + $sth = $dbh->prepare(q{ + SELECT status, priority FROM holds WHERE item_id = ? AND completed = 0 order by priority LIMIT 1 + }); $sth->execute($itemnumber); - ($found, $priority) = $sth->fetchrow_array; - } - - if(defined $found) { - return 'Waiting' if $found eq 'W' and $priority == 0; - return 'Finished' if $found eq 'F'; + my $h = Koha::Holds->search({item_id => $itemnumber}); + ($status, $priority) = $sth->fetchrow_array; } + return 'Waiting' if defined $status and $status eq 'waiting' and $priority == 0; + return 'Finished' if defined $status and $status eq 'fulfilled'; return 'Reserved' if defined $priority && $priority > 0; return ''; # empty string here will remove need for checking undef, or less log lines @@ -748,7 +747,7 @@ sub CheckReserves { my $sth; my $select; if (C4::Context->preference('item-level_itypes')){ - $select = " + $select = " SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, @@ -763,7 +762,7 @@ sub CheckReserves { "; } else { - $select = " + $select = " SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, @@ -810,8 +809,8 @@ sub CheckReserves { my $priority = 10000000; foreach my $res (@reserves) { - if ( $res->{'itemnumber'} && $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) { - if ($res->{'found'} eq 'W') { + if ( $res->{item_id} && $res->{item_id} == $itemnumber && $res->{priority} == 0) { + if ($res->{status} eq 'waiting') { return ( "Waiting", $res, \@reserves ); # Found it, it is waiting } else { return ( "Reserved", $res, \@reserves ); # Found determinated hold, e. g. the tranferred one @@ -822,14 +821,14 @@ sub CheckReserves { my $local_hold_match; if ($LocalHoldsPriority) { - $patron = Koha::Patrons->find( $res->{borrowernumber} ); + $patron = Koha::Patrons->find( $res->{patron_id} ); $item = Koha::Items->find($itemnumber); my $local_holds_priority_item_branchcode = $item->$LocalHoldsPriorityItemControl; my $local_holds_priority_patron_branchcode = ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' ) - ? $res->{branchcode} + ? $res->{pickup_library_id} : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' ) ? $patron->branchcode : undef; @@ -839,10 +838,10 @@ sub CheckReserves { } # See if this item is more important than what we've got so far - if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) { + if ( ( $res->{priority} && $res->{priority} < $priority ) || $local_hold_match ) { $item ||= Koha::Items->find($itemnumber); - next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype; - $patron ||= Koha::Patrons->find( $res->{borrowernumber} ); + next if $res->{item_type} && $res->{item_type} ne $item->effective_itemtype; + $patron ||= Koha::Patrons->find( $res->{patron_id} ); my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed ); my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype); next if ($branchitemrule->{'holdallowed'} == 0); @@ -850,10 +849,10 @@ sub CheckReserves { my $library = Koha::Libraries->find({branchcode=>$item->homebranch}); next if (($branchitemrule->{'holdallowed'} == 3) && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) )); my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy}; - next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{branchcode}})) ); - next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) ); - next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) ); - next unless $item->can_be_transferred( { to => Koha::Libraries->find( $res->{branchcode} ) } ); + next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{pickup_library_id}})) ); + next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{pickup_library_id} ne $item->$hold_fulfillment_policy) ); + next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{pickup_library_id} ne $item->$hold_fulfillment_policy) ); + next unless $item->can_be_transferred( { to => Koha::Libraries->find( $res->{pickup_library_id} ) } ); $priority = $res->{'priority'}; $highest = $res; last if $local_hold_match; @@ -886,19 +885,19 @@ sub CancelExpiredReserves { my $expireWaiting = C4::Context->preference('ExpireReservesMaxPickUpDelay'); my $dtf = Koha::Database->new->schema->storage->datetime_parser; - my $params = { expirationdate => { '<', $dtf->format_date($today) } }; - $params->{found} = [ { '!=', 'W' }, undef ] unless $expireWaiting; + my $params = { expiration_date => { '<', $dtf->format_date($today) } }; + $params->{status} = [ { '!=', 'waiting' }, undef ] unless $expireWaiting; # FIXME To move to Koha::Holds->search_expired (?) my $holds = Koha::Holds->search( $params ); while ( my $hold = $holds->next ) { - my $calendar = Koha::Calendar->new( branchcode => $hold->branchcode ); + my $calendar = Koha::Calendar->new( branchcode => $hold->pickup_library_id ); next if !$cancel_on_holidays && $calendar->is_holiday( $today ); my $cancel_params = {}; - if ( $hold->found eq 'W' ) { + if ( $hold->status eq 'waiting' ) { $cancel_params->{charge_cancel_fee} = 1; } $hold->cancel( $cancel_params ); @@ -909,14 +908,14 @@ sub CancelExpiredReserves { AutoUnsuspendReserves(); -Unsuspends all suspended reserves with a suspend_until date from before today. +Unsuspends all suspended reserves with a suspended_until date from before today. =cut sub AutoUnsuspendReserves { my $today = dt_from_string(); - my @holds = Koha::Holds->search( { suspend_until => { '<=' => $today->ymd() } } ); + my @holds = Koha::Holds->search( { suspended_until_date => { '<=' => $today->ymd() } } ); map { $_->resume() } @holds; } @@ -975,10 +974,10 @@ sub ModReserve { my $hold; unless ( $reserve_id ) { - my $holds = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber }); + my $holds = Koha::Holds->search({ biblio_id => $biblionumber, patron_id => $borrowernumber, item_id => $itemnumber }); return unless $holds->count; # FIXME Should raise an exception $hold = $holds->next; - $reserve_id = $hold->reserve_id; + $reserve_id = $hold->id; } $hold ||= Koha::Holds->find($reserve_id); @@ -987,21 +986,21 @@ sub ModReserve { $hold->cancel; } elsif ($rank =~ /^\d+/ and $rank > 0) { - logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) ) + logaction( 'HOLDS', 'MODIFY', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); my $properties = { - priority => $rank, - branchcode => $branchcode, - itemnumber => $itemnumber, - found => undef, - waitingdate => undef + priority => $rank, + pickup_library_id => $branchcode, + item_id => $itemnumber, + status => 'placed', + waiting_date => undef }; if (exists $params->{reservedate}) { - $properties->{reservedate} = $params->{reservedate} || undef; + $properties->{placing_date} = $params->{reservedate} || undef; } if (exists $params->{expirationdate}) { - $properties->{expirationdate} = $params->{expirationdate} || undef; + $properties->{expiration_date} = $params->{expirationdate} || undef; } $hold->set($properties)->store(); @@ -1013,7 +1012,7 @@ sub ModReserve { } else { # If the hold is suspended leave the hold suspended, but convert it to an indefinite hold. # If the hold is not suspended, this does nothing. - $hold->set( { suspend_until => undef } )->store(); + $hold->set( { suspended_until_date => undef } )->store(); } } @@ -1035,37 +1034,37 @@ whose keys are fields from the reserves table in the Koha database. sub ModReserveFill { my ($res) = @_; - my $reserve_id = $res->{'reserve_id'}; + my $reserve_id = $res->{'id'}; my $hold = Koha::Holds->find($reserve_id); # get the priority on this record.... my $priority = $hold->priority; # update the hold statuses, no need to store it though, we will be deleting it anyway + # FIXME: Add Koha::Hold->set_filled and add the correct log $hold->set( { - found => 'F', - priority => 0, + status => 'fulfilled', + priority => 0, + completed => 1, + completion_date => \'NOW()' } ); - logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) ) + logaction( 'HOLDS', 'MODIFY', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); - # FIXME Must call Koha::Hold->cancel ? => No, should call ->filled and add the correct log - Koha::Old::Hold->new( $hold->unblessed() )->store(); - - $hold->delete(); + $hold->store(); if ( C4::Context->preference('HoldFeeMode') eq 'any_time_is_collected' ) { - my $reserve_fee = GetReserveFee( $hold->borrowernumber, $hold->biblionumber ); - ChargeReserveFee( $hold->borrowernumber, $reserve_fee, $hold->biblio->title ); + my $reserve_fee = GetReserveFee( $hold->patron_id, $hold->biblio_id ); + ChargeReserveFee( $hold->patron_id, $reserve_fee, $hold->biblio->title ); } # now fix the priority on the others (if the priority wasn't # already sorted!).... unless ( $priority == 0 ) { - _FixPriority( { reserve_id => $reserve_id, biblionumber => $hold->biblionumber } ); + _FixPriority( { reserve_id => $reserve_id, biblionumber => $hold->biblio_id } ); } } @@ -1087,7 +1086,7 @@ sub ModReserveStatus { my ($itemnumber, $newstatus) = @_; my $dbh = C4::Context->dbh; - my $query = "UPDATE reserves SET found = ?, waitingdate = NOW() WHERE itemnumber = ? AND found IS NULL AND priority = 0"; + my $query = "UPDATE holds SET status = ?, waiting_date = NOW() WHERE item_id = ? AND status='placed' AND priority = 0"; my $sth_set = $dbh->prepare($query); $sth_set->execute( $newstatus, $itemnumber ); @@ -1131,19 +1130,19 @@ sub ModReserveAffect { # Find hold by id if we have it $hold = Koha::Holds->find( $reserve_id ) if $reserve_id; # Find item level hold for this item if there is one - $hold ||= Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->next(); + $hold ||= Koha::Holds->search( { patron_id => $borrowernumber, item_id => $itemnumber } )->next(); # Find record level hold if there is no item level hold - $hold ||= Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->next(); + $hold ||= Koha::Holds->search( { patron_id => $borrowernumber, biblio_id => $biblionumber } )->next(); return unless $hold; - my $already_on_shelf = $hold->found && $hold->found eq 'W'; + my $already_on_shelf = $hold->status eq 'waiting'; - $hold->itemnumber($itemnumber); + $hold->item_id($itemnumber); $hold->set_waiting($transferToDo); if( !$transferToDo ){ - _koha_notify_reserve( $hold->reserve_id ) unless $already_on_shelf; + _koha_notify_reserve( $hold->id ) unless $already_on_shelf; my $transfers = Koha::Item::Transfers->search({ itemnumber => $itemnumber, datearrived => undef @@ -1161,7 +1160,7 @@ sub ModReserveAffect { CartToShelf( $itemnumber ); } - logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) ) + logaction( 'HOLDS', 'MODIFY', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); return; @@ -1178,15 +1177,15 @@ function to cancel reserv,check other reserves, and transfer document if it's ne sub ModReserveCancelAll { my $messages; my $nextreservinfo; - my ( $itemnumber, $borrowernumber ) = @_; + my ( $item_id, $patron_id ) = @_; #step 1 : cancel the reservation - my $holds = Koha::Holds->search({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); + my $holds = Koha::Holds->search({ item_id => $item_id, patron_id => $patron_id }); return unless $holds->count; $holds->next->cancel; #step 2 launch the subroutine of the others reserves - ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber); + ( $messages, $nextreservinfo ) = GetOtherReserves($item_id); return ( $messages, $nextreservinfo ); } @@ -1200,17 +1199,17 @@ Reduce the values of queued list =cut sub ModReserveMinusPriority { - my ( $itemnumber, $reserve_id ) = @_; + my ( $item_id, $reserve_id ) = @_; #first step update the value of the first person on reserv my $dbh = C4::Context->dbh; my $query = " - UPDATE reserves - SET priority = 0 , itemnumber = ? - WHERE reserve_id = ? + UPDATE holds + SET priority = 0 , item_id = ? + WHERE id = ? "; my $sth_upd = $dbh->prepare($query); - $sth_upd->execute( $itemnumber, $reserve_id ); + $sth_upd->execute( $item_id, $reserve_id ); # second step update all others reserves _FixPriority({ reserve_id => $reserve_id, rank => '0' }); } @@ -1348,8 +1347,8 @@ sub AlterPriority { my $hold = Koha::Holds->find( $reserve_id ); return unless $hold; - if ( $hold->cancellationdate ) { - warn "I cannot alter the priority for reserve_id $reserve_id, the reserve has been cancelled (" . $hold->cancellationdate . ')'; + if ( $hold->completion_date and $hold->status eq 'cancelled' ) { + warn "I cannot alter the priority for reserve_id $reserve_id, the reserve has been cancelled (" . $hold->completion_date . ')'; return; } @@ -1381,7 +1380,7 @@ sub ToggleLowestPriority { my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( "UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); + my $sth = $dbh->prepare( "UPDATE holds SET lowest_priority = NOT lowest_priority WHERE id = ?"); $sth->execute( $reserve_id ); _FixPriority({ reserve_id => $reserve_id, rank => '999999' }); @@ -1441,9 +1440,9 @@ sub SuspendAll { return unless ( $borrowernumber || $biblionumber ); my $params; - $params->{found} = undef; - $params->{borrowernumber} = $borrowernumber if $borrowernumber; - $params->{biblionumber} = $biblionumber if $biblionumber; + $params->{status} = 'placed'; + $params->{patron_id} = $borrowernumber if $borrowernumber; + $params->{biblio_id} = $biblionumber if $biblionumber; my @holds = Koha::Holds->search($params); @@ -1503,15 +1502,11 @@ sub _FixPriority { my $hold; if ( $reserve_id ) { $hold = Koha::Holds->find( $reserve_id ); - if (!defined $hold){ - # may have already been checked out and hold fulfilled - $hold = Koha::Old::Holds->find( $reserve_id ); - } return unless $hold; } unless ( $biblionumber ) { # FIXME This is a very weird API - $biblionumber = $hold->biblionumber; + $biblionumber = $hold->biblio_id; } if ( $rank eq "del" ) { # FIXME will crash if called without $hold @@ -1521,10 +1516,10 @@ sub _FixPriority { # make sure priority for waiting or in-transit items is 0 my $query = " - UPDATE reserves + UPDATE holds SET priority = 0 - WHERE reserve_id = ? - AND found IN ('W', 'T') + WHERE id = ? + AND status IN ('waiting', 'in_transit') "; my $sth = $dbh->prepare($query); $sth->execute( $reserve_id ); @@ -1533,10 +1528,11 @@ sub _FixPriority { # get whats left my $query = " - SELECT reserve_id, borrowernumber, reservedate - FROM reserves - WHERE biblionumber = ? - AND ((found <> 'W' AND found <> 'T') OR found IS NULL) + SELECT id + FROM holds + WHERE biblio_id = ? + AND completed = 0 + AND status = 'placed' ORDER BY priority ASC "; my $sth = $dbh->prepare($query); @@ -1550,7 +1546,7 @@ sub _FixPriority { my $i; my $key = -1; # to allow for 0 to be a valid result for ( $i = 0 ; $i < @priority ; $i++ ) { - if ( $reserve_id && $reserve_id == $priority[$i]->{'reserve_id'} ) { + if ( $reserve_id && $reserve_id == $priority[$i]->{'id'} ) { $key = $i; # save the index last; } @@ -1566,25 +1562,25 @@ sub _FixPriority { # now fix the priority on those that are left.... $query = " - UPDATE reserves + UPDATE holds SET priority = ? - WHERE reserve_id = ? + WHERE id = ? "; $sth = $dbh->prepare($query); for ( my $j = 0 ; $j < @priority ; $j++ ) { $sth->execute( $j + 1, - $priority[$j]->{'reserve_id'} + $priority[$j]->{'id'} ); } - $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" ); + $sth = $dbh->prepare( "SELECT id FROM holds WHERE lowest_priority = 1 ORDER BY priority" ); $sth->execute(); unless ( $ignoreSetLowestRank ) { while ( my $res = $sth->fetchrow_hashref() ) { _FixPriority({ - reserve_id => $res->{'reserve_id'}, + reserve_id => $res->{'id'}, rank => '999999', ignoreSetLowestRank => 1 }); @@ -1621,28 +1617,32 @@ sub _Findgroupreserve { # TODO: consolidate at least the SELECT portion of the first 2 queries to a common $select var. # check for exact targeted match my $item_level_target_query = qq{ - SELECT reserves.biblionumber AS biblionumber, - reserves.borrowernumber AS borrowernumber, - reserves.reservedate AS reservedate, - reserves.branchcode AS branchcode, - reserves.cancellationdate AS cancellationdate, - reserves.found AS found, - reserves.reservenotes AS reservenotes, - reserves.priority AS priority, - reserves.timestamp AS timestamp, + SELECT holds.biblio_id AS biblio_id, + holds.patron_id AS patron_id, + holds.placing_date AS placing_date, + holds.pickup_library_id AS pickup_library_id, + holds.completion_date AS completion_date, + holds.status AS status, + holds.notes AS notes, + holds.priority AS priority, + holds.timestamp AS timestamp, biblioitems.biblioitemnumber AS biblioitemnumber, - reserves.itemnumber AS itemnumber, - reserves.reserve_id AS reserve_id, - reserves.itemtype AS itemtype - FROM reserves - JOIN biblioitems USING (biblionumber) - JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber) - WHERE found IS NULL + holds.item_id AS item_id, + holds.id AS id, + holds.item_type AS item_type + FROM holds + JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber) + JOIN hold_fill_targets ON ( + holds.biblio_id=hold_fill_targets.biblionumber + AND holds.patron_id=hold_fill_targets.borrowernumber + AND holds.item_id=hold_fill_targets.itemnumber) + WHERE status='placed' AND priority > 0 - AND item_level_request = 1 - AND itemnumber = ? - AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) - AND suspend = 0 + AND completed = 0 + AND item_level = 1 + AND item_id = ? + AND placing_date <= DATE_ADD(NOW(),INTERVAL ? DAY) + AND suspended = 0 ORDER BY priority }; my $sth = $dbh->prepare($item_level_target_query); @@ -1650,34 +1650,37 @@ sub _Findgroupreserve { my @results; if ( my $data = $sth->fetchrow_hashref ) { push( @results, $data ) - unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; + unless any{ $data->{patron_id} eq $_ } @$ignore_borrowers ; } return @results if @results; # check for title-level targeted match my $title_level_target_query = qq{ - SELECT reserves.biblionumber AS biblionumber, - reserves.borrowernumber AS borrowernumber, - reserves.reservedate AS reservedate, - reserves.branchcode AS branchcode, - reserves.cancellationdate AS cancellationdate, - reserves.found AS found, - reserves.reservenotes AS reservenotes, - reserves.priority AS priority, - reserves.timestamp AS timestamp, + SELECT holds.biblio_id AS biblio_id, + holds.patron_id AS patron_id, + holds.placing_date AS placing_date, + holds.pickup_library_id AS pickup_library_id, + holds.completion_date AS completion_date, + holds.status AS status, + holds.notes AS notes, + holds.priority AS priority, + holds.timestamp AS timestamp, biblioitems.biblioitemnumber AS biblioitemnumber, - reserves.itemnumber AS itemnumber, - reserves.reserve_id AS reserve_id, - reserves.itemtype AS itemtype - FROM reserves - JOIN biblioitems USING (biblionumber) - JOIN hold_fill_targets USING (biblionumber, borrowernumber) - WHERE found IS NULL + holds.item_id AS item_id, + holds.id AS id, + holds.item_type AS item_type + FROM holds + JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber) + JOIN hold_fill_targets ON ( + holds.biblio_id=hold_fill_targets.biblionumber + AND holds.patron_id=hold_fill_targets.borrowernumber) + WHERE status='placed' AND priority > 0 - AND item_level_request = 0 + AND completed = 0 + AND item_level = 0 AND hold_fill_targets.itemnumber = ? - AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) - AND suspend = 0 + AND placing_date <= DATE_ADD(NOW(),INTERVAL ? DAY) + AND suspended = 0 ORDER BY priority }; $sth = $dbh->prepare($title_level_target_query); @@ -1685,29 +1688,30 @@ sub _Findgroupreserve { @results = (); if ( my $data = $sth->fetchrow_hashref ) { push( @results, $data ) - unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; + unless any{ $data->{patron_id} eq $_ } @$ignore_borrowers ; } return @results if @results; my $query = qq{ - SELECT reserves.biblionumber AS biblionumber, - reserves.borrowernumber AS borrowernumber, - reserves.reservedate AS reservedate, - reserves.waitingdate AS waitingdate, - reserves.branchcode AS branchcode, - reserves.cancellationdate AS cancellationdate, - reserves.found AS found, - reserves.reservenotes AS reservenotes, - reserves.priority AS priority, - reserves.timestamp AS timestamp, - reserves.itemnumber AS itemnumber, - reserves.reserve_id AS reserve_id, - reserves.itemtype AS itemtype - FROM reserves - WHERE reserves.biblionumber = ? - AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?) - AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) - AND suspend = 0 + SELECT holds.biblio_id AS biblio_id, + holds.patron_id AS patron_id, + holds.placing_date AS placing_date, + holds.waiting_date AS waiting_date, + holds.pickup_library_id AS pickup_library_id, + holds.completion_date AS completion_date, + holds.status AS status, + holds.notes AS notes, + holds.priority AS priority, + holds.timestamp AS timestamp, + holds.item_id AS item_id, + holds.id AS id, + holds.item_type AS item_type + FROM holds + WHERE holds.biblio_id = ? + AND (holds.item_id IS NULL OR holds.item_id = ?) + AND holds.placing_date <= DATE_ADD(NOW(),INTERVAL ? DAY) + AND suspended = 0 + AND completed = 0 ORDER BY priority }; $sth = $dbh->prepare($query); @@ -1715,7 +1719,7 @@ sub _Findgroupreserve { @results = (); while ( my $data = $sth->fetchrow_hashref ) { push( @results, $data ) - unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; + unless any{ $data->{patron_id} eq $_ } @$ignore_borrowers ; } return @results; } @@ -1751,7 +1755,7 @@ The following tables are availalbe witin the notice: sub _koha_notify_reserve { my $reserve_id = shift; my $hold = Koha::Holds->find($reserve_id); - my $borrowernumber = $hold->borrowernumber; + my $borrowernumber = $hold->patron_id; my $patron = Koha::Patrons->find( $borrowernumber ); @@ -1763,21 +1767,21 @@ sub _koha_notify_reserve { message_name => 'Hold_Filled' } ); - my $library = Koha::Libraries->find( $hold->branchcode )->unblessed; + my $library = Koha::Libraries->find( $hold->pickup_library_id )->unblessed; my $admin_email_address = $library->{branchemail} || C4::Context->preference('KohaAdminEmailAddress'); my %letter_params = ( module => 'reserves', - branchcode => $hold->branchcode, + branchcode => $hold->pickup_library_id, lang => $patron->lang, tables => { 'branches' => $library, 'borrowers' => $patron->unblessed, - 'biblio' => $hold->biblionumber, - 'biblioitems' => $hold->biblionumber, + 'biblio' => $hold->biblio_id, + 'biblioitems' => $hold->biblio_id, 'reserves' => $hold->unblessed, - 'items' => $hold->itemnumber, + 'items' => $hold->item_id, }, ); @@ -1840,7 +1844,7 @@ sub _ShiftPriorityByDateAndPriority { my ( $biblio, $resdate, $new_priority ) = @_; my $dbh = C4::Context->dbh; - my $query = "SELECT priority FROM reserves WHERE biblionumber = ? AND ( reservedate > ? OR priority > ? ) ORDER BY priority ASC LIMIT 1"; + my $query = "SELECT priority FROM holds WHERE biblio_id = ? AND ( placing_date > ? OR priority > ? ) AND completed = 0 ORDER BY priority ASC LIMIT 1"; my $sth = $dbh->prepare( $query ); $sth->execute( $biblio, $resdate, $new_priority ); my $min_priority = $sth->fetchrow; @@ -1848,20 +1852,20 @@ sub _ShiftPriorityByDateAndPriority { $new_priority = $min_priority if ( $min_priority ); # Shift the priority up by one; works in conjunction with the next SQL statement - $query = "UPDATE reserves + $query = "UPDATE holds SET priority = priority+1 - WHERE biblionumber = ? - AND borrowernumber = ? - AND reservedate = ? - AND found IS NULL"; + WHERE biblio_id = ? + AND patron_id = ? + AND placing_date = ? + AND status = 'placed'"; my $sth_update = $dbh->prepare( $query ); # Select all reserves for the biblio with priority greater than $new_priority, and order greatest to least - $query = "SELECT borrowernumber, reservedate FROM reserves WHERE priority >= ? AND biblionumber = ? ORDER BY priority DESC"; + $query = "SELECT patron_id, placing_date FROM holds WHERE priority >= ? AND biblio_id = ? ORDER BY priority DESC"; $sth = $dbh->prepare( $query ); $sth->execute( $new_priority, $biblio ); while ( my $row = $sth->fetchrow_hashref ) { - $sth_update->execute( $biblio, $row->{borrowernumber}, $row->{reservedate} ); + $sth_update->execute( $biblio, $row->{patron_id}, $row->{placing_date} ); } return $new_priority; # so the caller knows what priority they wind up receiving @@ -1885,9 +1889,9 @@ sub MoveReserve { my ( $restype, $res, undef ) = CheckReserves( $itemnumber, undef, $lookahead ); return unless $res; - my $biblionumber = $res->{biblionumber}; + my $biblionumber = $res->{biblio_id}; - if ($res->{borrowernumber} == $borrowernumber) { + if ($res->{patron_id} == $borrowernumber) { ModReserveFill($res); } else { @@ -1896,8 +1900,8 @@ sub MoveReserve { # Find this item in the reserves my $borr_res = Koha::Holds->search({ - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, + patron_id => $borrowernumber, + biblio_id => $biblionumber, },{ order_by => 'priority' })->next(); @@ -1911,7 +1915,7 @@ sub MoveReserve { RevertWaitingStatus({ itemnumber => $itemnumber }); } elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item - my $hold = Koha::Holds->find( $res->{reserve_id} ); + my $hold = Koha::Holds->find( $res->{id} ); $hold->cancel; } } @@ -1928,33 +1932,31 @@ This shifts the holds from C<$from_biblio> to C<$to_biblio> and reorders them by sub MergeHolds { my ( $dbh, $to_biblio, $from_biblio ) = @_; my $sth = $dbh->prepare( - "SELECT count(*) as reserve_count FROM reserves WHERE biblionumber = ?" + "SELECT count(*) as reserve_count FROM holds WHERE biblio_id = ? AND completed = 0" ); $sth->execute($from_biblio); if ( my $data = $sth->fetchrow_hashref() ) { # holds exist on old record, if not we don't need to do anything $sth = $dbh->prepare( - "UPDATE reserves SET biblionumber = ? WHERE biblionumber = ?"); + "UPDATE holds SET biblio_id = ? WHERE biblio_id = ?"); $sth->execute( $to_biblio, $from_biblio ); # Reorder by date # don't reorder those already waiting $sth = $dbh->prepare( -"SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC" + "SELECT id FROM holds WHERE biblio_id = ? AND status = 'placed' AND completed = 0 ORDER BY placing_date ASC" ); my $upd_sth = $dbh->prepare( -"UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ? - AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) " + "UPDATE holds SET priority = ? WHERE id = ? " ); - $sth->execute( $to_biblio, 'W', 'T' ); + $sth->execute( $to_biblio ); my $priority = 1; while ( my $reserve = $sth->fetchrow_hashref() ) { $upd_sth->execute( - $priority, $to_biblio, - $reserve->{'borrowernumber'}, $reserve->{'reservedate'}, - $reserve->{'itemnumber'} + $priority, + $reserve->{'id'} ); $priority++; } @@ -1985,40 +1987,40 @@ sub RevertWaitingStatus { ## Get the waiting reserve we want to revert my $query = " - SELECT * FROM reserves - WHERE itemnumber = ? - AND found IS NOT NULL + SELECT * FROM holds + WHERE item_id = ? + AND status <> 'placed' AND completed = 0 "; my $sth = $dbh->prepare( $query ); $sth->execute( $itemnumber ); my $reserve = $sth->fetchrow_hashref(); - my $hold = Koha::Holds->find( $reserve->{reserve_id} ); # TODO Remove the next raw SQL statements and use this instead + my $hold = Koha::Holds->find( $reserve->{id} ); # TODO Remove the next raw SQL statements and use this instead ## Increment the priority of all other non-waiting ## reserves for this bib record $query = " - UPDATE reserves + UPDATE holds SET priority = priority + 1 WHERE - biblionumber = ? + biblio_id = ? AND priority > 0 "; $sth = $dbh->prepare( $query ); - $sth->execute( $reserve->{'biblionumber'} ); + $sth->execute( $reserve->{'biblio_id'} ); $hold->set( { - priority => 1, - found => undef, - waitingdate => undef, - itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, + priority => 1, + status => 'placed', + waiting_date => undef, + item_id => $hold->item_level ? $hold->item_id : undef, } )->store(); - _FixPriority( { biblionumber => $reserve->{biblionumber} } ); + _FixPriority( { biblionumber => $reserve->{biblio_id} } ); return $hold; } @@ -2066,17 +2068,19 @@ sub ReserveSlip { $hold = Koha::Holds->search( { - biblionumber => $biblionumber, - borrowernumber => $borrowernumber, - itemnumber => $itemnumber + biblio_id => $biblionumber, + patron_id => $borrowernumber, + item_id => $itemnumber, + completed => 0 } )->next; } else { $hold = Koha::Holds->search( { - biblionumber => $biblionumber, - borrowernumber => $borrowernumber + biblio_id => $biblionumber, + patron_id => $borrowernumber, + completed => 0 } )->next; } @@ -2091,11 +2095,11 @@ sub ReserveSlip { lang => $patron->lang, tables => { 'reserves' => $reserve, - 'branches' => $reserve->{branchcode}, - 'borrowers' => $reserve->{borrowernumber}, - 'biblio' => $reserve->{biblionumber}, - 'biblioitems' => $reserve->{biblionumber}, - 'items' => $reserve->{itemnumber}, + 'branches' => $reserve->{pickup_library_id}, + 'borrowers' => $reserve->{patron_id}, + 'biblio' => $reserve->{biblio_id}, + 'biblioitems' => $reserve->{biblio_id}, + 'items' => $reserve->{item_id}, }, ); } @@ -2149,17 +2153,17 @@ sub CalculatePriority { my ( $biblionumber, $resdate ) = @_; my $sql = q{ - SELECT COUNT(*) FROM reserves - WHERE biblionumber = ? + SELECT COUNT(*) FROM holds + WHERE biblio_id = ? AND priority > 0 - AND (found IS NULL OR found = '') + AND status='placed' }; - #skip found==W or found==T (waiting or transit holds) + #skip status==W or status==T (waiting or transit holds) if( $resdate ) { - $sql.= ' AND ( reservedate <= ? )'; + $sql.= ' AND ( placing_date <= ? )'; } else { - $sql.= ' AND ( reservedate < NOW() )'; + $sql.= ' AND ( placing_date < NOW() )'; } my $dbh = C4::Context->dbh(); my @row = $dbh->selectrow_array( @@ -2176,19 +2180,19 @@ sub CalculatePriority { my $bool = IsItemFoundHold( $itemnumber ); Returns true if the item is currently on hold - and that hold has a non-null found status ( W, T, etc. ) + and that hold has a status of waiting or in_transit =cut sub IsItemOnHoldAndFound { my ($itemnumber) = @_; - my $rs = Koha::Database->new()->schema()->resultset('Reserve'); + my $rs = Koha::Database->new()->schema()->resultset('Hold'); my $found = $rs->count( { - itemnumber => $itemnumber, - found => { '!=' => undef } + item_id => $itemnumber, + status => { '-in' => ['waiting', 'in_transit'] } } ); diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index b528501f39..7568e02871 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -505,11 +505,11 @@ sub _get_outstanding_holds { my $borrowernumber = shift; my $patron = Koha::Patrons->find( $borrowernumber ); - my $holds = $patron->holds->search( { -or => [ { found => undef }, { found => { '!=' => 'W' } } ] } ); + my $holds = $patron->holds->search( { completed => 0, status => {'!=' => 'waiting'} } ); my @holds; while ( my $hold = $holds->next ) { my $item; - if ($hold->itemnumber) { + if ($hold->item_id) { $item = $hold->item; } else { diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm index d51ebde199..86924c0511 100644 --- a/C4/SIP/ILS/Transaction/Hold.pm +++ b/C4/SIP/ILS/Transaction/Hold.pm @@ -96,7 +96,7 @@ sub drop_hold { } my $item = Koha::Items->find({ barcode => $self->{item}->id }); - my $holds = $item->holds->search({ borrowernumber => $patron->borrowernumber }); + my $holds = $item->holds->search({ patron_id => $patron->borrowernumber }); return $self unless $holds->count; diff --git a/C4/XSLT.pm b/C4/XSLT.pm index 1b3941c7fb..79f075496b 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -303,7 +303,7 @@ sub buildKohaItemsNamespace { 'me.biblionumber' => $biblionumber, 'me.itemnumber' => { not_in => $hidden_items } }, - { prefetch => [ 'branchtransfers', 'reserves' ] } + { prefetch => [ 'branchtransfers', 'holds' ] } ); my $shelflocations = diff --git a/Koha/Acquisition/Order.pm b/Koha/Acquisition/Order.pm index 0d59f5989d..0ac535d29d 100644 --- a/Koha/Acquisition/Order.pm +++ b/Koha/Acquisition/Order.pm @@ -201,7 +201,7 @@ sub current_item_level_holds { return $biblio->current_holds->search( { - itemnumber => { + item_id => { -in => \@item_numbers } } diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 4fe30113e3..befeafbc59 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -442,7 +442,8 @@ return the current holds placed on this record sub holds { my ( $self, $params, $attributes ) = @_; $attributes->{order_by} = 'priority' unless exists $attributes->{order_by}; - my $hold_rs = $self->_result->reserves->search( $params, $attributes ); + $params->{completed} = 0 unless exists $params->{completed}; + my $hold_rs = $self->_result->holds->search( $params, $attributes ); return Koha::Holds->_new_from_dbic($hold_rs); } @@ -459,7 +460,7 @@ sub current_holds { my ($self) = @_; my $dtf = Koha::Database->new->schema->storage->datetime_parser; return $self->holds( - { reservedate => { '<=' => $dtf->format_date(dt_from_string) } } ); + { completed => 0, placing_date => { '<=' => $dtf->format_date(dt_from_string) } } ); } =head3 biblioitem @@ -520,8 +521,8 @@ Tells if this bibliographic record has items waiting or in transit. sub has_items_waiting_or_intransit { my ( $self ) = @_; - if ( Koha::Holds->search({ biblionumber => $self->id, - found => ['W', 'T'] })->count ) { + if ( Koha::Holds->search({ biblio_id => $self->id, + status => ['waiting', 'in_transit'] })->count ) { return 1; } diff --git a/Koha/Item.pm b/Koha/Item.pm index 9e1bca4ef5..7ead199395 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -249,7 +249,7 @@ sub safe_to_delete { # check it doesn't have a waiting reserve return "book_reserved" - if $self->holds->search( { found => [ 'W', 'T' ] } )->count; + if $self->holds->search( { status => [ 'waiting', 'in_transit' ], completed => 0 } )->count; return "linked_analytics" if C4::Items::GetAnalyticsCount( $self->itemnumber ) > 0; @@ -365,7 +365,8 @@ Return holds attached to an item, optionally accept a hashref of params to pass sub holds { my ( $self,$params ) = @_; - my $holds_rs = $self->_result->reserves->search($params); + $params->{completed} = 0 unless exists $params->{completed}; + my $holds_rs = $self->_result->holds->search($params); return Koha::Holds->_new_from_dbic( $holds_rs ); } @@ -616,14 +617,15 @@ sub current_holds { my $attributes = { order_by => 'priority' }; my $dtf = Koha::Database->new->schema->storage->datetime_parser; my $params = { - itemnumber => $self->itemnumber, - suspend => 0, + item_id => $self->itemnumber, + suspended => 0, -or => [ - reservedate => { '<=' => $dtf->format_date(dt_from_string) }, - waitingdate => { '!=' => undef }, + placing_date => { '<=' => $dtf->format_date(dt_from_string) }, + waiting_date => { '!=' => undef }, ], + completed => 0 }; - my $hold_rs = $self->_result->reserves->search( $params, $attributes ); + my $hold_rs = $self->_result->holds->search( $params, $attributes ); return Koha::Holds->_new_from_dbic($hold_rs); } diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 5436ccc448..7985987b33 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -71,9 +71,8 @@ our $RESULTSET_PATRON_ID_MAPPING = { Message => 'borrowernumber', MessageQueue => 'borrowernumber', OldIssue => 'borrowernumber', - OldReserve => 'borrowernumber', Rating => 'borrowernumber', - Reserve => 'borrowernumber', + Hold => 'patron_id', Review => 'borrowernumber', SearchHistory => 'userid', Statistic => 'borrowernumber', @@ -1129,8 +1128,9 @@ Return all the holds placed by this patron =cut sub holds { - my ($self) = @_; - my $holds_rs = $self->_result->reserves->search( {}, { order_by => 'reservedate' } ); + my ($self, $params) = @_; + $params->{completed} = 0 unless exists $params->{completed}; + my $holds_rs = $self->_result->holds->search( $params, { order_by => 'placing_date' } ); return Koha::Holds->_new_from_dbic($holds_rs); } @@ -1144,8 +1144,8 @@ Return all the historical holds for this patron sub old_holds { my ($self) = @_; - my $old_holds_rs = $self->_result->old_reserves->search( {}, { order_by => 'reservedate' } ); - return Koha::Old::Holds->_new_from_dbic($old_holds_rs); + my $old_holds_rs = $self->_result->holds->search( { completed => 1 }, { order_by => 'placing_date' } ); + return Koha::Holds->_new_from_dbic($old_holds_rs); } =head3 return_claims diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index 884fd99ee0..7b20d2f2da 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -300,10 +300,10 @@ sub validate_query_parameters { Allows access to object for its owner. There are endpoints that should allow access for the object owner even if they -do not have the required permission, e.g. access an own reserve. This can be +do not have the required permission, e.g. access an own hold. This can be achieved by defining the operation as follows: -"/holds/{reserve_id}": { +"/holds/{hold_id}": { "get": { ..., "x-koha-authorization": { @@ -367,7 +367,7 @@ sub check_object_ownership { borrowernumber => \&_object_ownership_by_patron_id, patron_id => \&_object_ownership_by_patron_id, checkout_id => \&_object_ownership_by_checkout_id, - reserve_id => \&_object_ownership_by_reserve_id, + hold_id => \&_object_ownership_by_hold_id, }; foreach my $param ( keys %{ $parameters } ) { @@ -431,20 +431,18 @@ sub _object_ownership_by_checkout_id { && $user->borrowernumber == $issue->borrowernumber; } -=head3 _object_ownership_by_reserve_id +=head3 _object_ownership_by_hold_id -Finds a Koha::Hold-object by C<$reserve_id> and checks if it +Finds a Koha::Hold-object by C<$hold_id> and checks if it belongs to C<$user>. -TODO: Also compare against old_reserves - =cut -sub _object_ownership_by_reserve_id { - my ($c, $user, $reserve_id) = @_; +sub _object_ownership_by_hold_id { + my ($c, $user, $hold_id) = @_; - my $reserve = Koha::Holds->find($reserve_id); - return $reserve && $user->borrowernumber == $reserve->borrowernumber; + my $hold = Koha::Holds->find($hold_id); + return $hold && $user->borrowernumber == $hold->borrowernumber; } =head3 _basic_auth diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 87d7c05336..6213f6538e 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -43,7 +43,7 @@ sub list { my $c = shift->openapi->valid_input or return; return try { - my $holds_set = Koha::Holds->new; + my $holds_set = Koha::Holds->search({completed=>0}); my $holds = $c->objects->search( $holds_set ); return $c->render( status => 200, openapi => $holds ); } @@ -73,9 +73,9 @@ sub add { my $item_type = $body->{item_type}; my $expiration_date = $body->{expiration_date}; my $notes = $body->{notes}; - my $hold_date = $body->{hold_date}; + my $placing_date = $body->{placing_date}; - if(!C4::Context->preference( 'AllowHoldDateInFuture' ) && $hold_date) { + if(!C4::Context->preference( 'AllowHoldDateInFuture' ) && $placing_date) { return $c->render( status => 400, openapi => { error => "Hold date in future not allowed" } @@ -155,7 +155,7 @@ sub add { borrowernumber => $patron_id, biblionumber => $biblio_id, priority => $priority, - reservation_date => $hold_date, + reservation_date => $placing_date, expiration_date => $expiration_date, notes => $notes, title => $biblio->title, @@ -208,7 +208,7 @@ sub edit { return try { my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find( $hold_id ); + my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( status => 404, @@ -217,17 +217,17 @@ sub edit { my $body = $c->req->json; - my $pickup_library_id = $body->{pickup_library_id} // $hold->branchcode; + my $pickup_library_id = $body->{pickup_library_id} // $hold->pickup_library_id; my $priority = $body->{priority} // $hold->priority; # suspended_until can also be set to undef - my $suspended_until = exists $body->{suspended_until} ? $body->{suspended_until} : $hold->suspend_until; + my $suspended_until = exists $body->{suspended_until_date} ? $body->{suspended_until_date} : $hold->suspended_until_date; my $params = { reserve_id => $hold_id, branchcode => $pickup_library_id, rank => $priority, suspend_until => $suspended_until ? output_pref(dt_from_string($suspended_until, 'rfc3339')) : '', - itemnumber => $hold->itemnumber + itemnumber => $hold->item_id }; C4::Reserves::ModReserve($params); @@ -253,7 +253,7 @@ sub delete { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find($hold_id); + my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( status => 404, openapi => { error => "Hold not found." } ); @@ -282,7 +282,7 @@ sub suspend { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find($hold_id); + my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; my $body = $c->req->json; my $end_date = ($body) ? $body->{end_date} : undef; @@ -296,9 +296,9 @@ sub suspend { $hold->discard_changes; $c->res->headers->location( $c->req->url->to_string ); my $suspend_end_date; - if ($hold->suspend_until) { + if ($hold->suspended_until_date) { $suspend_end_date = output_pref({ - dt => dt_from_string( $hold->suspend_until ), + dt => dt_from_string( $hold->suspended_until_date ), dateformat => 'rfc3339', dateonly => 1 } @@ -330,7 +330,7 @@ sub resume { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find($hold_id); + my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; my $body = $c->req->json; unless ($hold) { @@ -356,7 +356,7 @@ sub update_priority { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find($hold_id); + my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( diff --git a/Koha/Schema/Result/Hold.pm b/Koha/Schema/Result/Hold.pm index 65ffe5989a..caf8d0d5a8 100644 --- a/Koha/Schema/Result/Hold.pm +++ b/Koha/Schema/Result/Hold.pm @@ -97,7 +97,7 @@ __PACKAGE__->table("holds"); data_type: 'enum' default_value: 'placed' extra: {list => ["placed","fulfilled","waiting","in_transit","cancelled"]} - is_nullable: 1 + is_nullable: 0 =head2 timestamp @@ -184,7 +184,7 @@ __PACKAGE__->add_columns( extra => { list => ["placed", "fulfilled", "waiting", "in_transit", "cancelled"], }, - is_nullable => 1, + is_nullable => 0, }, "timestamp", { @@ -331,8 +331,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2020-04-22 19:21:03 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:rpFBfRe1swRgNQsdQPiTtg +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-04-23 12:55:08 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:hACjwpQwyEtJ694l+rtpLw __PACKAGE__->add_columns( '+completed' => { is_boolean => 1 }, diff --git a/Koha/Schema/Result/OldReserve.pm b/Koha/Schema/Result/OldReserve.pm index 37c2fa6f7d..a2e6f888e2 100644 --- a/Koha/Schema/Result/OldReserve.pm +++ b/Koha/Schema/Result/OldReserve.pm @@ -21,7 +21,7 @@ __PACKAGE__->table_class("DBIx::Class::ResultSource::View"); =cut __PACKAGE__->table("old_reserves"); -__PACKAGE__->result_source_instance->view_definition("select `dbic`.`holds`.`id` AS `reserve_id`,`dbic`.`holds`.`patron_id` AS `borrowernumber`,`dbic`.`holds`.`placing_date` AS `reservedate`,`dbic`.`holds`.`biblio_id` AS `biblionumber`,`dbic`.`holds`.`pickup_library_id` AS `branchcode`,(case when (`dbic`.`holds`.`status` = 'cancelled') then `dbic`.`holds`.`completion_date` else NULL end) AS `cancellationdate`,`dbic`.`holds`.`notes` AS `reservenotes`,`dbic`.`holds`.`priority` AS `priority`,(case when (`dbic`.`holds`.`status` = 'fulfilled') then 'F' else NULL end) AS `found`,`dbic`.`holds`.`timestamp` AS `timestamp`,`dbic`.`holds`.`item_id` AS `itemnumber`,`dbic`.`holds`.`waiting_date` AS `waitingdate`,`dbic`.`holds`.`expiration_date` AS `expirationdate`,`dbic`.`holds`.`lowest_priority` AS `lowestPriority`,`dbic`.`holds`.`suspended` AS `suspend`,`dbic`.`holds`.`suspended_until_date` AS `suspend_until`,`dbic`.`holds`.`item_type` AS `itemtype`,`dbic`.`holds`.`item_level` AS `item_level_hold` from `dbic`.`holds` where (`dbic`.`holds`.`completed` = 1)"); +__PACKAGE__->result_source_instance->view_definition("select `dbic`.`holds`.`id` AS `reserve_id`,`dbic`.`holds`.`patron_id` AS `borrowernumber`,`dbic`.`holds`.`placing_date` AS `reservedate`,`dbic`.`holds`.`biblio_id` AS `biblionumber`,`dbic`.`holds`.`pickup_library_id` AS `branchcode`,case when `dbic`.`holds`.`status` = 'cancelled' then `dbic`.`holds`.`completion_date` else NULL end AS `cancellationdate`,`dbic`.`holds`.`notes` AS `reservenotes`,`dbic`.`holds`.`priority` AS `priority`,case when `dbic`.`holds`.`status` = 'fulfilled' then 'F' else NULL end AS `found`,`dbic`.`holds`.`timestamp` AS `timestamp`,`dbic`.`holds`.`item_id` AS `itemnumber`,`dbic`.`holds`.`waiting_date` AS `waitingdate`,`dbic`.`holds`.`expiration_date` AS `expirationdate`,`dbic`.`holds`.`lowest_priority` AS `lowestPriority`,`dbic`.`holds`.`suspended` AS `suspend`,`dbic`.`holds`.`suspended_until_date` AS `suspend_until`,`dbic`.`holds`.`item_type` AS `itemtype`,`dbic`.`holds`.`item_level` AS `item_level_hold` from `dbic`.`holds` where `dbic`.`holds`.`completed` = 1"); =head1 ACCESSORS @@ -82,7 +82,7 @@ __PACKAGE__->result_source_instance->view_definition("select `dbic`.`holds`.`id` data_type: 'timestamp' datetime_undef_if_invalid: 1 - default_value: '0000-00-00 00:00:00' + default_value: current_timestamp is_nullable: 0 =head2 itemnumber @@ -158,7 +158,7 @@ __PACKAGE__->add_columns( { data_type => "timestamp", datetime_undef_if_invalid => 1, - default_value => "0000-00-00 00:00:00", + default_value => \"current_timestamp", is_nullable => 0, }, "itemnumber", @@ -189,8 +189,8 @@ __PACKAGE__->add_columns( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2020-04-22 19:21:04 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:j84bBLVB23oDE0csBcI8+A +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-04-23 12:55:08 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3TEDHR07dWzKIII7kRWa9g __PACKAGE__->add_columns( '+item_level_hold' => { is_boolean => 1 }, diff --git a/Koha/Schema/Result/Reserve.pm b/Koha/Schema/Result/Reserve.pm index 8925d233c0..b316d157b4 100644 --- a/Koha/Schema/Result/Reserve.pm +++ b/Koha/Schema/Result/Reserve.pm @@ -21,7 +21,7 @@ __PACKAGE__->table_class("DBIx::Class::ResultSource::View"); =cut __PACKAGE__->table("reserves"); -__PACKAGE__->result_source_instance->view_definition("select `dbic`.`holds`.`id` AS `reserve_id`,`dbic`.`holds`.`patron_id` AS `borrowernumber`,`dbic`.`holds`.`placing_date` AS `reservedate`,`dbic`.`holds`.`biblio_id` AS `biblionumber`,`dbic`.`holds`.`pickup_library_id` AS `branchcode`,NULL AS `cancellationdate`,`dbic`.`holds`.`notes` AS `reservenotes`,`dbic`.`holds`.`priority` AS `priority`,(case when (`dbic`.`holds`.`status` = 'waiting') then 'W' when (`dbic`.`holds`.`status` = 'in_transit') then 'T' else NULL end) AS `found`,`dbic`.`holds`.`timestamp` AS `timestamp`,`dbic`.`holds`.`item_id` AS `itemnumber`,`dbic`.`holds`.`waiting_date` AS `waitingdate`,`dbic`.`holds`.`expiration_date` AS `expirationdate`,`dbic`.`holds`.`lowest_priority` AS `lowestPriority`,`dbic`.`holds`.`suspended` AS `suspend`,`dbic`.`holds`.`suspended_until_date` AS `suspend_until`,`dbic`.`holds`.`item_type` AS `itemtype`,`dbic`.`holds`.`item_level` AS `item_level_hold` from `dbic`.`holds` where (`dbic`.`holds`.`completed` = 0)"); +__PACKAGE__->result_source_instance->view_definition("select `dbic`.`holds`.`id` AS `reserve_id`,`dbic`.`holds`.`patron_id` AS `borrowernumber`,`dbic`.`holds`.`placing_date` AS `reservedate`,`dbic`.`holds`.`biblio_id` AS `biblionumber`,`dbic`.`holds`.`pickup_library_id` AS `branchcode`,NULL AS `cancellationdate`,`dbic`.`holds`.`notes` AS `reservenotes`,`dbic`.`holds`.`priority` AS `priority`,case when `dbic`.`holds`.`status` = 'waiting' then 'W' when `dbic`.`holds`.`status` = 'in_transit' then 'T' else NULL end AS `found`,`dbic`.`holds`.`timestamp` AS `timestamp`,`dbic`.`holds`.`item_id` AS `itemnumber`,`dbic`.`holds`.`waiting_date` AS `waitingdate`,`dbic`.`holds`.`expiration_date` AS `expirationdate`,`dbic`.`holds`.`lowest_priority` AS `lowestPriority`,`dbic`.`holds`.`suspended` AS `suspend`,`dbic`.`holds`.`suspended_until_date` AS `suspend_until`,`dbic`.`holds`.`item_type` AS `itemtype`,`dbic`.`holds`.`item_level` AS `item_level_hold` from `dbic`.`holds` where `dbic`.`holds`.`completed` = 0"); =head1 ACCESSORS @@ -81,7 +81,7 @@ __PACKAGE__->result_source_instance->view_definition("select `dbic`.`holds`.`id` data_type: 'timestamp' datetime_undef_if_invalid: 1 - default_value: '0000-00-00 00:00:00' + default_value: current_timestamp is_nullable: 0 =head2 itemnumber @@ -157,7 +157,7 @@ __PACKAGE__->add_columns( { data_type => "timestamp", datetime_undef_if_invalid => 1, - default_value => "0000-00-00 00:00:00", + default_value => \"current_timestamp", is_nullable => 0, }, "itemnumber", @@ -188,8 +188,8 @@ __PACKAGE__->add_columns( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2020-04-22 19:21:04 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:5rN+zc7MxGkvUsCfihsq2Q +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-04-23 12:55:08 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:CQKx3fKQhJO9UQrLf92PrQ __PACKAGE__->add_columns( '+item_level_hold' => { is_boolean => 1 }, diff --git a/api/v1/swagger/definitions/hold.json b/api/v1/swagger/definitions/hold.json index dda32600b3..a4c571067c 100644 --- a/api/v1/swagger/definitions/hold.json +++ b/api/v1/swagger/definitions/hold.json @@ -9,7 +9,7 @@ "type": "integer", "description": "Internal patron identifier" }, - "hold_date": { + "placing_date": { "type": ["string", "null"], "format": "date", "description": "The date the hold was placed" @@ -37,7 +37,7 @@ }, "status": { "type": ["string", "null"], - "description": "A one letter code defining what the status of the hold is after it has been confirmed" + "description": "the status the hold is ('placed', 'fulfilled', 'waiting', 'in_transit', 'cancelled')" }, "timestamp": { "type": "string", @@ -66,7 +66,7 @@ "type": "boolean", "description": "Controls if the hold is suspended" }, - "suspended_until": { + "suspended_until_date": { "type": ["string", "null"], "format": "date-time", "description": "Date until which the hold has been suspended" @@ -78,6 +78,14 @@ "item_level": { "type": "boolean", "description": "If the hold is placed at item level" + }, + "completed": { + "type": "boolean", + "description": "If it has been completed (i.e. either 'fulfilled' or 'cancelled')" + }, + "completion_date": { + "type": ["string", "null"], + "description": "the date this hold was completed (fulfilled or cancelled)" } } } diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json index e4e925d8ef..0ffd50726f 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -280,11 +280,11 @@ "type": "integer", "minimum": 1 }, - "branchcode": { + "pickup_library_id": { "description": "Pickup location", "type": "string" }, - "suspend_until": { + "suspend_until_date": { "description": "Suspend until", "type": "string", "format": "date" diff --git a/reserve/request.pl b/reserve/request.pl index 61f853d085..333da3139d 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -635,7 +635,7 @@ foreach my $biblionumber (@biblionumbers) { # existingreserves building my @reserveloop; - my @reserves = Koha::Holds->search( { biblionumber => $biblionumber }, { order_by => 'priority' } ); + my @reserves = Koha::Holds->search( { biblio_id => $biblionumber }, { order_by => 'priority' } ); foreach my $res ( sort { my $a_found = $a->found() || ''; diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index b504f3ba5f..f2fb7888c3 100644 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -108,7 +108,7 @@ get '/dbic_merge_sorting_date' => sub { $attributes = $c->dbic_merge_sorting( { attributes => $attributes, - params => { _match => 'exact', _order_by => [ '-hold_date' ] }, + params => { _match => 'exact', _order_by => [ '-placing_date' ] }, result_set => $result_set } ); @@ -331,7 +331,7 @@ subtest 'dbic_merge_sorting() tests' => sub { ->json_is( '/a' => 'a', 'Existing values are kept (a)' ) ->json_is( '/b' => 'b', 'Existing values are kept (b)' )->json_is( '/order_by' => [ - { -desc => 'reservedate' } + { -desc => 'placing_date' } ] ); diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 7ae3b9f694..960db63dd2 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -415,31 +415,31 @@ subtest "CanBookBeRenewed tests" => sub { is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); # Now let's add an item level hold, we should no longer be able to renew the item - my $hold = Koha::Database->new()->schema()->resultset('Reserve')->create( + my $hold = Koha::Hold->new( { - borrowernumber => $hold_waiting_borrowernumber, - biblionumber => $biblio->biblionumber, - itemnumber => $item_1->itemnumber, - branchcode => $branch, - priority => 3, + patron_id => $hold_waiting_borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_1->itemnumber, + pickup_library_id => $branch, + priority => 3, } - ); + )->store; ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); is( $renewokay, 0, 'Bug 13919 - Renewal possible with item level hold on item'); $hold->delete(); # Now let's add a waiting hold on the 3rd item, it's no longer available tp check out by just anyone, so we should no longer # be able to renew these items - $hold = Koha::Database->new()->schema()->resultset('Reserve')->create( + $hold = Koha::Hold->new( { - borrowernumber => $hold_waiting_borrowernumber, - biblionumber => $biblio->biblionumber, - itemnumber => $item_3->itemnumber, - branchcode => $branch, + patron_id => $hold_waiting_borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_3->itemnumber, + pickup_library_id => $branch, priority => 0, - found => 'W' + status => 'waiting' } - ); + )->store; ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); @@ -454,15 +454,15 @@ subtest "CanBookBeRenewed tests" => sub { is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); - my $reserveid = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next->reserve_id; + my $reserveid = Koha::Holds->search({ biblio_id => $biblio->biblionumber, patron_id => $reserving_borrowernumber })->next->id; my $reserving_borrower = Koha::Patrons->find( $reserving_borrowernumber )->unblessed; AddIssue($reserving_borrower, $item_3->barcode); my $reserve = $dbh->selectrow_hashref( - 'SELECT * FROM old_reserves WHERE reserve_id = ?', + 'SELECT * FROM holds WHERE id = ? AND completed = 1', { Slice => {} }, $reserveid ); - is($reserve->{found}, 'F', 'hold marked completed when checking out item that fills it'); + is($reserve->{status}, 'fulfilled', 'hold marked completed when checking out item that fills it'); # Item-level hold, renewal test AddReserve( @@ -606,7 +606,7 @@ subtest "CanBookBeRenewed tests" => sub { is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); - $hold = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next; + $hold = Koha::Holds->search({ biblio_id => $biblio->biblionumber, patron_id => $reserving_borrowernumber })->next; $hold->cancel; # Bug 14101 @@ -2965,7 +2965,7 @@ subtest 'Set waiting flag' => sub { my ( $res, $rr ) = AddReturn( $item->{barcode}, $library_1->{branchcode} ); ModReserveAffect( $item->{itemnumber}, undef, $do_transfer, $reserve_id ); my $hold = Koha::Holds->find( $reserve_id ); - is( $hold->found, 'T', 'Hold is in transit' ); + is( $hold->status, 'in_transit', 'Hold is in transit' ); my ( $status ) = CheckReserves($item->{itemnumber}); is( $status, 'Reserved', 'Hold is not waiting yet'); @@ -2975,7 +2975,7 @@ subtest 'Set waiting flag' => sub { AddReturn( $item->{barcode}, $library_2->{branchcode} ); ModReserveAffect( $item->{itemnumber}, undef, $do_transfer, $reserve_id ); $hold = Koha::Holds->find( $reserve_id ); - is( $hold->found, 'W', 'Hold is waiting' ); + is( $hold->status, 'waiting', 'Hold is waiting' ); ( $status ) = CheckReserves($item->{itemnumber}); is( $status, 'Waiting', 'Now the hold is waiting'); @@ -2983,12 +2983,12 @@ subtest 'Set waiting flag' => sub { set_userenv( $library_1 ); (undef, my $messages, undef, undef ) = AddReturn ( $item->{barcode}, $library_1->{branchcode} ); $hold = Koha::Holds->find( $reserve_id ); - is( $hold->found, undef, 'Hold is no longer marked waiting' ); + is( $hold->status, 'placed', 'Hold is no longer marked waiting' ); is( $hold->priority, 1, "Hold is now priority one again"); - is( $hold->waitingdate, undef, "Hold no longer has a waiting date"); - is( $hold->itemnumber, $item->{itemnumber}, "Hold has retained its' itemnumber"); + is( $hold->waiting_date, undef, "Hold no longer has a waiting date"); + is( $hold->item_id, $item->{itemnumber}, "Hold has retained its' itemnumber"); is( $messages->{ResFound}->{ResFound}, "Reserved", "Hold is still returned"); - is( $messages->{ResFound}->{found}, undef, "Hold is no longer marked found in return message"); + is( $messages->{ResFound}->{status}, 'placed', "Hold is no longer marked found in return message"); is( $messages->{ResFound}->{priority}, 1, "Hold is priority 1 in return message"); }; @@ -3022,7 +3022,7 @@ subtest 'Cancel transfers on lost items' => sub { ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id ); C4::Circulation::transferbook( $library_2->{branchcode}, $item->barcode ); my $hold = Koha::Holds->find( $reserve_id ); - is( $hold->found, 'T', 'Hold is in transit' ); + is( $hold->status, 'in_transit', 'Hold is in transit' ); #Check transfer exists and the items holding branch is the transfer destination branch before marking it as lost my ($datesent,$frombranch,$tobranch) = GetTransfers($item->itemnumber); @@ -3844,13 +3844,13 @@ subtest 'Filling a hold should cancel existing transfer' => sub { biblionumber => $item->biblionumber, itemnumber => $item->itemnumber }); - my $reserves = Koha::Holds->search({ itemnumber => $item->itemnumber }); + my $reserves = Koha::Holds->search({ item_id => $item->itemnumber }); is( $reserves->count, 1, "Reserve is placed"); ( undef, $message ) = AddReturn( $item->barcode, $libraryA->branchcode, undef, undef ); my $reserve = $reserves->next; - ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 0, $reserve->reserve_id ); + ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 0, $reserve->id ); $reserve->discard_changes; - ok( $reserve->found eq 'W', "Reserve is marked waiting" ); + ok( $reserve->status eq 'waiting', "Reserve is marked waiting" ); is( Koha::Item::Transfers->search({ itemnumber => $item->itemnumber, datearrived => undef })->count, 0, "No outstanding transfers when hold is waiting"); }; diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index 36c56bb427..5093e19542 100644 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -69,8 +69,7 @@ $dbh->do(q|DELETE FROM borrowers|); $dbh->do(q|DELETE FROM categories|); $dbh->do(q|DELETE FROM accountlines|); $dbh->do(q|DELETE FROM circulation_rules|); -$dbh->do(q|DELETE FROM reserves|); -$dbh->do(q|DELETE FROM old_reserves|); +$dbh->do(q|DELETE FROM holds|); $dbh->do(q|DELETE FROM statistics|); # Generate sample datas @@ -478,7 +477,7 @@ my $reserve_id = AddReserve( ok( $reserve_id, 'The reserve should have been inserted' ); AddIssue( $borrower_2, $barcode_1, dt_from_string, 'cancel' ); my $hold = Koha::Holds->find( $reserve_id ); -is( $hold, undef, 'The reserve should have been correctly cancelled' ); +is( $hold->status, 'cancelled', 'The reserve should have been correctly cancelled' ); #End transaction $schema->storage->txn_rollback; diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index f0bf9b61d0..30506d5ef6 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -86,9 +86,9 @@ for my $i ( 0 .. 5 ) { my $patron = $patrons[$i]; my $hold = Koha::Hold->new( { - borrowernumber => $patron->id, - biblionumber => $biblio->id, - branchcode => $library->{branchcode}, + patron_id => $patron->id, + biblio_id => $biblio->id, + pickup_library_id => $library->{branchcode}, } )->store(); } @@ -146,9 +146,9 @@ for my $i ( 5 .. 10 ) { my $patron = $patrons[$i]; my $hold = Koha::Hold->new( { - borrowernumber => $patron->id, - biblionumber => $biblio->id, - branchcode => $library->{branchcode}, + patron_id => $patron->id, + biblio_id => $biblio->id, + pickup_library_id => $library->{branchcode}, } )->store(); } diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index 674f58ae3e..dce0379b17 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -66,13 +66,14 @@ $item->store(); my $hold = Koha::Hold->new( { - biblionumber => $biblionumber, - itemnumber => $item->id(), - reservedate => '2017-01-01', - waitingdate => '2000-01-01', - borrowernumber => $borrower->{borrowernumber}, - branchcode => $branches[1]->{branchcode}, - suspend => 0, + biblio_id => $biblionumber, + item_id => $item->id(), + placing_date => '2017-01-01', + waiting_date => '2000-01-01', + patron_id => $borrower->{borrowernumber}, + pickup_library_id => $branches[1]->{branchcode}, + suspended => 0, + status => 'placed' } ); $hold->store(); @@ -83,22 +84,22 @@ my $today = dt_from_string; is( $hold->age(), $today->delta_days( dt_from_string( '2017-01-01' ) )->in_units( 'days') , "Age of hold is days from reservedate to now if calendar ignored"); is( $hold->age(1), $today->delta_days( dt_from_string( '2017-01-01' ) )->in_units( 'days' ) - 1 , "Age of hold is days from reservedate to now minus 1 if calendar used"); -is( $hold->suspend, 0, "Hold is not suspended" ); +is( $hold->suspended, 0, "Hold is not suspended" ); $hold->suspend_hold(); -is( $hold->suspend, 1, "Hold is suspended" ); +is( $hold->suspended, 1, "Hold is suspended" ); $hold->resume(); -is( $hold->suspend, 0, "Hold is not suspended" ); +is( $hold->suspended, 0, "Hold is not suspended" ); my $dt = dt_from_string(); $hold->suspend_hold( $dt ); $dt->truncate( to => 'day' ); -is( $hold->suspend, 1, "Hold is suspended" ); -is( $hold->suspend_until, "$dt", "Hold is suspended with a date, truncation takes place automatically" ); +is( $hold->suspended, 1, "Hold is suspended" ); +is( $hold->suspended_until_date, "$dt", "Hold is suspended with a date, truncation takes place automatically" ); $hold->suspend_hold; -is( $hold->suspend, 1, "Hold is suspended" ); -is( $hold->suspend_until, undef, "Hold is suspended without a date" ); +is( $hold->suspended, 1, "Hold is suspended" ); +is( $hold->suspended_until_date, undef, "Hold is suspended without a date" ); $hold->resume(); -is( $hold->suspend, 0, "Hold is not suspended" ); -is( $hold->suspend_until, undef, "Hold no longer has suspend_until date" ); +is( $hold->suspended, 0, "Hold is not suspended" ); +is( $hold->suspended_until_date, undef, "Hold no longer has suspend_until date" ); $item = $hold->item(); @@ -107,30 +108,30 @@ ok( $hold_borrower, 'Got hold borrower' ); is( $hold_borrower->borrowernumber(), $borrower->{borrowernumber}, 'Hold borrower matches correct borrower' ); t::lib::Mocks::mock_preference( 'ReservesMaxPickUpDelay', '5' ); -$hold->found('T'); +$hold->status('in_transit'); isnt( $hold->is_waiting, 1, 'The hold is not waiting (T)' ); is( $hold->is_found, 1, 'The hold is found'); is( $hold->is_in_transit, 1, 'The hold is in transit' ); -$hold->found(q{}); +$hold->status(q{}); isnt( $hold->is_waiting, 1, 'The hold is not waiting (W)' ); is( $hold->is_found, 0, 'The hold is not found' ); ok( !$hold->is_in_transit, 'The hold is not in transit' ); # Test method is_cancelable_from_opac -$hold->found(undef); +$hold->status('placed'); is( $hold->is_cancelable_from_opac, 1, "Unfound hold is cancelable" ); -$hold->found('W'); +$hold->status('waiting'); is( $hold->is_cancelable_from_opac, 0, "Waiting hold is not cancelable" ); -$hold->found('T'); +$hold->status('in_transit'); is( $hold->is_cancelable_from_opac, 0, "In transit hold is not cancelable" ); # Test method is_at_destination -$hold->found(undef); +$hold->status('placed'); ok( !$hold->is_at_destination(), "Unfound hold cannot be at destination" ); -$hold->found('T'); +$hold->status('in_transit'); ok( !$hold->is_at_destination(), "In transit hold cannot be at destination" ); -$hold->found('W'); +$hold->status('waiting'); ok( !$hold->is_at_destination(), "Waiting hold where hold branchcode is not the same as the item's holdingbranch is not at destination" ); $item->holdingbranch( $branches[1]->{branchcode} ); ok( $hold->is_at_destination(), "Waiting hold where hold branchcode is the same as the item's holdingbranch is at destination" ); @@ -146,31 +147,31 @@ subtest "delete() tests" => sub { # Disable logging t::lib::Mocks::mock_preference( 'HoldsLog', 0 ); - my $hold = $builder->build({ source => 'Reserve' }); + my $hold = $builder->build_object({ class => 'Koha::Holds' }); + my $hold_id = $hold->id; + my $deleted = $hold->delete; - my $hold_object = Koha::Holds->find( $hold->{ reserve_id } ); - my $deleted = $hold_object->delete; is( ref($deleted), 'Koha::Hold', 'Koha::Hold->delete should return the Koha::Hold object if the hold has been correctly deleted' ); - is( Koha::Holds->search({ reserve_id => $hold->{ reserve_id } })->count, 0, + is( Koha::Holds->search({ id => $hold_id })->count, 0, "Koha::Hold->delete should have deleted the hold" ); my $number_of_logs = $schema->resultset('ActionLog')->search( - { module => 'HOLDS', action => 'DELETE', object => $hold->{ reserve_id } } )->count; + { module => 'HOLDS', action => 'DELETE', object => $hold_id } )->count; is( $number_of_logs, 0, 'With HoldsLogs, Koha::Hold->delete shouldn\'t have been logged' ); # Enable logging t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); - $hold = $builder->build({ source => 'Reserve' }); + $hold = $builder->build_object({ class => 'Koha::Holds' }); + $hold_id = $hold->id; + $deleted = $hold->delete; - $hold_object = Koha::Holds->find( $hold->{ reserve_id } ); - $deleted = $hold_object->delete; is( ref($deleted), 'Koha::Hold', 'Koha::Hold->delete should return a Koha::Hold object if the hold has been correctly deleted' ); - is( Koha::Holds->search({ reserve_id => $hold->{ reserve_id } })->count, 0, + is( Koha::Holds->search({ id => $hold_id })->count, 0, "Koha::Hold->delete should have deleted the hold" ); $number_of_logs = $schema->resultset('ActionLog')->search( - { module => 'HOLDS', action => 'DELETE', object => $hold->{ reserve_id } } )->count; + { module => 'HOLDS', action => 'DELETE', object => $hold_id } )->count; is( $number_of_logs, 1, 'With HoldsLogs, Koha::Hold->delete should have been logged' ); $schema->storage->txn_rollback(); @@ -186,8 +187,14 @@ subtest 'suspend() tests' => sub { t::lib::Mocks::mock_preference( 'HoldsLog', 0 ); my $hold = $builder->build_object( - { class => 'Koha::Holds', - value => { found => undef, suspend => 0, suspend_until => undef, waitingdate => undef } + { + class => 'Koha::Holds', + value => { + status => 'placed', + suspended => 0, + suspended_until_date => undef, + waiting_date => undef + } } ); @@ -196,7 +203,7 @@ subtest 'suspend() tests' => sub { is( ref($suspended) , 'Koha::Hold', 'suspend returns the Koha::Hold object' ); is( $suspended->id, $hold->id, 'suspend returns the same object' ); ok( $suspended->is_suspended, 'The hold is suspended' ); - is( $suspended->suspend_until, undef, 'It is an indefinite suspension' ); + is( $suspended->suspended_until_date, undef, 'It is an indefinite suspension' ); # resume the hold $suspended->resume; @@ -208,7 +215,7 @@ subtest 'suspend() tests' => sub { is( ref($suspended) , 'Koha::Hold', 'suspend returns the Koha::Hold object' ); is( $suspended->id, $hold->id, 'suspend returns the same object' ); ok( $suspended->is_suspended, 'The hold is suspended' ); - is( $suspended->suspend_until, $date->truncate( to => 'day' ), 'It is an indefinite suspension' ); + is( $suspended->suspended_until_date, $date->truncate( to => 'day' ), 'It is an indefinite suspension' ); # resume the hold $suspended->resume; @@ -221,7 +228,7 @@ subtest 'suspend() tests' => sub { 'Koha::Exceptions::Hold::CannotSuspendFound', 'Exception is thrown when a found hold is tried to suspend'; - is( $@->status, 'W', 'Exception gets the \'status\' parameter set correctly' ); + is( $@->status, 'waiting', 'Exception gets the \'status\' parameter set correctly' ); # set hold found=T $hold->set_waiting(1); @@ -230,13 +237,13 @@ subtest 'suspend() tests' => sub { 'Koha::Exceptions::Hold::CannotSuspendFound', 'Exception is thrown when a found hold is tried to suspend'; - is( $@->status, 'T', 'Exception gets the \'status\' parameter set correctly' ); + is( $@->status, 'in_transit', 'Exception gets the \'status\' parameter set correctly' ); my $holds_module = Test::MockModule->new('Koha::Hold'); $holds_module->mock( 'is_found', 1 ); # bad data case - $hold->found('X'); + $hold->status('cancelled'); throws_ok { $hold->suspend_hold } 'Koha::Exceptions::Hold::CannotSuspendFound', @@ -244,8 +251,8 @@ subtest 'suspend() tests' => sub { is( $@->error, 'Unhandled data exception on found hold (id=' . $hold->id - . ', found=' - . $hold->found + . ', status=' + . $hold->status . ')' , 'Exception gets the \'status\' parameter set correctly' ); $holds_module->unmock( 'is_found' ); @@ -257,8 +264,14 @@ subtest 'suspend() tests' => sub { { module => 'HOLDS', action => 'SUSPEND', object => $hold->id } )->count; $hold = $builder->build_object( - { class => 'Koha::Holds', - value => { suspend => 0, suspend_until => undef, waitingdate => undef, found => undef } + { + class => 'Koha::Holds', + value => { + suspended => 0, + suspended_until_date => undef, + waiting_date => undef, + status => 'placed' + } } ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 69b2071284..7ec146ff54 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -46,7 +46,7 @@ my $category = $builder->build({ source => 'Category' }); my $borrowers_count = 5; $dbh->do('DELETE FROM itemtypes'); -$dbh->do('DELETE FROM reserves'); +$dbh->do('DELETE FROM holds'); $dbh->do('DELETE FROM circulation_rules'); my $insert_sth = $dbh->prepare('INSERT INTO itemtypes (itemtype) VALUES (?)'); $insert_sth->execute('CAN'); @@ -96,10 +96,10 @@ is( $holds->next->priority, 5, "Reserve 5 has a priority of 5" ); my $item = Koha::Items->find( $itemnumber ); $holds = $item->current_holds; 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; +my $reservedate = $first_hold->placing_date; +my $borrowernumber = $first_hold->patron_id; +my $branch_1code = $first_hold->pickup_library_id; +my $reserve_id = $first_hold->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"); @@ -116,21 +116,21 @@ ok( $hold_item == $hold->item(), "item method returns stashed item" ); my $hold_branch = $hold->branch(); ok( $hold_branch, "Got branch using branch() method" ); ok( $hold_branch == $hold->branch(), "branch method returns stashed branch" ); -my $hold_found = $hold->found(); -$hold->set({ found => 'W'})->store(); +my $hold_found = $hold->status(); +$hold->set({ status => 'waiting' })->store(); is( Koha::Holds->waiting()->count(), 1, "Koha::Holds->waiting returns waiting holds" ); is( Koha::Holds->unfilled()->count(), 4, "Koha::Holds->unfilled returns unfilled holds" ); my $patron = Koha::Patrons->find( $borrowernumbers[0] ); $holds = $patron->holds; -is( $holds->next->borrowernumber, $borrowernumbers[0], "Test Koha::Patron->holds"); +is( $holds->next->patron_id, $borrowernumbers[0], "Test Koha::Patron->holds"); $holds = $item->current_holds; $first_hold = $holds->next; -$borrowernumber = $first_hold->borrowernumber; -$branch_1code = $first_hold->branchcode; -$reserve_id = $first_hold->reserve_id; +$borrowernumber = $first_hold->patron_id; +$branch_1code = $first_hold->pickup_library_id; +$reserve_id = $first_hold->id; ModReserve({ reserve_id => $reserve_id, @@ -142,8 +142,8 @@ ModReserve({ $hold = Koha::Holds->find( $reserve_id ); ok( $hold->priority eq '4', "Test ModReserve, priority changed correctly" ); -ok( $hold->suspend, "Test ModReserve, suspend hold" ); -is( $hold->suspend_until, '2013-01-01 00:00:00', "Test ModReserve, suspend until date" ); +ok( $hold->suspended, "Test ModReserve, suspend hold" ); +is( $hold->suspended_until_date, '2013-01-01 00:00:00', "Test ModReserve, suspend until date" ); ModReserve({ # call without reserve_id rank => '3', @@ -156,15 +156,15 @@ ok( $hold->priority eq '3', "Test ModReserve, priority changed correctly" ); ToggleSuspend( $reserve_id ); $hold = Koha::Holds->find( $reserve_id ); -ok( ! $hold->suspend, "Test ToggleSuspend(), no date" ); +ok( ! $hold->suspended, "Test ToggleSuspend(), no date" ); ToggleSuspend( $reserve_id, '2012-01-01' ); $hold = Koha::Holds->find( $reserve_id ); -is( $hold->suspend_until, '2012-01-01 00:00:00', "Test ToggleSuspend(), with date" ); +is( $hold->suspended_until_date, '2012-01-01 00:00:00', "Test ToggleSuspend(), with date" ); AutoUnsuspendReserves(); $hold = Koha::Holds->find( $reserve_id ); -ok( ! $hold->suspend, "Test AutoUnsuspendReserves()" ); +ok( ! $hold->suspended, "Test AutoUnsuspendReserves()" ); SuspendAll( borrowernumber => $borrowernumber, @@ -173,8 +173,8 @@ SuspendAll( suspend_until => '2012-01-01', ); $hold = Koha::Holds->find( $reserve_id ); -is( $hold->suspend, 1, "Test SuspendAll()" ); -is( $hold->suspend_until, '2012-01-01 00:00:00', "Test SuspendAll(), with date" ); +is( $hold->suspended, 1, "Test SuspendAll()" ); +is( $hold->suspended_until_date, '2012-01-01 00:00:00', "Test SuspendAll(), with date" ); SuspendAll( borrowernumber => $borrowernumber, @@ -182,8 +182,8 @@ SuspendAll( suspend => 0, ); $hold = Koha::Holds->find( $reserve_id ); -is( $hold->suspend, 0, "Test resuming with SuspendAll()" ); -is( $hold->suspend_until, undef, "Test resuming with SuspendAll(), should have no suspend until date" ); +is( $hold->suspended, 0, "Test resuming with SuspendAll()" ); +is( $hold->suspended_until_date, undef, "Test resuming with SuspendAll(), should have no suspend until date" ); # Add a new hold for the borrower whose hold we canceled earlier, this time at the bib level AddReserve( @@ -196,26 +196,26 @@ is( $hold->suspend_until, undef, "Test resuming with SuspendAll(), should have n $patron = Koha::Patrons->find( $borrowernumber ); $holds = $patron->holds; -my $reserveid = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $borrowernumbers[0] })->next->reserve_id; +my $reserveid = Koha::Holds->search({ biblio_id => $biblio->biblionumber, patron_id => $borrowernumbers[0] })->next->id; ModReserveMinusPriority( $itemnumber, $reserveid ); $holds = $patron->holds; -is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" ); +is( $holds->next->item_id, $itemnumber, "Test ModReserveMinusPriority()" ); $holds = $biblio->holds; $hold = $holds->next; -AlterPriority( 'top', $hold->reserve_id, undef, 2, 1, 6 ); +AlterPriority( 'top', $hold->id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '1', "Test AlterPriority(), move to top" ); -AlterPriority( 'down', $hold->reserve_id, undef, 2, 1, 6 ); +AlterPriority( 'down', $hold->id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '2', "Test AlterPriority(), move down" ); -AlterPriority( 'up', $hold->reserve_id, 1, 3, 1, 6 ); +AlterPriority( 'up', $hold->id, 1, 3, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '1', "Test AlterPriority(), move up" ); -AlterPriority( 'bottom', $hold->reserve_id, undef, 2, 1, 6 ); +AlterPriority( 'bottom', $hold->id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '6', "Test AlterPriority(), move to bottom" ); @@ -315,7 +315,7 @@ ok( } ); - my $hhh = Koha::Holds->search({ biblionumber => $biblio->biblionumber }); + my $hhh = Koha::Holds->search({ biblio_id => $biblio->biblionumber }); my $hold3 = Koha::Holds->find( $reserveid3 ); is( $hold3->priority, 3, "The 3rd hold should have a priority set to 3" ); ModReserve({ reserve_id => $reserveid1, rank => 'del' }); @@ -330,9 +330,9 @@ ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damag $hold = Koha::Hold->new( { - borrowernumber => $borrowernumbers[0], - itemnumber => $itemnumber, - biblionumber => $biblio->biblionumber, + patron_id => $borrowernumbers[0], + item_id => $itemnumber, + biblio_id => $biblio->biblionumber, } )->store(); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, @@ -431,7 +431,7 @@ is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, 'OK', t::lib::Mocks::mock_preference( 'item-level_itypes', 1 ); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); -$dbh->do('DELETE FROM reserves'); +$dbh->do('DELETE FROM holds'); $dbh->do('DELETE FROM issues'); $dbh->do('DELETE FROM items'); $dbh->do('DELETE FROM biblio'); @@ -475,7 +475,7 @@ t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); subtest 'Test max_holds per library/patron category' => sub { plan tests => 6; - $dbh->do('DELETE FROM reserves'); + $dbh->do('DELETE FROM holds'); $biblio = $builder->build_sample_biblio; $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber; @@ -503,7 +503,7 @@ subtest 'Test max_holds per library/patron category' => sub { } my $count = - Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count(); + Koha::Holds->search( { patron_id => $borrowernumbers[0] } )->count(); is( $count, 3, 'Patron now has 3 holds' ); my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); @@ -694,7 +694,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { # Update last hold so reservedate is in the past, so 2 holds, but different day $hold = Koha::Holds->find($res_id); my $yesterday = dt_from_string() - DateTime::Duration->new( days => 1 ); - $hold->reservedate($yesterday)->store; + $hold->placing_date($yesterday)->store; is_deeply( CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t index ae5e96cb60..cf3c17ae8f 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -259,11 +259,12 @@ my $itemtype2 = $builder->build({ })->{itemtype}; my $item3 = $builder->build_sample_item({ itype => $itemtype2 }); -my $hold = $builder->build({ - source => 'Reserve', +my $hold = $builder->build_object({ + class => 'Koha::Holds', value =>{ - itemnumber => $item3->itemnumber, - found => 'T' + item_id => $item3->itemnumber, + status => 'in_transit', + completed => 0 } }); diff --git a/t/db_dependent/Holds/LocalHoldsPriority.t b/t/db_dependent/Holds/LocalHoldsPriority.t index 98ac219976..12a7b4da05 100755 --- a/t/db_dependent/Holds/LocalHoldsPriority.t +++ b/t/db_dependent/Holds/LocalHoldsPriority.t @@ -85,26 +85,26 @@ my ($status, $reserve, $all_reserves); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 0 ); ($status, $reserve, $all_reserves) = CheckReserves($itemnumber); -ok( $reserve->{borrowernumber} eq $borrowernumbers[0], "Received expected results with LocalHoldsPriority disabled" ); +ok( $reserve->{patron_id} eq $borrowernumbers[0], "Received expected results with LocalHoldsPriority disabled" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); ($status, $reserve, $all_reserves) = CheckReserves($itemnumber); -ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with PickupLibrary/homebranch" ); +ok( $reserve->{patron_id} eq $borrowernumbers[2], "Received expected results with PickupLibrary/homebranch" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); ($status, $reserve, $all_reserves) = CheckReserves($itemnumber); -ok( $reserve->{borrowernumber} eq $borrowernumbers[1], "Received expected results with PickupLibrary/holdingbranch" ); +ok( $reserve->{patron_id} eq $borrowernumbers[1], "Received expected results with PickupLibrary/holdingbranch" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); ($status, $reserve, $all_reserves) = CheckReserves($itemnumber); -ok( $reserve->{borrowernumber} eq $borrowernumbers[1], "Received expected results with HomeLibrary/holdingbranch" ); +ok( $reserve->{patron_id} eq $borrowernumbers[1], "Received expected results with HomeLibrary/holdingbranch" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); ($status, $reserve, $all_reserves) = CheckReserves($itemnumber); -ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with HomeLibrary/homebranch" ); +ok( $reserve->{patron_id} eq $borrowernumbers[2], "Received expected results with HomeLibrary/homebranch" ); diff --git a/t/db_dependent/Holds/RevertWaitingStatus.t b/t/db_dependent/Holds/RevertWaitingStatus.t index 252d277a0f..b5304232a7 100755 --- a/t/db_dependent/Holds/RevertWaitingStatus.t +++ b/t/db_dependent/Holds/RevertWaitingStatus.t @@ -34,8 +34,7 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; my $dbh = C4::Context->dbh; -$dbh->do("DELETE FROM reserves"); -$dbh->do("DELETE FROM old_reserves"); +$dbh->do("DELETE FROM holds"); my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; my $itemtype = $builder->build( diff --git a/t/db_dependent/Holds/WaitingReserves.t b/t/db_dependent/Holds/WaitingReserves.t index 292061def3..6a36b2a139 100644 --- a/t/db_dependent/Holds/WaitingReserves.t +++ b/t/db_dependent/Holds/WaitingReserves.t @@ -18,7 +18,7 @@ $schema->storage->txn_begin; my $dbh = C4::Context->dbh; $dbh->do(q{DELETE FROM special_holidays}); $dbh->do(q{DELETE FROM repeatable_holidays}); -$dbh->do("DELETE FROM reserves"); +$dbh->do(q{DELETE FROM holds}); my $builder = t::lib::TestBuilder->new(); @@ -119,16 +119,17 @@ $reserve1_reservedate->subtract(days => 20); my $reserve1_expirationdate = $today->clone; $reserve1_expirationdate->add(days => 6); -my $reserve1 = $builder->build({ - source => 'Reserve', +my $r = $builder->build_object({ + class => 'Koha::Holds', value => { - borrowernumber => $patron1->{borrowernumber}, - reservedate => $reserve1_reservedate->ymd, - expirationdate => undef, - biblionumber => $biblio->{biblionumber}, - branchcode => 'LIB1', + patron_id => $patron1->{borrowernumber}, + placing_date => $reserve1_reservedate->ymd, + expiration_date => undef, + biblio_id => $biblio->{biblionumber}, + pickup_library_id => 'LIB1', priority => 1, - found => '', + status => 'placed', + completed => 0 }, }); @@ -136,44 +137,46 @@ t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelay', 1); t::lib::Mocks::mock_preference('ReservesMaxPickUpDelay', 6); ModReserveAffect( $item1->{itemnumber}, $patron1->{borrowernumber}); -my $r = Koha::Holds->find($reserve1->{reserve_id}); +$r->discard_changes; -is($r->waitingdate, $today->ymd, 'Waiting date should be set to today' ); -is($r->expirationdate, $reserve1_expirationdate->ymd, 'Expiration date should be set to today + 6' ); -is($r->found, 'W', 'Reserve status is now "waiting"' ); +is($r->waiting_date, $today->ymd, 'Waiting date should be set to today' ); +is($r->expiration_date, $reserve1_expirationdate->ymd, 'Expiration date should be set to today + 6' ); +is($r->status, 'waiting', 'Reserve status is now "waiting"' ); is($r->priority, 0, 'Priority should be 0' ); -is($r->itemnumber, $item1->{itemnumber}, 'Item number should be set correctly' ); +is($r->item_id, $item1->{itemnumber}, 'Item number should be set correctly' ); -my $reserve2 = $builder->build({ - source => 'Reserve', +my $r2 = $builder->build_object({ + class => 'Koha::Holds', value => { - borrowernumber => $patron2->{borrowernumber}, - reservedate => $reserve1_reservedate->ymd, - expirationdate => undef, - biblionumber => $biblio2->{biblionumber}, - branchcode => 'LIB1', + patron_id => $patron2->{borrowernumber}, + placing_date => $reserve1_reservedate->ymd, + expiration_date => undef, + biblio_id => $biblio2->{biblionumber}, + pickup_library_id => 'LIB1', priority => 1, - found => '', + status => 'placed', + completed => 0 }, }); ModReserveAffect( $item2->{itemnumber}, $patron2->{borrowernumber}, 1); -my $r2 = Koha::Holds->find($reserve2->{reserve_id}); +$r2->discard_changes; -is($r2->found, 'T', '2nd reserve - Reserve status is now "To transfer"' ); +is($r2->status, 'in_transit', '2nd reserve - Reserve status is now "To transfer"' ); is($r2->priority, 0, '2nd reserve - Priority should be 0' ); -is($r2->itemnumber, $item2->{itemnumber}, '2nd reserve - Item number should be set correctly' ); +is($r2->item_id, $item2->{itemnumber}, '2nd reserve - Item number should be set correctly' ); -my $reserve3 = $builder->build({ - source => 'Reserve', +my $r3 = $builder->build_object({ + class => 'Koha::Holds', value => { - borrowernumber => $patron2->{borrowernumber}, - reservedate => $reserve1_reservedate->ymd, - expirationdate => undef, - biblionumber => $biblio3->{biblionumber}, - branchcode => 'LIB1', + patron_id => $patron2->{borrowernumber}, + placing_date => $reserve1_reservedate->ymd, + expiration_date => undef, + biblio_id => $biblio3->{biblionumber}, + pickup_library_id => 'LIB1', priority => 1, - found => '', + status => 'placed', + completed => 0 }, }); @@ -216,29 +219,30 @@ ModReserveAffect( $item3->{itemnumber}, $patron2->{borrowernumber}); my $expected_expiration = $today->clone; $expected_expiration->add(days => 8); -my $r3 = Koha::Holds->find($reserve3->{reserve_id}); -is($r3->expirationdate, $expected_expiration->ymd, 'Expiration date should be set to today + 7' ); +$r3->discard_changes; +is($r3->expiration_date, $expected_expiration->ymd, 'Expiration date should be set to today + 7' ); my $reserve4_reservedate = $today->clone; my $requested_expiredate = $today->clone()->add(days => 6); -my $reserve4 = $builder->build({ - source => 'Reserve', +my $r4 = $builder->build_object({ + class => 'Koha::Holds', value => { - borrowernumber => $patron2->{borrowernumber}, - reservedate => $reserve4_reservedate->ymd, - expirationdate => $requested_expiredate->ymd, - biblionumber => $biblio4->{biblionumber}, - branchcode => 'LIB1', + patron_id => $patron2->{borrowernumber}, + placing_date => $reserve4_reservedate->ymd, + expiration_date => $requested_expiredate->ymd, + biblio_id => $biblio4->{biblionumber}, + pickup_library_id => 'LIB1', priority => 1, - found => '', + status => 'placed', + completed => 0 }, }); t::lib::Mocks::mock_preference('ReservesMaxPickUpDelay', 10); -ModReserveAffect( $item4->{itemnumber}, $patron2->{borrowernumber}, 0, $reserve4->{reserve_id}); +ModReserveAffect( $item4->{itemnumber}, $patron2->{borrowernumber}, 0, $r4->id ); -my $r4 = Koha::Holds->find($reserve4->{reserve_id}); -is($r4->expirationdate, $requested_expiredate->ymd, 'Requested expiration date should be kept' ); +$r4->discard_changes; +is($r4->expiration_date, $requested_expiredate->ymd, 'Requested expiration date should be kept' ); $schema->storage->txn_rollback; diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index ef5d89324d..2872b254e4 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -110,7 +110,7 @@ foreach ( $borrower_branchcode, $least_cost_branch_code, @other_branches ) { } # Remove existing reserves, makes debugging easier -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); my $bibitems = undef; my $priority = 1; # Make a reserve @@ -123,7 +123,7 @@ AddReserve( } ); # $resdate, $expdate, $notes, $title, $checkitem, $found -$dbh->do("UPDATE reserves SET reservedate = DATE_SUB( reservedate, INTERVAL 1 DAY )"); +$dbh->do("UPDATE holds SET placing_date = DATE_SUB( placing_date, INTERVAL 1 DAY )"); # Tests my $use_cost_matrix_sth = $dbh->prepare("UPDATE systempreferences SET value = ? WHERE variable = 'UseTransportCostMatrix'"); @@ -281,14 +281,14 @@ $items_insert_sth->execute( $barcode + 0, $branchcodes[0], $branchcodes[0] ); $items_insert_sth->execute( $barcode + 1, $branchcodes[1], $branchcodes[1] ); $items_insert_sth->execute( $barcode + 2, $branchcodes[1], $branchcodes[1] ); -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); my $sth = $dbh->prepare(q{ - INSERT INTO reserves ( - borrowernumber, - biblionumber, - branchcode, + INSERT INTO holds ( + patron_id, + biblio_id, + pickup_library_id, priority, - reservedate + placing_date ) VALUES ( ?,?,?,?, CURRENT_DATE() ) }); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); @@ -376,7 +376,7 @@ $dbh->do("DELETE FROM issues"); t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'homebranch'); C4::Context->clear_syspref_cache(); -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); $sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 ); @@ -394,7 +394,7 @@ t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary') t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); C4::Context->clear_syspref_cache(); -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); $sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[1], 2 ); @@ -436,7 +436,7 @@ t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); C4::Context->clear_syspref_cache(); -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); $sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 ); @@ -453,7 +453,7 @@ is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue givi t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); C4::Context->clear_syspref_cache(); -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); $sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[2], 3 ); @@ -470,7 +470,7 @@ is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue givi t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'homebranch'); C4::Context->clear_syspref_cache(); -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); $sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[0], 2 ); $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[2], 3 ); @@ -493,7 +493,7 @@ $borrowernumber = $borrower3->{borrowernumber}; my $library_A = $library1->{branchcode}; my $library_B = $library2->{branchcode}; my $library_C = $borrower3->{branchcode}; -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $dbh->do("DELETE FROM issues"); $dbh->do("DELETE FROM items"); $dbh->do("DELETE FROM biblio"); @@ -558,7 +558,7 @@ $itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => 0 } $borrowernumber = $borrower2->{borrowernumber}; $library_A = $library1->{branchcode}; $library_B = $library2->{branchcode}; -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $dbh->do("DELETE FROM issues"); $dbh->do("DELETE FROM items"); $dbh->do("DELETE FROM biblio"); @@ -611,7 +611,7 @@ $borrowernumber = $borrower3->{borrowernumber}; $library_A = $library1->{branchcode}; $library_B = $library2->{branchcode}; $library_C = $library3->{branchcode}; -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $dbh->do("DELETE FROM issues"); $dbh->do("DELETE FROM items"); $dbh->do("DELETE FROM biblio"); @@ -819,7 +819,7 @@ my $wrong_itemtype = $builder->build({ source => 'Itemtype', value => { notforlo my $right_itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => 0 } })->{itemtype}; $borrowernumber = $borrower3->{borrowernumber}; my $branchcode = $library1->{branchcode}; -$dbh->do("DELETE FROM reserves"); +$dbh->do("DELETE FROM holds"); $dbh->do("DELETE FROM issues"); $dbh->do("DELETE FROM items"); $dbh->do("DELETE FROM biblio"); @@ -1103,7 +1103,7 @@ subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue $dbh->do("DELETE FROM tmp_holdsqueue"); $dbh->do("DELETE FROM hold_fill_targets"); - $dbh->do("DELETE FROM reserves"); + $dbh->do("DELETE FROM holds"); $dbh->do("DELETE FROM circulation_rules"); Koha::Biblios->delete(); diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index fdce3f4a0b..98c911f717 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -340,11 +340,12 @@ subtest 'Holds test' => sub { $item_o->damaged(0)->store; my $hold = $builder->build({ - source => 'Reserve', + source => 'Hold', value => { - borrowernumber => $patron->{borrowernumber}, - biblionumber => $biblio->{biblionumber}, - itemnumber => $item->{itemnumber} + patron_id => $patron->{borrowernumber}, + biblio_id => $biblio->{biblionumber}, + item_id => $item->{itemnumber}, + completed => 0 } }); @@ -586,19 +587,19 @@ subtest 'Holds test with start_date and end_date' => sub { my $reply = C4::ILSDI::Services::HoldItem( $query ); is ($reply->{pickup_location}, $pickup_library->branchname, "Item hold with date parameters was placed"); - my $hold = Koha::Holds->search({ biblionumber => $item->biblionumber})->next(); - is( $hold->biblionumber, $item->biblionumber, "correct biblionumber"); - is( $hold->reservedate, '2020-03-20', "Item hold has correct start date" ); - is( $hold->expirationdate, '2020-04-22', "Item hold has correct end date" ); + my $hold = Koha::Holds->search({ biblio_id => $item->biblionumber})->next(); + is( $hold->biblio_id, $item->biblionumber, "correct biblionumber"); + is( $hold->placing_date, '2020-03-20', "Item hold has correct start date" ); + is( $hold->expiration_date, '2020-04-22', "Item hold has correct end date" ); $hold->delete(); $reply = C4::ILSDI::Services::HoldTitle( $query ); is ($reply->{pickup_location}, $pickup_library->branchname, "Record hold with date parameters was placed"); - $hold = Koha::Holds->search({ biblionumber => $item->biblionumber})->next(); - is( $hold->biblionumber, $item->biblionumber, "correct biblionumber"); - is( $hold->reservedate, '2020-03-20', "Record hold has correct start date" ); - is( $hold->expirationdate, '2020-04-22', "Record hold has correct end date" ); + $hold = Koha::Holds->search({ biblio_id => $item->biblionumber})->next(); + is( $hold->biblio_id, $item->biblionumber, "correct biblionumber"); + is( $hold->placing_date, '2020-03-20', "Record hold has correct start date" ); + is( $hold->expiration_date, '2020-04-22', "Record hold has correct end date" ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Items/GetItemsForInventory.t b/t/db_dependent/Items/GetItemsForInventory.t index 20993514ab..6c885d3539 100755 --- a/t/db_dependent/Items/GetItemsForInventory.t +++ b/t/db_dependent/Items/GetItemsForInventory.t @@ -107,7 +107,7 @@ subtest 'Skip items with waiting holds' => sub { biblionumber => $item_1->biblionumber, priority => 1, itemnumber => $item_1->itemnumber, - found => 'W' + found => 'waiting' } ); C4::Reserves::AddReserve( diff --git a/t/db_dependent/Items/MoveItemFromBiblio.t b/t/db_dependent/Items/MoveItemFromBiblio.t index fec7e51442..e8d7664332 100644 --- a/t/db_dependent/Items/MoveItemFromBiblio.t +++ b/t/db_dependent/Items/MoveItemFromBiblio.t @@ -57,18 +57,18 @@ my $item3 = $builder->build( ); my $bib_level_hold_not_to_move = $builder->build( - { source => 'Reserve', - value => { biblionumber => $from_biblio->{biblionumber}, }, + { source => 'Hold', + value => { biblio_id => $from_biblio->{biblionumber}, completed => 0}, } ); my $item_level_hold_not_to_move = $builder->build( - { source => 'Reserve', - value => { biblionumber => $from_biblio->{biblionumber}, itemnumber => $item1->{itemnumber} }, + { source => 'Hold', + value => { biblio_id => $from_biblio->{biblionumber}, item_id => $item1->{itemnumber}, completed => 0 }, } ); my $item_level_hold_to_move = $builder->build( - { source => 'Reserve', - value => { biblionumber => $from_biblio->{biblionumber}, itemnumber => $item2->{itemnumber} }, + { source => 'Hold', + value => { biblio_id => $from_biblio->{biblionumber}, item_id => $item2->{itemnumber}, completed => 0 }, } ); @@ -87,13 +87,13 @@ is( $get_item2->biblionumber, $to_biblio->{biblionumber}, 'The item2 should have my $get_item3 = Koha::Items->find( $item3->{itemnumber} ); is( $get_item3->biblionumber, $to_biblio->{biblionumber}, 'The item3 should not have been moved' ); -my $get_bib_level_hold = Koha::Holds->find( $bib_level_hold_not_to_move->{reserve_id} ); -my $get_item_level_hold_1 = Koha::Holds->find( $item_level_hold_not_to_move->{reserve_id} ); -my $get_item_level_hold_2 = Koha::Holds->find( $item_level_hold_to_move->{reserve_id} ); +my $get_bib_level_hold = Koha::Holds->find( $bib_level_hold_not_to_move->{id} ); +my $get_item_level_hold_1 = Koha::Holds->find( $item_level_hold_not_to_move->{id} ); +my $get_item_level_hold_2 = Koha::Holds->find( $item_level_hold_to_move->{id} ); -is( $get_bib_level_hold->biblionumber, $from_biblio->{biblionumber}, 'MoveItemFromBiblio should not have moved the biblio-level hold' ); -is( $get_item_level_hold_1->biblionumber, $from_biblio->{biblionumber}, 'MoveItemFromBiblio should not have moved the item-level hold placed on item 1' ); -is( $get_item_level_hold_2->biblionumber, $to_biblio->{biblionumber}, 'MoveItemFromBiblio should have moved the item-level hold placed on item 2' ); +is( $get_bib_level_hold->biblio_id, $from_biblio->{biblionumber}, 'MoveItemFromBiblio should not have moved the biblio-level hold' ); +is( $get_item_level_hold_1->biblio_id, $from_biblio->{biblionumber}, 'MoveItemFromBiblio should not have moved the item-level hold placed on item 1' ); +is( $get_item_level_hold_2->biblio_id, $to_biblio->{biblionumber}, 'MoveItemFromBiblio should have moved the item-level hold placed on item 2' ); $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index a6af46d288..230331cc18 100644 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -74,7 +74,7 @@ subtest 'holds + current_holds' => sub { my $holds = $biblio->holds; is( ref($holds), 'Koha::Holds', '->holds should return a Koha::Holds object' ); is( $holds->count, 1, '->holds should only return 1 hold' ); - is( $holds->next->borrowernumber, $patron->borrowernumber, '->holds should return the correct hold' ); + is( $holds->next->patron_id, $patron->borrowernumber, '->holds should return the correct hold' ); $holds->delete; # Add a hold in the future @@ -103,23 +103,23 @@ subtest 'waiting_or_in_transit' => sub { biblionumber => $biblio->{biblionumber} } }); - my $reserve = $builder->build({ - source => 'Reserve', + my $reserve = $builder->build_object({ + class => 'Koha::Holds', value => { - biblionumber => $biblio->{biblionumber}, - found => undef + biblio_id => $biblio->{biblionumber}, + status => 'placed', + completed => 0 } }); - $reserve = Koha::Holds->find($reserve->{reserve_id}); $biblio = Koha::Biblios->find($biblio->{biblionumber}); is($biblio->has_items_waiting_or_intransit, 0, 'Item is neither waiting nor in transit'); - $reserve->found('W')->store; + $reserve->status('waiting')->store; is($biblio->has_items_waiting_or_intransit, 1, 'Item is waiting'); - $reserve->found('T')->store; + $reserve->status('in_transit')->store; is($biblio->has_items_waiting_or_intransit, 1, 'Item is in transit'); my $transfer = $builder->build({ @@ -130,7 +130,7 @@ subtest 'waiting_or_in_transit' => sub { } }); my $t = Koha::Database->new()->schema()->resultset( 'Branchtransfer' )->find($transfer->{branchtransfer_id}); - $reserve->found(undef)->store; + $reserve->status('placed')->store; is($biblio->has_items_waiting_or_intransit, 1, 'Item has transfer'); }; diff --git a/t/db_dependent/Koha/Club/Hold.t b/t/db_dependent/Koha/Club/Hold.t index 6aee1e4dcf..d48ac0f2a7 100644 --- a/t/db_dependent/Koha/Club/Hold.t +++ b/t/db_dependent/Koha/Club/Hold.t @@ -80,7 +80,7 @@ subtest 'add' => sub { my $hold = Koha::Holds->find($patron_hold->hold_id); - is($patron_hold->patron_id, $hold->borrowernumber, 'Patron must be the same'); + is($patron_hold->patron_id, $hold->patron_id, 'Patron must be the same'); $schema->storage->txn_rollback; } diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t index 055dd18d11..752d5304a4 100644 --- a/t/db_dependent/Koha/Holds.t +++ b/t/db_dependent/Koha/Holds.t @@ -59,7 +59,9 @@ subtest 'DB constraints' => sub { }; subtest 'cancel' => sub { - plan tests => 12; + + plan tests => 13; + my $biblioitem = $builder->build_object( { class => 'Koha::Biblioitems' } ); my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $itemtype = $builder->build_object( { class => 'Koha::ItemTypes', value => { rentalcharge => 0 } } ); @@ -100,7 +102,7 @@ subtest 'cancel' => sub { # There are 3 holds on this records my $nb_of_holds = - Koha::Holds->search( { biblionumber => $item->biblionumber } )->count; + Koha::Holds->search( { biblio_id => $item->biblionumber } )->count; is( $nb_of_holds, 3, 'There should have 3 holds placed on this biblio record' ); my $first_hold = $holds[0]; @@ -114,14 +116,18 @@ subtest 'cancel' => sub { # Remove the second hold, only 2 should still exist in DB and priorities must have been updated my $is_cancelled = $second_hold->cancel; is( ref($is_cancelled), 'Koha::Hold', - 'Koha::Hold->cancel should return the Koha::Hold (?)' ) + 'Koha::Hold->cancel should return the Koha::Hold (?)' ); + $second_hold->discard_changes; ; # This is can reconsidered - is( $second_hold->in_storage, 0, - 'The hold has been cancelled and does not longer exist in DB' ); + is( $second_hold->in_storage, 1, + 'The hold has been cancelled and still exists on the DB' ); + is( $second_hold->status, 'cancelled', "Status is 'cancelled'" ); + ok( $second_hold->completed, 'Hold marked as comleted' ); + $nb_of_holds = - Koha::Holds->search( { biblionumber => $item->biblionumber } )->count; + Koha::Holds->search( { biblio_id => $item->biblionumber, completed => 0 } )->count; is( $nb_of_holds, 2, - 'a hold has been cancelled, there should have only 2 holds placed on this biblio record' + 'a hold has been cancelled, there should have only 2 (not completed) holds placed on this biblio record' ); # discard_changes to refetch @@ -164,7 +170,7 @@ subtest 'cancel' => sub { }; subtest 'waiting hold' => sub { - plan tests => 1; + plan tests => 2; my $patron = $builder->build_object({ class => 'Koha::Patrons' }); my $reserve_id = C4::Reserves::AddReserve( { @@ -174,12 +180,13 @@ subtest 'cancel' => sub { priority => 1, title => "title for fee", itemnumber => $item->itemnumber, - found => 'W', + status => 'W', } ); - Koha::Holds->find( $reserve_id )->cancel; - my $hold_old = Koha::Old::Holds->find( $reserve_id ); - is( $hold_old->found, 'W', 'The found column should have been kept and a hold is cancelled' ); + my $hold = Koha::Holds->find( $reserve_id ); + $hold->cancel->discard_changes; + is( $hold->status, 'cancelled', 'The found column should have been kept and a hold is cancelled' ); + ok( $hold->completed, 'Hold is marked as completed' ); }; subtest 'HoldsLog' => sub { @@ -206,50 +213,6 @@ subtest 'cancel' => sub { $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'HOLDS', action => 'CANCEL', object => $reserve_id } )->count; is( $number_of_logs, 1, 'With HoldsLog, Koha::Hold->cancel should have logged' ); }; - - subtest 'rollback' => sub { - plan tests => 3; - my $patron_category = $builder->build_object( - { - class => 'Koha::Patron::Categories', - value => { reservefee => 0 } - } - ); - my $patron = $builder->build_object( - { - class => 'Koha::Patrons', - value => { categorycode => $patron_category->categorycode } - } - ); - my $hold_info = { - branchcode => $library->branchcode, - borrowernumber => $patron->borrowernumber, - biblionumber => $item->biblionumber, - priority => 1, - title => "title for fee", - itemnumber => $item->itemnumber, - }; - - t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelayCharge',42 ); - my $reserve_id = C4::Reserves::AddReserve($hold_info); - my $hold = Koha::Holds->find($reserve_id); - - # Add a row with the same id to make the cancel fails - Koha::Old::Hold->new( $hold->unblessed )->store; - - warning_like { - eval { $hold->cancel( { charge_cancel_fee => 1 } ) }; - } - qr{.*DBD::mysql::st execute failed: Duplicate entry.*}, - 'DBD should have raised an error about dup primary key'; - - $hold = Koha::Holds->find($reserve_id); - is( ref($hold), 'Koha::Hold', 'The hold should not have been deleted' ); - is( $patron->account->balance, 0, -'If the hold has not been cancelled, the patron should not have been charged' - ); - }; - }; $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index d04559a16d..ca7c3a377b 100644 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -109,14 +109,14 @@ subtest 'holds' => sub { }); $nb_of_items++; is($item->holds->count, 0, "Nothing returned if no holds"); - my $hold1 = $builder->build({ source => 'Reserve', value => { itemnumber=>$item->itemnumber, found => 'T' }}); - my $hold2 = $builder->build({ source => 'Reserve', value => { itemnumber=>$item->itemnumber, found => 'W' }}); - my $hold3 = $builder->build({ source => 'Reserve', value => { itemnumber=>$item->itemnumber, found => 'W' }}); + my $hold1 = $builder->build({ source => 'Hold', value => { item_id=>$item->itemnumber, status => 'in_transit', completed => 0 }}); + my $hold2 = $builder->build({ source => 'Hold', value => { item_id=>$item->itemnumber, status => 'waiting', completed => 0 }}); + my $hold3 = $builder->build({ source => 'Hold', value => { item_id=>$item->itemnumber, status => 'waiting', completed => 0 }}); is($item->holds()->count,3,"Three holds found"); - is($item->holds({found => 'W'})->count,2,"Two waiting holds found"); - is_deeply($item->holds({found => 'T'})->next->unblessed,$hold1,"Found transit holds matches the hold"); - is($item->holds({found => undef})->count, 0,"Nothing returned if no matching holds"); + is($item->holds({status => 'waiting'})->count,2,"Two waiting holds found"); + is_deeply($item->holds({status => 'in_transit'})->next->unblessed,$hold1,"Found transit holds matches the hold"); + is($item->holds({status => 'placed'})->count, 0,"Nothing returned if no matching holds"); }; subtest 'biblio' => sub { diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index c23f023cd4..50c8ac5a23 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -264,7 +264,7 @@ subtest "to_api() tests" => sub { my $biblio = $builder->build_sample_biblio(); my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); - my $hold = $builder->build_object({ class => 'Koha::Holds', value => { itemnumber => $item->itemnumber } }); + my $hold = $builder->build_object({ class => 'Koha::Holds', value => { item_id => $item->itemnumber, completed => 0 } }); my $embeds = { 'items' => {} }; @@ -289,7 +289,7 @@ subtest "to_api() tests" => sub { ok(exists $biblio_api->{items}, 'Items where embedded in biblio results'); is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item still matches'); ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded'); - is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $hold->reserve_id, 'Hold matches'); + is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $hold->id, 'Hold matches'); is_deeply($biblio_api->{biblioitem}, $biblio->biblioitem->to_api, 'More than one root'); my $hold_api = $hold->to_api( diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index 0baf3618f3..8a668c4789 100644 --- a/t/db_dependent/Koha/Objects.t +++ b/t/db_dependent/Koha/Objects.t @@ -310,7 +310,7 @@ subtest "to_api() tests" => sub { my $hold_1 = $builder->build_object( { class => 'Koha::Holds', - value => { itemnumber => $item_1->itemnumber } + value => { item_id => $item_1->itemnumber, completed => 0 } } ); @@ -319,7 +319,7 @@ subtest "to_api() tests" => sub { my $hold_2 = $builder->build_object( { class => 'Koha::Holds', - value => { itemnumber => $item_2->itemnumber } + value => { item_id => $item_2->itemnumber, completed => 0 } } ); @@ -363,7 +363,7 @@ subtest "to_api() tests" => sub { ok(exists $biblio_api->{items}, 'Items where embedded in biblio results'); is($biblio_api->{items}->[0]->{item_id}, $items[$i]->itemnumber, 'Item still matches'); ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded'); - is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $holds[$i]->reserve_id, 'Hold matches'); + is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $holds[$i]->id, 'Hold matches'); $i++; } diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 4f434adc3c..25c3f8e76f 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -480,8 +480,8 @@ subtest "delete" => sub { my $patron = $builder->build( { source => 'Borrower' } ); my $retrieved_patron = Koha::Patrons->find( $patron->{borrowernumber} ); my $hold = $builder->build( - { source => 'Reserve', - value => { borrowernumber => $patron->{borrowernumber} } + { source => 'Hold', + value => { patron_id => $patron->{borrowernumber}, completed => 0 } } ); my $list = $builder->build( @@ -496,9 +496,9 @@ subtest "delete" => sub { is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron' ); - is (Koha::Old::Holds->search( { reserve_id => $hold->{ reserve_id } } )->count, 1, q|Koha::Patron->delete should have cancelled patron's holds| ); + is (Koha::Holds->search( { id => $hold->{id}, status => 'cancelled' } )->count, 1, q|Koha::Patron->delete should have cancelled patron's holds| ); - is( Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have cancelled patron's holds 2| ); + is( Koha::Holds->search( { patron_id => $patron->{borrowernumber}, completed => 0 } )->count, 0, q|Koha::Patron->delete should have cancelled patron's holds 2| ); is( Koha::Virtualshelves->search( { owner => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's lists| ); @@ -1033,8 +1033,8 @@ subtest 'holds and old_holds' => sub { is( $holds->count, 2, 'There should be 2 holds placed by this patron' ); my $old_holds = $patron->old_holds; - is( ref($old_holds), 'Koha::Old::Holds', - 'Koha::Patron->old_holds should return a Koha::Old::Holds objects' ); + is( ref($old_holds), 'Koha::Holds', + 'Koha::Patron->old_holds should return a Koha::Holds objects' ); is( $old_holds->count, 0, 'There should not be any old holds yet'); my $hold = $holds->next; @@ -1683,7 +1683,7 @@ subtest 'BorrowersLog tests' => sub { $schema->storage->txn_rollback; subtest 'Test Koha::Patrons::merge' => sub { - plan tests => 110; + plan tests => 106; my $schema = Koha::Database->new()->schema(); @@ -1966,7 +1966,7 @@ subtest 'lock' => sub { my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); my $hold = $builder->build_object({ class => 'Koha::Holds', - value => { borrowernumber => $patron1->borrowernumber }, + value => { patron_id => $patron1->borrowernumber, completed => 0 }, }); t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 ); diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index 1d726ce708..4a3e7f3dd2 100644 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -234,7 +234,7 @@ subtest 'objects.search helper, with path parameters and _match' => sub { $builder->build_object( { class => "Koha::Holds", - value => { borrowernumber => $patron->borrowernumber } + value => { patron_id => $patron->borrowernumber, completed => 0 } } ); diff --git a/t/db_dependent/Koha/Z3950Responder/Session.t b/t/db_dependent/Koha/Z3950Responder/Session.t index 33bfd4f527..90f5df01ac 100644 --- a/t/db_dependent/Koha/Z3950Responder/Session.t +++ b/t/db_dependent/Koha/Z3950Responder/Session.t @@ -32,7 +32,7 @@ subtest 'add_item_status' => sub { }); my $item_marc_1 = C4::Items::GetMarcItem( $item_1->{biblionumber}, $item_1->{itemnumber} ); my $item_field_1 = scalar $item_marc_1->field('952'); - $builder->build({ source => 'Reserve', value=> { itemnumber => $item_1->{itemnumber} } }); + $builder->build({ source => 'Hold', value=> { item_id => $item_1->{itemnumber}, completed => 0 } }); $builder->build({ source => 'Branchtransfer', value=> { itemnumber => $item_1->{itemnumber}, datearrived => undef } }); ## END FIRST ITEM ## diff --git a/t/db_dependent/Koha/Z3950Responder/Session2.t b/t/db_dependent/Koha/Z3950Responder/Session2.t index ece9bded6f..4075c47905 100644 --- a/t/db_dependent/Koha/Z3950Responder/Session2.t +++ b/t/db_dependent/Koha/Z3950Responder/Session2.t @@ -56,7 +56,7 @@ subtest 'add_item_status' => sub { }); my $item_marc_1 = C4::Items::GetMarcItem( $item_1->{biblionumber}, $item_1->{itemnumber} ); my $item_field_1 = scalar $item_marc_1->field('952'); - $builder->build({ source => 'Reserve', value=> { itemnumber => $item_1->{itemnumber} } }); + $builder->build({ source => 'Hold', value=> { item_id => $item_1->{itemnumber}, completed => 0 } }); $builder->build({ source => 'Branchtransfer', value=> { itemnumber => $item_1->{itemnumber}, datearrived => undef } }); ## END FIRST ITEM ## diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t index 6f55a8a93e..ff2db5c9c9 100644 --- a/t/db_dependent/Letters/TemplateToolkit.t +++ b/t/db_dependent/Letters/TemplateToolkit.t @@ -70,8 +70,8 @@ my $hold = $builder->build_object( { class => 'Koha::Holds', value => { - borrowernumber => $patron->{borrowernumber}, - biblionumber => $item->biblionumber + patron_id => $patron->{borrowernumber}, + biblio_id => $item->biblionumber } } ); @@ -190,7 +190,10 @@ $prepared_letter = GetPreparedLetter( module => 'test', letter_code => 'TEST_HOLD', tables => { - reserves => { borrowernumber => $patron->{borrowernumber}, biblionumber => $item->biblionumber }, + holds => { + patron_id => $patron->{borrowernumber}, + biblio_id => $item->biblionumber + }, }, ) ); @@ -202,13 +205,13 @@ eval { module => 'test', letter_code => 'TEST_HOLD', tables => { - reserves => [ $patron->{borrowernumber}, $item->biblionumber ], + holds => [ $patron->{borrowernumber}, $item->biblionumber ], }, ) ) }; my $croak = $@; -like( $croak, qr{^Multiple foreign keys \(table reserves\) should be passed using an hashref.*}, "GetPreparedLetter should not be called with arrayref for multiple FK" ); +like( $croak, qr{^Multiple foreign keys \(table holds\) should be passed using an hashref.*}, "GetPreparedLetter should not be called with arrayref for multiple FK" ); # Bug 16942 $prepared_letter = GetPreparedLetter( @@ -220,8 +223,8 @@ $prepared_letter = GetPreparedLetter( 'borrowers' => $patron, 'biblio' => $item->biblionumber, 'biblioitems' => $item->biblioitemnumber, - 'reserves' => $hold->unblessed, - 'items' => $hold->itemnumber, + 'holds' => $hold->unblessed, + 'items' => $hold->item_id, } ) ); @@ -529,7 +532,7 @@ You have [% count %] items due biblionumber => $biblio1->{biblionumber}, notes => "a note", itemnumber => $item1->{itemnumber}, - found => 'W' + found => 'waiting' } ); C4::Reserves::AddReserve( diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index c75a69ad95..3f4e659853 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -125,7 +125,7 @@ my ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber, $barcod is($status, "Reserved", "CheckReserves Test 1"); -ok(exists($reserve->{reserve_id}), 'CheckReserves() include reserve_id in its response'); +ok(exists($reserve->{id}), 'CheckReserves() include reserve_id in its response'); ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber); is($status, "Reserved", "CheckReserves Test 2"); @@ -269,14 +269,14 @@ ModReserveAffect($itemnum_cpl, $requesters{$branch_3}, 0); # Now it should have different priorities. my $biblio = Koha::Biblios->find( $bibnum2 ); -my $holds = $biblio->holds({}, { order_by => 'reserve_id' });; +my $holds = $biblio->holds({}, { order_by => 'id' });; is($holds->next->priority, 0, 'Item is correctly waiting'); is($holds->next->priority, 1, 'Item is correctly priority 1'); is($holds->next->priority, 2, 'Item is correctly priority 2'); -my @reserves = Koha::Holds->search({ borrowernumber => $requesters{$branch_3} })->waiting(); +my @reserves = Koha::Holds->search({ patron_id => $requesters{$branch_3} })->waiting(); is( @reserves, 1, 'GetWaiting got only the waiting reserve' ); -is( $reserves[0]->borrowernumber(), $requesters{$branch_3}, 'GetWaiting got the reserve for the correct borrower' ); +is( $reserves[0]->patron_id(), $requesters{$branch_3}, 'GetWaiting got the reserve for the correct borrower' ); $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); @@ -314,7 +314,7 @@ my $messages; # the one placed by the CPL patron, as the other two patron's hold # requests cannot be filled by that item per policy. (undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2); -is( $messages->{ResFound}->{borrowernumber}, +is( $messages->{ResFound}->{patron_id}, $requesters{$branch_1}, 'restrictive library\'s items only fill requests by own patrons (bug 10272)'); @@ -326,14 +326,14 @@ is( $messages->{ResFound}->{borrowernumber}, t::lib::Mocks::mock_preference( 'LocalHoldsPriority', '' ); (undef, $messages, undef, undef) = AddReturn('bug10272_FPL', $branch_2); -is( $messages->{ResFound}->{borrowernumber}, +is( $messages->{ResFound}->{patron_id}, $requesters{$branch_3}, 'for generous library, its items fill first hold request in line (bug 10272)'); $biblio = Koha::Biblios->find( $biblionumber ); $holds = $biblio->holds; is($holds->count, 1, "Only one reserves for this biblio"); -my $reserve_id = $holds->next->reserve_id; +my $reserve_id = $holds->next->id; # 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) @@ -406,7 +406,7 @@ is($new_count, $hold_notice_count + 1, 'patron not notified a second time (bug 1 t::lib::Mocks::mock_preference('IndependentBranches', 0); $item = Koha::Items->find($item->itemnumber); is( - $item->safe_delete, + $item->safe_to_delete, 'book_reserved', 'item that is captured to fill a hold cannot be deleted', ); @@ -434,7 +434,7 @@ AddReserve( $holds = $item->current_holds; my $dtf = Koha::Database->new->schema->storage->datetime_parser; -my $future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +my $future_holds = $holds->search({ placing_date => { '>' => $dtf->format_date( dt_from_string ) } } ); is( $future_holds->count, 0, 'current_holds does not return a future next available hold'); # 9788b: current_holds does not return future item level hold $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); @@ -448,11 +448,11 @@ AddReserve( itemnumber => $item->itemnumber, } ); #item level hold -$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +$future_holds = $holds->search({ placing_date => { '>' => $dtf->format_date( dt_from_string ) } } ); is( $future_holds->count, 0, 'current_holds does not return a future item level hold' ); # 9788c: current_holds returns future wait (confirmed future hold) ModReserveAffect( $item->itemnumber, $requesters{$branch_1} , 0); #confirm hold -$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +$future_holds = $holds->search({ placing_date => { '>' => $dtf->format_date( dt_from_string ) } } ); is( $future_holds->count, 1, 'current_holds returns a future wait (confirmed future hold)' ); # End of tests for bug 9788 @@ -546,17 +546,17 @@ is( 'CanReserveBeCanceledFromOpac should return undef if called without borrowernumber' ); -my $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); +my $cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); is($cancancel, 1, 'Can user cancel its own reserve'); -$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_2}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_2}); is($cancancel, 0, 'Other user cant cancel reserve'); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 1); -$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); is($cancancel, 0, 'Reserve in transfer status cant be canceled'); -$dbh->do('DELETE FROM reserves', undef, ($bibnum)); +$dbh->do('DELETE FROM holds', undef, ($bibnum)); AddReserve( { branchcode => $branch_1, @@ -568,7 +568,7 @@ AddReserve( (undef, $canres, undef) = CheckReserves($item->itemnumber); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); -$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); is($cancancel, 0, 'Reserve in waiting status cant be canceled'); # End of tests for bug 12876 @@ -636,7 +636,7 @@ Koha::CirculationRules->set_rules( # tests for MoveReserve in relation to ConfirmFutureHolds (BZ 14526) # hold from A pos 1, today, no fut holds: MoveReserve should fill it -$dbh->do('DELETE FROM reserves', undef, ($bibnum)); +$dbh->do('DELETE FROM holds'); t::lib::Mocks::mock_preference('ConfirmFutureHolds', 0); t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); AddReserve( @@ -657,7 +657,7 @@ AddReserve( borrowernumber => $borrowernumber, biblionumber => $bibnum, priority => 1, - found => 'W', + found => 'waiting', } ); MoveReserve( $item->itemnumber, $borrowernumber ); @@ -831,13 +831,13 @@ subtest 'ReservesNeedReturns' => sub { t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # Test with feature disabled my $hold = place_item_hold( $patron, $item, $library, $priority ); is( $hold->priority, $priority, 'If ReservesNeedReturns is 1, priority must not have been set to changed' ); - is( $hold->found, undef, 'If ReservesNeedReturns is 1, found must not have been set waiting' ); + is( $hold->status, 'placed', 'If ReservesNeedReturns is 1, status must not have been set waiting' ); $hold->delete; - t::lib::Mocks::mock_preference('ReservesNeedReturns', 0); # '0' means 'Automatically mark a hold as found and waiting' + t::lib::Mocks::mock_preference('ReservesNeedReturns', 0); # '0' means 'Automatically mark a hold as status and waiting' $hold = place_item_hold( $patron, $item, $library, $priority ); is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' ); - is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); + is( $hold->status, 'waiting', 'If ReservesNeedReturns is 0 and no other status, status must have been set waiting' ); $hold->delete; $item->onloan('2010-01-01')->store; @@ -853,11 +853,11 @@ subtest 'ReservesNeedReturns' => sub { t::lib::Mocks::mock_preference('AllowHoldsOnDamagedItems', 1); # '0' means damaged holds not allowed $hold = place_item_hold( $patron, $item, $library, $priority ); is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and damaged holds allowed, priority must have been set to 0' ); - is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and damaged holds allowed, found must have been set waiting' ); + is( $hold->status, 'waiting', 'If ReservesNeedReturns is 0 and damaged holds allowed, status must have been set waiting' ); $hold->delete; my $hold_1 = place_item_hold( $patron, $item, $library, $priority ); - is( $hold_1->found, 'W', 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); + is( $hold_1->status, 'waiting', 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); is( $hold_1->priority, 0, 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); $hold = place_item_hold( $patron_2, $item, $library, $priority ); is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item already on hold priority must be set to 1' ); @@ -873,21 +873,21 @@ subtest 'ReservesNeedReturns' => sub { }); $item->damaged(0)->store; $hold = place_item_hold( $patron, $item, $library, $priority ); - is( $hold->found, undef, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); + is( $hold->status, 'placed', 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); $hold->delete; $transfer->delete; $hold = place_item_hold( $patron, $item, $library, $priority ); is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' ); - is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); + is( $hold->status, 'waiting', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); $hold_1 = place_item_hold( $patron, $item, $library, $priority ); is( $hold_1->priority, 1, 'If ReservesNeedReturns is 0 but item has a hold priority is 1' ); - $hold_1->suspend(1)->store; # We suspend the hold + $hold_1->suspended(1)->store; # We suspend the hold $hold->delete; # Delete the waiting hold $hold = place_item_hold( $patron, $item, $library, $priority ); is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and other hold(s) suspended, priority must have been set to 0' ); - is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and other hold(s) suspended, found must have been set waiting' ); + is( $hold->status, 'waiting', 'If ReservesNeedReturns is 0 and other hold(s) suspended, found must have been set waiting' ); @@ -944,7 +944,7 @@ subtest 'reserves.item_level_hold' => sub { ); my $hold = Koha::Holds->find($reserve_id); - is( $hold->item_level_hold, 1, 'item_level_hold should be set when AddReserve is called with a specific item' ); + is( $hold->item_level, 1, 'item_level should be set when AddReserve is called with a specific item' ); # Mark it waiting ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 1 ); @@ -955,7 +955,7 @@ subtest 'reserves.item_level_hold' => sub { $hold = Koha::Holds->find($reserve_id); - is( $hold->itemnumber, $item->itemnumber, 'Itemnumber should not be removed when the waiting status is revert' ); + is( $hold->item_id, $item->itemnumber, 'Itemnumber should not be removed when the waiting status is revert' ); $hold->delete; # cleanup }; @@ -972,19 +972,19 @@ subtest 'reserves.item_level_hold' => sub { ); my $hold = Koha::Holds->find($reserve_id); - is( $hold->item_level_hold, 0, 'item_level_hold should not be set when AddReserve is called without a specific item' ); + is( $hold->item_level, 0, 'item_level should not be set when AddReserve is called without a specific item' ); # Mark it waiting ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 1 ); $hold = Koha::Holds->find($reserve_id); - is( $hold->itemnumber, $item->itemnumber, 'Itemnumber should be set on hold confirmation' ); + is( $hold->item_id, $item->itemnumber, 'Itemnumber should be set on hold confirmation' ); # Revert the waiting status C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } ); $hold = Koha::Holds->find($reserve_id); - is( $hold->itemnumber, undef, 'Itemnumber should be removed when the waiting status is revert' ); + is( $hold->item_id, undef, 'Itemnumber should be removed when the waiting status is revert' ); $hold->delete; }; @@ -1022,16 +1022,24 @@ subtest 'MoveReserve additional test' => sub { itemnumber => $item_1->itemnumber, } ); - is($patron_1->holds->next()->reserve_id, $reserve_1, "The 1st patron has a hold"); - is($patron_2->holds->next()->reserve_id, $reserve_2, "The 2nd patron has a hold"); + is($patron_1->holds->next()->id, $reserve_1, "The 1st patron has a hold"); + is($patron_2->holds->next()->id, $reserve_2, "The 2nd patron has a hold"); # Fake the holds queue - $dbh->do(q{INSERT INTO hold_fill_targets VALUES (?, ?, ?, ?, ?)},undef,($patron_1->borrowernumber,$biblio->biblionumber,$item_1->itemnumber,$item_1->homebranch,0)); + $dbh->do( + q{INSERT INTO hold_fill_targets VALUES (?, ?, ?, ?, ?)}, + undef, + ( + $patron_1->borrowernumber, $biblio->biblionumber, + $item_1->itemnumber, $item_1->homebranch, + 0 + ) + ); # The 2nd hold should be filed even if the item is preselected for the first hold MoveReserve($item_1->itemnumber,$patron_2->borrowernumber); - is($patron_2->holds->count, 0, "The 2nd patrons no longer has a hold"); - is($patron_2->old_holds->next()->reserve_id, $reserve_2, "The 2nd patrons hold was filled and moved to old holds"); + is($patron_2->holds({ completed => 0 })->count, 0, "The 2nd patrons no longer has a hold"); + is($patron_2->holds({ completed => 1 })->next()->id, $reserve_2, "The 2nd patrons hold was filled and moved to old holds"); }; diff --git a/t/db_dependent/Reserves/AutoUnsuspendReserves.t b/t/db_dependent/Reserves/AutoUnsuspendReserves.t index d5703cc4e9..b4b92b0523 100644 --- a/t/db_dependent/Reserves/AutoUnsuspendReserves.t +++ b/t/db_dependent/Reserves/AutoUnsuspendReserves.t @@ -43,10 +43,10 @@ subtest 'AutoUnsuspendReserves test' => sub { my $hold_1 = $builder->build_object({ class => 'Koha::Holds', value => { - expirationdate => undef, - cancellationdate => undef, + expiration_date => undef, + completion_date => undef, priority => 5, - found => undef, + status => 'placed', }, }); @@ -56,10 +56,10 @@ subtest 'AutoUnsuspendReserves test' => sub { my $hold_2 = $builder->build_object({ class => 'Koha::Holds', value => { - expirationdate => undef, - cancellationdate => undef, + expiration_date => undef, + completion_date => undef, priority => 6, - found => undef, + status => 'placed', }, }); @@ -84,11 +84,11 @@ subtest 'AutoUnsuspendReserves test' => sub { my $hold_3 = $builder->build_object( { class => 'Koha::Holds', value => { - expirationdate => undef, - cancellationdate => undef, - priority => 5, - found => undef, - suspend_until => undef, + expiration_date => undef, + completion_date => undef, + priority => 5, + status => 'placed', + suspended_until_date => undef, } } ); @@ -121,10 +121,10 @@ subtest 'AutoUnsuspendReserves test' => sub { my $hold_4 = $builder->build_object( { class => 'Koha::Holds', value => { - expirationdate => undef, - cancellationdate => undef, - priority => 5, - found => undef + expiration_date => undef, + completion_date => undef, + priority => 5, + status => 'placed' } } ); @@ -137,7 +137,7 @@ subtest 'AutoUnsuspendReserves test' => sub { AutoUnsuspendReserves(); $hold_4->discard_changes; - ok(!defined($hold_4->suspend_until), 'Hold suspended until today should be unsuspended.'); + ok(!defined($hold_4->suspended_until_date), 'Hold suspended until today should be unsuspended.'); my $new_logs_count = $schema->resultset('ActionLog') ->search( { module => 'HOLDS', action => 'RESUME' } )->count; diff --git a/t/db_dependent/Reserves/CancelExpiredReserves.t b/t/db_dependent/Reserves/CancelExpiredReserves.t index e8f8df8cef..2951593c73 100644 --- a/t/db_dependent/Reserves/CancelExpiredReserves.t +++ b/t/db_dependent/Reserves/CancelExpiredReserves.t @@ -16,7 +16,7 @@ my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; subtest 'CancelExpiredReserves tests incl. holidays' => sub { - plan tests => 4; + plan tests => 6; my $builder = t::lib::TestBuilder->new(); @@ -33,52 +33,62 @@ subtest 'CancelExpiredReserves tests incl. holidays' => sub { $reserve1_expirationdate->add(days => 1); # Reserve not expired - my $reserve1 = $builder->build({ - source => 'Reserve', - value => { - reservedate => $reserve_reservedate, - expirationdate => $reserve1_expirationdate, - cancellationdate => undef, - priority => 0, - found => 'W', - }, - }); + my $reserve1 = $builder->build_object( + { + class => 'Koha::Holds', + value => { + placing_date => $reserve_reservedate, + expiration_date => $reserve1_expirationdate, + completion_date => undef, + priority => 0, + status => 'waiting', + completed => 0, + }, + } + ); CancelExpiredReserves(); - my $r1 = Koha::Holds->find($reserve1->{reserve_id}); + my $r1 = Koha::Holds->find($reserve1->id); ok($r1, 'Reserve 1 should not be canceled.'); my $reserve2_expirationdate = $today->clone; $reserve2_expirationdate->subtract(days => 1); # Reserve expired - my $reserve2 = $builder->build({ - source => 'Reserve', - value => { - reservedate => $reserve_reservedate, - expirationdate => $reserve2_expirationdate, - cancellationdate => undef, - priority => 0, - found => 'W', - }, - }); + my $reserve2 = $builder->build_object( + { + class => 'Koha::Holds', + value => { + placing_date => $reserve_reservedate, + expiration_date => $reserve2_expirationdate, + completion_date => undef, + priority => 0, + status => 'waiting', + completed => 0, + }, + } + ); CancelExpiredReserves(); - my $r2 = Koha::Holds->find($reserve2->{reserve_id}); - is($r2, undef,'reserve 2 should be canceled.'); + my $r2 = Koha::Holds->find($reserve2->id); + is(ref($r2), 'Koha::Hold', 'reserve 2 is still there.'); + is($r2->status, 'cancelled', 'reserve 2 should be canceled.'); + is($r2->completed, 1, 'reserve 2 should be completed.'); # Reserve expired on holiday - my $reserve3 = $builder->build({ - source => 'Reserve', - value => { - reservedate => $reserve_reservedate, - expirationdate => $reserve2_expirationdate, - branchcode => 'LIB1', - cancellationdate => undef, - priority => 0, - found => 'W', - }, - }); + my $reserve3 = $builder->build_object( + { + class => 'Koha::Holds', + value => { + placing_date => $reserve_reservedate, + expiration_date => $reserve2_expirationdate, + pickup_library_id => 'LIB1', + completion_date => undef, + priority => 0, + status => 'waiting', + }, + } + ); Koha::Caches->get_instance()->flush_all(); my $holiday = $builder->build({ @@ -94,13 +104,13 @@ subtest 'CancelExpiredReserves tests incl. holidays' => sub { }); CancelExpiredReserves(); - my $r3 = Koha::Holds->find($reserve3->{reserve_id}); - ok($r3,'Reserve 3 should not be canceled.'); + $reserve3->discard_changes; + is($reserve3->status, 'waiting','Reserve 3 should not be canceled.'); t::lib::Mocks::mock_preference('ExpireReservesOnHolidays', 1); CancelExpiredReserves(); - $r3 = Koha::Holds->find($reserve3->{reserve_id}); - is($r3, undef,'Reserve 3 should be canceled.'); + $reserve3->discard_changes; + is($reserve3->status, 'cancelled', 'Reserve 3 should be cancelled.'); }; subtest 'Test handling of waiting reserves by CancelExpiredReserves' => sub { @@ -121,45 +131,51 @@ subtest 'Test handling of waiting reserves by CancelExpiredReserves' => sub { my $expdate = dt_from_string->add( days => -2 ); my $notexpdate = dt_from_string->add( days => 2 ); - my $hold1 = Koha::Hold->new({ - branchcode => $branchcode, - borrowernumber => $borrowernumber, - biblionumber => $bibnum, - priority => 1, - reservedate => $resdate, - expirationdate => $notexpdate, - found => undef, - })->store; - - my $hold2 = Koha::Hold->new({ - branchcode => $branchcode, - borrowernumber => $borrowernumber, - biblionumber => $bibnum, - priority => 2, - reservedate => $resdate, - expirationdate => $expdate, - found => undef, - })->store; - - my $hold3 = Koha::Hold->new({ - branchcode => $branchcode, - borrowernumber => $borrowernumber, - biblionumber => $bibnum, - itemnumber => $itemnumber, - priority => 0, - reservedate => $resdate, - expirationdate => $expdate, - found => 'W', - })->store; + my $hold1 = Koha::Hold->new( + { + pickup_library_id => $branchcode, + patron_id => $borrowernumber, + biblio_id => $bibnum, + priority => 1, + placing_date => $resdate, + expiration_date => $notexpdate, + status => 'placed', + } + )->store; + + my $hold2 = Koha::Hold->new( + { + pickup_library_id => $branchcode, + patron_id => $borrowernumber, + biblio_id => $bibnum, + priority => 2, + placing_date => $resdate, + expiration_date => $expdate, + status => 'placed', + } + )->store; + + my $hold3 = Koha::Hold->new( + { + pickup_library_id => $branchcode, + patron_id => $borrowernumber, + biblio_id => $bibnum, + item_id => $itemnumber, + priority => 0, + placing_date => $resdate, + expiration_date => $expdate, + status => 'waiting', + } + )->store; t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 0 ); CancelExpiredReserves(); - my $count1 = Koha::Holds->search->count; + my $count1 = Koha::Holds->search({ completed => 0 })->count; is( $count1, 2, 'Only the non-waiting expired holds should be cancelled'); t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 ); CancelExpiredReserves(); - my $count2 = Koha::Holds->search->count; + my $count2 = Koha::Holds->search({ completed => 0 })->count; is( $count2, 1, 'Also the waiting expired hold should be cancelled now'); }; @@ -171,33 +187,38 @@ subtest 'Test handling of in transit reserves by CancelExpiredReserves' => sub { t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 ); my $expdate = dt_from_string->add( days => -2 ); - my $reserve = $builder->build({ - source => 'Reserve', + my $reserve = $builder->build_object({ + class => 'Koha::Holds', value => { - expirationdate => '2018-01-01', - found => 'T', - cancellationdate => undef, - suspend => 0, - suspend_until => undef + expiration_date => '2018-01-01', + status => 'in_transit', + suspended => 0, + completed => 0, + cancellation_date => undef, + suspended_until_date => undef, } }); - my $count = Koha::Holds->search->count; + + my $active = Koha::Holds->search({ completed => 0 }); + my $count = $active->count; + CancelExpiredReserves(); - is(Koha::Holds->search->count, $count-1, "Transit hold is cancelled if ExpireReservesMaxPickUpDelay set"); + is($active->count, $count-1, "Transit hold is cancelled if ExpireReservesMaxPickUpDelay set"); t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 0 ); - my $reserve2 = $builder->build({ - source => 'Reserve', + my $reserve2 = $builder->build_object({ + class => 'Koha::Holds', value => { - expirationdate => '2018-01-01', - found => 'T', - cancellationdate => undef, - suspend => 0, - suspend_until => undef + expiration_date => '2018-01-01', + status => 'in_transit', + cancellation_date => undef, + suspended => 0, + suspended_until_date => undef, + completed => 0, } }); CancelExpiredReserves(); - is(Koha::Holds->search->count, $count-1, "Transit hold is cancelled if ExpireReservesMaxPickUpDelay unset"); + is($active->count, $count-1, "Transit hold is cancelled if ExpireReservesMaxPickUpDelay unset"); }; diff --git a/t/db_dependent/Reserves/GetReserveFee.t b/t/db_dependent/Reserves/GetReserveFee.t index 4645c49901..8d92d560db 100755 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ b/t/db_dependent/Reserves/GetReserveFee.t @@ -126,19 +126,19 @@ subtest 'Integration with AddReserve' => sub { plan tests => 3; t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - No fee charged for patron 1 if not issued' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); is( acctlines( $patron1->{borrowernumber} ), 1, 'any_time_is_placed - Patron should be always charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_collected'); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); is( acctlines( $patron1->{borrowernumber} ), 0, 'any_time_is_collected - Patron should not be charged when placing a hold' ); @@ -150,12 +150,12 @@ subtest 'Integration with AddReserve' => sub { C4::Circulation::AddIssue( $patron2, $item1->{barcode}, '2015-12-31', 0, undef, 0, {} ); t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' ); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron3->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); @@ -163,7 +163,7 @@ subtest 'Integration with AddReserve' => sub { is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should be charged if all the items are not checked out and at least 1 hold is already placed' ); C4::Circulation::AddIssue( $patron3, $item2->{barcode}, '2015-12-31', 0, undef, 0, {} ); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should be charged if all items are checked out' ); @@ -174,7 +174,7 @@ subtest 'Integration with AddIssue' => sub { plan tests => 5; $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); - $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->{biblionumber} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); diff --git a/t/db_dependent/Reserves/MultiplePerRecord.t b/t/db_dependent/Reserves/MultiplePerRecord.t index 6fb431da09..7f9081b013 100755 --- a/t/db_dependent/Reserves/MultiplePerRecord.t +++ b/t/db_dependent/Reserves/MultiplePerRecord.t @@ -247,25 +247,25 @@ is( $rule->{holds_per_record}, 5, 'Got holds_per_record Koha::CirculationRules->delete(); -my $holds = Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } ); +my $holds = Koha::Holds->search( { patron_id => $patron->{borrowernumber} } ); is( $holds->forced_hold_level, undef, "No holds does not force an item or record level hold" ); # Test Koha::Holds::forced_hold_level my $hold = Koha::Hold->new({ - borrowernumber => $patron->{borrowernumber}, - reservedate => '1981-06-10', - biblionumber => $biblio->{biblionumber}, - branchcode => $library->{branchcode}, + patron_id => $patron->{borrowernumber}, + placing_date => '1981-06-10', + biblio_id => $biblio->{biblionumber}, + pickup_library_id => $library->{branchcode}, priority => 1, })->store(); -$holds = Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } ); +$holds = Koha::Holds->search( { patron_id => $patron->{borrowernumber} } ); is( $holds->forced_hold_level, 'record', "Record level hold forces record level holds" ); -$hold->itemnumber( $item1->{itemnumber} ); +$hold->item_id( $item1->{itemnumber} ); $hold->store(); -$holds = Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } ); +$holds = Koha::Holds->search( { patron_id => $patron->{borrowernumber} } ); is( $holds->forced_hold_level, 'item', "Item level hold forces item level holds" ); $hold->delete(); @@ -310,7 +310,7 @@ ok( $hold_id, 'Second hold was placed' ); $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber}); is( $can->{status}, 'tooManyHoldsForThisRecord', 'Third hold exceeds limit of holds per record' ); -Koha::Holds->find($hold_id)->found("W")->store; +Koha::Holds->find($hold_id)->set_waiting(); $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber}); is( $can->{status}, 'tooManyHoldsForThisRecord', 'Third hold exceeds limit of holds per record' ); diff --git a/t/db_dependent/Reserves/ReserveSlip.t b/t/db_dependent/Reserves/ReserveSlip.t index 9b28179fcc..52dd057f73 100644 --- a/t/db_dependent/Reserves/ReserveSlip.t +++ b/t/db_dependent/Reserves/ReserveSlip.t @@ -31,7 +31,7 @@ $schema->storage->txn_begin; my $dbh = C4::Context->dbh; $dbh->do(q|DELETE FROM letter|); -$dbh->do(q|DELETE FROM reserves|); +$dbh->do(q|DELETE FROM holds|); my $builder = t::lib::TestBuilder->new(); my $library = $builder->build( @@ -83,24 +83,25 @@ my $item2 = $builder->build( my $hold1 = Koha::Hold->new( { - biblionumber => $biblio->{biblionumber}, - itemnumber => $item1->{itemnumber}, - waitingdate => '2000-01-01', - borrowernumber => $patron->{borrowernumber}, - branchcode => $library->{branchcode}, + biblio_id => $biblio->{biblionumber}, + item_id => $item1->{itemnumber}, + waiting_date => '2000-01-01', + patron_id => $patron->{borrowernumber}, + pickup_library_id => $library->{branchcode}, } )->store; my $hold2 = Koha::Hold->new( { - biblionumber => $biblio->{biblionumber}, - itemnumber => $item2->{itemnumber}, - waitingdate => '2000-01-01', - borrowernumber => $patron->{borrowernumber}, - branchcode => $library->{branchcode}, + biblio_id => $biblio->{biblionumber}, + item_id => $item2->{itemnumber}, + waiting_date => '2000-01-01', + patron_id => $patron->{borrowernumber}, + pickup_library_id => $library->{branchcode}, } )->store; + my $letter = $builder->build( { source => 'Letter', diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t index e2fbba7a9b..85a4cd5b03 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -248,11 +248,11 @@ subtest 'Test build with NULL values' => sub { is( $info->{is_nullable} && $item && !defined( $item->{barcode} ), 1, 'Barcode can be NULL' ); # Nullable FK - $params = { source => 'Reserve', value => { itemnumber => undef }}; - my $reserve = $builder->build( $params ); - $info = $schema->source( 'Reserve' )->column_info( 'itemnumber' ); - is( $reserve && $info->{is_nullable} && $info->{is_foreign_key} && - !defined( $reserve->{itemnumber} ), 1, 'Nullable FK' ); + $params = { source => 'Hold', value => { item_id => undef }}; + my $hold = $builder->build( $params ); + $info = $schema->source( 'Hold' )->column_info( 'item_id' ); + is( $hold && $info->{is_nullable} && $info->{is_foreign_key} && + !defined( $hold->{item_id} ), 1, 'Nullable FK' ); }; @@ -394,6 +394,7 @@ subtest 'build_object() tests' => sub { $module =~ s|^.*/(Koha.*)\.pm$|$1|; $module =~ s|/|::|g; next if $module eq 'Koha::Objects'; + next if $module eq 'Koha::Old::Holds'; #This is a view now eval "require $module"; my $object = $builder->build_object( { class => $module } ); is( ref($object), $module->object_class, "Testing $module" ); diff --git a/t/db_dependent/XSLT.t b/t/db_dependent/XSLT.t index 2d9e9db1b3..759298fac9 100644 --- a/t/db_dependent/XSLT.t +++ b/t/db_dependent/XSLT.t @@ -96,10 +96,11 @@ subtest 'buildKohaItemsNamespace status tests' => sub { like($xml,qr{In transit},"In-transit status takes precedence over Damaged"); my $hold = $builder->build_object({ class => 'Koha::Holds', value => { - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - found => 'W', - priority => 0, + biblio_id => $item->biblionumber, + item_id => $item->itemnumber, + status => 'waiting', + priority => 0, + completed => 0, } }); $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index f7436da0ee..b875166f1d 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -114,7 +114,7 @@ my $biblio_2 = $builder->build_sample_biblio; my $item_2 = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber, itype => $itemtype }); my $dbh = C4::Context->dbh; -$dbh->do('DELETE FROM reserves'); +$dbh->do('DELETE FROM holds'); Koha::CirculationRules->search()->delete(); Koha::CirculationRules->set_rules( { @@ -162,7 +162,7 @@ my $post_data = { }; my $put_data = { priority => 2, - suspended_until => output_pref({ dt => $suspended_until, dateformat => 'rfc3339' }), + suspended_until_date => output_pref({ dt => $suspended_until, dateformat => 'rfc3339' }), }; subtest "Test endpoints without authentication" => sub { @@ -219,7 +219,7 @@ subtest "Test endpoints with permission" => sub { $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => $put_data ) ->status_is(200) ->json_is( '/hold_id', $reserve_id ) - ->json_is( '/suspended_until', $expected_suspended_until ) + ->json_is( '/suspended_until_date', $expected_suspended_until ) ->json_is( '/priority', 2 ) ->json_is( '/pickup_library_id', $branchcode ); @@ -227,15 +227,15 @@ subtest "Test endpoints with permission" => sub { $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => { pickup_library_id => $branchcode2 } ) ->status_is(200) ->json_is( '/hold_id', $reserve_id ) - ->json_is( '/suspended_until', $expected_suspended_until ) + ->json_is( '/suspended_until_date', $expected_suspended_until ) ->json_is( '/priority', 2 ) ->json_is( '/pickup_library_id', $branchcode2 ); # Reset suspended_until, everything else should remain - $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => { suspended_until => undef } ) + $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => { suspended_until_date => undef } ) ->status_is(200) ->json_is( '/hold_id', $reserve_id ) - ->json_is( '/suspended_until', undef ) + ->json_is( '/suspended_until_date', undef ) ->json_is( '/priority', 2 ) ->json_is( '/pickup_library_id', $branchcode2 ); @@ -267,6 +267,7 @@ subtest "Test endpoints with permission" => sub { $t->post_ok( "//$userid_3:$password@/api/v1/holds" => json => $post_data ) ->status_is(201) ->json_has('/hold_id'); + # Get id from response $reserve_id = $t->tx->res->json->{hold_id}; @@ -329,7 +330,7 @@ subtest 'test AllowHoldDateInFuture' => sub { item_id => int($item_1->itemnumber), pickup_library_id => $branchcode, expiration_date => output_pref({ dt => $expiration_date, dateformat => 'rfc3339', dateonly => 1 }), - hold_date => output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 }), + placing_date => output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 }), priority => 2, }; @@ -343,7 +344,7 @@ subtest 'test AllowHoldDateInFuture' => sub { $t->post_ok( "//$userid_3:$password@/api/v1/holds" => json => $post_data ) ->status_is(201) - ->json_is('/hold_date', output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 })); + ->json_is('/placing_date', output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 })); }; subtest 'test AllowHoldPolicyOverride' => sub { @@ -395,7 +396,7 @@ subtest 'suspend and resume tests' => sub { my $hold = $builder->build_object( { class => 'Koha::Holds', - value => { suspend => 0, suspend_until => undef, waitingdate => undef, found => undef } + value => { suspended => 0, suspended_until_date => undef, waiting_date => undef, status => 'placed', completed => 0 } } ); @@ -422,7 +423,7 @@ subtest 'suspend and resume tests' => sub { $t->json_is( '/end_date', output_pref({ - dt => dt_from_string( $hold->suspend_until ), + dt => dt_from_string( $hold->suspended_until_date ), dateformat => 'rfc3339', dateonly => 1 }), @@ -455,13 +456,13 @@ subtest 'suspend and resume tests' => sub { $t->post_ok( "//$userid:$password@/api/v1/holds/" . $hold->id . "/suspension" ) ->status_is( 400, 'Cannot suspend waiting hold' ) - ->json_is( '/error', 'Found hold cannot be suspended. Status=W' ); + ->json_is( '/error', 'Found hold cannot be suspended. Status=waiting' ); $hold->set_waiting(1)->discard_changes; $t->post_ok( "//$userid:$password@/api/v1/holds/" . $hold->id . "/suspension" ) ->status_is( 400, 'Cannot suspend waiting hold' ) - ->json_is( '/error', 'Found hold cannot be suspended. Status=T' ); + ->json_is( '/error', 'Found hold cannot be suspended. Status=in_transit' ); $schema->storage->txn_rollback; }; -- 2.25.0