Bug 27426 - Plugin barcode must keep leading zeros
Summary: Plugin barcode must keep leading zeros
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 30827 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-01-13 16:05 UTC by Jonathan Druart
Modified: 2022-07-16 19:48 UTC (History)
4 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 27426: Keep leading zeros for barcode plugin (incremental) (2.82 KB, patch)
2021-01-13 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-01-13 16:05:11 UTC
If autoBarcode is set to incremental, we are generating the next barcode adding 1, but casting to integer.

If you have 
"0001"
"0002"
the next barcode will be "3".

Should not we keep the leading zeros and generate "0003" instead?
Comment 1 Jonathan Druart 2021-01-13 16:08:04 UTC
Created attachment 115115 [details] [review]
Bug 27426: Keep leading zeros for barcode plugin (incremental)

If autoBarcode is set to incremental, we are generating the next barcode adding 1, but casting to integer.

If you have
"0001"
"0002"
the next barcode will be "3".

Should not we keep the leading zeros and generate "0003" instead?

This patch suggests to keep them.
If this behaviour is not the expected one we could create another
barcode plugin.

Test plan:
Create an item with barcode 0001, another one with 0002
Set autoBarcode=incremental
Create another item and click the barcode input
With this patch the new barcode will be 0003
Without this patch the barcode was cast to int and resulted in 3
Comment 2 David Cook 2021-01-14 04:40:53 UTC
It's an interesting idea, but I think that it is incompatible with Bug 26328. 

Explicitly casting to an integer is needed in Bug 26328 because barcode may not be an integer even in a database with autoBarcode set to "incremental", which can cause lots of problems.

Hmm one way to solve both Bug 26328 and Bug 27426 could be to use the following SQL:

SELECT barcode FROM items WHERE CAST(barcode AS unsigned) ORDER BY CAST(barcode AS unsigned) DESC LIMIT 1;

It's an ugly bit of SQL but it should work.
Comment 3 David Cook 2021-01-14 04:44:55 UTC
Btw, we can't use ABS() here because it interprets exponents. For instance, "select abs(978e0143019375)" will actually create an error or "Inf" if it's already stored in a database column.
Comment 4 Hakam Almotlak 2021-06-23 18:19:27 UTC
hey i applied the patch and it did`t work i still get the last bar code number plus one and without the zeros at the begging
Comment 5 David Cook 2021-06-23 23:31:41 UTC
I should've actually marked this as Failed QA previously.
Comment 6 Jonathan Druart 2021-06-24 08:25:29 UTC
(In reply to David Cook from comment #3)
> Btw, we can't use ABS() here because it interprets exponents. For instance,
> "select abs(978e0143019375)" will actually create an error or "Inf" if it's
> already stored in a database column.

How do we process then, David? The patch here has test to prove it is working. I can base it on top of yours, but you need to do what's needed to remove its FQA status.

(In reply to Hakam Almotlak from comment #4)
> hey i applied the patch and it did`t work i still get the last bar code
> number plus one and without the zeros at the begging

Please detail what you tried exactly.
Comment 7 David Cook 2021-06-25 04:37:15 UTC
(In reply to Jonathan Druart from comment #6)
> (In reply to David Cook from comment #3)
> > Btw, we can't use ABS() here because it interprets exponents. For instance,
> > "select abs(978e0143019375)" will actually create an error or "Inf" if it's
> > already stored in a database column.
> 
> How do we process then, David? The patch here has test to prove it is
> working. I can base it on top of yours, but you need to do what's needed to
> remove its FQA status.

Do you mean for Bug 26328? That tester never responded to my questions. I could re-test it myself, but it could be good to get someone new to test too.
Comment 8 Jonathan Druart 2021-06-25 06:34:26 UTC
It could be nice to provide a test first :)
Comment 9 Katrin Fischer 2022-07-16 19:48:43 UTC
*** Bug 30827 has been marked as a duplicate of this bug. ***