From 8cad1b9303b27570517a3d3cf63e3e06c8f1f38f 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 --- 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 11c3c7713db..ebc7988744a 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 64b481bc6fd..34465a7caf6 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 3cce38faa2c..08e527df101 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 f5b57d2489a..bd49af60519 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 7988782577f..55fe588706c 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 383155416c4..4a0db8a114d 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 914d5de35c2..045a30ee318 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 7eaa73b905b..2ca602238e8 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 40d591519b1..486b9d793d4 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 00000000000..5a6b156f2aa --- /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 18292a02a7e..fda58e013d3 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 5ef25f793e8..7d038bb4db9 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 69cbffdfde0..0d898f9d4eb 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 151af984763..89ecf603c79 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 0f57a2debd9..e3d26c60ffd 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 5eac6e4e743..52b05997032 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.25.1