From ba1b3d52dda0f64a16c4fc90b736b0c2c4e0d9c4 Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Thu, 25 Feb 2021 13:07:12 -0300 Subject: [PATCH] Bug 23678: Allow cancel holds in bulk This patch allows staff patrons to cancel multiple holds in bulk. To test: 1. Apply this patch 2. restart_all 3. In cataloge go to a book and place many holds CHECK => Holds table shows a column of checkboxes 4. Play with checkboxes (have some fun ;-P) CHECK => When you manually check all checkboxes, the checkbox in the header also gets checked. => When you uncheck one of the checkboxes, the one in the header also gets unchecked. => If no checkbox is checked and you check the one in the header, all checkboxes get checked. => If there are some checkboxes that are checked and others are not, when you click on the checkbox in the header all checkboxes get unchecked. => If all checkboxes are checked, when you uncheck the one in the header, all checkboxes get unchecked. => Every time you play with checkboxes, the number in the button "Cancel selected" changes. 5. Check some of the checkboxes and click on cancel selected. SUCCESS => A background job gets fired to cancel all selected holds. => A message should appear with a link to the job. 6. Wait a few seconds and click on the link SUCCESS => A message appears with the report of the execution of the background job. 7. Grab a patron and search to hold 8. Select multiple biblios and click on "place hold for " CHECK => After holds are confirmed, multiple holds table are shown.. one for each record. Checkboxes work exactly the same as before, but scoped for each individual table. Checkboxes from one table will not affect checkboxes from other tables. 9. Repeat steps 4 to 6. 10. Check In some of the items so the get in Waiting state. 11. Update expirationdate os some of those holds and set it to ReservesMaxPickUpDelay + 1 days earlier NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...) 12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs. Signed-off-by: Tomas Cohen Arazi --- Koha/BackgroundJob.pm | 3 + Koha/BackgroundJob/BatchCancelHold.pm | 144 ++++++++++++++++++ circ/waitingreserves.pl | 17 +++ .../prog/en/includes/holds_table.inc | 4 +- .../prog/en/includes/waiting_holds.inc | 8 +- .../prog/en/modules/admin/background_jobs.tt | 32 ++++ .../prog/en/modules/circ/waitingreserves.tt | 123 ++++++++++++++- .../prog/en/modules/reserve/request.tt | 74 ++++++++- misc/background_jobs_worker.pl | 2 +- reserve/request.pl | 14 ++ 10 files changed, 409 insertions(+), 12 deletions(-) create mode 100644 Koha/BackgroundJob/BatchCancelHold.pm diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index c303fa175e..0c2c89536c 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -26,6 +26,7 @@ use Koha::DateUtils qw( dt_from_string ); use Koha::Exceptions; use Koha::BackgroundJob::BatchUpdateBiblio; use Koha::BackgroundJob::BatchUpdateAuthority; +use Koha::BackgroundJob::BatchCancelHold; use base qw( Koha::Object ); @@ -155,6 +156,8 @@ sub process { ? Koha::BackgroundJob::BatchUpdateBiblio->process($args) : $job_type eq 'batch_authority_record_modification' ? Koha::BackgroundJob::BatchUpdateAuthority->process($args) + : $job_type eq 'batch_hold_cancel' + ? Koha::BackgroundJob::BatchCancelHold->process($args) : Koha::Exceptions::Exception->throw('->process called without valid job_type'); } diff --git a/Koha/BackgroundJob/BatchCancelHold.pm b/Koha/BackgroundJob/BatchCancelHold.pm new file mode 100644 index 0000000000..47426f8455 --- /dev/null +++ b/Koha/BackgroundJob/BatchCancelHold.pm @@ -0,0 +1,144 @@ +package Koha::BackgroundJob::BatchCancelHold; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; +use JSON qw( encode_json decode_json ); + +use Koha::BackgroundJobs; +use Koha::DateUtils qw( dt_from_string ); +use Koha::Holds; + +use base 'Koha::BackgroundJob'; + +=head1 NAME + +Koha::BackgroundJob::BatchCancelHold - Batch cancel holds + +This is a subclass of Koha::BackgroundJob. + +=head1 API + +=head2 Class methods + +=head3 job_type + +Define the job type of this job: batch_hold_cancel + +=cut + +sub job_type { + return 'batch_hold_cancel'; +} + +=head3 process + +Process the modification. + +=cut + +sub process { + my ( $self, $args ) = @_; + + my $job = Koha::BackgroundJobs->find( $args->{job_id} ); + + if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) { + return; + } + + my $job_progress = 0; + $job->started_on(dt_from_string) + ->progress($job_progress) + ->status('started') + ->store; + + my @hold_ids = @{ $args->{hold_ids} }; + + my $report = { + total_holds => scalar @hold_ids, + total_success => 0, + }; + my @messages; + HOLD_IDS: for my $hold_id ( sort { $a <=> $b } @hold_ids ) { + next unless $hold_id; + # Authorities + my ($hold, $patron, $biblio); + $hold = Koha::Holds->find($hold_id); + + my $error = eval { + $patron = $hold->patron; + $biblio = $hold->biblio; + $hold->cancel({cancellation_reason => $args->{reason}}); + }; + + if ( $error and $error != $hold or $@ ) { + push @messages, { + type => 'error', + code => 'hold_not_cancelled', + patron_id => defined $patron?$patron->borrowernumber:'', + patron_name => defined $patron?($patron->firstname?$patron->firstname.', ':'').$patron->surname:'', + biblio_id => defined $biblio?$biblio->biblionumber:'', + biblio_title => defined $biblio?$biblio->title:'', + hold_id => $hold_id, + error => defined $hold?($@ ? $@ : 0):'No hold with id '.$hold_id.' found', + }; + } else { + push @messages, { + type => 'success', + code => 'hold_cancelled', + patron_id => $patron->borrowernumber, + patron_name => ($patron->firstname?$patron->firstname.', ':'').$patron->surname, + biblio_id => $biblio->biblionumber, + biblio_title => $biblio->title, + hold_id => $hold_id, + }; + $report->{total_success}++; + } + $job->progress( ++$job_progress )->store; + } + + my $job_data = decode_json $job->data; + $job_data->{messages} = \@messages; + $job_data->{report} = $report; + + $job->ended_on(dt_from_string) + ->data(encode_json $job_data); + $job->status('finished') if $job->status ne 'cancelled'; + $job->store; + +} + +=head3 enqueue + +Enqueue the new job + +=cut + +sub enqueue { + my ( $self, $args) = @_; + + # TODO Raise exception instead + return unless exists $args->{hold_ids}; + + my @hold_ids = @{ $args->{hold_ids} }; + + $self->SUPER::enqueue({ + job_size => scalar @hold_ids, + job_args => {hold_ids => \@hold_ids, reason => $args->{reason}} + }); +} + +1; diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl index c9d4e84e82..ccbab78a2b 100755 --- a/circ/waitingreserves.pl +++ b/circ/waitingreserves.pl @@ -31,6 +31,7 @@ use Koha::BiblioFrameworks; use Koha::Items; use Koha::ItemTypes; use Koha::Patrons; +use Koha::BackgroundJob::BatchCancelHold; my $input = CGI->new; @@ -41,6 +42,7 @@ my $tbr = $input->param('tbr') || ''; my $all_branches = $input->param('allbranches') || ''; my $cancelall = $input->param('cancelall'); my $tab = $input->param('tab'); +my $cancelBulk = $input->param('cancelBulk'); my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( { @@ -71,6 +73,21 @@ if ( C4::Context->preference('IndependentBranches') ) { } $template->param( all_branches => 1 ) if $all_branches; +if ( $cancelBulk ) { + my $cancellation_reason = $input->param("cancellation-reason"); + my @hold_ids = split ',', $input->param("ids"); + my $params = { + reason => $cancellation_reason, + hold_ids => \@hold_ids, + }; + my $job_id = Koha::BackgroundJob::BatchCancelHold->new->enqueue($params); + + $template->param( + enqueued => 1, + job_id => $job_id + ) +} + my (@reserve_loop, @over_loop); # FIXME - Is priority => 0 useful? If yes it must be moved to waiting, otherwise we need to remove it from here. my $holds = Koha::Holds->waiting->search({ priority => 0, ( $all_branches ? () : ( branchcode => $default ) ) }, { order_by => ['waitingdate'] }); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index 59f33cd2a0..fee360c65c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -1,8 +1,9 @@ [% USE Koha %] [% SET hold_cancellation = AuthorisedValues.GetAuthValueDropbox('HOLD_CANCELLATION') %] [% USE AuthorisedValues %] - +
+ [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] @@ -28,6 +29,7 @@ [% FOREACH hold IN holds %] [% IF !hold.found && first_priority == 0 %][% first_priority = hold.priority %][% END %] +
Priority  
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc index bf47e822aa..27e7fb7724 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc @@ -1,8 +1,11 @@ [% USE ItemTypes %] [% USE AuthorisedValues %] - +
+ [% IF select_column %] + + [% END %] @@ -19,6 +22,9 @@ [% FOREACH reserveloo IN reserveloop %] + [% IF select_column %] + + [% END %]
Waiting since Date hold placed Title
[% reserveloo.waitingdate | $KohaDates %] [% reserveloo.reservedate | $KohaDates %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt index 64f2278c20..9470e5e5c5 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt @@ -108,6 +108,20 @@ Administration › Koha [% END %] [% END %] + [% CASE 'batch_hold_cancel' %] + [% SET report = job.report %] + [% IF report %] + [% IF report.total_holds == report.total_success %] +
+ All holds have successfully been cancelled! +
+ [% ELSE %] +
+ [% report.total_success | html %] / [% report.total_holds | html %] holds have successfully been modified. Some errors occurred. + [% IF job.status == 'cancelled' %]The job has been cancelled before it finished.[% END %] +
+ [% END %] + [% END %] [% CASE %]Job type "[% job.type | html %]" not handled in the template [% END %] @@ -149,6 +163,24 @@ Administration › Koha [% END %] [% END %] + [% CASE 'batch_hold_cancel' %] + [% FOR m IN job.messages %] +
+ [% IF m.type == 'success' %] + + [% ELSIF m.type == 'warning' %] + + [% ELSIF m.type == 'error' %] + + [% END %] + [% SWITCH m.code %] + [% CASE 'hold_not_cancelled' %] + [% UNLESS !m.biblio_id || !m.patron_id %] Hold on [% m.biblio_title | html %] for [% m.patron_name %] has not been cancelled. [% END %]An error occurred on cancelling.[% IF m.error %] ([% m.error | html %])[% END %]. + [% CASE 'hold_cancelled' %] + Hold on [% m.biblio_title | html %] for [% m.patron_name %] has successfully been cancelled. + [% END %] +
+ [% END %] [% CASE %]Job type "[% job.type | html %]" not handled in the template [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt index abee77bf56..5ee8efbb79 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt @@ -4,6 +4,7 @@ [% USE KohaDates %] [% USE Branches %] [% USE TablesSettings %] +[% USE AuthorisedValues %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] Holds awaiting pickup › Circulation › Koha @@ -72,6 +73,12 @@ [% END %] [% END %] [% ELSE %] + [% IF enqueued %] +
+

The job has been enqueued! It will be processed as soon as possible.

+

View detail of the enqueued job

+
+ [% END %]
[% IF ( reserveloop ) %] - [% INCLUDE waiting_holds.inc table_name='holdst' reserveloop=reserveloop tab='holdwaiting' %] +
+ +
+ [% INCLUDE waiting_holds.inc select_column='1' table_name='holdst' reserveloop=reserveloop tab='holdwaiting' %] [% ELSE %]
No holds found.
[% END %] @@ -92,6 +102,7 @@ [% IF ( ReservesMaxPickUpDelay ) %]

Holds listed here have been awaiting pickup for more than [% ReservesMaxPickUpDelay | html %] days.

[% END %] [% IF ( overloop ) %] +
@@ -105,8 +116,9 @@ [% UNLESS TransferWhenCancelAllWaitingHolds %] Only items that need not be transferred will be cancelled (TransferWhenCancelAllWaitingHolds syspref) [% END %] + - [% INCLUDE waiting_holds.inc table_name='holdso' reserveloop=overloop tab='holdsover' %] + [% INCLUDE waiting_holds.inc select_column='1' table_name='holdso' reserveloop=overloop tab='holdsover' %] [% ELSE %]
No holds found.
[% END %] @@ -128,25 +140,128 @@
+ + [% MACRO jsinclude BLOCK %] [% INCLUDE 'datatables.inc' %] [% INCLUDE 'columns_settings.inc' %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 5a8545c612..e27e941127 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -874,9 +874,16 @@ [% END %] + [% IF enqueued %] +
+

The job has been enqueued! It will be processed as soon as possible.

+

View detail of the enqueued job

+
+ [% END %] +

Existing holds

- +
[% SET hold_cancellation = AuthorisedValues.GetAuthValueDropbox('HOLD_CANCELLATION') %] [% IF hold_cancellation %] @@ -1081,6 +1088,8 @@ cannotBeTransferred: _("Cannot be transferred to pickup library"), pickupNotInHoldGroup: _("Only pickup locations within the same hold group are allowed") } + + var MSG_CANCEL_SELECTED = _("Cancel selected (%s)"); columns_settings_borrowers_table = [% TablesSettings.GetColumns( 'circ', 'circulation', 'table_borrowers', 'json' ) | $raw %]; $.fn.select2.defaults.set("width", "100%" ); $.fn.select2.defaults.set("dropdownAutoWidth", true ); @@ -1525,11 +1534,20 @@ return false; }); $("#cancelModalConfirmBtn").on("click",function(e) { - let borrowernumber = cancel_link.data('borrowernumber'); - let biblionumber = cancel_link.data('biblionumber'); - let reserve_id = cancel_link.data('id'); + let link; + if(cancel_link.data('bulk')) { + [% IF biblionumbers %] + link = `request.pl?biblionumbers=[% biblionumbers | url %]&action=cancelBulk&ids=${$('.holds_table .select_hold:checked').toArray().map(el => $(el).data('id')).join(',')}`; + [% ELSE %] + link = `request.pl?biblionumber=[% biblionumber | url %]&action=cancelBulk&ids=${$('.holds_table .select_hold:checked').toArray().map(el => $(el).data('id')).join(',')}`; + [% END %] + } else { + let borrowernumber = cancel_link.data('borrowernumber'); + let biblionumber = cancel_link.data('biblionumber'); + let reserve_id = cancel_link.data('id'); + link = `request.pl?action=cancel&borrowernumber=${ borrowernumber }&biblionumber=${ biblionumber }&reserve_id=${ reserve_id }`; + } let reason = $("#modal-cancellation-reason").val(); - let link = `request.pl?action=cancel&borrowernumber=${ borrowernumber }&biblionumber=${ biblionumber }&reserve_id=${ reserve_id }`; if ( reason ) { link += "&cancellation-reason=" + reason } @@ -1577,6 +1595,52 @@ stickTo: "#existing_holds", stickyClass: "floating" }); + + if(!localStorage.selectedHolds || document.referrer.replace(/\?.*/, '') !== document.location.origin+document.location.pathname) { + localStorage.selectedHolds = []; + } + + $('.holds_table .select_hold').each(function() { + if(localStorage.selectedHolds.includes($(this).data('id'))) { + $(this).prop('checked', true); + } + }); + + $('.holds_table .select_hold_all').each(function() { + var table = $(this).parents('.holds_table'); + var count = $('.select_hold:not(:checked)', table).length; + $('.select_hold_all', table).prop('checked', !count); + }); + + $('.cancel_selected_holds').html(MSG_CANCEL_SELECTED.format($('.holds_table .select_hold:checked').length)); + + $('.holds_table .select_hold_all').click(function() { + var table = $(this).parents('.holds_table'); + var count = $('.select_hold:checked', table).length; + $('.select_hold', table).prop('checked', !count); + $(this).prop('checked', !count); + $('.cancel_selected_holds').html(MSG_CANCEL_SELECTED.format($('.holds_table .select_hold:checked').length)); + localStorage.selectedHolds = $('.holds_table .select_hold:checked').toArray().map(el => $(el).data('id')); + }); + + $('.holds_table .select_hold').click(function() { + var table = $(this).parents('.holds_table'); + var count = $('.select_hold:not(:checked)', table).length; + $('.select_hold_all', table).prop('checked', !count); + $('.cancel_selected_holds').html(MSG_CANCEL_SELECTED.format($('.holds_table .select_hold:checked').length)); + localStorage.selectedHolds = $('.holds_table .select_hold:checked').toArray().map(el => $(el).data('id')); + }); + + $('.cancel_selected_holds').click(function(e) { + e.preventDefault(); + if($('.holds_table .select_hold:checked').length) { + cancel_link = $(this); + delete localStorage.selectedHolds; + $('#cancelModal').modal(); + } + return false; + }); + }); [% END %] diff --git a/misc/background_jobs_worker.pl b/misc/background_jobs_worker.pl index 7987289895..5f3e4ce13b 100755 --- a/misc/background_jobs_worker.pl +++ b/misc/background_jobs_worker.pl @@ -28,7 +28,7 @@ try { warn sprintf "Cannot connect to the message broker, the jobs will be processed anyway (%s)", $_; }; -my @job_types = qw( batch_biblio_record_modification batch_authority_record_modification ); +my @job_types = qw( batch_biblio_record_modification batch_authority_record_modification batch_hold_cancel ); if ( $conn ) { # FIXME cf note in Koha::BackgroundJob about $namespace diff --git a/reserve/request.pl b/reserve/request.pl index a6fbe528f4..363a51ec31 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -52,6 +52,7 @@ use Koha::ItemTypes; use Koha::Libraries; use Koha::Patrons; use Koha::Clubs; +use Koha::BackgroundJob::BatchCancelHold; my $dbh = C4::Context->dbh; my $input = CGI->new; @@ -113,6 +114,19 @@ if ( $action eq 'move' ) { my $reserve_id = $input->param('reserve_id'); my $suspend_until = $input->param('suspend_until'); ToggleSuspend( $reserve_id, $suspend_until ); +} elsif ( $action eq 'cancelBulk') { + my $cancellation_reason = $input->param("cancellation-reason"); + my @hold_ids = split ',', $input->param("ids"); + my $params = { + reason => $cancellation_reason, + hold_ids => \@hold_ids, + }; + my $job_id = Koha::BackgroundJob::BatchCancelHold->new->enqueue($params); + + $template->param( + enqueued => 1, + job_id => $job_id + ) } if ($findborrower) { -- 2.32.0