From b2b1483f4d829512b7d92723b587a7f060de57bb Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 2 Sep 2020 09:17:31 -0400 Subject: [PATCH] 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 Signed-off-by: Martin Renvoize 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 Bug 26351: Add unit tests Signed-off-by: Martin Renvoize Bug 26351: (QA follow-up) Remove unused method barcode_transform Signed-off-by: Martin Renvoize Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform Signed-off-by: Martin Renvoize Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode Signed-off-by: Martin Renvoize Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter Signed-off-by: Martin Renvoize Bug 26351: (QA follow-up) Fix Checkouts.t Signed-off-by: Martin Renvoize 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 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 Signed-off-by: Jonathan Druart --- C4/Circulation.pm | 2 + C4/ILSDI/Services.pm | 2 +- Koha/Item.pm | 4 +- cataloguing/additem.pl | 4 +- circ/circulation.pl | 4 +- circ/renew.pl | 2 +- circ/returns.pl | 4 +- offline_circ/process_koc.pl | 7 +- opac/sco/sco-main.pl | 5 +- .../Koha/Plugins/Barcode_transform_hooks.t | 78 +++++++++++++++++++ .../Plugins/Biblio_and_Items_plugin_hooks.t | 3 + .../Koha/Plugins/Circulation_hooks.t | 1 + t/db_dependent/Koha/Plugins/Plugins.t | 3 +- t/lib/Koha/Plugin/Test.pm | 7 +- tools/batchMod.pl | 5 +- tools/inventory.pl | 6 +- 16 files changed, 122 insertions(+), 15 deletions(-) create mode 100755 t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 11c3c7713d..ebc7988744 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -61,6 +61,7 @@ use Koha::Config::SysPref; use Koha::Checkouts::ReturnClaims; use Koha::SearchEngine::Indexer; use Koha::Exceptions::Checkout; +use Koha::Plugins; use Carp qw( carp ); use List::MoreUtils qw( any ); use Scalar::Util qw( looks_like_number ); @@ -165,6 +166,7 @@ sub barcodedecode { my ($barcode, $filter) = @_; my $branch = C4::Context::mybranch(); $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter; + ($barcode) = Koha::Plugins->call_recursive('item_barcode_transform', $barcode ); $filter or return $barcode; # ensure filter is defined, else return untouched barcode if ($filter eq 'whitespace') { $barcode =~ s/\s//g; diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 64b481bc6f..34465a7caf 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -642,7 +642,7 @@ sub GetServices { # Issuing management my $barcode = $item->barcode || ''; - $barcode = barcodedecode($barcode) if ( $barcode && C4::Context->preference('itemBarcodeInputFilter') ); + $barcode = barcodedecode($barcode) if $barcode; if ($barcode) { my ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $barcode ); diff --git a/Koha/Item.pm b/Koha/Item.pm index 3cce38faa2..08e527df10 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -26,7 +26,7 @@ use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); use C4::Context; -use C4::Circulation qw( GetBranchItemRule ); +use C4::Circulation qw( barcodedecode GetBranchItemRule ); use C4::Reserves; use C4::ClassSource qw( GetClassSort ); use C4::Log qw( logaction ); @@ -89,6 +89,8 @@ sub store { $self->itype($self->biblio->biblioitem->itemtype); } + $self->barcode( C4::Circulation::barcodedecode( $self->barcode ) ); + my $today = dt_from_string; my $action = 'create'; diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index f5b57d2489..bd49af6051 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -33,7 +33,7 @@ use C4::Biblio qw( ModBiblio ); use C4::Context; -use C4::Circulation qw( LostItem ); +use C4::Circulation qw( barcodedecode LostItem ); use C4::Koha qw( GetAuthorisedValues ); use C4::ClassSource qw( GetClassSources GetClassSource ); use C4::Barcodes; @@ -462,6 +462,8 @@ if ($op eq "additem") { $item->barcode($barcode); } + $item->barcode(barcodedecode($item->barcode)); + # If we have to add or add & duplicate, we add the item if ( $add_submit || $prefillitem) { diff --git a/circ/circulation.pl b/circ/circulation.pl index 7988782577..55fe588706 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -46,6 +46,7 @@ use Koha::CsvProfiles; use Koha::Patrons; use Koha::Patron::Debarments qw( GetDebarments ); use Koha::DateUtils qw( dt_from_string output_pref ); +use Koha::Plugins; use Koha::Database; use Koha::BiblioFrameworks; use Koha::Items; @@ -157,8 +158,7 @@ if (C4::Context->preference("DisplayClearScreenButton")) { for my $barcode ( @$barcodes ) { $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace - $barcode = barcodedecode($barcode) - if( $barcode && C4::Context->preference('itemBarcodeInputFilter')); + $barcode = barcodedecode( $barcode ) if $barcode; } my $stickyduedate = $query->param('stickyduedate') || $session->param('stickyduedate'); diff --git a/circ/renew.pl b/circ/renew.pl index 383155416c..4a0db8a114 100755 --- a/circ/renew.pl +++ b/circ/renew.pl @@ -44,7 +44,7 @@ my $schema = Koha::Database->new()->schema(); my $barcode = $cgi->param('barcode') // ''; my $unseen = $cgi->param('unseen') || 0; $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespae -$barcode = barcodedecode($barcode) if( $barcode && C4::Context->preference('itemBarcodeInputFilter')); +$barcode = barcodedecode($barcode) if $barcode; my $override_limit = $cgi->param('override_limit'); my $override_holds = $cgi->param('override_holds'); my $hard_due_date = $cgi->param('hard_due_date'); diff --git a/circ/returns.pl b/circ/returns.pl index 914d5de35c..045a30ee31 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -113,7 +113,7 @@ foreach ( $query->param ) { # decode barcode ## Didn't we already decode them before passing them back last time?? $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace - $barcode = barcodedecode($barcode) if(C4::Context->preference('itemBarcodeInputFilter')); + $barcode = barcodedecode($barcode) if $barcode; ###################### #Are these lines still useful ? @@ -247,7 +247,7 @@ if ($transit) { my $returnbranch; if ($barcode) { $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace - $barcode = barcodedecode($barcode) if C4::Context->preference('itemBarcodeInputFilter'); + $barcode = barcodedecode($barcode) if $barcode; my $item = Koha::Items->find({ barcode => $barcode }); if ( $item ) { diff --git a/offline_circ/process_koc.pl b/offline_circ/process_koc.pl index 7eaa73b905..2ca602238e 100755 --- a/offline_circ/process_koc.pl +++ b/offline_circ/process_koc.pl @@ -242,7 +242,8 @@ sub arguments_for_command { sub kocIssueItem { my $circ = shift; - $circ->{ 'barcode' } = barcodedecode($circ->{'barcode'}) if( $circ->{'barcode'} && C4::Context->preference('itemBarcodeInputFilter')); + $circ->{barcode} = barcodedecode( $circ->{barcode} ) if $circ->{barcode}; + my $branchcode = C4::Context->userenv->{branch}; my $patron = Koha::Patrons->find( { cardnumber => $circ->{cardnumber} } ); my $borrower = $patron->unblessed; @@ -322,7 +323,9 @@ sub kocIssueItem { sub kocReturnItem { my ( $circ ) = @_; - $circ->{'barcode'} = barcodedecode($circ->{'barcode'}) if( $circ->{'barcode'} && C4::Context->preference('itemBarcodeInputFilter')); + + $circ->{barcode} = barcodedecode( $circ->{barcode} ) if $circ->{barcode}; + my $item = Koha::Items->find({ barcode => $circ->{barcode} }); my $biblio = $item->biblio; my $borrowernumber = _get_borrowernumber_from_barcode( $circ->{'barcode'} ); diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl index 40d591519b..486b9d793d 100755 --- a/opac/sco/sco-main.pl +++ b/opac/sco/sco-main.pl @@ -36,7 +36,7 @@ use Modern::Perl; use CGI qw ( -utf8 ); use C4::Auth qw( in_iprange get_template_and_user checkpw ); -use C4::Circulation qw( AddReturn CanBookBeIssued AddIssue CanBookBeRenewed AddRenewal ); +use C4::Circulation qw( barcodedecode AddReturn CanBookBeIssued AddIssue CanBookBeRenewed AddRenewal ); use C4::Reserves; use C4::Output qw( output_html_with_http_headers ); use C4::Members; @@ -46,6 +46,7 @@ use Koha::Items; use Koha::Patrons; use Koha::Patron::Images; use Koha::Patron::Messages; +use Koha::Plugins; use Koha::Token; my $query = CGI->new; @@ -105,6 +106,8 @@ my ($op, $patronid, $patronlogin, $patronpw, $barcode, $confirmed, $newissues) = $query->param("newissues") || '', ); +$barcode = barcodedecode( $barcode ) if $barcode; + my @newissueslist = split /,/, $newissues; my $issuenoconfirm = 1; #don't need to confirm on issue. my $issuer = Koha::Patrons->find( $issuerid )->unblessed; diff --git a/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t b/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t new file mode 100755 index 0000000000..5a6b156f2a --- /dev/null +++ b/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t @@ -0,0 +1,78 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 4; +use Test::MockModule; +use Test::Warn; + +use File::Basename; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +BEGIN { + # Mock pluginsdir before loading Plugins module + my $path = dirname(__FILE__) . '/../../../lib'; + t::lib::Mocks::mock_config( 'pluginsdir', $path ); + + use_ok('Koha::Plugins'); + use_ok('Koha::Plugins::Handler'); + use_ok('Koha::Plugin::Test'); +} + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +t::lib::Mocks::mock_config( 'enable_plugins', 1 ); + +subtest '() hook tests' => sub { + + plan tests => 1; + + $schema->storage->txn_begin; + + my $plugins = Koha::Plugins->new; + $plugins->InstallPlugins; + + my $plugin = Koha::Plugin::Test->new->enable; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + + t::lib::Mocks::mock_userenv( + { + patron => $patron, + branchcode => $patron->branchcode + } + ); + + # Avoid testing useless warnings + my $test_plugin = Test::MockModule->new('Koha::Plugin::Test'); + $test_plugin->mock( 'after_item_action', undef ); + $test_plugin->mock( 'after_biblio_action', undef ); + + my $biblio = $builder->build_sample_biblio(); + my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + $item_1->barcode('THISISATEST'); + + warning_like { $item_1->store(); } + qr/item_barcode_transform called with parameter: THISISATEST/, + 'AddReserve calls the after_hold_create hook'; + + $schema->storage->txn_rollback; + Koha::Plugins::Methods->delete; +}; diff --git a/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t b/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t index 18292a02a7..fda58e013d 100755 --- a/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t +++ b/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t @@ -52,6 +52,9 @@ subtest 'after_biblio_action() and after_item_action() hooks tests' => sub { my $plugin = Koha::Plugin::Test->new->enable; + my $test_plugin = Test::MockModule->new('Koha::Plugin::Test'); + $test_plugin->mock( 'item_barcode_transform', undef ); + my $biblio_id; warning_like { ( $biblio_id, undef ) = C4::Biblio::AddBiblio( MARC::Record->new(), '' ); } diff --git a/t/db_dependent/Koha/Plugins/Circulation_hooks.t b/t/db_dependent/Koha/Plugins/Circulation_hooks.t index 5ef25f793e..7d038bb4db 100755 --- a/t/db_dependent/Koha/Plugins/Circulation_hooks.t +++ b/t/db_dependent/Koha/Plugins/Circulation_hooks.t @@ -66,6 +66,7 @@ subtest 'after_circ_action() hook tests' => sub { my $test_plugin = Test::MockModule->new('Koha::Plugin::Test'); $test_plugin->mock( 'after_item_action', undef ); $test_plugin->mock( 'after_biblio_action', undef ); + $test_plugin->mock( 'item_barcode_transform', sub { my ( $self, $barcode ) = @_; return $barcode; } ); my $biblio = $builder->build_sample_biblio(); my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); diff --git a/t/db_dependent/Koha/Plugins/Plugins.t b/t/db_dependent/Koha/Plugins/Plugins.t index 69cbffdfde..0d898f9d4e 100755 --- a/t/db_dependent/Koha/Plugins/Plugins.t +++ b/t/db_dependent/Koha/Plugins/Plugins.t @@ -25,7 +25,7 @@ use File::Temp qw( tempdir tempfile ); use FindBin qw($Bin); use Module::Load::Conditional qw(can_load); use Test::MockModule; -use Test::More tests => 59; +use Test::More tests => 60; use Test::Warn; use C4::Context; @@ -222,6 +222,7 @@ ok( $plugin->can('opac_head'), 'Test plugin can opac_head' ); ok( $plugin->can('opac_js'), 'Test plugin can opac_js' ); ok( $plugin->can('intranet_head'), 'Test plugin can intranet_head' ); ok( $plugin->can('intranet_js'), 'Test plugin can intranet_js' ); +ok( $plugin->can('item_barcode_transform'), 'Test plugin can barcode_transform' ); ok( $plugin->can('configure'), 'Test plugin can configure' ); ok( $plugin->can('install'), 'Test plugin can install' ); ok( $plugin->can('upgrade'), 'Test plugin can upgrade' ); diff --git a/t/lib/Koha/Plugin/Test.pm b/t/lib/Koha/Plugin/Test.pm index 151af98476..89ecf603c7 100644 --- a/t/lib/Koha/Plugin/Test.pm +++ b/t/lib/Koha/Plugin/Test.pm @@ -93,9 +93,14 @@ sub intranet_js { return "Koha::Plugin::Test::intranet_js"; } +sub item_barcode_transform { + my ( $self, $barcode ) = @_; + Koha::Exceptions::Exception->throw("item_barcode_transform called with parameter: $barcode"); +} + sub configure { my ( $self, $args ) = @_; - return "Koha::Plugin::Test::configure";; + return "Koha::Plugin::Test::configure"; } sub install { diff --git a/tools/batchMod.pl b/tools/batchMod.pl index 0f57a2debd..e3d26c60ff 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -33,7 +33,7 @@ use C4::Biblio qw( TransformHtmlToXml ); use C4::Items qw( GetItemsInfo Item2Marc ModItemFromMarc ); -use C4::Circulation qw( LostItem IsItemIssued ); +use C4::Circulation qw( barcodedecode LostItem IsItemIssued ); use C4::Context; use C4::Koha; use C4::BackgroundJob; @@ -372,6 +372,9 @@ if ($op eq "show"){ if ( my $list = $input->param('barcodelist') ) { my @barcodelist = grep /\S/, ( split /[$split_chars]/, $list ); @barcodelist = uniq @barcodelist; + + @barcodelist = map { barcodedecode( $_ ) } @barcodelist; + # Note: adding lc for case insensitivity my %itemdata = map { lc($_->{barcode}) => $_->{itemnumber} } @{ Koha::Items->search({ barcode => \@barcodelist }, { columns => [ 'itemnumber', 'barcode' ] } )->unblessed }; @itemnumbers = map { exists $itemdata{lc $_} ? $itemdata{lc $_} : () } @barcodelist; diff --git a/tools/inventory.pl b/tools/inventory.pl index 5eac6e4e74..52b0599703 100755 --- a/tools/inventory.pl +++ b/tools/inventory.pl @@ -31,7 +31,7 @@ use C4::Context; use C4::Output qw( output_html_with_http_headers ); use C4::Items qw( GetItemsForInventory ); use C4::Koha qw( GetAuthorisedValues ); -use C4::Circulation qw( AddReturn ); +use C4::Circulation qw( barcodedecode AddReturn ); use C4::Reports::Guided qw( ); use C4::Charset qw( NormalizeString ); @@ -41,6 +41,7 @@ use Koha::AuthorisedValues; use Koha::BiblioFrameworks; use Koha::ClassSources; use Koha::Items; + use List::MoreUtils qw( none ); my $minlocation=$input->param('minlocation') || ''; @@ -186,6 +187,9 @@ if ( ($uploadbarcodes && length($uploadbarcodes) > 0) || ($barcodelist && length } for my $barcode (@uploadedbarcodes) { next unless $barcode; + + $barcode = barcodedecode($barcode); + ++$lines_read; if (length($barcode)>$barcode_size) { $err_length += 1; -- 2.20.1