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>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

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 835be89..1845229 100644 (file)
--- 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 32577c8..24f273b 100644 (file)
--- 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 f24b2f6..0e985a2 100755 (executable)
--- 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"