We are migrating one library from custom software to Koha, and they are using EAN-13 zero-padded barcodes (primary key in legacy system) instead of ISBN or ISSN. Since books allready have barcodes on them, we can't change it. To make things more insteresting, at least one barcode reader reports it as UPC-A without first leading zero, since EAN-13 has backwards compatibility with it.
Created attachment 8668 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This filter which check validity of EAN-13 barcode and padd it with zeros up to full 13 digit number. This will also expand 12 digit UPC-A barcodes to EAN-13 automatically which is useful for older barcode readers which tend to ignore first zero in EAN-13 if they have just UPC-A support. It should be noted that EAN-13 or UPC-A product codes printed on books are not good choice for barcodes in Koha since each item has to have unique barcode. Test scenario: 1. prove t/Circulation_barcodedecode.t this checks expansion of 12 digit UPC-A to 13 digit EAN-13 and zero padding 2. in systempreferences search for itemBarcodeInputFilter and select EAN-13 3. edit one item and assign it valid EAN-13 barcode, eg. 0000000695152, check it out 4. test checkin with just 695152 to test leading zero expansion
Created attachment 8669 [details] [review] Bug 6448 [2/3] Barcodes::EAN13 autoBarcode Implement auto-incrementing EAN-13 barcodes To make this work, C4::Barcodes::next was modified to call process_tail with new incremented value so that process_tail can generate correct checksum. Since process_tail is currenlty not used by any barcodes, this change is safe. C4::Barcodes is used by addbiblio.pl when adding multiple records, while value_builder is used in all other cases. Test scenario: 1. prove t/Barcodes_EAN13.t 2. KOHA_CONF=/etc/koha/sites/fer/koha-conf.xml prove t/db_dependent/Barcodes.t this will check C4::Barcode implementataion 3. in systempreference change autoBarcode to incremental EAN-13 barcode 4. edit two items of any biblio assigning barcodes and verify that numbers are increasing. Have in mind that last digit is check digit, and it doesn't increment, but is calculated from barcode itself. Example with checksum in brackets: 000000086275[2], 000000086276[9], 000000086277[6] 5. Add Item and verify that it gets assigned next barcode 6. Add & Duplicate item and verify barcode increase 7. Add Multiple Copies and verify that barcode increase for each copy
Created attachment 8670 [details] [review] Bug 6448 [3/3] EAN-13 barcode labels Current implementation doesn't scale barcodes because low-price CCD barcode readers are very sensitive about size Test scenario: 1. in Tools > Labels create or edit Layout and select EAN13 as barcode type 2. export one of existing batches using EAN13 layout and verify that generated pdf file contains barcodes 3. print pdf file and test it with barcode reader
Applying: Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter error: patch failed: C4/Circulation.pm:36 error: C4/Circulation.pm: patch does not apply error: patch failed: installer/data/mysql/updatedatabase.pl:5086 error: installer/data/mysql/updatedatabase.pl: patch does not apply Patch failed at 0001 Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter
Created attachment 9192 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This filter which check validity of EAN-13 barcode and padd it with zeros up to full 13 digit number. This will also expand 12 digit UPC-A barcodes to EAN-13 automatically which is useful for older barcode readers which tend to ignore first zero in EAN-13 if they have just UPC-A support. It should be noted that EAN-13 or UPC-A product codes printed on books are not good choice for barcodes in Koha since each item has to have unique barcode. Test scenario: 1. prove t/Circulation_barcodedecode.t this checks expansion of 12 digit UPC-A to 13 digit EAN-13 and zero padding 2. in systempreferences search for itemBarcodeInputFilter and select EAN-13 3. edit one item and assign it valid EAN-13 barcode, eg. 0000000695152, check it out 4. test checkin with just 695152 to test leading zero expansion
Comment on attachment 8668 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This patch was obsoleted by new one which is rebased on current master
Marc, thanks for reporting bitrot since master moved quite a bit since I wrote it :-) I can't re-order patches on bugzilla, and since only first one required change and now it's last one if you are using git bz to apply them you have to do that in correct order (which is included in commit message). Patches will be in correct order once signed off, so I'm reluctant to re-upload last two patches again just to fix order, so I'm including example of how to apply it using git bz to make it easier: dpavlin@koha-dev:/srv/koha$ git bz apply 6448 Bug 6448 - EAN-13 barcode support Bug 6448 [2/3] Barcodes::EAN13 autoBarcode Apply? [yn] n Bug 6448 [3/3] EAN-13 barcode labels Apply? [yn] n Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter Apply? [yn] y Applying: Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter dpavlin@koha-dev:/srv/koha$ git bz apply 6448 Bug 6448 - EAN-13 barcode support Bug 6448 [2/3] Barcodes::EAN13 autoBarcode Apply? [yn] y Applying: Bug 6448 [2/3] Barcodes::EAN13 autoBarcode Bug 6448 [3/3] EAN-13 barcode labels Apply? [yn] y Applying: Bug 6448 [3/3] EAN-13 barcode labels Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter Apply? [yn] n
Created attachment 10019 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This filter which check validity of EAN-13 barcode and padd it with zeros up to full 13 digit number. This will also expand 12 digit UPC-A barcodes to EAN-13 automatically which is useful for older barcode readers which tend to ignore first zero in EAN-13 if they have just UPC-A support. It should be noted that EAN-13 or UPC-A product codes printed on books are not good choice for barcodes in Koha since each item has to have unique barcode. Test scenario: 1. prove t/Circulation_barcodedecode.t this checks expansion of 12 digit UPC-A to 13 digit EAN-13 and zero padding 2. in systempreferences search for itemBarcodeInputFilter and select EAN-13 3. edit one item and assign it valid EAN-13 barcode, eg. 0000000695152, check it out 4. test checkin with just 695152 to test leading zero expansion
First patch again bit-rotted because development moved on. Instruction how to apply diffs in correct order still apply
Created attachment 10127 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This filter which check validity of EAN-13 barcode and padd it with zeros up to full 13 digit number. This will also expand 12 digit UPC-A barcodes to EAN-13 automatically which is useful for older barcode readers which tend to ignore first zero in EAN-13 if they have just UPC-A support. It should be noted that EAN-13 or UPC-A product codes printed on books are not good choice for barcodes in Koha since each item has to have unique barcode. Test scenario: 1. prove t/Circulation_barcodedecode.t this checks expansion of 12 digit UPC-A to 13 digit EAN-13 and zero padding 2. in systempreferences search for itemBarcodeInputFilter and select EAN-13 3. edit one item and assign it valid EAN-13 barcode, eg. 0000000695152, check it out 4. test checkin with just 695152 to test leading zero expansion
Created attachment 10128 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This filter which check validity of EAN-13 barcode and padd it with zeros up to full 13 digit number. This will also expand 12 digit UPC-A barcodes to EAN-13 automatically which is useful for older barcode readers which tend to ignore first zero in EAN-13 if they have just UPC-A support. It should be noted that EAN-13 or UPC-A product codes printed on books are not good choice for barcodes in Koha since each item has to have unique barcode. Test scenario: 1. prove t/Circulation_barcodedecode.t this checks expansion of 12 digit UPC-A to 13 digit EAN-13 and zero padding 2. in systempreferences search for itemBarcodeInputFilter and select EAN-13 3. edit one item and assign it valid EAN-13 barcode, eg. 0000000695152, check it out 4. test checkin with just 695152 to test leading zero expansion Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 10129 [details] [review] Bug 6448 [2/3] Barcodes::EAN13 autoBarcode Implement auto-incrementing EAN-13 barcodes To make this work, C4::Barcodes::next was modified to call process_tail with new incremented value so that process_tail can generate correct checksum. Since process_tail is currenlty not used by any barcodes, this change is safe. C4::Barcodes is used by addbiblio.pl when adding multiple records, while value_builder is used in all other cases. Test scenario: 1. prove t/Barcodes_EAN13.t 2. KOHA_CONF=/etc/koha/sites/fer/koha-conf.xml prove t/db_dependent/Barcodes.t this will check C4::Barcode implementataion 3. in systempreference change autoBarcode to incremental EAN-13 barcode 4. edit two items of any biblio assigning barcodes and verify that numbers are increasing. Have in mind that last digit is check digit, and it doesn't increment, but is calculated from barcode itself. Example with checksum in brackets: 000000086275[2], 000000086276[9], 000000086277[6] 5. Add Item and verify that it gets assigned next barcode 6. Add & Duplicate item and verify barcode increase 7. Add Multiple Copies and verify that barcode increase for each copy Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 10130 [details] [review] Bug 6448 [3/3] EAN-13 barcode labels Current implementation doesn't scale barcodes because low-price CCD barcode readers are very sensitive about size Test scenario: 1. in Tools > Labels create or edit Layout and select EAN13 as barcode type 2. export one of existing batches using EAN13 layout and verify that generated pdf file contains barcodes 3. print pdf file and test it with barcode reader Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 10134 [details] [review] Bug 6448 [3/3] EAN-13 barcode labels Current implementation doesn't scale barcodes because low-price CCD barcode readers are very sensitive about size Test scenario: 1. in Tools > Labels create or edit Layout and select EAN13 as barcode type 2. export one of existing batches using EAN13 layout and verify that generated pdf file contains barcodes 3. print pdf file and test it with barcode reader
Please ignore last patch which was git bz gone wrong and just concentrate on Chirs' patches
First patch does not update sysprefs.sql to reflect the new EAN13 option. DB Rev is also at 3.07.... instead of 3.09.. (though the RM has to fix that manually, anyway). In Patch 2, C4/Barcodes/EAN13 is missing copyright statement. Also, subroutines are prototyped, which fails our perlcritic. Another syspref altered without changes to sysprefs.sql. Marking Failed QA for above reasons.
Created attachment 11041 [details] [review] Bug 6448 [3/3] EAN-13 barcode labels Current implementation doesn't scale barcodes because low-price CCD barcode readers are very sensitive about size Test scenario: 1. in Tools > Labels create or edit Layout and select EAN13 as barcode type 2. export one of existing batches using EAN13 layout and verify that generated pdf file contains barcodes 3. print pdf file and test it with barcode reader Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 11042 [details] [review] Bug 6448 [1/3] EAN-13/UPC-A itemBarcodeInputFilter This filter which check validity of EAN-13 barcode and padd it with zeros up to full 13 digit number. This will also expand 12 digit UPC-A barcodes to EAN-13 automatically which is useful for older barcode readers which tend to ignore first zero in EAN-13 if they have just UPC-A support. It should be noted that EAN-13 or UPC-A product codes printed on books are not good choice for barcodes in Koha since each item has to have unique barcode. Test scenario: 1. prove t/Circulation_barcodedecode.t this checks expansion of 12 digit UPC-A to 13 digit EAN-13 and zero padding 2. in systempreferences search for itemBarcodeInputFilter and select EAN-13 3. edit one item and assign it valid EAN-13 barcode, eg. 0000000695152, check it out 4. test checkin with just 695152 to test leading zero expansion Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 11043 [details] [review] Bug 6448 [2/3] Barcodes::EAN13 autoBarcode Implement auto-incrementing EAN-13 barcodes To make this work, C4::Barcodes::next was modified to call process_tail with new incremented value so that process_tail can generate correct checksum. Since process_tail is currenlty not used by any barcodes, this change is safe. C4::Barcodes is used by addbiblio.pl when adding multiple records, while value_builder is used in all other cases. Test scenario: 1. prove t/Barcodes_EAN13.t 2. KOHA_CONF=/etc/koha/sites/fer/koha-conf.xml prove t/db_dependent/Barcodes.t this will check C4::Barcode implementataion 3. in systempreference change autoBarcode to incremental EAN-13 barcode 4. edit two items of any biblio assigning barcodes and verify that numbers are increasing. Have in mind that last digit is check digit, and it doesn't increment, but is calculated from barcode itself. Example with checksum in brackets: 000000086275[2], 000000086276[9], 000000086277[6] 5. Add Item and verify that it gets assigned next barcode 6. Add & Duplicate item and verify barcode increase 7. Add Multiple Copies and verify that barcode increase for each copy Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
I addressed QA issues in first two patches
QA comment: * passes test-qa.pl * mostly include new feature using a lot of elsif, depending on the itemBarcodeInputFilter syspref. I tried to use "Remove spaces", still work well (dunno how to tests other cases) passed QA
Jenkins warning = this patch makes db_dependant test failed: perl t/db_dependent/Barcodes.t 1..66 ok 1 - use C4::Barcodes; 3.436456908560981e17 not valid EAN-13 barcode at /home/paul/koha.dev/koha-community//C4/Barcodes/EAN13.pm line 43. # Looks like you planned 66 tests but ran 1. # Looks like your test exited with 255 just after 1; The test dies because the 25 ok($obj1 = C4::Barcodes->new($_), "$pre Barcode Creation : new($_)"); calls C4::Barcodes::EAN13::parse, and the barcode is invalid I thought it was because of my database, but I'm not sure it is. Is it because of the database jenkins uses or something else, I can't say. But for sure, you're welcomed to investigate.
(In reply to comment #22) > I thought it was because of my database, but I'm not sure it is. Is it > because of the database jenkins uses or something else, I can't say. OK, it is my database, but also the jenkin's one ! I activated $debug, and see: autoBarcode format = EAN13 at t/db_dependent/Barcodes.t line 26 trying to create new EAN13 Retrieving max database query. (current) max barcode found: 3.436456908560981e17 3.436456908560981e17 not valid EAN-13 barcode at /home/paul/koha.dev/koha-community//C4/Barcodes/EAN13.pm line 43. I see that the (current) max barcode found comes from SELECT max(abs(barcode)) FROM items LIMIT 1; which is wrong for me because I don't have EAN13 as barcodes, but seems it's also wrong for the jenkins database. Dobrica, please provide needed information to chris_c !
Yes, the tests should work with the sample db, do you need to add an EAN-13 to the sample data. Either that or use Test::MockModule and DBD::Mock to mock a database. Otherwise Jenkins is going to keep complaining at an ever increasing number of people.
Adding EAN13 bar-code to database might help, but only if all other bar-codes are shorter than 13 digits. Excuse my ignorance, but where is the sample data and how can I add to it? I tried sandbox databases from http://git.koha-community.org/gitweb/?p=contrib/global.git;a=tree;f=sandbox and I can reproduce problem with them. Other bar-code plugins seems to carp on invalid input instead of die, so that might be another possible solution.
I think for the test the best option would be to use DBD::Mock, because there is no reason to think that all barcodes will be shorter than 13 digits on any arbitrary installation.
Created attachment 11349 [details] [review] Bug 6448 - carp on invalid barcode This is consistent with implementation of other barcode types which all carp instead of die on invalid barcodes.
I would almost prefer DBD::Mock, but it's another dependency, we should really require it for production deployment so it seems like overkill to introduce it for this case.
We are already using DBD::Mock in tests (or, if we aren't, we will be soon). I think people will be willing to install one additional dependency in order to ensure a more stable system.
This still needs to be fixed (the test that is) or it is going to continue to fail.
(In reply to comment #27) > Created attachment 11349 [details] [review] > Bug 6448 - carp on invalid barcode > > This is consistent with implementation of other barcode types > which all carp instead of die on invalid barcodes. Hey, it seems no-one (including me) spotted that you've submitted a fix that should please jenkins ! Note = when you submit a follow-up to a pushed patch, please switch back to "need signoff" to let everyone see there's something new. Anyway: I applied the patch, and prove t/db_dependent/Barcodes.t now returns a correct All tests successful. (even if this is a workaround, because the test is not actually testing correctly. It just don't die)