The current barcode plugin uses the autobarcode syspref to decide which type of barcode to generate, then *always* inserts a barcode. There should be a plugin which will only insert the barcode when the plugin link is clicked.
Created attachment 11271 [details] [review] Bug 8524: Add a non-automatic barcode plugin In addition to adding a new barcode plugin, this commit begins refactoring the barcode generation code using a new module, C4::Barcodes::ValueBuilder. From the POD: This module is intended as a shim to ease the eventual transition from having all barcode-related code in the value builder plugin barcode.pl file to using C4::Barcodes. Since the shift will require a rather significant amount of refactoring, this module will return value builder-formatted results, at first by merely running the code that was formerly in the barcodes.pl value builder, but later by using C4::Barcodes.
To test: 1) Set the plugin for 952$p (or UNIMARC equivalent to items.barcode) for one of your frameworks to barcode_manual.pl instead of barcode.pl. 2) Turn on autobarcode. 3) Observe that when you click in the barcode field, nothing happens. 4) Observe that when you click the value builder link next to the plugin, the barcode field is filled in. 5) Run t/Barcodes_ValueBuilder.t, ensure that the tests complete. That's it, really.
Created attachment 11343 [details] [review] Bug 8524: Add a non-automatic barcode plugin In addition to adding a new barcode plugin, this commit begins refactoring the barcode generation code using a new module, C4::Barcodes::ValueBuilder. From the POD: This module is intended as a shim to ease the eventual transition from having all barcode-related code in the value builder plugin barcode.pl file to using C4::Barcodes. Since the shift will require a rather significant amount of refactoring, this module will return value builder-formatted results, at first by merely running the code that was formerly in the barcodes.pl value builder, but later by using C4::Barcodes. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
You plan 15 tests in t/Barcodes_ValueBuilder.t but only 10 are run. You can replace use Test::More tests => 15; with use Test::More; and at the end of the file: done_testing; or use Test::More tests => 15; => use Test::More tests => 10; + 2 warnings: t/Barcodes_ValueBuilder.t .. 1/15 Use of uninitialized value $scr in numeric eq (==) at t/Barcodes_ValueBuilder.t line 43. C4::Barcodes::ValueBuilder::incremental::get_barcode returns 1 results not 2. $src is always undefined. Same thing for C4::Barcodes::ValueBuilder::annual::get_barcode Marking as Failed QA.
Created attachment 11481 [details] [review] Bug 8524 follow-up: fix broken test The t/Barcodes_ValueBuilder.t test had the wrong number of tests declared, and rather than checking that a variable was undefined I was checking that it had a length of zero. Fixed both issues.
Created attachment 11484 [details] [review] Bug 8524 follow-up: fix broken test The t/Barcodes_ValueBuilder.t test had the wrong number of tests declared, and rather than checking that a variable was undefined I was checking that it had a length of zero. Fixed both issues. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 11485 [details] [review] Bug 8524: Followup FIX an undeclared variable $query
Jared, Sorry, but another problem already exists. When I click twice on '...' to fill with the barcode value, a JS error occured: "force is not defined". The issue only exist for the plugin barcode_manual.pl.
(In reply to comment #8) > Jared, > Sorry, but another problem already exists. When I click twice on '...' to > fill with the barcode value, a JS error occured: "force is not defined". > The issue only exist for the plugin barcode_manual.pl. Ooh, good test. It never occurred to me to click twice.
Created attachment 11491 [details] [review] Bug 8524 follow-up: fix Javascript syntax error
Created attachment 11492 [details] [review] Bug 8524 follow-up: fix Javascript syntax error Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
(In reply to comment #9) > Ooh, good test. It never occurred to me to click twice. I didn't do it on purpose :) I just forgot to reload the page... Marked as Passed QA
Created attachment 11493 [details] [review] Bug 8524 follow-up: fix broken test The t/Barcodes_ValueBuilder.t test had the wrong number of tests declared, and rather than checking that a variable was undefined I was checking that it had a length of zero. Fixed both issues. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11494 [details] [review] Bug 8524: Followup FIX an undeclared variable $query Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11495 [details] [review] Bug 8524 follow-up: fix Javascript syntax error Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Last question :) Why didn't you remove the other plugin (barcode.pl) ? It seems they do exacly the same (except for $autoBarcodeType eq 'EAN13').
(In reply to comment #16) > Last question :) Why didn't you remove the other plugin (barcode.pl) ? It > seems they do exacly the same (except for $autoBarcodeType eq 'EAN13'). barcode.pl works exactly the way it always has: whenever the barcode field has focus, it is populated. For many libraries this is not the desired functionality. barcode_manual.pl will not populate the barcode unless the ellipsis is clicked.
I haven't tested that the new plugin work, but the old plugin still work, I've tested that.
After no objections, this was pushed to 3.8.x and will be in 3.8.5