From 92462ec187f33de6690848215bac0ab055b27f41 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$") # finally 8. Sign off Sponsored-by: ByWater Solutions Sponsored-by: Theke Solutions Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 12 +- C4/Members.pm | 2 +- C4/Reserves.pm | 514 +++++++++--------- 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/Hold.pm | 9 +- Koha/Item.pm | 18 +- Koha/Patron.pm | 12 +- Koha/REST/V1/Auth.pm | 20 +- Koha/REST/V1/Holds.pm | 38 +- Koha/Template/Plugin/Biblio.pm | 11 +- api/v1/swagger/definitions/hold.json | 14 +- api/v1/swagger/paths/holds.json | 4 +- circ/circulation.pl | 2 +- .../prog/en/modules/members/holdshistory.tt | 32 +- members/holdshistory.pl | 12 +- reserve/request.pl | 73 ++- t/Koha/REST/Plugin/Query.t | 4 +- t/db_dependent/Circulation.t | 71 +-- t/db_dependent/Circulation/issue.t | 5 +- t/db_dependent/DecreaseLoanHighHolds.t | 12 +- t/db_dependent/Hold.t | 107 ++-- t/db_dependent/Holds.t | 82 ++- .../Holds/DisallowHoldIfItemsAvailable.t | 6 +- t/db_dependent/Holds/HoldItemtypeLimit.t | 2 +- t/db_dependent/Holds/LocalHoldsPriority.t | 12 +- 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 | 33 +- 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/Z3950Responder/Session.t | 2 +- t/db_dependent/Koha/Z3950Responder/Session2.t | 2 +- t/db_dependent/Letters/TemplateToolkit.t | 19 +- t/db_dependent/Reserves.t | 133 +++-- .../Reserves/AutoUnsuspendReserves.t | 32 +- .../Reserves/CancelExpiredReserves.t | 170 ++++-- t/db_dependent/Reserves/GetReserveFee.t | 29 + t/db_dependent/Reserves/MultiplePerRecord.t | 22 +- t/db_dependent/Reserves/ReserveSlip.t | 19 +- t/db_dependent/TestBuilder.t | 11 +- t/db_dependent/XSLT.t | 9 +- t/db_dependent/api/v1/holds.t | 78 +-- 54 files changed, 1047 insertions(+), 899 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 2229f3edb0..e1fba75d93 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1273,7 +1273,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(); @@ -2850,8 +2850,12 @@ sub CanBookBeRenewed { # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber. if ( $resfound && $resrec->{non_priority} ) { $resfound = Koha::Holds->search( - { biblionumber => $resrec->{biblionumber}, non_priority => 0 } ) - ->count > 0; + { + biblio_id => $resrec->{biblionumber}, + non_priority => 0, + completed => 0 + } + )->count > 0; } @@ -2863,7 +2867,7 @@ sub CanBookBeRenewed { { my $schema = Koha::Database->new()->schema(); - my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count(); + my $item_holds = $schema->resultset('Reserve')->search( { item_id => $itemnumber, status => 'placed' } )->count(); if ($item_holds) { # There is an item level hold on this item, no other item can fill the hold $resfound = 1; diff --git a/C4/Members.pm b/C4/Members.pm index 05056345e3..dadcba657b 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 d5f90494eb..e7f567c292 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 Koha::Plugins; @@ -186,7 +184,7 @@ sub AddReserve { my $notes = $params->{notes}; my $title = $params->{title}; my $checkitem = $params->{itemnumber}; - my $found = $params->{found}; + my $found = $params->{found} // 'placed'; my $itemtype = $params->{itemtype}; my $non_priority = $params->{non_priority}; @@ -217,7 +215,7 @@ sub AddReserve { && !$item->current_holds->count ) { $priority = 0; - $found = 'W'; + $found = 'waiting'; } } @@ -230,7 +228,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 and $found eq 'waiting' ) { $waitingdate = $resdate; } @@ -240,22 +238,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, + created_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, + waiting_date => $waitingdate, + expiration_date => $expdate, + item_type => $itemtype, + item_level => $checkitem ? 1 : 0, non_priority => $non_priority ? 1 : 0, } )->store(); - $hold->set_waiting() if $found && $found eq 'W'; + $hold->set_waiting() if $found eq 'waiting'; logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); @@ -391,7 +388,7 @@ 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(); # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') @@ -403,15 +400,15 @@ sub CanItemBeReserved { 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"; @@ -448,10 +445,11 @@ sub CanItemBeReserved { my $holds_per_day = $rights->{holds_per_day}; my $search_params = { - borrowernumber => $borrowernumber, - biblionumber => $item->biblionumber, + patron_id => $borrowernumber, + biblio_id => $item->biblionumber, + completed => 0, }; - $search_params->{found} = undef if $params->{ignore_found_holds}; + $search_params->{status} = 'placed' if $params->{ignore_found_holds}; my $holds = Koha::Holds->search($search_params); if ( defined $holds_per_record && $holds_per_record ne '' @@ -460,8 +458,9 @@ sub CanItemBeReserved { } my $today_holds = Koha::Holds->search({ - borrowernumber => $borrowernumber, - reservedate => dt_from_string->date + patron_id => $borrowernumber, + created_date => dt_from_string->date, + completed => 0 }); if ( defined $holds_per_day && $holds_per_day ne '' @@ -513,7 +512,8 @@ 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}, + completed => 0 } )->count(); @@ -591,10 +591,10 @@ sub CanReserveBeCanceledFromOpac { my ($reserve_id, $borrowernumber) = @_; return unless $reserve_id and $borrowernumber; - my $reserve = Koha::Holds->find($reserve_id); + my $hold = Koha::Holds->find($reserve_id); - return 0 unless $reserve->borrowernumber == $borrowernumber; - return $reserve->is_cancelable_from_opac; + return 0 unless $hold->patron_id == $borrowernumber; + return $hold->is_cancelable_from_opac; } =head2 GetOtherReserves @@ -612,19 +612,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' ), ; @@ -635,9 +635,9 @@ sub GetOtherReserves { $messages->{'waiting'} = 1; ModReserveMinusPriority( $itemnumber, - $checkreserves->{'reserve_id'}, + $checkreserves->{id}, ); - ModReserveStatus($itemnumber,'W'); + ModReserveStatus($itemnumber,'waiting'); } $nextreservinfo = $checkreserves; @@ -728,25 +728,20 @@ If several reserves exist, the reserve with the lower priority is given. ## multiple reserves for that bib can have the itemnumber set ## the sub is only used once in the codebase. sub GetReserveStatus { - my ($itemnumber) = @_; + my ($item_id) = @_; - my $dbh = C4::Context->dbh; + my $holds_rs = Koha::Holds->search({ item_id => $item_id, completed => 0 }, { order_by => 'priority' }); - my ($sth, $found, $priority); - if ( $itemnumber ) { - $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE itemnumber = ? order by priority LIMIT 1"); - $sth->execute($itemnumber); - ($found, $priority) = $sth->fetchrow_array; - } + if ( $holds_rs->count > 0 ) { + # They are sorted by priority, pick the first one + my $hold = $holds_rs->next; - if(defined $found) { - return 'Waiting' if $found eq 'W' and $priority == 0; - return 'Processing' if $found eq 'P'; - return 'Finished' if $found eq 'F'; + return 'Waiting' if $hold->is_waiting and $hold->priority == 0; + return 'Processing' if $hold->is_in_processing; + return 'Finished' if $hold->status eq 'fulfilled'; + return 'Reserved' if $hold->priority > 0; } - return 'Reserved' if defined $priority && $priority > 0; - return ''; # empty string here will remove need for checking undef, or less log lines } @@ -785,7 +780,7 @@ sub CheckReserves { my $sth; my $select; if (C4::Context->preference('item-level_itypes')){ - $select = " + $select = " SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, @@ -800,7 +795,7 @@ sub CheckReserves { "; } else { - $select = " + $select = " SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, @@ -853,10 +848,10 @@ 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 - } elsif ($res->{'found'} eq 'P') { + } elsif ($res->{'status'} eq 'processing') { return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one } else { return ( "Reserved", $res, \@reserves ); # Found determinated hold, e. g. the transferred one @@ -867,7 +862,7 @@ 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); unless ($item->exclude_from_local_holds_priority || $patron->category->exclude_from_local_holds_priority) { @@ -875,7 +870,7 @@ sub CheckReserves { $item->$LocalHoldsPriorityItemControl; my $local_holds_priority_patron_branchcode = ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' ) - ? $res->{branchcode} + ? $res->{pickup_library_id} : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' ) ? $patron->branchcode : undef; @@ -886,10 +881,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); @@ -897,10 +892,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; @@ -934,20 +929,20 @@ 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 = {}; $cancel_params->{cancellation_reason} = $cancellation_reason if defined($cancellation_reason); - if ( defined($hold->found) && $hold->found eq 'W' ) { + if ( $hold->is_waiting ) { $cancel_params->{charge_cancel_fee} = 1; } $hold->cancel( $cancel_params ); @@ -958,14 +953,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; } @@ -1025,10 +1020,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); @@ -1037,21 +1032,21 @@ sub ModReserve { $hold->cancel({ cancellation_reason => $cancellation_reason }); } 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->{created_date} = $params->{reservedate} || undef; } if (exists $params->{expirationdate}) { - $properties->{expirationdate} = $params->{expirationdate} || undef; + $properties->{expiration_date} = $params->{expirationdate} || undef; } $hold->set($properties)->store(); @@ -1063,7 +1058,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(); } } @@ -1085,37 +1080,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, + completed_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 } ); } } @@ -1137,7 +1132,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 ); @@ -1181,15 +1176,15 @@ 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); if ($transferToDo) { $hold->set_transfer(); @@ -1199,7 +1194,7 @@ sub ModReserveAffect { $hold->set_processing(); } else { $hold->set_waiting($desk_id); - _koha_notify_reserve( $hold->reserve_id ) unless $already_on_shelf; + _koha_notify_reserve( $hold->id ) unless $already_on_shelf; # Complete transfer if one exists my $transfer = $hold->item->get_transfer; $transfer->receive if $transfer; @@ -1212,7 +1207,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; @@ -1229,15 +1224,15 @@ function to cancel reserv,check other reserves, and transfer document if it's ne sub ModReserveCancelAll { my $messages; my $nextreservinfo; - my ( $itemnumber, $borrowernumber, $cancellation_reason ) = @_; + my ( $item_id, $patron_id, $cancellation_reason ) = @_; #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({ cancellation_reason => $cancellation_reason }); #step 2 launch the subroutine of the others reserves - ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber); + ( $messages, $nextreservinfo ) = GetOtherReserves($item_id); return ( $messages, $nextreservinfo->{borrowernumber} ); } @@ -1251,17 +1246,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' }); } @@ -1405,8 +1400,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->completed_date and $hold->status eq 'cancelled' ) { + warn "I cannot alter the priority for reserve_id $reserve_id, the reserve has been cancelled (" . $hold->completed_date . ')'; return; } @@ -1438,7 +1433,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' }); @@ -1498,9 +1493,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); @@ -1560,15 +1555,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 @@ -1578,10 +1569,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', 'P') + WHERE id = ? + AND status IN ('waiting', 'in_transit', 'processing') "; my $sth = $dbh->prepare($query); $sth->execute( $reserve_id ); @@ -1590,10 +1581,11 @@ sub _FixPriority { # get whats left my $query = " - SELECT reserve_id, borrowernumber, reservedate - FROM reserves - WHERE biblionumber = ? - AND ((found <> 'W' AND found <> 'T' AND found <> 'P') 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); @@ -1607,7 +1599,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; } @@ -1623,25 +1615,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 }); @@ -1678,29 +1670,33 @@ 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.created_date AS created_date, + holds.pickup_library_id AS pickup_library_id, + holds.completed_date AS completed_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, - reserves.non_priority AS non_priority - FROM reserves - JOIN biblioitems USING (biblionumber) - JOIN hold_fill_targets USING (reserve_id) - WHERE found IS NULL + holds.item_id AS item_id, + holds.id AS id, + holds.item_type AS item_type, + holds.non_priority AS non_priority + 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 hold_fill_targets.itemnumber = ? - AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) - AND suspend = 0 + AND completed = 0 + AND item_level = 1 + AND item_id = ? + AND created_date <= DATE_ADD(NOW(),INTERVAL ? DAY) + AND suspended = 0 ORDER BY priority }; my $sth = $dbh->prepare($item_level_target_query); @@ -1708,35 +1704,38 @@ 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.created_date AS created_date, + holds.pickup_library_id AS pickup_library_id, + holds.completed_date AS completed_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, - reserves.non_priority AS non_priority - FROM reserves - JOIN biblioitems USING (biblionumber) - JOIN hold_fill_targets USING (reserve_id) - WHERE found IS NULL + holds.item_id AS item_id, + holds.id AS id, + holds.item_type AS item_type, + holds.non_priority AS non_priority + 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 created_date <= DATE_ADD(NOW(),INTERVAL ? DAY) + AND suspended = 0 ORDER BY priority }; $sth = $dbh->prepare($title_level_target_query); @@ -1744,30 +1743,31 @@ 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, - reserves.non_priority AS non_priority - 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.created_date AS created_date, + holds.waiting_date AS waiting_date, + holds.pickup_library_id AS pickup_library_id, + holds.completed_date AS completed_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, + holds.non_priority AS non_priority + FROM holds + WHERE holds.biblio_id = ? + AND (holds.item_id IS NULL OR holds.item_id = ?) + AND holds.created_date <= DATE_ADD(NOW(),INTERVAL ? DAY) + AND suspended = 0 + AND completed = 0 ORDER BY priority }; $sth = $dbh->prepare($query); @@ -1775,7 +1775,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; } @@ -1811,7 +1811,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 ); @@ -1823,21 +1823,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, }, ); @@ -1901,7 +1901,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 ( created_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; @@ -1909,20 +1909,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 created_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, created_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->{created_date} ); } return $new_priority; # so the caller knows what priority they wind up receiving @@ -1946,9 +1946,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 { @@ -1957,8 +1957,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(); @@ -1972,7 +1972,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; } } @@ -1989,33 +1989,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 NOT IN ('W', 'T', 'P') OR found is NULL) ORDER BY reservedate ASC" + "SELECT id FROM holds WHERE biblio_id = ? AND status = 'placed' AND completed = 0 ORDER BY created_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 ); 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++; } @@ -2038,36 +2036,34 @@ sub MergeHolds { sub RevertWaitingStatus { my ( $params ) = @_; - my $itemnumber = $params->{'itemnumber'}; + my $item_id = $params->{'itemnumber'}; - return unless ( $itemnumber ); - - my $dbh = C4::Context->dbh; + return unless ( $item_id ); ## Get the waiting reserve we want to revert my $hold = Koha::Holds->search( { - itemnumber => $itemnumber, - found => { not => undef }, + item_id => $item_id, + status => 'waiting', } )->next; ## Increment the priority of all other non-waiting ## reserves for this bib record - my $holds = Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } }) + my $holds = Koha::Holds->search({ biblio_id => $hold->biblio_id, priority => { '>' => 0 } }) ->update({ priority => \'priority + 1' }, { no_triggers => 1 }); ## Fix up the currently waiting reserve $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 => $hold->biblionumber } ); + _FixPriority( { biblionumber => $hold->biblio_id } ); return $hold; } @@ -2100,13 +2096,13 @@ available within the slip: sub ReserveSlip { my ($args) = @_; - my $branchcode = $args->{branchcode}; + my $branchcode = $args->{branchcode}; my $reserve_id = $args->{reserve_id}; my $hold = Koha::Holds->find($reserve_id); return unless $hold; - my $patron = $hold->borrower; + my $patron = $hold->patron; my $reserve = $hold->unblessed; return C4::Letters::GetPreparedLetter ( @@ -2116,11 +2112,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}, }, ); } @@ -2174,17 +2170,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 or found==P (waiting, transit or processing holds) + #skip status==waiting or status==in_transit or status==processing if( $resdate ) { - $sql.= ' AND ( reservedate <= ? )'; + $sql.= ' AND ( created_date <= ? )'; } else { - $sql.= ' AND ( reservedate < NOW() )'; + $sql.= ' AND ( created_date < NOW() )'; } my $dbh = C4::Context->dbh(); my @row = $dbh->selectrow_array( @@ -2198,26 +2194,24 @@ sub CalculatePriority { =head2 IsItemOnHoldAndFound - my $bool = IsItemFoundHold( $itemnumber ); + my $bool = IsItemFoundHold( $item_id ); 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 in_transit, processing + or waiting. =cut sub IsItemOnHoldAndFound { - my ($itemnumber) = @_; - - my $rs = Koha::Database->new()->schema()->resultset('Reserve'); + my ($item_id) = @_; - my $found = $rs->count( + return Koha::Holds->search( { - itemnumber => $itemnumber, - found => { '!=' => undef } + item_id => $item_id, + status => [ 'in_transit', 'processing', 'waiting' ], + completed => 0 } - ); - - return $found; + )->count; } =head2 GetMaxPatronHoldsForRecord diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index d5ac629023..76450f23a3 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -525,11 +525,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 2bc1133e8d..39663954b4 100644 --- a/C4/SIP/ILS/Transaction/Hold.pm +++ b/C4/SIP/ILS/Transaction/Hold.pm @@ -86,7 +86,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 0484c2b733..38a9727265 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 07dea68939..417c92cafb 100644 --- a/Koha/Acquisition/Order.pm +++ b/Koha/Acquisition/Order.pm @@ -299,7 +299,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 0e17862d20..80fe213708 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, created_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/Hold.pm b/Koha/Hold.pm index 244c443a7b..ec3f537335 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -355,7 +355,7 @@ sub is_in_processing { Returns true if hold is a cancelable hold -Holds may be only canceled if they are not found. +Holds may be only canceled if they are not found or completed. This is used from the OPAC. @@ -364,8 +364,11 @@ This is used from the OPAC. sub is_cancelable_from_opac { my ($self) = @_; - return 1 unless $self->is_found(); - return 0; # if ->is_in_transit or if ->is_waiting or ->is_in_processing + if ( $self->completed or $self->is_found ) { + return 0; + } + + return 1; } =head3 is_at_destination diff --git a/Koha/Item.pm b/Koha/Item.pm index 7bbc87b95b..b017c66795 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -274,7 +274,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; @@ -283,7 +283,7 @@ sub safe_to_delete { if $self->biblio->items->count == 1 && $self->biblio->holds->search( { - itemnumber => undef, + item_id => undef, } )->count; @@ -398,7 +398,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 ); } @@ -750,14 +751,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 }, + created_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 8c1592c3f2..dbbb812808 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -72,9 +72,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', @@ -1196,8 +1195,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 => 'created_date' } ); return Koha::Holds->_new_from_dbic($holds_rs); } @@ -1211,8 +1211,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 => 'created_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 a28fb5920c..7557cadda1 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -316,10 +316,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": { @@ -383,7 +383,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 } ) { @@ -447,20 +447,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 9020c57a47..b05279f6b8 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -46,7 +46,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 ); } @@ -77,10 +77,10 @@ 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 $created_date = $body->{created_date}; my $non_priority = $body->{non_priority}; - if(!C4::Context->preference( 'AllowHoldDateInFuture' ) && $hold_date) { + if(!C4::Context->preference( 'AllowHoldDateInFuture' ) && $created_date) { return $c->render( status => 400, openapi => { error => "Hold date in future not allowed" } @@ -199,7 +199,7 @@ sub add { borrowernumber => $patron_id, biblionumber => $biblio_id, priority => $priority, - reservation_date => $hold_date, + reservation_date => $created_date, expiration_date => $expiration_date, notes => $notes, title => $biblio->title, @@ -253,11 +253,13 @@ 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, - openapi => {error => "Hold not found"} ); + unless ( $hold ) { + return $c->render( + status => 404, + openapi => { error => "Hold not found" } + ); } my $body = $c->req->json; @@ -277,17 +279,17 @@ sub edit { ); } - $pickup_library_id //= $hold->branchcode; + $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); @@ -313,7 +315,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." } ); @@ -342,7 +344,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; @@ -356,9 +358,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 } @@ -390,7 +392,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) { @@ -416,7 +418,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( @@ -464,7 +466,7 @@ sub pickup_locations { return try { my $ps_set; - if ( $hold->itemnumber ) { + if ( $hold->item_id ) { $ps_set = $hold->item->pickup_locations( { patron => $hold->patron } ); } else { diff --git a/Koha/Template/Plugin/Biblio.pm b/Koha/Template/Plugin/Biblio.pm index e9d7a73009..d07b184edc 100644 --- a/Koha/Template/Plugin/Biblio.pm +++ b/Koha/Template/Plugin/Biblio.pm @@ -29,11 +29,14 @@ use Koha::ArticleRequests; use Koha::ArticleRequest::Status; sub HoldsCount { - my ( $self, $biblionumber ) = @_; - - my $holds = Koha::Holds->search( { biblionumber => $biblionumber } ); + my ( $self, $biblio_id ) = @_; - return $holds->count(); + return Koha::Holds->search( + { + biblio_id => $biblio_id, + completed => 0 + } + )->count; } sub ArticleRequestsActiveCount { diff --git a/api/v1/swagger/definitions/hold.json b/api/v1/swagger/definitions/hold.json index 6a018e916f..1a5e6509ac 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": { + "created_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" @@ -82,6 +82,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')" + }, + "completed_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 c71fe1cacf..33e9175607 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -293,11 +293,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/circ/circulation.pl b/circ/circulation.pl index ee3878ca13..7e4b27e202 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -457,7 +457,7 @@ if (@$barcodes) { # BUILD HTML # show all reserves of this borrower, and the position of the reservation .... if ($patron) { - my $holds = Koha::Holds->search( { borrowernumber => $borrowernumber } ); # FIXME must be Koha::Patron->holds + my $holds = Koha::Holds->search( { patron_id => $borrowernumber } ); # FIXME must be Koha::Patron->holds my $waiting_holds = $holds->waiting; $template->param( holds_count => $holds->count(), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt index 5efdf309d6..7c10bae8fc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -60,43 +60,44 @@ [% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %] [% hold.biblio.author | html %] [% hold.item.barcode | html %] - [% Branches.GetName( hold.branchcode ) | html %] - [% hold.reservedate | $KohaDates %] + [% Branches.GetName( hold.pickup_library_id ) | html %] + [% hold.created_date | $KohaDates %] - [% IF hold.expirationdate %] - [% hold.expirationdate | $KohaDates %] + [% IF hold.expiration_date %] + [% hold.expiration_date | $KohaDates %] [% ELSE %] [% END %] - [% IF hold.waitingdate %] - [% hold.waitingdate | $KohaDates %] + [% IF hold.waiting_date %] + [% hold.waiting_date | $KohaDates %] [% ELSE %] [% END %] - [% IF hold.cancellationdate %] - [% hold.cancellationdate | $KohaDates %] + [% IF hold.status == 'cancelled' && hold.completed_date %] + [% hold.completed_date | $KohaDates %] [% ELSE %] [% END %] [% IF show_itemtype_column %] - [% IF hold.itemtype %] - [% ItemTypes.GetDescription( hold.itemtype ) | html %] + [% IF hold.item_type %] + [% ItemTypes.GetDescription( hold.item_type ) | html %] [% ELSE %] Any item type [% END %] [% END %] - [% IF hold.found == 'F' %] + [% IF hold.status == 'fulfilled' %] Fulfilled - [% ELSIF hold.cancellationdate %] + [% ELSIF hold.status == 'cancelled' %] Cancelled +<<<<<<< HEAD [% IF hold.cancellation_reason %] ([% AuthorisedValues.GetByCode('HOLD_CANCELLATION', hold.cancellation_reason) | html %]) [% END %] @@ -105,8 +106,13 @@ [% ELSIF hold.found == 'P' %] Processing [% ELSIF hold.found == 'T' %] +======= + [% ELSIF hold.status == 'waiting' %] + Waiting + [% ELSIF hold.status == 'in_transit' %] +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) In transit - [% ELSE %] + [% ELSIF hold.status == 'placed' %] Pending [% END %] diff --git a/members/holdshistory.pl b/members/holdshistory.pl index 2b2db160d2..84561e0ef8 100755 --- a/members/holdshistory.pl +++ b/members/holdshistory.pl @@ -54,7 +54,6 @@ unless ( $patron ) { } my $holds; -my $old_holds; if ( $borrowernumber eq C4::Context->preference('AnonymousPatron') ){ # use of 'eq' in the above comparison is intentional -- the @@ -62,21 +61,12 @@ if ( $borrowernumber eq C4::Context->preference('AnonymousPatron') ){ $template->param( is_anonymous => 1 ); } else { $holds = $patron->holds; - $old_holds = $patron->old_holds; - - while (my $hold = $holds->next) { - push @all_holds, $hold; - } - - while (my $hold = $old_holds->next) { - push @all_holds, $hold; - } } $template->param( holdshistoryview => 1, patron => $patron, - holds => \@all_holds, + holds => $holds ); output_html_with_http_headers $input, $cookie, $template->output; diff --git a/reserve/request.pl b/reserve/request.pl index 1751a0aa92..892ab04aac 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -330,9 +330,9 @@ foreach my $biblionumber (@biblionumbers) { # the patron places must be item level my $holds = Koha::Holds->search( { - borrowernumber => $patron->borrowernumber, - biblionumber => $biblionumber, - found => undef, + patron_id => $patron->borrowernumber, + biblio_id => $biblionumber, + status => 'placed', } ); $force_hold_level = $holds->forced_hold_level(); @@ -349,7 +349,12 @@ foreach my $biblionumber (@biblionumbers) { } - my $count = Koha::Holds->search( { biblionumber => $biblionumber } )->count(); + my $count = Koha::Holds->search( + { + biblio_id => $biblionumber, + completed => 0 + } + )->count(); my $totalcount = $count; # FIXME think @optionloop, is maybe obsolete, or must be switchable by a systeme preference fixed rank or not @@ -494,13 +499,13 @@ foreach my $biblionumber (@biblionumbers) { my $item_object = Koha::Items->find( $itemnumber ); my $holds = $item_object->current_holds; if ( my $first_hold = $holds->next ) { - my $p = Koha::Patrons->find( $first_hold->borrowernumber ); + my $p = Koha::Patrons->find( $first_hold->patron_id ); $item->{backgroundcolor} = 'reserved'; - $item->{reservedate} = output_pref({ dt => dt_from_string( $first_hold->reservedate ), dateonly => 1 }); # FIXME Should be formatted in the template + $item->{reservedate} = output_pref({ dt => dt_from_string( $first_hold->created_date ), dateonly => 1 }); # FIXME Should be formatted in the template $item->{ReservedFor} = $p; - $item->{ExpectedAtLibrary} = $first_hold->branchcode; - $item->{waitingdate} = $first_hold->waitingdate; + $item->{ExpectedAtLibrary} = $first_hold->pickup_library_id; + $item->{waitingdate} = $first_hold->waiting_date; } # Management of the notforloan document @@ -629,7 +634,13 @@ 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, + completed => 0 + }, + { order_by => 'priority' } + ); foreach my $res ( sort { my $a_found = $a->found() || ''; @@ -655,11 +666,27 @@ foreach my $biblionumber (@biblionumbers) { $reserve{'holdingbranch'} = $res->item()->holdingbranch(); $reserve{'biblionumber'} = $res->item()->biblionumber(); $reserve{'barcodenumber'} = $res->item()->barcode(); - $reserve{'wbrcode'} = $res->branchcode(); - $reserve{'itemnumber'} = $res->itemnumber(); + $reserve{'wbrcode'} = $res->pickup_library_id; + $reserve{'itemnumber'} = $res->item_id; $reserve{'wbrname'} = $res->branch()->branchname(); +<<<<<<< HEAD $reserve{'atdestination'} = $res->is_at_destination(); $reserve{'desk_name'} = ( $res->desk() ) ? $res->desk()->desk_name() : '' ; +======= + + if ( $reserve{'holdingbranch'} eq $reserve{'wbrcode'} ) { + + # Just because the holdingbranch matches the reserve branch doesn't mean the item + # has arrived at the destination, check for an open transfer for the item as well + my ( $transfertwhen, $transfertfrom, $transferto ) = + C4::Circulation::GetTransfers( $res->item_id ); + if ( not $transferto or $transferto ne $res->pickup_library_id ) { + $reserve{'atdestination'} = 1; + } + } + + # set found to 1 if reserve is waiting for patron pickup +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $reserve{'found'} = $res->is_found(); $reserve{'inprocessing'} = $res->is_in_processing(); $reserve{'intransit'} = $res->is_in_transit(); @@ -672,24 +699,32 @@ foreach my $biblionumber (@biblionumbers) { } } - $reserve{'expirationdate'} = $res->expirationdate; - $reserve{'date'} = $res->reservedate; - $reserve{'borrowernumber'} = $res->borrowernumber(); - $reserve{'biblionumber'} = $res->biblionumber(); + $reserve{'expirationdate'} = $res->expiration_date; + $reserve{'date'} = $res->created_date; + $reserve{'borrowernumber'} = $res->patron_id; + $reserve{'biblionumber'} = $res->biblio_id; $reserve{'patron'} = $res->borrower; - $reserve{'notes'} = $res->reservenotes(); - $reserve{'waiting_date'} = $res->waitingdate(); + $reserve{'notes'} = $res->notes; + $reserve{'waiting_date'} = $res->waiting_date; $reserve{'ccode'} = $res->item() ? $res->item()->ccode() : undef; $reserve{'barcode'} = $res->item() ? $res->item()->barcode() : undef; - $reserve{'priority'} = $res->priority(); - $reserve{'lowestPriority'} = $res->lowestPriority(); + $reserve{'priority'} = $res->priority; + $reserve{'lowestPriority'} = $res->lowest_priority; $reserve{'optionloop'} = \@optionloop; +<<<<<<< HEAD $reserve{'suspend'} = $res->suspend(); $reserve{'suspend_until'} = $res->suspend_until(); $reserve{'reserve_id'} = $res->reserve_id(); $reserve{itemtype} = $res->itemtype(); $reserve{branchcode} = $res->branchcode(); $reserve{non_priority} = $res->non_priority(); +======= + $reserve{'suspend'} = $res->suspended; + $reserve{'suspend_until'} = $res->suspended_until_date; + $reserve{'reserve_id'} = $res->id; + $reserve{itemtype} = $res->item_type; + $reserve{branchcode} = $res->pickup_library_id; +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $reserve{object} = $res; push( @reserveloop, \%reserve ); diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index e213787783..23a1786da5 100755 --- 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 => [ '-created_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 => 'created_date' } ] ); diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index d40e586e6b..9d64c30b98 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -421,7 +421,7 @@ subtest "CanBookBeRenewed tests" => sub { my $expdate = undef; my $notes = ''; my $checkitem = undef; - my $found = undef; + my $found = 'placed'; my $issue = AddIssue( $renewing_borrower, $item_1->barcode); my $datedue = dt_from_string( $issue->date_due() ); @@ -449,8 +449,7 @@ subtest "CanBookBeRenewed tests" => sub { reservation_date => $resdate, expiration_date => $expdate, notes => $notes, - itemnumber => $checkitem, - found => $found, + itemnumber => $checkitem } ); @@ -480,31 +479,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); @@ -519,15 +518,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( @@ -686,7 +685,13 @@ subtest "CanBookBeRenewed tests" => sub { $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } ); $fines->delete(); - $hold = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next; + t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); + + $hold = Koha::Holds->search({ biblio_id => $biblio->biblionumber, patron_id => $reserving_borrowernumber })->next; $hold->cancel; # Bug 14101 @@ -3361,7 +3366,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'); @@ -3371,7 +3376,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'); @@ -3379,12 +3384,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"); }; @@ -3423,7 +3428,7 @@ subtest 'Cancel transfers on lost items' => sub { trigger => 'Reserve', }); 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); @@ -4225,13 +4230,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"); }; @@ -4688,7 +4693,7 @@ subtest 'Checkout should correctly terminate a transfer' => sub { GetOtherReserves( $item->itemnumber ) ; # To put the Reason, it's what does returns.pl... 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 $transfer = $item->get_transfer; is( $transfer->frombranch, $library_1->branchcode ); is( $transfer->tobranch, $library_2->branchcode ); @@ -4699,7 +4704,7 @@ subtest 'Checkout should correctly terminate a transfer' => sub { $transfer = $transfer->get_from_storage; isnt( $transfer->datearrived, undef ); $hold = $hold->get_from_storage; - is( $hold->found, undef, 'Hold is waiting' ); + is( $hold->status, 'placed', 'Hold is waiting' ); is( $hold->priority, 1, ); }; diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index 0ded4142fb..f1f9dfbd4a 100755 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -68,8 +68,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 @@ -484,7 +483,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' ); # Unseen rewnewals t::lib::Mocks::mock_preference('UnseenRenewals', 1); diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index 9e25fafba9..fd0b11304a 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -88,9 +88,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(); } @@ -169,9 +169,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 5b71b64292..46778792af 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(), + created_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,37 +108,37 @@ 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('P'); +$hold->status('processing'); is( $hold->is_found, 1, 'The hold is found'); is( $hold->is_in_processing, 1, 'The hold is in processing' ); -$hold->found(q{}); +$hold->status('placed'); 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' ); ok( !$hold->is_in_processing, 'The hold is not in processing' ); # 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" ); -$hold->found('P'); +$hold->status('processing'); is( $hold->is_cancelable_from_opac, 0, "In processing 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" ); @@ -153,31 +154,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(); @@ -193,8 +194,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 + } } ); @@ -203,7 +210,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; @@ -215,7 +222,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; @@ -228,7 +235,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_transfer; @@ -237,7 +244,7 @@ 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' ); # set hold found=T $hold->set_processing(); @@ -252,7 +259,7 @@ subtest 'suspend() tests' => sub { $holds_module->mock( 'is_found', 1 ); # bad data case - $hold->found('X'); + $hold->status('cancelled'); throws_ok { $hold->suspend_hold } 'Koha::Exceptions::Hold::CannotSuspendFound', @@ -260,8 +267,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' ); @@ -273,8 +280,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 3e736f6774..4ce352b20c 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -48,7 +48,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'); @@ -98,10 +98,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->created_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"); @@ -118,21 +118,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, @@ -144,8 +144,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', @@ -158,15 +158,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, @@ -175,8 +175,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, @@ -184,8 +184,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( @@ -198,26 +198,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->search({ itemnumber => $itemnumber })->count, 1, "Test ModReserveMinusPriority()" ); +is( $holds->search({ item_id => $itemnumber })->count, 1, "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" ); @@ -317,7 +317,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' }); @@ -332,9 +332,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}, @@ -349,17 +349,15 @@ ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for d # Items that are not for loan, but holdable should not be trapped until they are available for loan t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 0 ); Koha::Items->find($itemnumber)->damaged(0)->notforloan(-1)->store; -Koha::Holds->search({ biblionumber => $biblio->id })->delete(); +Koha::Holds->search({ biblio_id => $biblio->id })->delete(); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can place hold on item that is not for loan but holdable ( notforloan < 0 )" ); $hold = Koha::Hold->new( { - borrowernumber => $borrowernumbers[0], - itemnumber => $itemnumber, - biblionumber => $biblio->biblionumber, - found => undef, - priority => 1, - reservedate => dt_from_string, - branchcode => $branch_1, + patron_id => $borrowernumbers[0], + item_id => $itemnumber, + biblio_id => $biblio->biblionumber, + priority => 1, + pickup_library_id => $branch_1, } )->store(); ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); @@ -458,7 +456,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'); @@ -502,7 +500,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; @@ -530,7 +528,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 ); @@ -715,7 +713,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->created_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 a815713d43..0a4ff17221 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -290,10 +290,10 @@ my $itemtype2 = $builder->build( my $item3 = $builder->build_sample_item( { itype => $itemtype2 } ); my $hold = $builder->build( - { source => 'Reserve', + { source => 'Hold', value => { - itemnumber => $item3->itemnumber, - found => 'T' + item_id => $item3->itemnumber, + status => 'in_transit' } } ); diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t index 13958e6241..69a6a20ef8 100755 --- a/t/db_dependent/Holds/HoldItemtypeLimit.t +++ b/t/db_dependent/Holds/HoldItemtypeLimit.t @@ -57,7 +57,7 @@ my $right_itemtype = $itemtype1->{itemtype}; my $wrong_itemtype = $itemtype2->{itemtype}; my $borrowernumber = $borrower->{borrowernumber}; my $branchcode = $library->{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"); diff --git a/t/db_dependent/Holds/LocalHoldsPriority.t b/t/db_dependent/Holds/LocalHoldsPriority.t index 26c052b6cd..942f9c5435 100755 --- a/t/db_dependent/Holds/LocalHoldsPriority.t +++ b/t/db_dependent/Holds/LocalHoldsPriority.t @@ -85,29 +85,29 @@ 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[2], "Received expected results with HomeLibrary/holdingbranch" ); +ok( $reserve->{patron_id} eq $borrowernumbers[2], "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[3], "Received expected results with HomeLibrary/homebranch" ); +ok( $reserve->{patron_id} eq $borrowernumbers[3], "Received expected results with HomeLibrary/homebranch" ); $schema->storage->txn_rollback; @@ -208,4 +208,4 @@ subtest "exclude from local holds" => sub { is($reserve->{borrowernumber}, $patron_nex_l2->borrowernumber, "Patron from other library is next checkout because item is excluded"); $schema->storage->txn_rollback; -}; \ No newline at end of file +}; 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 50b6ed8573..920bd188d7 100755 --- 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(); @@ -77,16 +77,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}, + created_date => $reserve1_reservedate->ymd, + expiration_date => undef, + biblio_id => $biblio->biblionumber, + pickup_library_id => 'LIB1', priority => 1, - found => '', + status => 'placed', + completed => 0 }, }); @@ -94,44 +95,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}, + created_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}, + created_date => $reserve1_reservedate->ymd, + expiration_date => undef, + biblio_id => $biblio3->biblionumber, + pickup_library_id => 'LIB1', priority => 1, - found => '', + status => 'placed', + completed => 0 }, }); @@ -174,29 +177,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}, + created_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 4ffac93358..7a44340fe7 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 created_date = DATE_SUB( created_date, INTERVAL 1 DAY )"); # Tests my $use_cost_matrix_sth = $dbh->prepare("UPDATE systempreferences SET value = ? WHERE variable = 'UseTransportCostMatrix'"); @@ -288,14 +288,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 + created_date ) VALUES ( ?,?,?,?, CURRENT_DATE() ) }); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); @@ -383,7 +383,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 ); @@ -401,7 +401,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 ); @@ -443,7 +443,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 ); @@ -460,7 +460,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 ); @@ -477,7 +477,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 ); @@ -500,7 +500,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"); @@ -565,7 +565,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"); @@ -618,7 +618,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"); @@ -828,7 +828,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"); @@ -1204,7 +1204,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 37fd3fbdfb..7e46aa6a03 100755 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -326,11 +326,12 @@ subtest 'Holds test' => sub { $item->damaged(0)->store; my $hold = $builder->build({ - source => 'Reserve', + source => 'Hold', value => { - borrowernumber => $patron->{borrowernumber}, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber + patron_id => $patron->{borrowernumber}, + biblio_id => $item->biblionumber, + item_id => $item->itemnumber, + completed => 0 } }); @@ -533,15 +534,15 @@ subtest 'Holds test for branch transfer limits' => sub { $reply = C4::ILSDI::Services::HoldItem( $query ); is( $reply->{code}, undef, "Item hold, Item can be transferred" ); - my $hold = Koha::Holds->search({ itemnumber => $item->itemnumber, borrowernumber => $patron->{borrowernumber} })->next; - is( $hold->branchcode, $pickup_branch->{branchcode}, 'The library id is correctly set' ); + my $hold = Koha::Holds->search({ item_id => $item->itemnumber, patron_id => $patron->{borrowernumber} })->next; + is( $hold->pickup_library_id, $pickup_branch->{branchcode}, 'The library id is correctly set' ); Koha::Holds->search()->delete(); $reply = C4::ILSDI::Services::HoldTitle( $query ); is( $reply->{code}, undef, "Record hold, Item con be transferred" ); - $hold = Koha::Holds->search({ biblionumber => $item->biblionumber, borrowernumber => $patron->{borrowernumber} })->next; - is( $hold->branchcode, $pickup_branch->{branchcode}, 'The library id is correctly set' ); + $hold = Koha::Holds->search({ biblio_id => $item->biblionumber, patron_id => $patron->{borrowernumber} })->next; + is( $hold->pickup_library_id, $pickup_branch->{branchcode}, 'The library id is correctly set' ); $schema->storage->txn_rollback; }; @@ -587,19 +588,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->created_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->created_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 5e3ea7834c..be122c2189 100755 --- a/t/db_dependent/Items/GetItemsForInventory.t +++ b/t/db_dependent/Items/GetItemsForInventory.t @@ -112,7 +112,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 75ae92afe0..a698792320 100755 --- a/t/db_dependent/Items/MoveItemFromBiblio.t +++ b/t/db_dependent/Items/MoveItemFromBiblio.t @@ -43,18 +43,18 @@ my $item3 = $builder->build_sample_item( my $bib_level_hold_not_to_move = $builder->build( - { source => 'Reserve', - value => { biblionumber => $from_biblio->biblionumber, }, + { source => 'Hold', + value => { biblio_id => $from_biblio->biblionumber, }, } ); 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 }, } ); 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 }, } ); @@ -73,13 +73,13 @@ is( $get_item2->biblionumber, $to_biblio->biblionumber, 'The item2 should have b 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 994bd9611b..22eae0a870 100755 --- 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 @@ -95,25 +95,25 @@ subtest 'holds + current_holds' => sub { }; subtest 'waiting_or_in_transit' => sub { + plan tests => 4; + my $item = $builder->build_sample_item; - my $reserve = $builder->build({ - source => 'Reserve', + my $hold = $builder->build_object({ + class => 'Koha::Holds', value => { - biblionumber => $item->biblionumber, - found => undef + biblio_id => $item->biblionumber, } }); - $reserve = Koha::Holds->find($reserve->{reserve_id}); $biblio = $item->biblio; is($biblio->has_items_waiting_or_intransit, 0, 'Item is neither waiting nor in transit'); - $reserve->found('W')->store; + $hold->set_waiting(); is($biblio->has_items_waiting_or_intransit, 1, 'Item is waiting'); - $reserve->found('T')->store; + $hold->set_transfer(); is($biblio->has_items_waiting_or_intransit, 1, 'Item is in transit'); my $transfer = $builder->build({ @@ -125,7 +125,7 @@ subtest 'waiting_or_in_transit' => sub { } }); my $t = Koha::Database->new()->schema()->resultset( 'Branchtransfer' )->find($transfer->{branchtransfer_id}); - $reserve->found(undef)->store; + $hold->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 9c88f1049e..5ec23a4bbd 100755 --- a/t/db_dependent/Koha/Club/Hold.t +++ b/t/db_dependent/Koha/Club/Hold.t @@ -131,7 +131,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 1163bac03d..5eaf13c42b 100755 --- a/t/db_dependent/Koha/Holds.t +++ b/t/db_dependent/Koha/Holds.t @@ -61,7 +61,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 } } ); @@ -102,7 +104,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]; @@ -116,14 +118,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 @@ -166,7 +172,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( { @@ -176,12 +182,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 { @@ -208,50 +215,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' - ); - }; - }; subtest 'cancel with reason' => sub { diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index 115be2ab58..b44130ceb9 100755 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -1267,14 +1267,14 @@ subtest 'holds' => sub { biblionumber => $biblio->biblionumber, }); 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 b78283ce25..ac22c5b0fc 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -272,7 +272,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' => {} }; @@ -297,7 +297,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 afb00ee142..f10b781fcc 100755 --- a/t/db_dependent/Koha/Objects.t +++ b/t/db_dependent/Koha/Objects.t @@ -338,7 +338,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 } } ); @@ -347,7 +347,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 } } ); @@ -391,7 +391,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 c2824082e7..e10787acfb 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -468,8 +468,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( @@ -484,9 +484,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| ); @@ -992,8 +992,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; @@ -1605,7 +1605,7 @@ subtest 'BorrowersLog tests' => sub { $schema->storage->txn_rollback; subtest 'Test Koha::Patrons::merge' => sub { - plan tests => 113; + plan tests => 109; my $schema = Koha::Database->new()->schema(); @@ -1903,7 +1903,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/Z3950Responder/Session.t b/t/db_dependent/Koha/Z3950Responder/Session.t index 279aafe197..c91d398f09 100755 --- 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 ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); my $item_marc_1 = C4::Items::GetMarcItem( $item_1->biblionumber, $item_1->itemnumber ); my $item_field_1 = scalar $item_marc_1->field($itemtag); - $builder->build({ source => 'Reserve', value=> { itemnumber => $item_1->itemnumber } }); + $builder->build({ source => 'Hold', value=> { item_id => $item_1->itemnumber } }); $builder->build( { source => 'Branchtransfer', diff --git a/t/db_dependent/Koha/Z3950Responder/Session2.t b/t/db_dependent/Koha/Z3950Responder/Session2.t index e6e8e23862..41b2e49996 100755 --- a/t/db_dependent/Koha/Z3950Responder/Session2.t +++ b/t/db_dependent/Koha/Z3950Responder/Session2.t @@ -55,7 +55,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 } }); $builder->build( { source => 'Branchtransfer', diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t index 2489ebe35c..4051da1242 100755 --- 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 } } ); @@ -197,12 +197,15 @@ $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 + }, }, ) ); is( $prepared_letter->{content}, $hold->id(), 'Hold object used correctly for content' ); -is( $prepared_letter->{title}, $hold->borrowernumber, 'Hold object used correctly for title' ); +is( $prepared_letter->{title}, $hold->patron_id, 'Hold object used correctly for title' ); eval { $prepared_letter = GetPreparedLetter( @@ -210,13 +213,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( @@ -228,8 +231,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, } ) ); diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 2b47ebce5e..2bc44c547a 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -122,7 +122,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"); @@ -266,14 +266,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)); @@ -311,7 +311,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)'); @@ -323,19 +323,18 @@ 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"); -$holds->next->reserve_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) # Test 9761a: Add a reserve without date, CheckReserve should return it -$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); +$dbh->do("DELETE FROM holds WHERE biblio_id=?",undef,($bibnum)); AddReserve( { branchcode => $branch_1, @@ -403,7 +402,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', ); @@ -431,7 +430,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({ created_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)); @@ -445,11 +444,11 @@ AddReserve( itemnumber => $item->itemnumber, } ); #item level hold -$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); +$future_holds = $holds->search({ created_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({ created_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 @@ -543,17 +542,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, @@ -565,7 +564,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 @@ -633,7 +632,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( @@ -654,7 +653,7 @@ AddReserve( borrowernumber => $borrowernumber, biblionumber => $bibnum, priority => 1, - found => 'W', + found => 'waiting', } ); MoveReserve( $item->itemnumber, $borrowernumber ); @@ -828,13 +827,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; @@ -850,11 +849,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' ); @@ -871,21 +870,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' ); @@ -931,6 +930,9 @@ subtest 'reserves.item_level_hold' => sub { subtest 'item level hold' => sub { plan tests => 2; + + $schema->storage->txn_begin; + my $reserve_id = AddReserve( { branchcode => $item->homebranch, @@ -942,20 +944,19 @@ 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 ); + ModReserveAffect( $item->itemnumber, $patron->borrowernumber ); # Revert the waiting status C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } ); - $hold = Koha::Holds->find($reserve_id); + $hold->discard_changes; - 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 + $schema->storage->txn_rollback; }; subtest 'biblio level hold' => sub { @@ -970,19 +971,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 ); + ModReserveAffect( $item->itemnumber, $patron->borrowernumber ); $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; }; @@ -1020,16 +1021,16 @@ 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,$reserve_1)); # 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"); }; @@ -1085,18 +1086,12 @@ subtest 'CheckReserves additional test' => sub { { class => "Koha::Holds", value => { - found => undef, priority => 1, - itemnumber => undef, - biblionumber => $item->biblionumber, - waitingdate => undef, - cancellationdate => undef, - item_level_hold => 0, - lowestPriority => 0, - expirationdate => undef, - suspend_until => undef, - suspend => 0, - itemtype => undef, + item_id => undef, + biblio_id => $item->biblionumber, + item_level => 0, + lowest_priority => 0, + expiration_date => undef, } } ); @@ -1104,19 +1099,13 @@ subtest 'CheckReserves additional test' => sub { { class => "Koha::Holds", value => { - found => undef, priority => 2, - biblionumber => $item->biblionumber, - borrowernumber => $reserve1->borrowernumber, - itemnumber => undef, - waitingdate => undef, - cancellationdate => undef, - item_level_hold => 0, - lowestPriority => 0, - expirationdate => undef, - suspend_until => undef, - suspend => 0, - itemtype => undef, + item_id => $item->biblionumber, + patron_id => $reserve1->patron_id, + item_id => undef, + suspended_until => undef, + suspended => 0, + item_type => undef, } } ); @@ -1125,8 +1114,8 @@ subtest 'CheckReserves additional test' => sub { { source => 'TmpHoldsqueue', value => { - borrowernumber => $reserve1->borrowernumber, - biblionumber => $reserve1->biblionumber, + borrowernumber => $reserve1->patron_id, + biblionumber => $reserve1->biblio_id, } } ); @@ -1134,22 +1123,22 @@ subtest 'CheckReserves additional test' => sub { { source => 'HoldFillTarget', value => { - borrowernumber => $reserve1->borrowernumber, - biblionumber => $reserve1->biblionumber, + borrowernumber => $reserve1->patron_id, + biblionumber => $reserve1->biblio_id, itemnumber => $item->itemnumber, item_level_request => 0, } } ); - ModReserveAffect( $item->itemnumber, $reserve1->borrowernumber, 1, - $reserve1->reserve_id ); + ModReserveAffect( $item->itemnumber, $reserve1->patron_id, 1, + $reserve1->id ); my ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item->itemnumber ); is( $status, 'Reserved', "We found a reserve" ); is( $matched_reserve->{reserve_id}, - $reserve1->reserve_id, "We got the Transit reserve" ); + $reserve1->id, "We got the Transit reserve" ); is( scalar @$possible_reserves, 2, 'We do get both reserves' ); }; diff --git a/t/db_dependent/Reserves/AutoUnsuspendReserves.t b/t/db_dependent/Reserves/AutoUnsuspendReserves.t index d5703cc4e9..f44fe520b8 100755 --- 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, + completed_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, + completed_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, + completed_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, + completed_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 6822a04358..6dafd98c7d 100755 --- 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 => { + created_date => $reserve_reservedate, + expiration_date => $reserve1_expirationdate, + completed_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 => { + created_date => $reserve_reservedate, + expiration_date => $reserve2_expirationdate, + completed_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 => { + created_date => $reserve_reservedate, + expiration_date => $reserve2_expirationdate, + pickup_library_id => 'LIB1', + completed_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 { @@ -119,6 +129,7 @@ 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 ); +<<<<<<< HEAD my $hold1 = Koha::Hold->new({ branchcode => $branchcode, borrowernumber => $borrowernumber, @@ -149,15 +160,53 @@ subtest 'Test handling of waiting reserves by CancelExpiredReserves' => sub { expirationdate => $expdate, found => 'W', })->store; +======= + my $hold1 = Koha::Hold->new( + { + pickup_library_id => $branchcode, + patron_id => $borrowernumber, + biblio_id => $bibnum, + priority => 1, + created_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, + created_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, + created_date => $resdate, + expiration_date => $expdate, + status => 'waiting', + } + )->store; +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) 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'); }; @@ -169,33 +218,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 bb9b4dacba..786d459ca0 100755 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ b/t/db_dependent/Reserves/GetReserveFee.t @@ -119,19 +119,31 @@ subtest 'Integration with AddReserve' => sub { plan tests => 3; t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); +<<<<<<< HEAD $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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'); +<<<<<<< HEAD $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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'); +<<<<<<< HEAD $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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' ); @@ -143,20 +155,33 @@ 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'); +<<<<<<< HEAD $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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' ); +<<<<<<< HEAD $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron3->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); # FIXME Are we sure it's the expected behavior? 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' ); +<<<<<<< HEAD C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + C4::Circulation::AddIssue( $patron3, $item2->{barcode}, '2015-12-31', 0, undef, 0, {} ); + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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' ); @@ -167,7 +192,11 @@ subtest 'Integration with AddIssue' => sub { plan tests => 5; $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); +<<<<<<< HEAD $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); +======= + $dbh->do( "DELETE FROM holds WHERE biblio_id=?", undef, $biblio->{biblionumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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 6e88740702..85b88bae6c 100755 --- a/t/db_dependent/Reserves/MultiplePerRecord.t +++ b/t/db_dependent/Reserves/MultiplePerRecord.t @@ -184,25 +184,36 @@ is( $max, 9, 'GetMaxPatronHoldsForRecord returns max of 9 because Library specif 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({ +<<<<<<< HEAD borrowernumber => $patron->{borrowernumber}, reservedate => '1981-06-10', biblionumber => $biblio->biblionumber, branchcode => $library->{branchcode}, +======= + patron_id => $patron->{borrowernumber}, + created_date => '1981-06-10', + biblio_id => $biblio->{biblionumber}, + pickup_library_id => $library->{branchcode}, +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) 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" ); +<<<<<<< HEAD $hold->itemnumber( $item1->itemnumber ); +======= +$hold->item_id( $item1->{itemnumber} ); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) $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(); @@ -247,8 +258,13 @@ 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' ); +<<<<<<< HEAD Koha::Holds->find($hold_id)->found("W")->store; $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->biblionumber); +======= +Koha::Holds->find($hold_id)->set_waiting(); +$can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber}); +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) is( $can->{status}, 'tooManyHoldsForThisRecord', 'Third hold exceeds limit of holds per record' ); $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->biblionumber, undef, { ignore_found_holds => 1 }); diff --git a/t/db_dependent/Reserves/ReserveSlip.t b/t/db_dependent/Reserves/ReserveSlip.t index 03b5731644..dbd7be38f7 100755 --- 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( @@ -66,24 +66,41 @@ my $item2 = $builder->build_sample_item( my $hold1 = Koha::Hold->new( { +<<<<<<< HEAD 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}, +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) } )->store; my $hold2 = Koha::Hold->new( { +<<<<<<< HEAD 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}, +>>>>>>> 324514e305 (Bug 25260: Adapt all the things) } )->store; + my $letter = $builder->build( { source => 'Letter', diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t index e2fbba7a9b..85a4cd5b03 100755 --- 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 befce88e5c..c943fac81d 100755 --- a/t/db_dependent/XSLT.t +++ b/t/db_dependent/XSLT.t @@ -97,10 +97,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 41cf559ca9..1b9b31976d 100755 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -164,7 +164,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 { @@ -331,7 +331,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 }), + created_date => output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 }), priority => 2, }; @@ -351,7 +351,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('/created_date', output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 })); }; subtest 'test AllowHoldPolicyOverride' => sub { @@ -416,7 +416,12 @@ 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, + waiting_date => undef, + suspended_until_date => undef, + completed => 0 + } } ); @@ -427,7 +432,7 @@ subtest 'suspend and resume tests' => sub { $hold->discard_changes; # refresh object ok( $hold->is_suspended, 'Hold is suspended' ); - $t->json_is('/end_date', undef, 'Hold suspension has no end date'); + $t->json_is('/suspended_until_date', undef, 'Hold suspension has no end date'); my $end_date = output_pref({ dt => dt_from_string( undef ), @@ -443,7 +448,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 }), @@ -476,13 +481,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_transfer->discard_changes; $t->post_ok( "//$userid:$password@/api/v1/holds/" . $hold->id . "/suspension" ) ->status_is( 400, 'Cannot suspend hold on transfer' ) - ->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; }; @@ -769,20 +774,23 @@ subtest 'pickup_locations() tests' => sub { { class => 'Koha::Holds', value => { - itemnumber => undef, - biblionumber => $item->biblionumber, - borrowernumber => $patron->borrowernumber + item_id => undef, + biblio_id => $item->biblionumber, + patron_id => $patron->borrowernumber, + completed => 0, } } ); + # item-level hold my $hold_2 = $builder->build_object( { class => 'Koha::Holds', value => { - itemnumber => $item->itemnumber, - biblionumber => $item->biblionumber, - borrowernumber => $patron->borrowernumber + item_id => $item->itemnumber, + biblio_id => $item->biblionumber, + patron_id => $patron->borrowernumber, + completed => 0, } } ); @@ -859,10 +867,11 @@ subtest 'edit() tests' => sub { { class => "Koha::Holds", value => { - biblionumber => $biblio->biblionumber, - branchcode => $library_3->branchcode, - itemnumber => undef, - priority => 1, + biblio_id => $biblio->biblionumber, + pickup_library_id => $library_3->branchcode, + item_id => undef, + priority => 1, + completed => 0, } } ); @@ -877,7 +886,7 @@ subtest 'edit() tests' => sub { ->json_is({ error => 'The supplied pickup location is not valid' }); $biblio_hold->discard_changes; - is( $biblio_hold->branchcode, $library_3->branchcode, 'branchcode remains untouched' ); + is( $biblio_hold->pickup_library_id, $library_3->branchcode, 'pickup_library_id remains untouched' ); $biblio_hold_data->{pickup_library_id} = $library_2->branchcode; $t->put_ok( "//$userid:$password@/api/v1/holds/" @@ -886,17 +895,18 @@ subtest 'edit() tests' => sub { ->status_is(200); $biblio_hold->discard_changes; - is( $biblio_hold->branchcode, $library_2->id, 'Pickup location changed correctly' ); + is( $biblio_hold->pickup_library_id, $library_2->id, 'Pickup location changed correctly' ); # Test item-level holds my $item_hold = $builder->build_object( { class => "Koha::Holds", value => { - biblionumber => $biblio->biblionumber, - branchcode => $library_3->branchcode, - itemnumber => $item->itemnumber, - priority => 1, + biblio_id => $biblio->biblionumber, + pickup_library_id => $library_3->branchcode, + item_id => $item->itemnumber, + priority => 1, + completed => 0, } } ); @@ -911,7 +921,7 @@ subtest 'edit() tests' => sub { ->json_is({ error => 'The supplied pickup location is not valid' }); $item_hold->discard_changes; - is( $item_hold->branchcode, $library_3->branchcode, 'branchcode remains untouched' ); + is( $item_hold->pickup_library_id, $library_3->branchcode, 'branchcode remains untouched' ); $item_hold_data->{pickup_library_id} = $library_2->branchcode; $t->put_ok( "//$userid:$password@/api/v1/holds/" @@ -920,7 +930,7 @@ subtest 'edit() tests' => sub { ->status_is(200); $item_hold->discard_changes; - is( $item_hold->branchcode, $library_2->id, 'Pickup location changed correctly' ); + is( $item_hold->pickup_library_id, $library_2->id, 'Pickup location changed correctly' ); $schema->storage->txn_rollback; }; @@ -977,10 +987,10 @@ subtest 'add() tests' => sub { { class => "Koha::Holds", value => { - biblionumber => $biblio->biblionumber, - branchcode => $library_3->branchcode, - itemnumber => undef, - priority => 1, + biblio_id => $biblio->biblionumber, + pickup_library_id => $library_3->branchcode, + item_id => undef, + priority => 1, } } ); @@ -1003,10 +1013,10 @@ subtest 'add() tests' => sub { { class => "Koha::Holds", value => { - biblionumber => $biblio->biblionumber, - branchcode => $library_3->branchcode, - itemnumber => $item->itemnumber, - priority => 1, + biblio_id => $biblio->biblionumber, + pickup_library_id => $library_3->branchcode, + item_id => $item->itemnumber, + priority => 1, } } ); -- 2.20.1