From 506754b526b8914be6b8b68bff9d53a45c6233b1 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Thu, 28 Mar 2024 08:19:20 +0200 Subject: [PATCH] Bug 36135: Fix QA issues and add improvements This patch: - Fixes issues reported by qa-tool - Adds info about how many holds were modified - Add title and barcode columns to table of modified holds - Adds div with class "page-section" to "Modify holds" table Sponsored-by: Koha-Suomi Oy --- .../data/mysql/atomicupdate/bug_36135.pl | 10 +- .../en/modules/tools/batch_modify_holds.tt | 128 +++++++++--------- tools/batch_modify_holds.pl | 85 ++++++------ 3 files changed, 115 insertions(+), 108 deletions(-) diff --git a/installer/data/mysql/atomicupdate/bug_36135.pl b/installer/data/mysql/atomicupdate/bug_36135.pl index 4bd58cbac7..40aa318e1e 100755 --- a/installer/data/mysql/atomicupdate/bug_36135.pl +++ b/installer/data/mysql/atomicupdate/bug_36135.pl @@ -1,13 +1,15 @@ use Modern::Perl; return { - bug_number => "36135", + bug_number => "36135", description => "Add new permission batch_modify_holds", - up => sub { + up => sub { my ($args) = @_; - my ($dbh, $out) = @$args{qw(dbh out)}; + my ( $dbh, $out ) = @$args{qw(dbh out)}; - $dbh->do(q{INSERT IGNORE permissions (module_bit, code, description) VALUES (13, 'batch_modify_holds', 'Perform batch modification of holds')}); + $dbh->do( + q{INSERT IGNORE permissions (module_bit, code, description) VALUES (13, 'batch_modify_holds', 'Perform batch modification of holds')} + ); say $out "Added new permission 'batch_modify_holds'"; }, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt index d9a6ddef49..9ccba1fc48 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt @@ -48,15 +48,12 @@ NOTE! Column patron_expiration_date is used as a main filter for expiration date field. Otherwise column expirationdate is used.
  1. - -
  2. - - -
    3. -
  3. - - -
    4. + + + +
  4. + +
  5. @@ -82,15 +79,12 @@
  6. - -
  7. - - -
    8. -
  8. - - -
    9. + + + +
  9. + +
  10. @@ -108,7 +102,7 @@
    [% INCLUDE 'csrf-token.inc' %] [% IF holds.count %] -

    Found [% holds.count | html %][% IF holds.count > 1 %] holds [% ELSE %] hold [% END %]to modify.

    +

    Found [% holds.count | html %][% IF holds.count > 1 %] holds [% ELSE %] hold [% END %]to modify.

    Select all @@ -120,7 +114,7 @@   Expiration date Patron expiration date - Title + Title Barcode Status Hold pickup library @@ -131,12 +125,16 @@ [% FOREACH hold IN holds %] - + [% IF hold.found %] + + [% ELSE %] + + [% END %] [% hold.expirationdate | $KohaDates %] [% hold.patron_expiration_date | $KohaDates %] - [% hold.item.biblio.title | html %] - [% hold.item.barcode | html %] + [% hold.biblio.title | html %] + [% hold.item.barcode | html %] [% IF hold.found == "T" %] In transit @@ -158,43 +156,45 @@

    Modify holds

    - - - - - - - - - - - - - - - - - - - -
    New expiration dateNew pickup librarySuspend holdsSuspend untilNew reserve note
    - - - - - - - - - -
    +
    + + + + + + + + + + + + + + + + + + + +
    New expiration dateNew pickup librarySuspend holdsSuspend untilNew reserve note
    + + + + + + + + + +
    +
    @@ -209,13 +209,15 @@ [% END %] [% ELSIF view == 'report' %]
    - Holds have been modified! + [% updated_holds.count | html %] holds have been modified!
    + + @@ -226,6 +228,8 @@ [% FOREACH hold IN updated_holds %] + + @@ -309,7 +313,7 @@ } if( ( !new_suspend_status || new_suspend_status == "not_suspended") && new_suspend_date ){ e.preventDefault(); - alert(_('You have to suspend holds if new suspend until date is set.')); + alert(_("You have to suspend holds if new suspend until date is set.")); return false; } diff --git a/tools/batch_modify_holds.pl b/tools/batch_modify_holds.pl index c1438c3c61..8d32b96513 100755 --- a/tools/batch_modify_holds.pl +++ b/tools/batch_modify_holds.pl @@ -29,14 +29,14 @@ use Koha::DateUtils qw( dt_from_string ); use Koha::Holds; my $input = CGI->new; -my $op = $input->param('op') // q|form|; +my $op = $input->param('op') // q|form|; my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { - template_name => 'tools/batch_modify_holds.tt', - query => $input, - type => "intranet", - flagsrequired => { tools => 'batch_modify_holds.tt' }, + template_name => 'tools/batch_modify_holds.tt', + query => $input, + type => "intranet", + flagsrequired => { tools => 'batch_modify_holds.tt' }, } ); @@ -44,8 +44,7 @@ my @hold_ids; if ( $op eq 'form' ) { $template->param( view => 'form', ); -} -elsif ( $op eq 'list' ) { +} elsif ( $op eq 'list' ) { my @reserve_ids = $input->multi_param('reserve_ids'); my $expirationdate_from = $input->param('expirationdate_from'); @@ -59,36 +58,39 @@ elsif ( $op eq 'list' ) { my $search_params; - if( @reserve_ids ){ + if (@reserve_ids) { $search_params->{'me.reserve_id'} = { -in => \@reserve_ids }; } else { - if( $expirationdate_from && $expirationdate_to ){ - my $expirationdate_params = { -or => { - patron_expiration_date => { - -between => [ - $expirationdate_from, - $expirationdate_to - ] - }, - expirationdate => { - -between => [ - $expirationdate_from, - $expirationdate_to - ] + if ( $expirationdate_from && $expirationdate_to ) { + my $expirationdate_params = { + -or => { + patron_expiration_date => { + -between => [ + $expirationdate_from, + $expirationdate_to + ] + }, + expirationdate => { + -between => [ + $expirationdate_from, + $expirationdate_to + ] + } } - }}; + }; $search_params = $expirationdate_params; } - if ( @branchcodes ) { + if (@branchcodes) { $search_params->{'me.branchcode'} = { -in => \@branchcodes }; } - if ( @found_status ) { + if (@found_status) { my $found_params = {}; + # if NULL is used as filter, use it as "is" param - if( grep {$_ eq "NULL"} @found_status ) { + if ( grep { $_ eq "NULL" } @found_status ) { $found_params->{'-or'}{'-is'} = undef; $found_params->{'-or'}{'-in'} = \@found_status; } else { @@ -98,11 +100,11 @@ elsif ( $op eq 'list' ) { $search_params->{'me.found'} = $found_params; } - if( $suspend_status ne "none" ){ + if ( $suspend_status ne "none" ) { $search_params->{'me.suspend'} = $suspend_status; } - if( $suspend_until_from && $suspend_until_to ){ + if ( $suspend_until_from && $suspend_until_to ) { $search_params->{'me.suspend_until'} = { -between => [ $suspend_until_from, @@ -111,16 +113,14 @@ elsif ( $op eq 'list' ) { }; } - if( $reservenotes ){ - $search_params->{'me.reservenotes'} = { -like => "%".$reservenotes."%" } + if ($reservenotes) { + $search_params->{'me.reservenotes'} = { -like => "%" . $reservenotes . "%" }; } } my $holds = Koha::Holds->search( $search_params, - { - join => ["item"] - } + { join => [ "item", "biblio" ] } ); $template->param( @@ -139,33 +139,34 @@ if ( $op eq 'cud-modify' ) { @hold_ids = $input->multi_param('hold_id'); - my $holds_to_update = Koha::Holds->search( { reserve_id => { -in => \@hold_ids } } ); + my $holds_to_update = + Koha::Holds->search( { reserve_id => { -in => \@hold_ids } }, { join => [ "item", "biblio" ] } ); while ( my $hold = $holds_to_update->next ) { - if( $new_expiration_date ){ + if ($new_expiration_date) { $hold->expirationdate($new_expiration_date)->store; } - if($new_pickup_loc && ($hold->branchcode ne $new_pickup_loc)){ + if ( $new_pickup_loc && ( $hold->branchcode ne $new_pickup_loc ) ) { $hold->branchcode($new_pickup_loc)->store; } - if( $new_suspend_status && !$hold->is_found ){ - if( $new_suspend_status eq "suspend" ){ + if ( $new_suspend_status && !$hold->is_found ) { + if ( $new_suspend_status eq "suspend" ) { $hold->suspend(1)->store; - if( $new_suspend_date ){ - $hold->suspend_until( $new_suspend_date )->store; + if ($new_suspend_date) { + $hold->suspend_until($new_suspend_date)->store; } else { - $hold->suspend_until( undef )->store; + $hold->suspend_until(undef)->store; } } else { $hold->suspend(0)->store; - $hold->suspend_until( undef )->store; + $hold->suspend_until(undef)->store; } } - if( $new_reserve_note ){ + if ($new_reserve_note) { $hold->reservenotes($new_reserve_note)->store; } } -- 2.34.1
    Expiration dateTitleBarcode Hold pickup library Suspended Suspended until
    [% hold.expirationdate | $KohaDates %][% hold.biblio.title | html %][% hold.item.barcode | html %] [% Branches.GetName( hold.branchcode ) | html %] [% IF hold.suspend == 0 %]No[% ELSE %]Yes[% END %] [% hold.suspend_until | $KohaDates %]