From 391cb02d456d3e764c00bdd89e098f27f05246f0 Mon Sep 17 00:00:00 2001 From: Petro Vashchuk Date: Sun, 3 Apr 2022 00:02:40 +0300 Subject: [PATCH] Bug 30409: barcodedecode() should always trim barcode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Joonas Kylmälä Signed-off-by: Tomas Cohen Arazi (cherry picked from commit e2611c919dd5287402b89f219d75d3a013407123) Signed-off-by: Lucas Gass --- C4/Circulation.pm | 2 ++ circ/branchtransfers.pl | 4 ++-- circ/circulation.pl | 1 - circ/renew.pl | 3 +-- circ/returns.pl | 2 -- course_reserves/add_items.pl | 1 - .../prog/en/modules/admin/preferences/circulation.pref | 2 +- t/Circulation_barcodedecode.t | 10 +++++----- 8 files changed, 11 insertions(+), 14 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d92cdcc0b2..71b549b6e3 100644 --- a/C4/Circulation.pm +++ b/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 diff --git a/circ/branchtransfers.pl b/circ/branchtransfers.pl index 4e23ae6d7d..cb10b5b0f1 100755 --- a/circ/branchtransfers.pl +++ b/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) { diff --git a/circ/circulation.pl b/circ/circulation.pl index 6aaf831ba6..d7f51e44ef 100755 --- a/circ/circulation.pl +++ b/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; } diff --git a/circ/renew.pl b/circ/renew.pl index 5a1ca18ef8..e7d49fd23a 100755 --- a/circ/renew.pl +++ b/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) { diff --git a/circ/returns.pl b/circ/returns.pl index e2d2317a37..340efb4779 100755 --- a/circ/returns.pl +++ b/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 }); diff --git a/course_reserves/add_items.pl b/course_reserves/add_items.pl index f8972f88ec..d430cf58da 100755 --- a/course_reserves/add_items.pl +++ b/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 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index b4d6a761b2..01d9ecb877 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/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: diff --git a/t/Circulation_barcodedecode.t b/t/Circulation_barcodedecode.t index 8a57dcd984..094d4aafc5 100755 --- a/t/Circulation_barcodedecode.t +++ b/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;