Bug 30409: barcodedecode() should always trim barcode

Barcode is trimmed of leading/trailing whitespaces in many instances
before the barcodedecode sub was called. This patch instead makes that
barcodedecode sub is going to trim it itself and removes unnecessary,
and repetitive code that was used before barcodedecode was called.

Steps to test:
1. Edit item with any barcode, add a bunch of whitespaces at the start
and at the bottom of it. Save the item. Ensure that this action ruins
the barcode and ensure that the spaces are still there by editing the
same item again.
2. Apply the patch.
3. Edit the same item again in the same fashion. Ensure that now all
whitespaces are getting trimmed and it doesn't affect the barcode in
any negative way.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit e2611c919d)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

C4/Circulation.pm

@@ -154,6 +154,7 @@ Will do some manipulation of the barcode for systems that deliver a barcode
 to circulation.pl that differs from the barcode stored for the item.
 For proper functioning of this filter, calling the function on the 
 correct barcode string (items.barcode) should return an unaltered barcode.
+Barcode is going to be automatically trimmed of leading/trailing whitespaces.
 
 The optional $filter argument is to allow for testing or explicit 
 behavior that ignores the System Pref.  Valid values are the same as the 
@@ -167,6 +168,7 @@ System Pref options.
 sub barcodedecode {
     my ($barcode, $filter) = @_;
     my $branch = C4::Context::mybranch();
+    $barcode =~ s/^\s+|\s+$//g;
     $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
     Koha::Plugins->call('item_barcode_transform',  \$barcode );
     $filter or return $barcode;     # ensure filter is defined, else return untouched barcode

circ/branchtransfers.pl

@@ -22,7 +22,7 @@
 
 use Modern::Perl;
 use CGI qw ( -utf8 );
-use C4::Circulation qw( transferbook );
+use C4::Circulation qw( transferbook barcodedecode );
 use C4::Output qw( output_html_with_http_headers );
 use C4::Reserves qw( ModReserve ModReserveAffect );
 use C4::Auth qw( get_session get_template_and_user );
@@ -121,7 +121,7 @@ my @trsfitemloop;
 my $transferred;
 my $barcode = $query->param('barcode');
 # remove leading/trailing whitespace
-defined $barcode and $barcode =~ s/^\s*|\s*$//g;  # FIXME: barcodeInputFilter
+$barcode = barcodedecode($barcode) if $barcode;
 # warn "barcode : $barcode";
 if ($barcode) {
 

circ/circulation.pl

@@ -158,7 +158,6 @@ if (C4::Context->preference("DisplayClearScreenButton")) {
 }
 
 for my $barcode ( @$barcodes ) {
-    $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace
     $barcode = barcodedecode( $barcode ) if $barcode;
 }
 

circ/renew.pl

@@ -43,7 +43,6 @@ my $schema = Koha::Database->new()->schema();
 
 my $barcode        = $cgi->param('barcode') // '';
 my $unseen         = $cgi->param('unseen') || 0;
-$barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespae
 $barcode = barcodedecode($barcode) if $barcode;
 my $override_limit = $cgi->param('override_limit');
 my $override_holds = $cgi->param('override_holds');
@@ -54,7 +53,7 @@ my $error = q{};
 my ( $soonest_renew_date, $latest_auto_renew_date );
 
 if ($barcode) {
-    $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace
+    $barcode = barcodedecode($barcode) if $barcode;
     $item = $schema->resultset("Item")->single( { barcode => $barcode } );
 
     if ($item) {

circ/returns.pl

@@ -119,7 +119,6 @@ foreach ( $query->param ) {
     $counter++;
 
     # decode barcode    ## Didn't we already decode them before passing them back last time??
-    $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace
     $barcode = barcodedecode($barcode) if $barcode;
 
     ######################
@@ -285,7 +284,6 @@ if ($transit) {
 # actually return book and prepare item table.....
 my $returnbranch;
 if ($barcode) {
-    $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace
     $barcode = barcodedecode($barcode) if $barcode;
     my $item = Koha::Items->find({ barcode => $barcode });
 

course_reserves/add_items.pl

@@ -43,7 +43,6 @@ my $itemnumber   = $cgi->param('itemnumber')   || '';
 my $is_edit      = $cgi->param('is_edit')      || '';
 my $biblionumber = $cgi->param('biblionumber') || '';
 
-$barcode =~ s/^\s*|\s*$//g; #remove leading/trailing whitespace
 $barcode = barcodedecode($barcode) if $barcode;
 $biblionumber =~ s/^\s*|\s*$//g; #remove leading/trailing whitespace
 

koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref

@@ -35,7 +35,7 @@ Circulation:
                   T-prefix: Remove the first number from T-prefix style
                   libsuite8: Convert from Libsuite8 form
                   EAN13: EAN-13 or zero-padded UPC-A form
-            - scanned item barcodes.
+            - scanned item barcodes. Mind that barcode whitespaces always trimmed from both ends before this filter.
         -
             - pref: itemBarcodeFallbackSearch
               choices:

t/Circulation_barcodedecode.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 26;
+use Test::More tests => 28;
 
 use C4::Context;
 use t::lib::Mocks;
@@ -29,19 +29,19 @@ t::lib::Mocks::mock_userenv({ branchcode => 'IMS' });
 our %inputs = (
     cuecat     => ["26002315", '.C3nZC3nZC3nYD3b6ENnZCNnY.fHmc.C3D1Dxr2C3nZE3n7.', ".C3nZC3nZC3nYD3b6ENnZCNnY.fHmc.C3D1Dxr2C3nZE3n7.\r\n",
                     'q.C3nZC3nZC3nWDNzYDxf2CNnY.fHmc.C3DWC3nZCNjXD3nW.', '.C3nZC3nZC3nWCxjWE3D1C3nX.cGf2.ENr7C3v7D3T3ENj3C3zYDNnZ.' ],
-    whitespace => [" 26002315", "26002315 ", "\n\t26002315\n"],
+    whitespace => [" 26002315", "26002315 ", "\n\t26002315\n", "whitespace removed"],
     'T-prefix' => [qw(T0031472 T32)],
     'libsuite8' => ['b000126', 'b12', 'B0126', 'IMS-B-126', 'ims-b-126','CD0000024','00123','11998'],
     EAN13      => [qw(892685001928 695152)],
-    other      => [qw(26002315 T0031472 T32 Alphanum123), "Alpha Num 345"],
+    other      => [qw(26002315 T0031472 T32 Alphanum123), "Alpha Num 345", " side spaces removed \n"],
 );
 our %outputs = (
     cuecat     => ["26002315", "046675000808", "046675000808", "043000112403", "978068484914051500"],
-    whitespace => [qw(26002315 26002315 26002315)],
+    whitespace => [qw(26002315 26002315 26002315 whitespaceremoved)],
     'T-prefix' => [qw(T0031472 T0000002         )],
     'libsuite8' => ['IMS-b-126', 'IMS-b-12', 'IMS-B-126', 'IMS-B-126', 'ims-b-126','IMS-CD-24','IMS-b-123','IMS-b-11998'],
     EAN13      => [qw(0892685001928 0000000695152)],
-    other      => [qw(26002315 T0031472 T32 Alphanum123), "Alpha Num 345"],
+    other      => [qw(26002315 T0031472 T32 Alphanum123), "Alpha Num 345", "side spaces removed"],
 );
 
 my @filters = sort keys %inputs;