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.
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>
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>
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).
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
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.
(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?
(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.
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
Created attachment 110844 [details] [review] Bug 26351: Add unit tests
Created attachment 110846 [details] [review] Bug 26351: Add unit tests
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
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>
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>
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
Created attachment 111579 [details] [review] Bug 26351: Add unit tests
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.
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.
> * 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!
Created attachment 111862 [details] [review] Bug 26351: (QA follow-up) Remove unused method barcode_transform
Created attachment 111863 [details] [review] Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform
Created attachment 111894 [details] [review] Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode
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>
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>
Created attachment 111918 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
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
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!
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.
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>
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>
Created attachment 113573 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 113578 [details] [review] Bug 26351: (QA follow-up) Fix Checkouts.t
I update the doc in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
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.
Isn't there a problem if several plugins has the method defined?
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>
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>
Created attachment 115174 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 115179 [details] [review] Bug 26351: (QA follow-up) Fix Checkouts.t
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.
(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!
Sorry dude, this needs a rebase..
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
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>
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>
Created attachment 118651 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 118656 [details] [review] Bug 26351: (QA follow-up) Fix Checkouts.t
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.
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.
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>
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>
Created attachment 118683 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 118688 [details] [review] Bug 26351: (QA follow-up) Fix Checkouts.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
Passed QA again.. as it all works well.
Blocked by bug 28026.
Kyle, this is still blocked by bug 28026.
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>
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>
Created attachment 122435 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 122440 [details] [review] Bug 26351: (QA follow-up) Fix Checkouts.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
> Created attachment 122442 [details] [review] [review] > Bug 26351: Clean up includes in Circulation.pm This patch does not belong here.
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>
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>
Created attachment 124260 [details] [review] Bug 26351: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 124265 [details] [review] Bug 26351: (QA follow-up) Fix Checkouts.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
t/db_dependent/Koha/Plugins/Circulation_hooks.t is failing.
Created attachment 124313 [details] [review] Bug 26351: Fix t/db_dependent/Koha/Plugins/Circulation_hooks.t
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.
Created attachment 125447 [details] [review] Bug 26351: Revert improper change to unit test, fix number of tests
Created attachment 125763 [details] [review] Bug 26351: Remove uneeded use Koha::Plugins statements Left over from previous changes
Created attachment 125764 [details] [review] Bug 26351: Add missing barcodedecode import
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
I've squashed all the patches (the diff is much more easier to read!) and rebased it on top of bug 27526.
(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!
Pushed to master for 21.11, thanks to everybody involved!
This is great. I've edited "First version" in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks