Bug 4162 The inventory tool lacks validation for barcodes

The inventory tool had no form of validating on what was assumed
to be a valid barcode number.

To solve this, an extra loop to read before processing was added.
This allows to validate length and content. By using a check
of \p{Print}, this includes Unicode characters such as umlauts,
but excludes unusual control characters.

The template was modified to accomodate validation messages
related to the length and content errors. Additionally, it says
how many "barcodes" were read. Barcodes are supposed to be on
separate lines.

TEST PLAN
---------
1) Attempt to select a file which does not contain barcodes and
   is not a text file.
   -- a horrible lack of validation and spamminess ensues.
2) Apply patch
3) Create three files.
   a) One containing valid barcodes on each line
      -- this file should trigger no errors. Attempt a valid
         barcode with an umlaut.
   b) A copy of the first with an extra line of >20 characters
      (e.g. The Quick Red Fox Jumped Over The Brown Fence^A^B^C)
      -- this file should trigger the singular error message case.
         ^A^B^C are actually CTRL-A,CTRL-B,CTRL-C, and it is left
         as an exercise to the reader to add them to the line.
   c) A copy of the second with the last line duplicated
      -- this file should trigger the plural error message case.
4) Attempt each of the three files.
5) Run koha-qa tools.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
I have a feeling that the column size could be better fetched from
Koha::Database. But it is an improvement in functionality signing off

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, no problems found.
Passes tests and QA script.

Another thing for another day:
Empty lines are counted for the potential barcodes.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Works as expected, tried with arab strings, umlauts and no regressions found.

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt
index c751230..6ec6c2e 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt
@@ -102,8 +102,13 @@ $(document).ready(function(){
     <div id="yui-main">
     <div class="yui-b">
     <h1>Inventory/Stocktaking</h1>
-    [% IF (moddatecount) %]<div class="dialog message">[% moddatecount %] items modified : datelastseen set to [% date | $KohaDates %]</div>[% END %]
+    [% IF (moddatecount) %]<div class="dialog message">[% moddatecount %] items modified : datelastseen set to [% date | $KohaDates %]</div>
+    <div class="dialog alert">Number of potential barcodes read: [% LinesRead %]</div>[% END %]
     [% IF (errorfile) %]<div class="dialog alert">[% errorfile %] can't be opened</div>[% END %]
+    [% IF (err_length && err_length==1) %]<div class="dialog alert">There was 1 barcode that was too long.</div>[% END %]
+    [% IF (err_length && err_length>1) %]<div class="dialog alert">There were [% err_length %] barcodes that were too long.</div>[% END %]
+    [% IF (err_data && err_data==1) %]<div class="dialog alert">There was 1 barcode that contained at least one unprintable character.</div>[% END %]
+    [% IF (err_data && err_data>1) %]<div class="dialog alert">There were [% err_data %] barcodes that contained at least one unprintable character.</div>[% END %]
     [% FOREACH error IN errorloop %]
         <div class="dialog alert">
             [% error.barcode %]

diff --git a/tools/inventory.pl b/tools/inventory.pl
index f79f9d6..0255949 100755 (executable)
--- a/tools/inventory.pl
+++ b/tools/inventory.pl
@@ -160,9 +160,41 @@ if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) {
 
     my $count = 0;
 
+    my @barcodes;
+
+    my $sth = $dbh->column_info(undef,undef,"items","barcode");
+    my $barcode_def = $sth->fetchall_hashref('COLUMN_NAME');
+    my $barcode_size = $barcode_def->{barcode}->{COLUMN_SIZE};
+    my $err_length=0;
+    my $err_data=0;
+    my $lines_read=0;
+    binmode($uploadbarcodes, ":encoding(UTF-8)");
     while (my $barcode=<$uploadbarcodes>){
+        ++$lines_read;
         $barcode =~ s/\r?\n$//;
         next unless $barcode;
+        if (length($barcode)>$barcode_size) {
+            $err_length += 1;
+        }
+        my $check_barcode = $barcode;
+        $check_barcode =~ s/\p{Print}//g;
+        if (length($check_barcode)>0) { # Only printable unicode characters allowed.
+            $err_data += 1;
+        }
+        next if length($barcode)>$barcode_size;
+        next if ( length($check_barcode)>0 );
+        push @barcodes,$barcode;
+    }
+    $template->param( LinesRead => $lines_read );
+    if (! @barcodes) {
+        push @errorloop, {'barcode'=>'No valid barcodes!'};
+        $op=''; # force the initial inventory screen again.
+    }
+    else {
+        $template->param( err_length => $err_length,
+                          err_data   => $err_data );
+    }
+    foreach my $barcode (@barcodes) {
         if ( $qwithdrawn->execute($barcode) && $qwithdrawn->rows ) {
             push @errorloop, { 'barcode' => $barcode, 'ERR_WTHDRAWN' => 1 };
         } else {