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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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;

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 );
 

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';
 

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) {
 

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');

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');

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 ) {

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'} );

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;

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 <http://www.gnu.org/licenses>.
+
+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;
+};

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(), '' ); }

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 } );

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' );

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 {

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;

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;