Bugzilla – Attachment 125765 Details for
Bug 26351
Add plugin hooks to transform item barcodes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26351: Add plugin hooks to transform item barcodes
Bug-26351-Add-plugin-hooks-to-transform-item-barco.patch (text/plain), 18.75 KB, created by
Jonathan Druart
on 2021-10-06 10:27:30 UTC
(
hide
)
Description:
Bug 26351: Add plugin hooks to transform item barcodes
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2021-10-06 10:27:30 UTC
Size:
18.75 KB
patch
obsolete
>From 8cad1b9303b27570517a3d3cf63e3e06c8f1f38f Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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 <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 >--- > 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 <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; >+}; >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26351
:
109532
|
109548
|
109671
|
110844
|
110846
|
111575
|
111577
|
111578
|
111579
|
111862
|
111863
|
111894
|
111916
|
111917
|
111918
|
111919
|
111920
|
111921
|
111922
|
113571
|
113572
|
113573
|
113574
|
113575
|
113576
|
113577
|
113578
|
115172
|
115173
|
115174
|
115175
|
115176
|
115177
|
115178
|
115179
|
115180
|
118649
|
118650
|
118651
|
118652
|
118653
|
118654
|
118655
|
118656
|
118657
|
118658
|
118681
|
118682
|
118683
|
118684
|
118685
|
118686
|
118687
|
118688
|
118689
|
118690
|
122433
|
122434
|
122435
|
122436
|
122437
|
122438
|
122439
|
122440
|
122441
|
122442
|
124258
|
124259
|
124260
|
124261
|
124262
|
124263
|
124264
|
124265
|
124266
|
124313
|
125447
|
125763
|
125764
| 125765