[Bug 42221] New: autoarcode set to incremental EAN-13 barcodes do not increment
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Bug ID: 42221 Summary: autoarcode set to incremental EAN-13 barcodes do not increment Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Cataloging Assignee: koha-bugs@lists.koha-community.org Reporter: caroline.cyr-la-rose@inlibro.com QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl I don't know if this is the same as bug 41765, because Pavel says they increment in 7s but mine don't increment at all. I found this on 25.05 and confirmed it's still the case on main. To recreate : 1. Change the autobarcode system preference to incremental EAN-13 barcodes 1.1. Go to Administration > System preferences 1.2. Search for autobarcode -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|autoarcode set to |autoBarcode set to |incremental EAN-13 barcodes |incremental EAN-13 barcodes |do not increment |do not increment --- Comment #1 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Oops, I don't know what I did but I didn't want to submit right away... To recreate : 1. Change the autobarcode system preference to incremental EAN-13 barcodes 1.1. Go to Administration > System preferences 1.2. Search for autoBarcode 1.3. Change the value to "incremental EAN-13 barcodes" 1.4. Click "Save all Cataloging preferences" 2. Add an item to an existing bibliographic record 2.1. Search for a bibliographic record (I used http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=285) 2.2. Click New > New item 2.3. Click inside the p- barcode field --> Generates 3999900001927 2.4. Fill out any other mandatory field (should be already done in KTD) 2.5. Click "Add item" --> Item is saved correctly (OK) 3. Try to add a second item 3.3. Redo steps 2.3 to 2.5 --> Same barcode is generated again (NOT OK) --> Error saving item: Barcode must be unique. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41765 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 196653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196653&action=edit 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal CC| |martin.renvoize@openfifth.c | |o.uk Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Bug 8425 (merged July 2024) is where the regression crept in, here's exactly what changed: Before Bug 8425, the Click handler called set_barcode(id, true) — with force=true. The JS just wrote '$nextnum' verbatim (as a string). With any 14-digit plus barcodes, like for example those in KTD, $nextnum came out as one more than the max (39999000019194 in KTD) (14-plus-digit, increments each save because the saved 14-digit barcode eventually becomes the new max). Ugly, but it incremented. After Bug 8425, the Click handler passes force=false and routes EAN13 through incrementEAN13($nextnum, offset). That function slices $nextnum to 12 chars and recomputes a check digit, producing a valid-looking 13-digit barcode. But the saved 13-digit barcode (~3.9×10¹²) is always smaller than the KTD 14-plus-digit sample barcodes (~3.9×10¹³), so max(abs(barcode)) never picks up the saved EAN-13. $nextnum stays frozen at the same value every page load → same barcode every time. So the SQL query was always wrong, but before Bug 8425 it accidentally worked because saving a 14-digit barcode would push the max forward. Bug 8425 changed the JS output to 13-digit EAN-13, which broke that accidental increment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196653|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #4 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Created attachment 196674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196674&action=edit 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 <caroline.cyr-la-rose@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #5 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Thank you Martin for the patch and research! There was no test plan in the patch so I just redid my test plan with the patch applied and it fixed the problem I had. As I mentioned, I noticed this on a client's installation that's on 25.05. Plus, bug 8425 was also backported to 24.11. So this fix needs to be backported all the way to 24.11 if possible. Thanks! :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Thanks for testing Caroline. Sorry for the lack of test plan, I think I'm losing my marbles as I clearly completely forgot to add it. It was pretty much as you did, ktd comes with some "bad" data that would prompt the issue so it's basically try before and after apply the patch and note it works after. There's also some unit tests, but QA can run those happily 🙂 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #7 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- I did run prove t/db_dependent/Barcodes_ValueBuilder.t (after signing off) and there was nothing to report. Here's a revised test plan if you want to update the commit message To test : 1. Change the autobarcode system preference to incremental EAN-13 barcodes 1.1. Go to Administration > System preferences 1.2. Search for autoBarcode 1.3. Change the value to "incremental EAN-13 barcodes" 1.4. Click "Save all Cataloging preferences" 2. Add an item to an existing bibliographic record 2.1. Search for a bibliographic record 2.2. Click New > New item 2.3. Click inside the p- barcode field --> Generates 3999900001927 2.4. Fill out any other mandatory field (should be already done in KTD) 2.5. Click "Add item" --> Item is saved correctly (OK) 3. Try to add a second item 3.3. Redo steps 2.3 to 2.5 --> Same barcode is generated again (NOT OK) --> Error saving item: Barcode must be unique. 4. Apply patch and restart_all 5. Redo steps 2 and 3 --> Both items should be saved correctly (OK!) 6. Run prove t/db_dependent/Barcodes_ValueBuilder.t --> Test should pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com QA Contact|testopia@bugs.koha-communit |andrew@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196674|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #8 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 197531 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197531&action=edit 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 <caroline.cyr-la-rose@inlibro.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #9 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00 |26.05.00,25.11.05 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42221 --- Comment #10 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org