Bug 8524 - Add barcode plugin that does not autofill
Summary: Add barcode plugin that does not autofill
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jared Camins-Esakov
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-30 15:41 UTC by Jared Camins-Esakov
Modified: 2013-12-05 19:59 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8524: Add a non-automatic barcode plugin (15.60 KB, patch)
2012-08-01 16:26 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8524: Add a non-automatic barcode plugin (15.70 KB, patch)
2012-08-03 15:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8524 follow-up: fix broken test (1.59 KB, patch)
2012-08-09 11:47 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8524 follow-up: fix broken test (1.61 KB, patch)
2012-08-09 12:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8524: Followup FIX an undeclared variable $query (966 bytes, patch)
2012-08-09 12:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8524 follow-up: fix Javascript syntax error (1.35 KB, patch)
2012-08-09 12:38 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8524 follow-up: fix Javascript syntax error (1.37 KB, patch)
2012-08-09 12:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8524 follow-up: fix broken test (1.67 KB, patch)
2012-08-09 13:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8524: Followup FIX an undeclared variable $query (1021 bytes, patch)
2012-08-09 13:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8524 follow-up: fix Javascript syntax error (1.42 KB, patch)
2012-08-09 13:06 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-07-30 15:41:07 UTC
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.
Comment 1 Jared Camins-Esakov 2012-08-01 16:26:18 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-08-01 16:30:08 UTC
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.
Comment 3 Kyle M Hall 2012-08-03 15:50:53 UTC
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>
Comment 4 Jonathan Druart 2012-08-09 09:35:58 UTC
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.
Comment 5 Jared Camins-Esakov 2012-08-09 11:47:31 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2012-08-09 12:03:31 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2012-08-09 12:08:20 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2012-08-09 12:29:22 UTC
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.
Comment 9 Jared Camins-Esakov 2012-08-09 12:32:16 UTC
(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.
Comment 10 Jared Camins-Esakov 2012-08-09 12:38:40 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2012-08-09 12:59:41 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2012-08-09 13:04:39 UTC
(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
Comment 13 Kyle M Hall 2012-08-09 13:05:38 UTC
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>
Comment 14 Kyle M Hall 2012-08-09 13:06:08 UTC
Created attachment 11494 [details] [review]
Bug 8524: Followup FIX an undeclared variable $query

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Kyle M Hall 2012-08-09 13:06:33 UTC
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>
Comment 16 Jonathan Druart 2012-08-09 13:17:41 UTC
Last question :) Why didn't you remove the other plugin (barcode.pl) ? It seems they do exacly the same (except for $autoBarcodeType eq 'EAN13').
Comment 17 Jared Camins-Esakov 2012-08-09 13:35:28 UTC
(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.
Comment 18 Paul Poulain 2012-08-31 15:53:41 UTC
I haven't tested that the new plugin work, but the old plugin still work, I've tested that.
Comment 19 Chris Cormack 2012-09-13 20:59:59 UTC
After no objections, this was pushed to 3.8.x and will be in 3.8.5