Bug 26351 - Add plugin hooks to transform item barcodes
Summary: Add plugin hooks to transform item barcodes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 27526 28026
Blocks: 31252 31253 26352
  Show dependency treegraph
 
Reported: 2020-09-02 13:31 UTC by Kyle M Hall
Modified: 2022-07-28 13:12 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a plugin hook to transform item barcodes scanned in to Koha. For example, if you need to alter your scanned item barcodes, but your scanners cannot be programmed to do so, a plugin could be written to handle that change in Koha instead. One example would be to drop the first and last characters of the scanned barcode, which may be check digits rather than part of the barcode itself.
Version(s) released in:
21.11.00


Attachments
Bug 26351: Add plugin hooks to transform item barcodes (11.88 KB, patch)
2020-09-02 13:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.06 KB, patch)
2020-09-02 15:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (992 bytes, patch)
2020-09-04 15:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (2.27 KB, patch)
2020-09-28 10:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (4.95 KB, patch)
2020-09-28 10:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.08 KB, patch)
2020-10-13 18:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.08 KB, patch)
2020-10-13 22:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (992 bytes, patch)
2020-10-13 22:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (4.95 KB, patch)
2020-10-13 22:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.90 KB, patch)
2020-10-16 11:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.05 KB, patch)
2020-10-16 11:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.21 KB, patch)
2020-10-16 14:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.13 KB, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.02 KB, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: Add unit tests (4.99 KB, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.95 KB, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.10 KB, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.26 KB, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1012 bytes, patch)
2020-10-16 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (14.06 KB, patch)
2020-11-12 17:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.03 KB, patch)
2020-11-12 17:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (5.01 KB, patch)
2020-11-12 17:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.98 KB, patch)
2020-11-12 17:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.11 KB, patch)
2020-11-12 17:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.27 KB, patch)
2020-11-12 17:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1.05 KB, patch)
2020-11-12 17:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix Checkouts.t (735 bytes, patch)
2020-11-12 17:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (14.05 KB, patch)
2021-01-14 15:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.03 KB, patch)
2021-01-14 15:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (5.01 KB, patch)
2021-01-14 15:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.98 KB, patch)
2021-01-14 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.11 KB, patch)
2021-01-14 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.26 KB, patch)
2021-01-14 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1.05 KB, patch)
2021-01-14 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix Checkouts.t (721 bytes, patch)
2021-01-14 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add call_recursive() as a replacement for call() (7.48 KB, patch)
2021-01-14 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.48 KB, patch)
2021-03-23 13:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.03 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (5.01 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.98 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.11 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.26 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1.06 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix Checkouts.t (721 bytes, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add call_recursive() as a replacement for call() (7.48 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Clean up includes in Circulation.pm (3.00 KB, patch)
2021-03-23 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.46 KB, patch)
2021-03-23 16:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.02 KB, patch)
2021-03-23 16:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: Add unit tests (4.99 KB, patch)
2021-03-23 16:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.96 KB, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.09 KB, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.24 KB, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1.04 KB, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix Checkouts.t (770 bytes, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: Use call_recursive() as a replacement for call() (2.06 KB, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: Clean up includes in Circulation.pm (3.05 KB, patch)
2021-03-23 16:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (12.48 KB, patch)
2021-06-25 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.03 KB, patch)
2021-06-25 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add unit tests (5.01 KB, patch)
2021-06-25 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.98 KB, patch)
2021-06-25 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.10 KB, patch)
2021-06-25 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.25 KB, patch)
2021-06-25 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1.06 KB, patch)
2021-06-25 12:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix Checkouts.t (786 bytes, patch)
2021-06-25 12:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Use call_recursive() as a replacement for call() (2.08 KB, patch)
2021-06-25 12:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Clean up includes in Circulation.pm (3.05 KB, patch)
2021-06-25 12:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (11.98 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix QA script issue (1.02 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: Add unit tests (5.00 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Remove unused method barcode_transform (1.97 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform (12.12 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode (7.25 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter (1.04 KB, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: (QA follow-up) Fix Checkouts.t (772 bytes, patch)
2021-08-31 07:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: Use call_recursive() as a replacement for call() (2.06 KB, patch)
2021-08-31 07:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: Fix t/db_dependent/Koha/Plugins/Circulation_hooks.t (1.16 KB, patch)
2021-08-31 18:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Revert improper change to unit test, fix number of tests (1.20 KB, patch)
2021-09-29 11:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26351: Remove uneeded use Koha::Plugins statements (2.11 KB, patch)
2021-10-06 09:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: Add missing barcodedecode import (2.77 KB, patch)
2021-10-06 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26351: Add plugin hooks to transform item barcodes (18.75 KB, patch)
2021-10-06 10:27 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 Kyle M Hall 2020-09-02 13:31:16 UTC
Moved from bug 25712

Some of our partners have unusual barcode requirements that have required us to transform scanned barcodes using javascript. This is not the most reliable method. It would make more sense to have Koha transform the barcodes on the backend using a plugin. We should add hooks to transform and generate new item and patron barcodes.
Comment 1 Kyle M Hall 2020-09-02 13:33:18 UTC
Created attachment 109532 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 2 Kyle M Hall 2020-09-02 15:29:26 UTC
Created attachment 109548 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 3 Katrin Fischer 2020-09-03 20:59:45 UTC
Lacking the time to test deeply here tonight, but there is a small fail on the QA tools:

 FAIL	t/lib/Koha/Plugin/Test.pm
   FAIL	  critic
		# Variables::ProhibitUnusedVariables: Got 1 violation(s).
Comment 4 Kyle M Hall 2020-09-04 15:29:46 UTC
Created attachment 109671 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method
Comment 5 Julian Maurice 2020-09-11 09:08:32 UTC
I would like to make a suggestion about the hook name: IMO hooks should be named after an event (what just happened in Koha) rather than what the plugin is supposed to do. From what I understood this hook will be called every time a barcode is scanned. A plugin can choose to do other things than transforming the barcode (log it somewhere, transmit it to an external webservice, ...), so it would be more generic to call it "barcode_scanned" or something like that.

Also, I took a quick look at the code and I have a concern: what will happen if two (or more) plugins implement this hook ? It looks like only the return value from the first plugin will be used. We can imagine one want to implement item barcode transform in one plugin and patron barcode transform in another.
Comment 6 Kyle M Hall 2020-09-11 12:16:27 UTC
(In reply to Julian Maurice from comment #5)
> I would like to make a suggestion about the hook name: IMO hooks should be
> named after an event (what just happened in Koha) rather than what the
> plugin is supposed to do. From what I understood this hook will be called
> every time a barcode is scanned. A plugin can choose to do other things than
> transforming the barcode (log it somewhere, transmit it to an external
> webservice, ...), so it would be more generic to call it "barcode_scanned"
> or something like that.

I'm happy to change that!
 
> Also, I took a quick look at the code and I have a concern: what will happen
> if two (or more) plugins implement this hook ? It looks like only the return
> value from the first plugin will be used. We can imagine one want to
> implement item barcode transform in one plugin and patron barcode transform
> in another.

In it's current state, the last plugin 'wins'. A couple solutions would be to:
A) Modify call() to feed the result of the last plugin call into the next one
B) Split the hooks into separate methods

I'm much more inclined to go with B. What do you think Julian?
Comment 7 Julian Maurice 2020-09-11 12:38:17 UTC
(In reply to Kyle M Hall from comment #6)
> In it's current state, the last plugin 'wins'. A couple solutions would be
> to:
> A) Modify call() to feed the result of the last plugin call into the next one
> B) Split the hooks into separate methods
> 
> I'm much more inclined to go with B. What do you think Julian?

I think A would not work in all cases. There can be more than one parameter and it's not necessarily the same thing as the return value.
B would work in the example I mentioned, but would not work in other cases, like two plugins implementing the same method, one for transforming, the other for doing something else.

I've seen a pattern somewhere else that could work : plugins modify directly the parameters

  my $params = { barcode => $barcode };
  Koha::Plugins->call($hook_name, $params);
  $barcode = $params->{barcode};

It's not very intuitive, but it should work as expected in all cases, and there's no need to modify Koha::Plugins->call.
Comment 8 Martin Renvoize 2020-09-25 10:54:46 UTC
I don't see a test that confirms the hook into Koha::Item works.. there's prior art for such tests I believe.

Failing QA
Comment 9 Kyle M Hall 2020-09-28 10:51:42 UTC
Created attachment 110844 [details] [review]
Bug 26351: Add unit tests
Comment 10 Kyle M Hall 2020-09-28 10:54:44 UTC
Created attachment 110846 [details] [review]
Bug 26351: Add unit tests
Comment 11 Katrin Fischer 2020-10-11 12:36:11 UTC
Patch on longer applies - please rebase!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 26351: Add plugin hooks to transform item barcodes
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	C4/Items.pm
M	Koha/Item.pm
M	cataloguing/additem.pl
M	circ/circulation.pl
M	circ/returns.pl
M	offline_circ/process_koc.pl
M	opac/sco/sco-main.pl
M	t/lib/Koha/Plugin/Test.pm
M	tools/batchMod.pl
M	tools/inventory.pl
Falling back to patching base and 3-way merge...
Auto-merging tools/inventory.pl
Auto-merging tools/batchMod.pl
CONFLICT (content): Merge conflict in tools/batchMod.pl
Auto-merging t/lib/Koha/Plugin/Test.pm
Auto-merging opac/sco/sco-main.pl
Auto-merging offline_circ/process_koc.pl
Auto-merging circ/returns.pl
Auto-merging circ/circulation.pl
Auto-merging cataloguing/additem.pl
CONFLICT (content): Merge conflict in cataloguing/additem.pl
Auto-merging Koha/Item.pm
CONFLICT (content): Merge conflict in Koha/Item.pm
Auto-merging C4/Items.pm
Auto-merging C4/Circulation.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 26351: Add plugin hooks to transform item barcodes
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-26351-Add-plugin-hooks-to-transform-item-barco-ho0gdo.patch
Comment 12 Kyle M Hall 2020-10-13 18:52:20 UTC
Created attachment 111575 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 13 Kyle M Hall 2020-10-13 22:24:50 UTC
Created attachment 111577 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 14 Kyle M Hall 2020-10-13 22:25:12 UTC
Created attachment 111578 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method
Comment 15 Kyle M Hall 2020-10-13 22:25:18 UTC
Created attachment 111579 [details] [review]
Bug 26351: Add unit tests
Comment 16 Martin Renvoize 2020-10-16 10:44:29 UTC
Sorry Kyle....

* I see a test for barcode_generate, but don't see any code related to this in the rest of the patchset.. does this belong in a different bug?
* You're a tad inconsistent with when you call 'barcode_transform'.. sometimes it's prior to a call to 'barcodedecode', sometimes it's just after.. I imagine that it should be just after (and only actually called if 'barcodedecode' is not run (as barcodedecode runs it internally already).
* Finally, I wonder about the name of the method 'barcode_transform'.. it made sense prior to splitting the bug in two.. but perhaps it would be clearer/easier for plugin developers to have a different name for the two options.. item_barcode_transform and patron_barcode_transfer perhaps?.

I'm continuing with testing, but feedback on the above would be great.
Comment 17 Katrin Fischer 2020-10-16 10:48:00 UTC
If you were to change that one, maybe patron_cardnumber_transfer? With chip cards and the likes that might be more accurate and stay close to our db name.
Comment 18 Kyle M Hall 2020-10-16 11:18:54 UTC
> * I see a test for barcode_generate, but don't see any code related to this
> in the rest of the patchset.. does this belong in a different bug?

Yes, it is only used in Bug 26352.

> * You're a tad inconsistent with when you call 'barcode_transform'..
> sometimes it's prior to a call to 'barcodedecode', sometimes it's just
> after.. I imagine that it should be just after (and only actually called if
> 'barcodedecode' is not run (as barcodedecode runs it internally already).

Would it make more sense to replace all the calls *with* calls to barcodedecode? If feels incorrect for barcodedecode to be used inconsistently.

> * Finally, I wonder about the name of the method 'barcode_transform'.. it
> made sense prior to splitting the bug in two.. but perhaps it would be
> clearer/easier for plugin developers to have a different name for the two
> options.. item_barcode_transform and patron_barcode_transfer perhaps?.

That is fine by me!
Comment 19 Kyle M Hall 2020-10-16 11:23:53 UTC
Created attachment 111862 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform
Comment 20 Kyle M Hall 2020-10-16 11:36:53 UTC
Created attachment 111863 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform
Comment 21 Kyle M Hall 2020-10-16 14:43:28 UTC
Created attachment 111894 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode
Comment 22 Martin Renvoize 2020-10-16 15:47:13 UTC
Created attachment 111916 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2020-10-16 15:47:17 UTC
Created attachment 111917 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2020-10-16 15:47:23 UTC
Created attachment 111918 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2020-10-16 15:47:28 UTC
Created attachment 111919 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2020-10-16 15:47:33 UTC
Created attachment 111920 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2020-10-16 15:47:36 UTC
Created attachment 111921 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2020-10-16 15:47:41 UTC
Created attachment 111922 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2020-10-16 15:48:37 UTC
All works well here, and it's great to see a little general cleanup at the end, thanks Kyle.

QA scripts are happy, no regressions found and Circulation tests all passing.

Passing QA
Comment 30 Kyle M Hall 2020-10-19 14:04:28 UTC
RM Note: I will need to update the plugins I've written take account for the changes in this bug series. I will do that once this bug has been pushed and bug 26342 has passed qa!
Comment 31 Jonathan Druart 2020-11-09 13:51:24 UTC
Please 1. rebase, 2. adjust author of 4th patch and 3. fix the failure on Checkouts.t

t/db_dependent/Koha/Checkouts.t .. Use of inherited AUTOLOAD for non-method Koha::Item::barcodedecode() is no longer allowed at /kohadevbox/koha/Koha/Item.pm line 92.
Comment 32 Kyle M Hall 2020-11-12 17:30:32 UTC
Created attachment 113571 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 33 Kyle M Hall 2020-11-12 17:30:45 UTC
Created attachment 113572 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 34 Kyle M Hall 2020-11-12 17:30:48 UTC
Created attachment 113573 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Kyle M Hall 2020-11-12 17:30:51 UTC
Created attachment 113574 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Kyle M Hall 2020-11-12 17:30:54 UTC
Created attachment 113575 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Kyle M Hall 2020-11-12 17:30:57 UTC
Created attachment 113576 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 38 Kyle M Hall 2020-11-12 17:31:00 UTC
Created attachment 113577 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Kyle M Hall 2020-11-12 17:31:03 UTC
Created attachment 113578 [details] [review]
Bug 26351: (QA follow-up) Fix Checkouts.t
Comment 40 Fridolin Somers 2020-11-20 12:57:58 UTC
I update the doc in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
Comment 41 Jonathan Druart 2021-01-14 10:19:26 UTC
Hum, there is something wrong here.

Plugin has:
sub item_barcode_transform {
    my ( $self, $barcode ) = @_; 
    $barcode =~ s|0|X|g;
    return $barcode;
}

Checkout "0101"

I get "Barcode not found The barcode was not found: 1"

I am expecting "Barcode not found The barcode was not found: X1X1"

It would be nice to have the example plugin updated.
Comment 42 Jonathan Druart 2021-01-14 10:20:24 UTC
Isn't there a problem if several plugins has the method defined?
Comment 43 Kyle M Hall 2021-01-14 15:59:42 UTC
Created attachment 115172 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Kyle M Hall 2021-01-14 15:59:53 UTC
Created attachment 115173 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Kyle M Hall 2021-01-14 15:59:56 UTC
Created attachment 115174 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Kyle M Hall 2021-01-14 16:00:02 UTC
Created attachment 115175 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Kyle M Hall 2021-01-14 16:00:06 UTC
Created attachment 115176 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Kyle M Hall 2021-01-14 16:00:09 UTC
Created attachment 115177 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Kyle M Hall 2021-01-14 16:00:12 UTC
Created attachment 115178 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Kyle M Hall 2021-01-14 16:00:15 UTC
Created attachment 115179 [details] [review]
Bug 26351: (QA follow-up) Fix Checkouts.t
Comment 51 Kyle M Hall 2021-01-14 16:00:19 UTC
Created attachment 115180 [details] [review]
Bug 26351: Add call_recursive() as a replacement for call()

The method call() is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. call_recursive() takes the output of the first plugin and
uses it as the input for the next plugin and so on. This allowes each
plugin to see the current version of the barcode and modify it if
necessary.
Comment 52 Kyle M Hall 2021-01-14 16:02:45 UTC
(In reply to Jonathan Druart from comment #42)
> Isn't there a problem if several plugins has the method defined?

Looks like call() get modified so it no longer works for this patch set. I've introduced call_recursive() ( not technically recursive, can be renamed ) that fixes this and allows for multiple barcode transformation plugins to be called in succession!
Comment 53 Martin Renvoize 2021-03-09 16:17:13 UTC
Sorry dude, this needs a rebase..
Comment 54 Martin Renvoize 2021-03-09 16:25:43 UTC
I'm also not entirely sure about the final followup.. it feels like the intention of ->call is to loop through available plugins too.. call_recursive is almost a match code wise.

I think it'll be difficult for developers to know which of the two methods to call when.. and perhaps difficult for plugin developers to understand what they should be returning... something that can be fed back into @args or something entirely different..

Hmm
Comment 55 Kyle M Hall 2021-03-23 13:43:59 UTC
Created attachment 118649 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Kyle M Hall 2021-03-23 13:44:10 UTC
Created attachment 118650 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Kyle M Hall 2021-03-23 13:44:13 UTC
Created attachment 118651 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Kyle M Hall 2021-03-23 13:44:16 UTC
Created attachment 118652 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Kyle M Hall 2021-03-23 13:44:19 UTC
Created attachment 118653 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Kyle M Hall 2021-03-23 13:44:22 UTC
Created attachment 118654 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 61 Kyle M Hall 2021-03-23 13:44:25 UTC
Created attachment 118655 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 62 Kyle M Hall 2021-03-23 13:44:29 UTC
Created attachment 118656 [details] [review]
Bug 26351: (QA follow-up) Fix Checkouts.t
Comment 63 Kyle M Hall 2021-03-23 13:44:32 UTC
Created attachment 118657 [details] [review]
Bug 26351: Add call_recursive() as a replacement for call()

The method call() is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. call_recursive() takes the output of the first plugin and
uses it as the input for the next plugin and so on. This allowes each
plugin to see the current version of the barcode and modify it if
necessary.
Comment 64 Kyle M Hall 2021-03-23 13:44:35 UTC
Created attachment 118658 [details] [review]
Bug 26351: Clean up includes in Circulation.pm

Circulation.pm has so many includes that the same module
is getting used multiple times without developers noticing it
is already beuing used.

This commit separates use lines by namespace and sorts them,
removing duplicate and unnecessary use lines.
Comment 65 Martin Renvoize 2021-03-23 16:31:50 UTC
Created attachment 118681 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 66 Martin Renvoize 2021-03-23 16:31:54 UTC
Created attachment 118682 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 67 Martin Renvoize 2021-03-23 16:31:58 UTC
Created attachment 118683 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 68 Martin Renvoize 2021-03-23 16:32:02 UTC
Created attachment 118684 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 69 Martin Renvoize 2021-03-23 16:32:06 UTC
Created attachment 118685 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 70 Martin Renvoize 2021-03-23 16:32:09 UTC
Created attachment 118686 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 71 Martin Renvoize 2021-03-23 16:32:13 UTC
Created attachment 118687 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 72 Martin Renvoize 2021-03-23 16:32:17 UTC
Created attachment 118688 [details] [review]
Bug 26351: (QA follow-up) Fix Checkouts.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 73 Martin Renvoize 2021-03-23 16:32:21 UTC
Created attachment 118689 [details] [review]
Bug 26351: Use call_recursive() as a replacement for call()

The method `call()` is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. The `call_recursive()` method takes the output of the first
plugin and uses it as the input for the next plugin and so on. This allowes
each plugin to see the current version of the barcode and modify it if
necessary.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 74 Martin Renvoize 2021-03-23 16:32:24 UTC
Created attachment 118690 [details] [review]
Bug 26351: Clean up includes in Circulation.pm

Circulation.pm has so many includes that the same module
is getting used multiple times without developers noticing it
is already beuing used.

This commit separates use lines by namespace and sorts them,
removing duplicate and unnecessary use lines.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 75 Martin Renvoize 2021-03-23 16:33:08 UTC
Passed QA again.. as it all works well.
Comment 76 Jonathan Druart 2021-04-08 09:58:56 UTC
Blocked by bug 28026.
Comment 77 Jonathan Druart 2021-06-25 09:06:52 UTC
Kyle, this is still blocked by bug 28026.
Comment 78 Kyle M Hall 2021-06-25 12:05:31 UTC
Created attachment 122433 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 79 Kyle M Hall 2021-06-25 12:05:44 UTC
Created attachment 122434 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 80 Kyle M Hall 2021-06-25 12:05:47 UTC
Created attachment 122435 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 81 Kyle M Hall 2021-06-25 12:05:51 UTC
Created attachment 122436 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 82 Kyle M Hall 2021-06-25 12:05:55 UTC
Created attachment 122437 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 83 Kyle M Hall 2021-06-25 12:05:59 UTC
Created attachment 122438 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 84 Kyle M Hall 2021-06-25 12:06:03 UTC
Created attachment 122439 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 85 Kyle M Hall 2021-06-25 12:06:07 UTC
Created attachment 122440 [details] [review]
Bug 26351: (QA follow-up) Fix Checkouts.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 86 Kyle M Hall 2021-06-25 12:06:11 UTC
Created attachment 122441 [details] [review]
Bug 26351: Use call_recursive() as a replacement for call()

The method `call()` is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. The `call_recursive()` method takes the output of the first
plugin and uses it as the input for the next plugin and so on. This allowes
each plugin to see the current version of the barcode and modify it if
necessary.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 87 Kyle M Hall 2021-06-25 12:06:15 UTC
Created attachment 122442 [details] [review]
Bug 26351: Clean up includes in Circulation.pm

Circulation.pm has so many includes that the same module
is getting used multiple times without developers noticing it
is already beuing used.

This commit separates use lines by namespace and sorts them,
removing duplicate and unnecessary use lines.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 88 Jonathan Druart 2021-08-31 07:04:10 UTC
> Created attachment 122442 [details] [review] [review]
> Bug 26351: Clean up includes in Circulation.pm

This patch does not belong here.
Comment 89 Jonathan Druart 2021-08-31 07:05:20 UTC
Created attachment 124258 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Jonathan Druart 2021-08-31 07:05:26 UTC
Created attachment 124259 [details] [review]
Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 91 Jonathan Druart 2021-08-31 07:05:32 UTC
Created attachment 124260 [details] [review]
Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Jonathan Druart 2021-08-31 07:05:37 UTC
Created attachment 124261 [details] [review]
Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Jonathan Druart 2021-08-31 07:05:42 UTC
Created attachment 124262 [details] [review]
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 94 Jonathan Druart 2021-08-31 07:05:47 UTC
Created attachment 124263 [details] [review]
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 95 Jonathan Druart 2021-08-31 07:05:52 UTC
Created attachment 124264 [details] [review]
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Jonathan Druart 2021-08-31 07:05:57 UTC
Created attachment 124265 [details] [review]
Bug 26351: (QA follow-up) Fix Checkouts.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 97 Jonathan Druart 2021-08-31 07:06:02 UTC
Created attachment 124266 [details] [review]
Bug 26351: Use call_recursive() as a replacement for call()

The method `call()` is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. The `call_recursive()` method takes the output of the first
plugin and uses it as the input for the next plugin and so on. This allowes
each plugin to see the current version of the barcode and modify it if
necessary.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 98 Jonathan Druart 2021-08-31 07:34:04 UTC
t/db_dependent/Koha/Plugins/Circulation_hooks.t is failing.
Comment 99 Kyle M Hall 2021-08-31 18:10:32 UTC
Created attachment 124313 [details] [review]
Bug 26351: Fix t/db_dependent/Koha/Plugins/Circulation_hooks.t
Comment 100 Jonathan Druart 2021-09-29 09:58:24 UTC
Failing test:

    #   Failed test 'Only four plugins found via a metadata tag'
    #   at /kohadevbox/koha/t/db_dependent/Koha/Plugins/Plugins.t line 129.
    #          got: '2'
    #     expected: '4'
    # Looks like you failed 1 test of 2.
Comment 101 Kyle M Hall 2021-09-29 11:00:12 UTC
Created attachment 125447 [details] [review]
Bug 26351: Revert improper change to unit test, fix number of tests
Comment 102 Jonathan Druart 2021-10-06 09:51:42 UTC
Created attachment 125763 [details] [review]
Bug 26351: Remove uneeded use Koha::Plugins statements

Left over from previous changes
Comment 103 Jonathan Druart 2021-10-06 10:08:33 UTC
Created attachment 125764 [details] [review]
Bug 26351: Add missing barcodedecode import
Comment 104 Jonathan Druart 2021-10-06 10:27:30 UTC
Created attachment 125765 [details] [review]
Bug 26351: Add plugin hooks to transform item barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Fix Checkouts.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: Use call_recursive() as a replacement for call()

The method `call()` is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. The `call_recursive()` method takes the output of the first
plugin and uses it as the input for the next plugin and so on. This allowes
each plugin to see the current version of the barcode and modify it if
necessary.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: Fix t/db_dependent/Koha/Plugins/Circulation_hooks.t

Bug 26351: Revert improper change to unit test, fix number of tests

Bug 26351: Remove uneeded use Koha::Plugins statements

Left over from previous changes

Bug 26351: Add missing barcodedecode import
Comment 105 Jonathan Druart 2021-10-06 10:28:20 UTC
I've squashed all the patches (the diff is much more easier to read!) and rebased it on top of bug 27526.
Comment 106 Kyle M Hall 2021-10-06 12:05:14 UTC
(In reply to Jonathan Druart from comment #105)
> I've squashed all the patches (the diff is much more easier to read!) and
> rebased it on top of bug 27526.

Thanks!
Comment 107 Jonathan Druart 2021-10-06 12:56:47 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 108 Fridolin Somers 2021-10-07 21:56:52 UTC
This is great.
I've edited "First version" in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks