From c165a690ee2f05b61d988f5015a5c0f3a51363da Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Mon, 6 Aug 2018 15:26:49 -0300 Subject: [PATCH] Bug 21141: Handle non-existent itemnumbers in batch item modification/deletion The itemnumber list passed from a file in the batch item modification/deletion tools is not tested. Like barcodes we should make sure they refer to a valid items in the DB. Test plan: - Create a file with barcodes, itemnumbers and random lines - Use the batch item modification and deletion tools with this file You should see a table with a list of invalid barcodes and itemnumbers. Without this patch you got the following error: Can't call method "title" on an undefined value at /home/vagrant/kohaclone/tools/batchMod.pl line 580 Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> --- .../prog/en/modules/tools/batchMod-del.tt | 50 ++++++++++++----- .../prog/en/modules/tools/batchMod-edit.tt | 54 +++++++++++++------ tools/batchMod.pl | 16 +++--- 3 files changed, 82 insertions(+), 38 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt index 835be89507..18452295c9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt @@ -28,26 +28,48 @@ [% UNLESS ( action ) %] - [% IF ( notfoundbarcodes ) %] + [% IF ( notfoundbarcodes.size ) %] <div class="dialog alert"> - [% IF ( itemresults ) %]<p>Warning, the following barcodes were not found:</p>[% ELSE %]<p>Warning: no barcodes were found</p>[% END %] + [% IF ( itemresults ) %]<p>Warning, the following barcodes were not found:</p>[% ELSE %]<p>Warning: no barcodes were found</p>[% END %] </div> <table style="margin:auto;"> - <thead> - <tr> - <th>Barcode</th> - </tr> - </thead> - <tbody> - [% FOREACH notfoundbarcode IN notfoundbarcodes %] - <tr><td>[% notfoundbarcode.barcode |html %]</td></tr> + <thead> + <tr><th>Barcodes not found</th></tr> + </thead> + <tbody> + [% FOREACH notfoundbarcode IN notfoundbarcodes %] + <tr><td>[% notfoundbarcode |html %]</td></td> + [% END %] + </tbody> + </table> + [% IF ( item_loop ) %] + [% UNLESS ( too_many_items ) %] + <h4>The following barcodes were found: </h4> [% END %] - </tbody> + [% END %] + [% END %] <!-- /notfoundbarcodes --> + [% IF ( notfounditemnumbers.size ) %] + <div class="dialog alert"> + [% IF ( itemresults ) %]<p>Warning, the following itemnumbers were not found:</p>[% ELSE %]<p>Warning: no itemnumbers were found</p>[% END %] + </div> + <table style="margin:auto;"> + <thead> + <tr><th>Itemnumbers not found</th></tr> + </thead> + <tbody> + [% FOREACH notfounditemnumber IN notfounditemnumbers %] + <tr><td>[% notfounditemnumber |html %]</td></td> + [% END %] + </tbody> </table> - [% UNLESS ( too_many_items ) %] - [% IF ( item_loop ) %]<h4>The following barcodes were found: </h4>[% END %] + [% IF ( item_loop ) %] + [% UNLESS ( too_many_items ) %] + <h4>The following itemnumbers were found: </h4> + [% END %] [% END %] - [% END %] + [% END %] <!-- /notfounditemnumbers --> + + <form name="f" action="batchMod.pl" method="post"> diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt index 32577c8674..24f273b095 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt @@ -90,24 +90,44 @@ $(document).ready(function(){ [% IF ( book_on_loan ) %]<div class="dialog alert"><strong>Cannot Delete</strong>: item is checked out.</div>[% END %] [% IF ( book_reserved ) %]<div class="dialogalert"><strong>Cannot Delete</strong>: item has a waiting hold.</div>[% END %] - [% IF ( notfoundbarcodes ) %] - <div class="dialog alert"><p>Warning, the following barcodes were not found:</p></div> - <table style="margin:auto;"> - <thead> - <tr><th>Barcodes not found</th></tr> - </thead> - <tbody> - [% FOREACH notfoundbarcode IN notfoundbarcodes %] - <tr><td>[% notfoundbarcode.barcode |html %]</td></td> + [% IF ( notfoundbarcodes.size ) %] + <div class="dialog alert"><p>Warning, the following barcodes were not found:</p></div> + <table style="margin:auto;"> + <thead> + <tr><th>Barcodes not found</th></tr> + </thead> + <tbody> + [% FOREACH notfoundbarcode IN notfoundbarcodes %] + <tr><td>[% notfoundbarcode |html %]</td></td> + [% END %] + </tbody> + </table> + [% IF ( item_loop ) %] + [% UNLESS ( too_many_items ) %] + <h4>The following barcodes were found: </h4> + [% END %] [% END %] - </tbody> - </table> - [% IF ( item_loop ) %] - [% UNLESS ( too_many_items ) %] - <h4>The following barcodes were found: </h4> - [% END %] - [% END %] - [% END %] <!-- /notfoundbarcodes --> + [% END %] <!-- /notfoundbarcodes --> + [% IF ( notfounditemnumbers.size ) %] + <div class="dialog alert"><p>Warning, the following itemnumbers were not found:</p></div> + <table style="margin:auto;"> + <thead> + <tr><th>Itemnumbers not found</th></tr> + </thead> + <tbody> + [% FOREACH notfounditemnumber IN notfounditemnumbers %] + <tr><td>[% notfounditemnumber |html %]</td></td> + [% END %] + </tbody> + </table> + [% IF ( item_loop ) %] + [% UNLESS ( too_many_items ) %] + <h4>The following itemnumbers were found: </h4> + [% END %] + [% END %] + [% END %] <!-- /notfounditemnumbers --> + + <form name="f" action="batchMod.pl" method="post"> <input type="hidden" name="op" value="[% op %]" /> diff --git a/tools/batchMod.pl b/tools/batchMod.pl index f24b2f6987..0e985a2c7b 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -233,7 +233,7 @@ if ($op eq "action") { if ($op eq "show"){ my $filefh = $input->upload('uploadfile'); my $filecontent = $input->param('filecontent'); - my @notfoundbarcodes; + my ( @notfoundbarcodes, @notfounditemnumbers); my @contentlist; if ($filefh){ @@ -255,7 +255,9 @@ if ($op eq "show"){ } } elsif ( $filecontent eq 'itemid_file') { - @itemnumbers = @contentlist; + @itemnumbers = Koha::Items->search({ itemnumber => \@contentlist })->get_column('itemnumber'); + my %exists = map {$_=>1} @itemnumbers; + @notfounditemnumbers = grep { !$exists{$_} } @contentlist; } } else { if (defined $biblionumber){ @@ -496,11 +498,11 @@ $authorised_values_sth->finish; # what's the next op ? it's what we are not in : an add if we're editing, otherwise, and edit. - $template->param(item => \@loop_data); - if (@notfoundbarcodes) { - my @notfoundbarcodesloop = map{{barcode=>$_}}@notfoundbarcodes; - $template->param(notfoundbarcodes => \@notfoundbarcodesloop); - } + $template->param( + item => \@loop_data, + notfoundbarcodes => \@notfoundbarcodes, + notfounditemnumbers => \@notfounditemnumbers + ); $nextop="action" } # -- End action="show" -- 2.17.1