From 939f1f389b848e06b7adcd7121ff7629a0ba4adf Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 2 Mar 2023 17:01:07 +0000 Subject: [PATCH] Bug 32565: Add unallocated option to holds queue Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected To test: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Confirm the second hold is added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 Signed-off-by: Pedro Amorim Signed-off-by: Katrin Fischer --- C4/HoldsQueue.pm | 38 +++++++++++++++++------- misc/cronjobs/holds/build_holds_queue.pl | 14 +++++++-- t/db_dependent/HoldsQueue.t | 2 +- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index ef021eaf48..c775421054 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -168,10 +168,14 @@ Top level function that turns reserves into tmp_holdsqueue and hold_fill_targets =cut sub CreateQueue { + my $params = shift; + my $unallocated = $params->{unallocated}; my $dbh = C4::Context->dbh; - $dbh->do("DELETE FROM tmp_holdsqueue"); # clear the old table for new info - $dbh->do("DELETE FROM hold_fill_targets"); + unless( $unallocated ){ + $dbh->do("DELETE FROM tmp_holdsqueue"); # clear the old table for new info + $dbh->do("DELETE FROM hold_fill_targets"); + } my $total_bibs = 0; my $total_requests = 0; @@ -201,6 +205,7 @@ sub CreateQueue { { biblio_id => $biblionumber, branches_to_use => $branches_to_use, transport_cost_matrix => $transport_cost_matrix, + unallocated => $unallocated } ); @@ -228,6 +233,7 @@ sub GetBibsWithPendingHoldRequests { AND priority > 0 AND reservedate <= CURRENT_DATE() AND suspend = 0 + AND reserve_id NOT IN (SELECT reserve_id FROM hold_fill_targets) "; my $sth = $dbh->prepare($bib_query); @@ -239,10 +245,10 @@ sub GetBibsWithPendingHoldRequests { =head2 GetPendingHoldRequestsForBib - my $requests = GetPendingHoldRequestsForBib($biblionumber); + my $requests = GetPendingHoldRequestsForBib({ biblionumber => $biblionumber, unallocated => $unallocated }); Returns an arrayref of hashrefs to pending, unfilled hold requests -on the bib identified by $biblionumber. The following keys +on the bib identified by $biblionumber. Optionally returns only unallocated holds. The following keys are present in each hashref: biblionumber @@ -259,7 +265,9 @@ The arrayref is sorted in order of increasing priority. =cut sub GetPendingHoldRequestsForBib { - my $biblionumber = shift; + my $params = shift; + my $biblionumber = $params->{biblionumber}; + my $unallocated = $params->{unallocated}; my $dbh = C4::Context->dbh; @@ -271,8 +279,9 @@ sub GetPendingHoldRequestsForBib { AND found IS NULL AND priority > 0 AND reservedate <= CURRENT_DATE() - AND suspend = 0 - ORDER BY priority"; + AND suspend = 0 "; + $request_query .= "AND reserve_id NOT IN (SELECT reserve_id FROM hold_fill_targets) " if $unallocated; + $request_query .= "ORDER BY priority"; my $sth = $dbh->prepare($request_query); $sth->execute($biblionumber); @@ -330,10 +339,15 @@ sub GetItemsAvailableToFillHoldRequestsForBib { AND itemnumber IS NOT NULL AND (found IS NOT NULL OR priority = 0) ) + AND items.itemnumber NOT IN ( + SELECT itemnumber + FROM tmp_holdsqueue + WHERE biblionumber = ? + ) AND items.biblionumber = ? AND branchtransfers.itemnumber IS NULL"; - my @params = ($biblionumber, $biblionumber); + my @params = ($biblionumber, $biblionumber, $biblionumber); if ($branches_to_use && @$branches_to_use) { $items_query .= " AND holdingbranch IN (" . join (",", map { "?" } @$branches_to_use) . ")"; push @params, @$branches_to_use; @@ -1120,7 +1134,8 @@ sub least_cost_branch { biblio_id => $biblio_id, [ branches_to_use => $branches_to_use, transport_cost_matrix => $transport_cost_matrix, - delete => $delete, ] + delete => $delete, + unallocated => $unallocated, ] } ); @@ -1151,6 +1166,9 @@ It return a hashref containing: =item I tells the method to delete prior entries on the related tables for the biblio_id. +=item I tells the method to limit the holds to those not in the holds queue, should not + be passed at the same time as delete. + =back Note: All the optional parameters will be calculated in the method if omitted. They @@ -1171,7 +1189,7 @@ sub update_queue_for_biblio { $dbh->do("DELETE FROM hold_fill_targets WHERE biblionumber=$biblio_id"); } - my $hold_requests = GetPendingHoldRequestsForBib($biblio_id); + my $hold_requests = GetPendingHoldRequestsForBib({ biblionumber => $biblio_id, unallocated => $args->{unallocated} }); $result->{requests} = scalar( @{$hold_requests} ); # No need to check anything else if there are no holds to fill return $result unless $result->{requests}; diff --git a/misc/cronjobs/holds/build_holds_queue.pl b/misc/cronjobs/holds/build_holds_queue.pl index 76c61a8e94..06b02041e5 100755 --- a/misc/cronjobs/holds/build_holds_queue.pl +++ b/misc/cronjobs/holds/build_holds_queue.pl @@ -41,9 +41,15 @@ Print a brief help message and exits. Prints the manual page and exits. -=item B<--force> +=item b<--force> -Allows this script to rebuild the entire holds queue even if the RealTimeHoldsQueue system preference is enabled. +allows this script to rebuild the entire holds queue even if the realtimeholdsqueue system preference is enabled. + +=item b<--unallocated> + +prevents deletion of current queue and allows the script to only deal with holds not currently in the queue. +This is useful when using the realtimeholdsqueue and skipping closed libraries, or allowing holds in the future +This allows the script to catch holds that may have become active without triggering a real time update. =back @@ -56,6 +62,7 @@ This script builds or rebuilds the entire holds queue. my $help = 0; my $man = 0; my $force = 0; +my $unallocated = 0; my $command_line_options = join( " ", @ARGV ); @@ -63,6 +70,7 @@ GetOptions( 'h|help' => \$help, 'm|man' => \$man, 'f|force' => \$force, + 'u|unallocated' => \$unallocated ); pod2usage(1) if $help; pod2usage( -exitval => 0, -verbose => 2 ) if $man; @@ -77,6 +85,6 @@ if ( $rthq && !$force ) { cronlogaction( { info => $command_line_options } ); -CreateQueue(); +CreateQueue({ unallocated => $unallocated }); cronlogaction( { action => 'End', info => "COMPLETED" } ); diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 87da308bdc..d0c9a74642 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -1710,7 +1710,7 @@ subtest "Test _checkHoldPolicy" => sub { } ); ok( $reserve_id, "Hold was created"); - my $requests = C4::HoldsQueue::GetPendingHoldRequestsForBib($biblio->biblionumber); + my $requests = C4::HoldsQueue::GetPendingHoldRequestsForBib({ biblionumber => $biblio->biblionumber}); is( @$requests, 1, "Got correct number of holds"); my $request = $requests->[0]; -- 2.39.5