From 42114ec4be6a76f05df8b6cf2e7bb4c9f3aea106 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 Signed-off-by: Martin Renvoize Bug 23678: (QA follow-up) Add missing template filter Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Bug 23678: (QA follow-up) Add missing filters Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Bug 23678: (QA follow-up) Use correct indentation Signed-off-by: Tomas Cohen Arazi JD amended patch: also Koha/BackgroundJob/BatchCancelHold.pm --- Koha/BackgroundJob.pm | 3 + Koha/BackgroundJob/BatchCancelHold.pm | 154 ++++++++++++++++++ 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 | 70 +++++--- 10 files changed, 451 insertions(+), 36 deletions(-) create mode 100644 Koha/BackgroundJob/BatchCancelHold.pm diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index c303fa175ec..0c2c89536cb 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 00000000000..fb009483f67 --- /dev/null +++ b/Koha/BackgroundJob/BatchCancelHold.pm @@ -0,0 +1,154 @@ +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 c9d4e84e822..d71c53bb778 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 $reason = $input->param("cancellation-reason"); + my @hold_ids = split ',', $input->param("ids"); + my $params = { + reason => $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 ef3ca483324..516b04f3228 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 ) %] @@ -48,6 +49,7 @@ [%- this_priority = loop.count() - found_holds -%] [%- 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 bf47e822aaa..27e7fb77245 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 4e8c171623f..7170a2ff7c3 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 @@ -159,6 +159,20 @@ [% 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 %] @@ -200,6 +214,24 @@ [% 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 | html %] 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 | html %] 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 abee77bf569..5ee8efbb79d 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 e238ff26160..14cc35a3190 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -885,9 +885,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 %] @@ -1092,6 +1099,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 ); @@ -1556,11 +1565,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 } @@ -1608,6 +1626,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 79872898954..5f3e4ce13b5 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 95259814bfa..95a7fd43d1a 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; @@ -89,30 +90,51 @@ my $action = $input->param('action'); $action ||= q{}; if ( $action eq 'move' ) { - my $where = $input->param('where'); - my $reserve_id = $input->param('reserve_id'); - my $prev_priority = $input->param('prev_priority'); - my $next_priority = $input->param('next_priority'); - my $first_priority = $input->param('first_priority'); - my $last_priority = $input->param('last_priority'); - my $hold_itemnumber = $input->param('itemnumber'); - if ( $prev_priority == 0 && $next_priority == 1 ){ - C4::Reserves::RevertWaitingStatus({ itemnumber => $hold_itemnumber }); - } else { - AlterPriority( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority ); - } -} elsif ( $action eq 'cancel' ) { - my $reserve_id = $input->param('reserve_id'); - my $cancellation_reason = $input->param("cancellation-reason"); - my $hold = Koha::Holds->find( $reserve_id ); - $hold->cancel({ cancellation_reason => $cancellation_reason }) if $hold; -} elsif ( $action eq 'setLowestPriority' ) { - my $reserve_id = $input->param('reserve_id'); - ToggleLowestPriority( $reserve_id ); -} elsif ( $action eq 'toggleSuspend' ) { - my $reserve_id = $input->param('reserve_id'); - my $suspend_until = $input->param('suspend_until'); - ToggleSuspend( $reserve_id, $suspend_until ); + my $where = $input->param('where'); + my $reserve_id = $input->param('reserve_id'); + my $prev_priority = $input->param('prev_priority'); + my $next_priority = $input->param('next_priority'); + my $first_priority = $input->param('first_priority'); + my $last_priority = $input->param('last_priority'); + my $hold_itemnumber = $input->param('itemnumber'); + if ( $prev_priority == 0 && $next_priority == 1 ) { + C4::Reserves::RevertWaitingStatus( { itemnumber => $hold_itemnumber } ); + } + else { + AlterPriority( + $where, $reserve_id, $prev_priority, + $next_priority, $first_priority, $last_priority + ); + } +} +elsif ( $action eq 'cancel' ) { + my $reserve_id = $input->param('reserve_id'); + my $cancellation_reason = $input->param("cancellation-reason"); + my $hold = Koha::Holds->find($reserve_id); + $hold->cancel( { cancellation_reason => $cancellation_reason } ) if $hold; +} +elsif ( $action eq 'setLowestPriority' ) { + my $reserve_id = $input->param('reserve_id'); + ToggleLowestPriority($reserve_id); +} +elsif ( $action eq 'toggleSuspend' ) { + 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.25.1