From cd8ca4717c8ee3055a36497db20f263832c83659 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Sat, 12 Jul 2014 09:42:52 -0400 Subject: [PATCH] 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 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 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 Works as expected, tried with arab strings, umlauts and no regressions found. --- .../prog/en/modules/tools/inventory.tt | 7 +++- tools/inventory.pl | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) 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 c75123033d..6ec6c2e9a1 100644 --- 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(){

Inventory/Stocktaking

- [% IF (moddatecount) %]
[% moddatecount %] items modified : datelastseen set to [% date | $KohaDates %]
[% END %] + [% IF (moddatecount) %]
[% moddatecount %] items modified : datelastseen set to [% date | $KohaDates %]
+
Number of potential barcodes read: [% LinesRead %]
[% END %] [% IF (errorfile) %]
[% errorfile %] can't be opened
[% END %] + [% IF (err_length && err_length==1) %]
There was 1 barcode that was too long.
[% END %] + [% IF (err_length && err_length>1) %]
There were [% err_length %] barcodes that were too long.
[% END %] + [% IF (err_data && err_data==1) %]
There was 1 barcode that contained at least one unprintable character.
[% END %] + [% IF (err_data && err_data>1) %]
There were [% err_data %] barcodes that contained at least one unprintable character.
[% END %] [% FOREACH error IN errorloop %]
[% error.barcode %] diff --git a/tools/inventory.pl b/tools/inventory.pl index f79f9d6f34..02559490f7 100755 --- 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 { -- 2.39.5