From fc6e5839d4f676ce22d2641a3bb23a623d67c8a7 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 2 Apr 2026 13:43:58 +0100 Subject: [PATCH] Bug 42221: Fix EAN-13 autoBarcode not incrementing The EAN-13 barcode query used max(abs(barcode)) which allowed non-EAN-13 barcodes (e.g. 14-digit KTD sample barcodes) to dominate the result. The saved 13-digit EAN-13 barcodes were always smaller, so $nextnum never advanced and the same barcode was generated on every page load. Fix by extracting EAN-13 logic into C4::Barcodes::ValueBuilder::EAN13 (consistent with other barcode type packages) and scoping the query to WHERE length(barcode) = 13 so only valid-length barcodes contribute to the max. Both barcode.pl and Koha::REST::V1::Biblios now call the shared function, eliminating the duplicated inline code. Tests: adds EAN-13 test cases to t/db_dependent/Barcodes_ValueBuilder.t Signed-off-by: Caroline Cyr La Rose Signed-off-by: Andrew Fuerste Henry Signed-off-by: Lucas Gass --- C4/Barcodes/ValueBuilder.pm | 27 ++++++++++++++++++++++++++ Koha/REST/V1/Biblios.pm | 22 +-------------------- cataloguing/value_builder/barcode.pl | 21 +------------------- t/db_dependent/Barcodes_ValueBuilder.t | 25 +++++++++++++++++++++++- 4 files changed, 53 insertions(+), 42 deletions(-) diff --git a/C4/Barcodes/ValueBuilder.pm b/C4/Barcodes/ValueBuilder.pm index b12fd5bacf..f28192f6fa 100644 --- a/C4/Barcodes/ValueBuilder.pm +++ b/C4/Barcodes/ValueBuilder.pm @@ -92,6 +92,33 @@ sub get_barcode { return $nextnum; } +package C4::Barcodes::ValueBuilder::EAN13; +use Modern::Perl; +use C4::Context; +use Algorithm::CheckDigits qw( CheckDigits ); + +sub get_barcode { + my ($args) = @_; + + # Filter to 13-digit barcodes only so non-EAN-13 items (e.g. 14-digit sample + # barcodes) do not dominate max() and prevent the sequence from advancing. + my $query = "SELECT max(cast(barcode as unsigned)) FROM items WHERE length(barcode) = 13"; + my $sth = C4::Context->dbh->prepare($query); + $sth->execute(); + my ($nextnum) = $sth->fetchrow_array; + + my $ean = CheckDigits('ean'); + if ( $nextnum && $ean->is_valid($nextnum) ) { + my $next = $ean->basenumber($nextnum) + 1; + $nextnum = $ean->complete($next); + $nextnum = '0' x ( 13 - length($nextnum) ) . $nextnum; # pad zeros + } else { + warn "ERROR: invalid EAN-13 " . ( $nextnum // '' ) . ", using increment"; + $nextnum = ( $nextnum // 0 ) + 1; + } + return $nextnum; +} + 1; =head1 Barcodes::ValueBuilder diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index c9d59e5860..18e29d6b14 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -332,27 +332,7 @@ sub add_item { ($barcode) = C4::Barcodes::ValueBuilder::hbyymmincr::get_barcode( { year => $year, mon => $month } ); $barcode = $homebranch . $barcode; } elsif ( $autoBarcode eq 'EAN13' ) { - - # not the best, two catalogers could add the same - # barcode easily this way :/ - my $query = "select max(abs(barcode)) from items"; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare($query); - $sth->execute(); - my $nextnum; - while ( my ($last) = $sth->fetchrow_array ) { - $nextnum = $last; - } - my $ean = CheckDigits('ean'); - if ( $ean->is_valid($nextnum) ) { - my $next = $ean->basenumber($nextnum) + 1; - $nextnum = $ean->complete($next); - $nextnum = '0' x ( 13 - length($nextnum) ) . $nextnum; # pad zeros - } else { - warn "ERROR: invalid EAN-13 $nextnum, using increment"; - $nextnum++; - } - $barcode = $nextnum; + ($barcode) = C4::Barcodes::ValueBuilder::EAN13::get_barcode(); } else { warn "ERROR: unknown autoBarcode: $autoBarcode"; } diff --git a/cataloguing/value_builder/barcode.pl b/cataloguing/value_builder/barcode.pl index 6ed165e260..adbfea83db 100755 --- a/cataloguing/value_builder/barcode.pl +++ b/cataloguing/value_builder/barcode.pl @@ -27,8 +27,6 @@ use C4::Barcodes::ValueBuilder; use C4::Biblio qw( GetMarcFromKohaField ); use Koha::DateUtils qw( dt_from_string ); -use Algorithm::CheckDigits qw( CheckDigits ); - use CGI qw ( -utf8 ); use C4::Auth qw( check_cookie_auth ); my $input = CGI->new; @@ -64,24 +62,7 @@ my $builder = sub { { # Generates a barcode where hb = home branch Code, yymm = year/month catalogued, incr = incremental number, reset yearly -fbcit ( $nextnum, $scr ) = C4::Barcodes::ValueBuilder::hbyymmincr::get_barcode( \%args ); } elsif ( $autoBarcodeType eq 'EAN13' ) { - - # not the best, two catalogers could add the same barcode easily this way :/ - my $query = "select max(abs(barcode)) from items"; - my $dbh = $params->{dbh}; - my $sth = $dbh->prepare($query); - $sth->execute(); - while ( my ($last) = $sth->fetchrow_array ) { - $nextnum = $last; - } - my $ean = CheckDigits('ean'); - if ( $ean->is_valid($nextnum) ) { - my $next = $ean->basenumber($nextnum) + 1; - $nextnum = $ean->complete($next); - $nextnum = '0' x ( 13 - length($nextnum) ) . $nextnum; # pad zeros - } else { - warn "ERROR: invalid EAN-13 $nextnum, using increment"; - $nextnum++; - } + ($nextnum) = C4::Barcodes::ValueBuilder::EAN13::get_barcode( \%args ); } else { warn "ERROR: unknown autoBarcode: $autoBarcodeType"; } diff --git a/t/db_dependent/Barcodes_ValueBuilder.t b/t/db_dependent/Barcodes_ValueBuilder.t index 83f4c3f33a..d56111ac52 100755 --- a/t/db_dependent/Barcodes_ValueBuilder.t +++ b/t/db_dependent/Barcodes_ValueBuilder.t @@ -17,7 +17,8 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 9; +use Test::More tests => 14; +use Test::Warn; use Test::MockModule; use t::lib::TestBuilder; @@ -63,4 +64,26 @@ my $item_5 = $builder->build_sample_item( { barcode => '978e0143019375' } ); is( $nextnum, '979', 'incremental barcode' ); is( $scr, undef, 'incremental javascript' ); +# EAN-13 tests +$dbh->do(q|DELETE FROM items|); +$builder->build_sample_item( { barcode => '3999900001927' } ); # valid 13-digit EAN-13 +( $nextnum, $scr ) = C4::Barcodes::ValueBuilder::EAN13::get_barcode( \%args ); +is( $nextnum, '3999900001934', 'EAN-13 increments correctly from valid EAN-13' ); +is( $scr, undef, 'EAN-13 javascript' ); + +$dbh->do(q|DELETE FROM items|); +$builder->build_sample_item( { barcode => '39999000019193' } ); # 14-digit, should be ignored +warning_like( + sub { ( $nextnum, $scr ) = C4::Barcodes::ValueBuilder::EAN13::get_barcode( \%args ) }, + qr/ERROR: invalid EAN-13/, + 'EAN-13 warns when no valid 13-digit barcode found' +); +is( $nextnum, 1, 'EAN-13 not confused by 14-digit barcode (falls back to increment)' ); + +$dbh->do(q|DELETE FROM items|); +$builder->build_sample_item( { barcode => '39999000019193' } ); # 14-digit +$builder->build_sample_item( { barcode => '3999900001927' } ); # 13-digit EAN-13 +( $nextnum, $scr ) = C4::Barcodes::ValueBuilder::EAN13::get_barcode( \%args ); +is( $nextnum, '3999900001934', 'EAN-13 picks max among 13-digit barcodes, ignores 14-digit' ); + $schema->storage->txn_rollback;