From 88616aa00b2e10cfd3f037c6a9ab5df84a01d783 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 | 45 +- C4/Items.pm | 5 +- C4/Letters.pm | 10 +- C4/Members.pm | 2 +- C4/Reserves.pm | 527 +++++++++--------- C4/SIP/ILS/Item.pm | 15 +- 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 | 31 +- Koha/Holds.pm | 31 +- Koha/Item.pm | 20 +- Koha/Patron.pm | 12 +- Koha/REST/V1/Auth.pm | 20 +- Koha/REST/V1/Holds.pm | 30 +- Koha/Template/Plugin/Biblio.pm | 11 +- api/v1/swagger/definitions/hold.json | 14 +- api/v1/swagger/paths/holds.json | 12 +- circ/circulation.pl | 2 +- circ/returns.pl | 2 +- .../mysql/en/mandatory/sample_notices.yml | 6 +- .../prog/en/modules/circ/article-requests.tt | 2 +- .../prog/en/modules/members/holdshistory.tt | 32 +- koha-tmpl/intranet-tmpl/prog/js/holds.js | 2 +- .../bootstrap/en/modules/opac-holdshistory.tt | 38 +- members/holdshistory.pl | 12 +- .../thirdparty/TalkingTech_itiva_outbound.pl | 10 +- reserve/request.pl | 61 +- t/Koha/Exceptions.t | 4 +- t/Koha/REST/Plugin/Query.t | 4 +- t/db_dependent/Circulation.t | 63 ++- t/db_dependent/Circulation/issue.t | 5 +- t/db_dependent/DecreaseLoanHighHolds.t | 18 +- t/db_dependent/Hold.t | 161 +++--- t/db_dependent/Holds.t | 108 ++-- .../Holds/DisallowHoldIfItemsAvailable.t | 6 +- t/db_dependent/Holds/HoldItemtypeLimit.t | 2 +- t/db_dependent/Holds/LocalHoldsPriority.t | 16 +- t/db_dependent/Holds/RevertWaitingStatus.t | 3 +- t/db_dependent/Holds/WaitingReserves.t | 96 ++-- t/db_dependent/HoldsQueue.t | 43 +- t/db_dependent/ILSDI_Services.t | 37 +- t/db_dependent/Items/GetItemsForInventory.t | 2 +- t/db_dependent/Items/MoveItemFromBiblio.t | 24 +- t/db_dependent/Koha/Acquisition/Order.t | 2 +- t/db_dependent/Koha/Biblios.t | 18 +- t/db_dependent/Koha/Club/Hold.t | 2 +- t/db_dependent/Koha/Hold.t | 2 +- t/db_dependent/Koha/Holds.t | 79 +-- 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 | 34 +- t/db_dependent/Reserves.t | 157 +++--- .../Reserves/AutoUnsuspendReserves.t | 32 +- .../Reserves/CancelExpiredReserves.t | 230 ++++---- t/db_dependent/Reserves/GetReserveFee.t | 14 +- t/db_dependent/Reserves/MultiplePerRecord.t | 18 +- t/db_dependent/Reserves/ReserveSlip.t | 27 +- t/db_dependent/TestBuilder.t | 17 +- t/db_dependent/XSLT.t | 9 +- t/db_dependent/api/v1/holds.t | 100 ++-- 67 files changed, 1201 insertions(+), 1145 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 10e17c045d..08fd28aba3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1097,7 +1097,7 @@ sub CanBookBeIssued { # See if the item is on reserve. my ( $restype, $res ) = C4::Reserves::CheckReserves( $item_object->itemnumber ); if ($restype) { - my $resbor = $res->{'borrowernumber'}; + my $resbor = $res->{patron_id}; if ( $resbor ne $patron->borrowernumber ) { my $patron = Koha::Patrons->find( $resbor ); if ( $restype eq "Waiting" ) @@ -1109,9 +1109,9 @@ sub CanBookBeIssued { $needsconfirmation{'ressurname'} = $patron->surname; $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $res->{branchcode}; - $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'}; - $needsconfirmation{'reserve_id'} = $res->{reserve_id}; + $needsconfirmation{'resbranchcode'} = $res->{pickup_library_id}; + $needsconfirmation{'reswaitingdate'} = $res->{waiting_date}; + $needsconfirmation{'reserve_id'} = $res->{id}; } elsif ( $restype eq "Reserved" ) { # The item is on reserve for someone else. @@ -1121,8 +1121,8 @@ sub CanBookBeIssued { $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{reservedate}; - $needsconfirmation{'reserve_id'} = $res->{reserve_id}; + $needsconfirmation{'resreservedate'} = $res->{created_date}; + $needsconfirmation{'reserve_id'} = $res->{id}; } elsif ( $restype eq "Transferred" ) { # The item is determined hold being transferred for someone else. @@ -1132,8 +1132,8 @@ sub CanBookBeIssued { $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{reservedate}; - $needsconfirmation{'reserve_id'} = $res->{reserve_id}; + $needsconfirmation{'resreservedate'} = $res->{created_date}; + $needsconfirmation{'reserve_id'} = $res->{id}; } elsif ( $restype eq "Processing" ) { # The item is determined hold being processed for someone else. @@ -1143,8 +1143,8 @@ sub CanBookBeIssued { $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{reservedate}; - $needsconfirmation{'reserve_id'} = $res->{reserve_id}; + $needsconfirmation{'resreservedate'} = $res->{created_date}; + $needsconfirmation{'reserve_id'} = $res->{id}; } } } @@ -1287,7 +1287,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(); @@ -1508,10 +1508,10 @@ sub AddIssue { } )->store; if ( $transfer->reason && $transfer->reason eq 'Reserve' ) { - my $hold = $item_object->holds->search( { found => 'T' } )->next; + my $hold = $item_object->holds->search( { status => 'in_transit' } )->next; if ( $hold ) { # Is this really needed? - $hold->set( { found => undef } )->store; - C4::Reserves::ModReserveMinusPriority($item_object->itemnumber, $hold->reserve_id); + $hold->set( { status => 'placed' } )->store; + C4::Reserves::ModReserveMinusPriority($item_object->itemnumber, $hold->id); } } } @@ -2218,7 +2218,7 @@ sub AddReturn { my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn ); # if a hold is found and is waiting at another branch, change the priority back to 1 and trigger the hold (this will trigger a transfer and update the hold status properly) - if ( $resfound and $resfound eq "Waiting" and $branch ne $resrec->{branchcode} ) { + if ( $resfound and $resfound eq "Waiting" and $branch ne $resrec->{pickup_library_id} ) { my $hold = C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } ); $resfound = 'Reserved'; $resrec = $hold->unblessed; @@ -2864,11 +2864,15 @@ sub CanBookBeRenewed { } my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber); + use Data::Printer colored => 1; + p( $resfound ); + p( $resrec ); + p( $possible_reserves ); # 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 } ) + { biblio_id => $resrec->{biblio_id}, non_priority => 0, completed => 0 } ) ->count > 0; } @@ -2879,7 +2883,7 @@ sub CanBookBeRenewed { if ( $resfound && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) { - my $item_holds = Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count(); + my $item_holds = Koha::Holds->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; @@ -2888,14 +2892,14 @@ sub CanBookBeRenewed { # Get all other items that could possibly fill reserves my $items = Koha::Items->search({ - biblionumber => $resrec->{biblionumber}, + biblionumber => $resrec->{biblio_id}, onloan => undef, notforloan => 0, -not => { itemnumber => $itemnumber } }); # Get all other reserves that could have been filled by this item - my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves; + my @borrowernumbers = map { $_->{patron_id} } @$possible_reserves; my $patrons = Koha::Patrons->search({ borrowernumber => { -in => \@borrowernumbers } }); @@ -2930,7 +2934,6 @@ sub CanBookBeRenewed { } return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed - return ( 1, undef ); } @@ -3371,7 +3374,7 @@ sub GetIssuingCharges { $sth->execute($itemnumber); if ( my $item_data = $sth->fetchrow_hashref ) { $item_type = $item_data->{itemtype}; - $charge = $item_data->{rentalcharge}; + $charge = $item_data->{rentalcharge} // 0; # FIXME This should follow CircControl my $branch = C4::Context::mybranch(); my $patron = Koha::Patrons->find( $borrowernumber ); diff --git a/C4/Items.pm b/C4/Items.pm index 14774e48e1..decc4cbd44 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -602,8 +602,9 @@ sub GetItemsForInventory { } if ( $ignore_waiting_holds ) { - $query .= "LEFT JOIN reserves ON items.itemnumber = reserves.itemnumber "; - push( @where_strings, q{(reserves.found != 'W' OR reserves.found IS NULL)} ); + $query .= "LEFT JOIN holds ON items.itemnumber = holds.item_id "; + # holds.status IS NULL <- means the join doesn't match any hold to the item + push( @where_strings, q{(holds.status != 'waiting' OR holds.status IS NULL)} ); } if ( @$itemtypes ) { diff --git a/C4/Letters.pm b/C4/Letters.pm index 177a4f51f6..55e1f48ef6 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -786,7 +786,7 @@ sub _parseletter_sth { ($table eq 'items' ) ? "SELECT * FROM $table WHERE itemnumber = ?" : ($table eq 'issues' ) ? "SELECT * FROM $table WHERE itemnumber = ?" : ($table eq 'old_issues' ) ? "SELECT * FROM $table WHERE itemnumber = ? ORDER BY timestamp DESC LIMIT 1" : - ($table eq 'reserves' ) ? "SELECT * FROM $table WHERE borrowernumber = ? and biblionumber = ?" : + ($table eq 'holds' ) ? "SELECT * FROM $table WHERE patron_id = ? AND biblio_id = ?" : ($table eq 'borrowers' ) ? "SELECT * FROM $table WHERE borrowernumber = ?" : ($table eq 'branches' ) ? "SELECT * FROM $table WHERE branchcode = ?" : ($table eq 'suggestions' ) ? "SELECT * FROM $table WHERE suggestionid = ?" : @@ -834,8 +834,8 @@ sub _parseletter { $values->{'dateexpiry'} = output_pref({ dt => dt_from_string( $values->{'dateexpiry'} ), dateonly => 1 }); } - if ( $table eq 'reserves' && $values->{'waitingdate'} ) { - $values->{'waitingdate'} = output_pref({ dt => dt_from_string( $values->{'waitingdate'} ), dateonly => 1 }); + if ( $table eq 'holds' && $values->{waiting_date} ) { + $values->{waiting_date} = output_pref({ dt => dt_from_string( $values->{waiting_date} ), dateonly => 1 }); } if ($letter->{content} && $letter->{content} =~ /<>/) { @@ -1665,11 +1665,11 @@ sub _get_tt_params { plural => 'orders', pk => 'ordernumber', }, - reserves => { + holds => { module => 'Koha::Holds', singular => 'hold', plural => 'holds', - pk => 'reserve_id', + pk => 'id', }, serial => { module => 'Koha::Serials', diff --git a/C4/Members.pm b/C4/Members.pm index 73b254cb5e..5f24b917db 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -227,7 +227,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 54d2b00cb0..35d28bdd9e 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -37,12 +37,10 @@ use Koha::Calendar; use Koha::CirculationRules; use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); -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; @@ -190,7 +188,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}; @@ -221,7 +219,7 @@ sub AddReserve { && !$item->current_holds->count ) { $priority = 0; - $found = 'W'; + $found = 'waiting'; } } @@ -234,7 +232,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; } @@ -244,22 +242,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'); @@ -398,7 +395,7 @@ sub CanItemBeReserved { # Check that the patron doesn't have an item level hold on this item already return { status =>'itemAlreadyOnHold' } - if ( !$params->{ignore_hold_counts} && Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count() ); + if ( !$params->{ignore_hold_counts} && 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') @@ -410,15 +407,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"; @@ -455,10 +452,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 '' ){ @@ -471,8 +469,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 (!$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne '' @@ -528,7 +527,8 @@ sub CanItemBeReserved { if (!$params->{ignore_hold_counts} && $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(); @@ -606,10 +606,11 @@ sub CanReserveBeCanceledFromOpac { my ($reserve_id, $borrowernumber) = @_; return unless $reserve_id and $borrowernumber; - my $reserve = Koha::Holds->find($reserve_id) or return; + my $hold = Koha::Holds->find($reserve_id); + return unless $hold; - 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 @@ -627,19 +628,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' ), ; @@ -650,9 +651,9 @@ sub GetOtherReserves { $messages->{'waiting'} = 1; ModReserveMinusPriority( $itemnumber, - $checkreserves->{'reserve_id'}, + $checkreserves->{id}, ); - ModReserveStatus($itemnumber,'W'); + ModReserveStatus($itemnumber,'waiting'); } $nextreservinfo = $checkreserves; @@ -743,25 +744,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 } @@ -800,7 +796,7 @@ sub CheckReserves { my $sth; my $select; if (C4::Context->preference('item-level_itypes')){ - $select = " + $select = " SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, @@ -815,7 +811,7 @@ sub CheckReserves { "; } else { - $select = " + $select = " SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, @@ -868,11 +864,11 @@ sub CheckReserves { my $priority = 10000000; foreach my $res (@reserves) { - if ($res->{'found'} && $res->{'found'} eq 'W') { + if ($res->{status} eq 'waiting') { return ( "Waiting", $res, \@reserves ); # Found it, it is waiting - } elsif ($res->{'found'} && $res->{'found'} eq 'P') { + } elsif ($res->{status} eq 'processing') { return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one - } elsif ($res->{'found'} && $res->{'found'} eq 'T') { + } elsif ($res->{status} eq 'in_transit') { return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one } else { my $patron; @@ -880,7 +876,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) { @@ -888,7 +884,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; @@ -899,10 +895,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'} eq 'not_allowed'); @@ -910,10 +906,10 @@ sub CheckReserves { my $library = Koha::Libraries->find({branchcode=>$item->homebranch}); next if (($branchitemrule->{'holdallowed'} eq 'from_local_hold_group') && (!$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; @@ -947,20 +943,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 ); @@ -971,14 +967,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; } @@ -1038,10 +1034,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); @@ -1050,21 +1046,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(); @@ -1076,7 +1072,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(); } } @@ -1098,37 +1094,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 } ); } } @@ -1150,7 +1146,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 ); @@ -1196,15 +1192,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(); @@ -1214,7 +1210,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; @@ -1238,9 +1234,9 @@ sub ModReserveAffect { AND q.holdingbranch = t.source_branchcode WHERE t.reserve_id = ? }); - $std->execute($hold->reserve_id); + $std->execute($hold->id); - logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->get_from_storage->unblessed) ) + logaction( 'HOLDS', 'MODIFY', $hold->id, Dumper($hold->get_from_storage->unblessed) ) if C4::Context->preference('HoldsLog'); return; @@ -1257,15 +1253,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} ); } @@ -1279,17 +1275,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' }); } @@ -1433,8 +1429,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; } @@ -1466,7 +1462,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' }); @@ -1526,9 +1522,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); @@ -1588,15 +1584,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 @@ -1606,10 +1598,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 ); @@ -1618,10 +1610,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); @@ -1635,7 +1628,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; } @@ -1651,25 +1644,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 }); @@ -1706,29 +1699,30 @@ 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.id=hold_fill_targets.reserve_id) + 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); @@ -1736,35 +1730,36 @@ 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.id=hold_fill_targets.reserve_id) + 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); @@ -1772,30 +1767,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); @@ -1803,7 +1799,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; } @@ -1839,7 +1835,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 ); @@ -1851,21 +1847,21 @@ sub _koha_notify_reserve { message_name => 'Hold_Filled' } ); - my $library = Koha::Libraries->find( $hold->branchcode ); + my $library = Koha::Libraries->find( $hold->pickup_library_id ); my $admin_email_address = $library->from_email_address; $library = $library->unblessed; 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, - 'reserves' => $hold->unblessed, - 'items' => $hold->itemnumber, + 'biblio' => $hold->biblio_id, + 'biblioitems' => $hold->biblio_id, + 'holds' => $hold->unblessed, + 'items' => $hold->item_id, }, ); @@ -1929,7 +1925,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; @@ -1937,20 +1933,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 @@ -1974,9 +1970,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 { @@ -1985,8 +1981,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(); @@ -2000,7 +1996,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; } } @@ -2017,33 +2013,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++; } @@ -2066,36 +2060,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; } @@ -2128,27 +2120,26 @@ 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 $reserve = $hold->unblessed; + my $patron = $hold->patron; - return C4::Letters::GetPreparedLetter ( - module => 'circulation', + return C4::Letters::GetPreparedLetter( + module => 'circulation', letter_code => 'HOLD_SLIP', - branchcode => $branchcode, - lang => $patron->lang, - tables => { - 'reserves' => $reserve, - 'branches' => $reserve->{branchcode}, - 'borrowers' => $reserve->{borrowernumber}, - 'biblio' => $reserve->{biblionumber}, - 'biblioitems' => $reserve->{biblionumber}, - 'items' => $reserve->{itemnumber}, + branchcode => $branchcode, + lang => $patron->lang, + tables => { + 'holds' => $hold->unblessed, + 'branches' => $hold->pickup_library_id, + 'borrowers' => $hold->patron_id, + 'biblio' => $hold->biblio_id, + 'biblioitems' => $hold->biblio_id, + 'items' => $hold->item_id, }, ); } @@ -2202,17 +2193,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( @@ -2226,26 +2217,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/Item.pm b/C4/SIP/ILS/Item.pm index 48df606cad..a9ae49e8c9 100644 --- a/C4/SIP/ILS/Item.pm +++ b/C4/SIP/ILS/Item.pm @@ -105,7 +105,18 @@ sub new { $self->{borrowernumber} = $patron->borrowernumber; } my $biblio = Koha::Biblios->find( $self->{biblionumber} ); - my $holds = $biblio->current_holds->unblessed; + + # Holds + my $view_query = q{ + SELECT * + FROM + reserves + WHERE + biblionumber = ? + }; + + my $holds = C4::Context->dbh->selectall_arrayref( $view_query, { 'Slice' => {} }, $biblio->biblionumber ); + $self->{hold_queue} = $holds; $self->{hold_attached} = [( grep { defined $_->{found} and ( $_->{found} eq 'W' or $_->{found} eq 'P' or $_->{found} eq 'T' ) } @{$self->{hold_queue}} )]; $self->{pending_queue} = [( grep {(! defined $_->{found}) or ( $_->{found} ne 'W' and $_->{found} ne 'P' and $_->{found} ne 'T' ) } @{$self->{hold_queue}} )]; @@ -353,7 +364,7 @@ sub recall_date { sub hold_pickup_date { my $self = shift; - my $hold = Koha::Holds->find({ itemnumber => $self->{itemnumber}, found => 'W' }); + my $hold = Koha::Holds->find({ item_id => $self->{itemnumber}, status => 'waiting' }); if ( $hold ) { return $hold->expirationdate || 0; } diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index e40dbdbf39..4838b325aa 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -520,11 +520,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 82fddf1e97..4eea0b5ad7 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 245d76b74f..2ab6a2063c 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -315,7 +315,7 @@ sub buildKohaItemsNamespace { $items_rs = Koha::Items->new; } - my $items = $items_rs->search( $query, { prefetch => [ 'branchtransfers', 'reserves' ] } ); + my $items = $items_rs->search( $query, { prefetch => [ 'branchtransfers', 'holds' ] } ); my $shelflocations = { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.location' } ) }; diff --git a/Koha/Acquisition/Order.pm b/Koha/Acquisition/Order.pm index 090920d92c..9495e83d6f 100644 --- a/Koha/Acquisition/Order.pm +++ b/Koha/Acquisition/Order.pm @@ -298,7 +298,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 26ee7abd96..dcf1605e54 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -444,7 +444,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); } @@ -461,7 +462,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 @@ -522,8 +523,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 aae7386d75..a03113b938 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -368,7 +368,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. @@ -377,8 +377,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 @@ -590,11 +593,11 @@ sub store { if ( !$self->in_storage ) { if ( C4::Context->preference('DefaultHoldExpirationdate') - and ( not defined $self->expirationdate - or $self->expirationdate eq '' ) + and ( not defined $self->expiration_date + or $self->expiration_date eq '' ) ) { - $self->_set_default_expirationdate; + $self->_set_default_expiration_date; } } else { @@ -602,15 +605,15 @@ sub store { my %updated_columns = $self->_result->get_dirty_columns; return $self->SUPER::store unless %updated_columns; - if ( exists $updated_columns{reservedate} ) { + if ( exists $updated_columns{created_date} ) { if ( C4::Context->preference('DefaultHoldExpirationdate') - and ( not exists $updated_columns{expirationdate} - or exists $updated_columns{expirationdate} - and $updated_columns{expirationdate} eq '' ) + and ( not exists $updated_columns{expiration_date} + or exists $updated_columns{expiration_date} + and $updated_columns{expiration_date} eq '' ) ) { - $self->_set_default_expirationdate; + $self->_set_default_expiration_date; } } } @@ -618,15 +621,15 @@ sub store { $self = $self->SUPER::store; } -sub _set_default_expirationdate { +sub _set_default_expiration_date { my $self = shift; my $period = C4::Context->preference('DefaultHoldExpirationdatePeriod') || 0; my $timeunit = C4::Context->preference('DefaultHoldExpirationdateUnitOfTime') || 'days'; - $self->expirationdate( - dt_from_string( $self->reservedate )->add( $timeunit => $period ) ); + $self->expiration_date( + dt_from_string( $self->created_date )->add( $timeunit => $period ) ); } =head3 to_api_mapping diff --git a/Koha/Holds.pm b/Koha/Holds.pm index 05edfe5f49..26cfaed4ca 100644 --- a/Koha/Holds.pm +++ b/Koha/Holds.pm @@ -36,6 +36,26 @@ Koha::Holds - Koha Hold object set class =cut + +=head3 filter_by_current + + my $holds_rs = Koha::Holds->filter_by_current( ... ); + +Filter the resultset by current (i.e. completed = 0). + +=cut + +sub filter_by_current { + my ( $self ) = @_; + + return $self->search( + { + completed => 0 + } + ); +} + + =head3 waiting returns a set of holds that are waiting from an existing set @@ -121,7 +141,7 @@ Items that are not: sub get_items_that_can_fill { my ( $self ) = @_; - my @biblionumbers = $self->get_column('biblionumber'); + my @biblionumbers = $self->get_column('biblio_id'); my @branchtransfers = map { $_->itemnumber } Koha::Item::Transfers->search( @@ -131,11 +151,14 @@ sub get_items_that_can_fill { collapse => 1, } ); - my @waiting_holds = map { $_->itemnumber } + my @waiting_holds = map { $_->item_id } Koha::Holds->search( - { 'found' => 'W' }, { - columns => ['itemnumber'], + status => 'waiting', + completed => 0 + }, + { + columns => ['item_id'], collapse => 1, } ); diff --git a/Koha/Item.pm b/Koha/Item.pm index c653dc2583..9862b091d1 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -281,7 +281,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; @@ -290,7 +290,7 @@ sub safe_to_delete { if $self->biblio->items->count == 1 && $self->biblio->holds->search( { - itemnumber => undef, + item_id => undef, } )->count; @@ -397,7 +397,7 @@ sub checkout { my $holds = $item->holds(); my $holds = $item->holds($params); -my $holds = $item->holds({ found => 'W'}); +my $holds = $item->holds({ status => 'waiting'}); Return holds attached to an item, optionally accept a hashref of params to pass to search @@ -405,7 +405,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 ); } @@ -789,14 +790,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 941dace0df..320938fe59 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -71,9 +71,8 @@ our $RESULTSET_PATRON_ID_MAPPING = { Message => 'borrowernumber', MessageQueue => 'borrowernumber', OldIssue => 'borrowernumber', - OldReserve => 'borrowernumber', Rating => 'borrowernumber', - Reserve => 'borrowernumber', + Hold => 'patron_id', Review => 'borrowernumber', SearchHistory => 'userid', Statistic => 'borrowernumber', @@ -1205,8 +1204,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); } @@ -1220,8 +1220,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 2361aae361..73a133b448 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -318,10 +318,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": { @@ -385,7 +385,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 } ) { @@ -449,20 +449,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 7541a5c653..285cb70f73 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -45,7 +45,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 ); } @@ -76,13 +76,13 @@ 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}; my $overrides = $c->stash('koha.overrides'); my $can_override = $overrides->{any} && C4::Context->preference('AllowHoldPolicyOverride'); - 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" } @@ -194,7 +194,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, @@ -248,7 +248,7 @@ sub edit { return try { my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find( $hold_id ); + my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( @@ -277,17 +277,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 +313,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 +342,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 +356,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 +390,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 +416,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 +464,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 2fe5b6de96..676591a016 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" @@ -45,7 +45,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", @@ -74,7 +74,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" @@ -90,6 +90,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)" } }, "additionalProperties": false diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json index ea5245a08e..ede93d52ea 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -99,7 +99,7 @@ "type": "boolean" }, { - "name": "suspended_until", + "name": "suspended_until_date", "in": "query", "description": "Suspended until", "type": "string" @@ -228,7 +228,11 @@ "non_priority": { "description": "Set this hold as non priority", "type": [ "boolean", "null" ] - } + }, + "created_date": { + "type": "string", + "format": "date", + "description": "The date the hold was placed" } }, "required": [ "patron_id", "pickup_library_id" ], "additionalProperties": false @@ -320,7 +324,7 @@ "description": "Internal library identifier for the pickup library", "type": "string" }, - "suspended_until": { + "suspended_until_date": { "description": "Date until which the hold has been suspended", "type": "string", "format": "date-time" @@ -407,7 +411,7 @@ "description": "Internal library identifier for the pickup library", "type": "string" }, - "suspended_until": { + "suspended_until_date": { "description": "Date until which the hold has been suspended", "type": "string", "format": "date-time" diff --git a/circ/circulation.pl b/circ/circulation.pl index 7988782577..b3bdad18ab 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -448,7 +448,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/circ/returns.pl b/circ/returns.pl index 914d5de35c..bcf3306f8b 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -335,7 +335,7 @@ if ($barcode) { if (C4::Context->preference("WaitingNotifyAtCheckin") ) { #Check for waiting holds my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); - my $waiting_holds = $patron->holds->search({ found => 'W', branchcode => $userenv_branch })->count; + my $waiting_holds = $patron->holds->waiting->search({ pickup_library_id => $userenv_branch })->count; if ($waiting_holds > 0) { $template->param( waiting_holds => $waiting_holds, diff --git a/installer/data/mysql/en/mandatory/sample_notices.yml b/installer/data/mysql/en/mandatory/sample_notices.yml index db40a3a4cc..6a03192dc7 100644 --- a/installer/data/mysql/en/mandatory/sample_notices.yml +++ b/installer/data/mysql/en/mandatory/sample_notices.yml @@ -459,10 +459,10 @@ tables: - "
    " - "
  • <>
  • " - "
  • <>
  • " - - "
  • <>
  • " + - "
  • <>
  • " - "
" - "

Notes:" - - "

<>
" + - "
<>
" - "

" - "" @@ -1261,7 +1261,7 @@ tables: message_transport_type: email lang: default content: - - "Dear [% borrower.firstname %] [% borrower.surname %],\n\nThe following holds are waiting at [% branch.branchname %]:\n\n[% FOREACH hold IN holds %]\n [% hold.biblio.title %] : waiting since [% hold.waitingdate | $KohaDates %]\n[% END %]" + - "Dear [% borrower.firstname %] [% borrower.surname %],\n\nThe following holds are waiting at [% branch.branchname %]:\n\n[% FOREACH hold IN holds %]\n [% hold.biblio.title %] : waiting since [% hold.waiting_date | $KohaDates %]\n[% END %]" - module: serial code: SERIAL_ALERT diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/article-requests.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/article-requests.tt index 61e94ca026..90e1d4bee0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/article-requests.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/article-requests.tt @@ -53,7 +53,7 @@ [% END %] [% BLOCK item_status %] [% IF myitem.onloan %]Checked out - [% ELSIF myitem.holds({ found => ['W', 'T'] }).count > 0 %]On hold + [% ELSIF myitem.holds({ status => ['waiting', 'in_transit'] }).count > 0 %]On hold [% ELSIF myitem.notforloan > 0 OR myitem.itemtype.notforloan %]Not for loan [% ELSIF myitem.notforloan < 0 %]On order [% END %] 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 bd2aad334f..058c269aa3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -74,41 +74,41 @@ [% 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 %] - - [% hold.expirationdate | $KohaDates %] + [% Branches.GetName( hold.pickup_library_id ) | html %] + [% hold.created_date | $KohaDates %] + + [% hold.expiration_date | $KohaDates %] - - [% hold.waitingdate | $KohaDates %] + + [% hold.waiting_date | $KohaDates %] - - [% hold.cancellationdate | $KohaDates %] + + [% hold.completed_date | $KohaDates %] [% 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 [% IF hold.cancellation_reason %] ([% AuthorisedValues.GetByCode('HOLD_CANCELLATION', hold.cancellation_reason) | html %]) [% END %] - [% ELSIF hold.found == 'W' %] + [% ELSIF hold.status == 'waiting' %] Waiting - [% ELSIF hold.found == 'P' %] + [% ELSIF hold.status == 'processing' %] Processing - [% ELSIF hold.found == 'T' %] + [% ELSIF hold.status == 'in_transit' %] In transit - [% ELSE %] + [% ELSIF hold.status == 'placed' %] Pending [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/js/holds.js b/koha-tmpl/intranet-tmpl/prog/js/holds.js index e076fa5536..998bd5aca0 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/holds.js +++ b/koha-tmpl/intranet-tmpl/prog/js/holds.js @@ -77,7 +77,7 @@ $(document).ready(function() { }, { "mDataProp": function( oObj ) { - if( oObj.branches.length > 1 && oObj.found !== 'W' && oObj.found !== 'T' ){ + if( oObj.branches.length > 1 && oObj.status !== 'waiting' && oObj.status !== 'in_transit' ){ var branchSelect='