Koha-community/Koha
15
7
Fork 0
Code Issues Releases Wiki Activity

Bug 28211: Replace use of call_recursive() with call()

Browse source 
This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Kyle Hall 2021-04-23 11:53:51 -04:00 committed by Jonathan Druart
parent b271e9f9db
commit 18c1500772
9 changed files with 36 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
C4/Circulation.pm
View file

@ -166,7 +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 );
Koha::Plugins->call('item_barcode_transform', \$barcode );
$filter or return $barcode; # ensure filter is defined, else return untouched barcode
if ($filter eq 'whitespace') {
$barcode =~ s/\s//g;

16
C4/SIP/Sip/MsgType.pm
View file

@ -522,7 +522,7 @@ sub handle_checkout {
$fields = $self->{fields};
$patron_id = $fields->{ (FID_PATRON_ID) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
$item_id = $fields->{ (FID_ITEM_ID) };
my $fee_ack = $fields->{ (FID_FEE_ACK) };
@ -750,7 +750,7 @@ sub handle_block_patron {
$patron_id = $fields->{ (FID_PATRON_ID) };
$terminal_pwd = $fields->{ (FID_TERMINAL_PWD) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
# Terminal passwords are different from account login
# passwords, but I have no idea what to do with them. So,
@ -976,7 +976,7 @@ sub handle_patron_info {
$start = $fields->{ (FID_START_ITEM) };
$end = $fields->{ (FID_END_ITEM) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
$patron = $ils->find_patron($patron_id);
@ -1135,7 +1135,7 @@ sub handle_fee_paid {
$fee_id = $fields->{ (FID_FEE_ID) };
$trans_id = $fields->{ (FID_TRANSACTION_ID) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
$ils->check_inst_id( $inst_id, "handle_fee_paid" );
@ -1339,7 +1339,7 @@ sub handle_patron_enable {
$patron_id = $fields->{ (FID_PATRON_ID) };
$patron_pwd = $fields->{ (FID_PATRON_PWD) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
siplog( "LOG_DEBUG", "handle_patron_enable: patron_id: '%s', patron_pwd: '%s'", $patron_id, $patron_pwd );
@ -1404,7 +1404,7 @@ sub handle_hold {
$title_id = $fields->{ (FID_TITLE_ID) } || '';
$fee_ack = $fields->{ (FID_FEE_ACK) } || 'N';
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
if ( $hold_mode eq '+' ) {
$status = $ils->add_hold( $patron_id, $patron_pwd, $item_id, $title_id, $expiry_date, $pickup_locn, $hold_type, $fee_ack );
@ -1471,7 +1471,7 @@ sub handle_renew {
$item_props = $fields->{ (FID_ITEM_PROPS) };
$fee_ack = $fields->{ (FID_FEE_ACK) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
$status = $ils->renew( $patron_id, $patron_pwd, $item_id, $title_id, $no_block, $nb_due_date, $third_party, $item_props, $fee_ack );
@ -1554,7 +1554,7 @@ sub handle_renew_all {
$terminal_pwd = $fields->{ (FID_TERMINAL_PWD) };
$fee_ack = $fields->{ (FID_FEE_ACK) };
($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id );
Koha::Plugins->call('patron_barcode_transform', \$patron_id );
$status = $ils->renew_all( $patron_id, $patron_pwd, $fee_ack );

6
Koha/Patron.pm
View file

@ -112,7 +112,8 @@ Autogenerate next cardnumber from highest value found in database
sub fixup_cardnumber {
my ( $self ) = @_;
my ( $max ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $self->cardnumber );
my $max = $self->cardnumber;
Koha::Plugins->call( 'patron_barcode_transform', \$max );
$max ||= Koha::Patrons->search({
cardnumber => {-regexp => '^-?[0-9]+$'}
@ -201,7 +202,8 @@ sub store {
$self->trim_whitespaces;
my ( $new_cardnumber ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $self->cardnumber );
my $new_cardnumber = $self->cardnumber;
Koha::Plugins->call( 'patron_barcode_transform', \$new_cardnumber );
$self->cardnumber( $new_cardnumber );
# Set surname to uppercase if uppercasesurname is true

35
Koha/Plugins.pm
View file

@ -85,41 +85,6 @@ sub call {
return @responses;
}
=head2 call_recursive
Calls a plugin method for all enabled plugins,
passing the return value from the previous plugin
to the next one.
@response = Koha::Plugins->call_recursive($method, @args)
=cut
sub call_recursive {
my ( $class, $method, @args ) = @_;
my @responses;
if ( C4::Context->config('enable_plugins') ) {
my @plugins = $class->new( { enable_plugins => 1 } )->GetPlugins( { method => $method } );
@plugins = grep { $_->can($method) } @plugins;
foreach my $plugin (@plugins) {
my @response = eval { $plugin->$method(@args) };
if ($@) {
warn sprintf( "Plugin error (%s): %s", $plugin->get_metadata->{name}, $@ );
next;
} else {
@args = @response;
}
}
}
return @args;
}
=head2 GetPlugins
This will return a list of all available plugins, optionally limited by

5
circ/circulation.pl
View file

@ -75,7 +75,8 @@ my $autoswitched;
my $borrowernumber = $query->param('borrowernumber');
if (C4::Context->preference("AutoSwitchPatron") && $barcode) {
my ( $new_barcode ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $findborrower );
my $new_barcode = $findborrower;
Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode );
if (Koha::Patrons->search( { cardnumber => $new_barcode} )->count() > 0) {
$findborrower = $barcode;
undef $barcode;
@ -221,7 +222,7 @@ if ( @$barcodes == 0 && $charges eq 'yes' ) {
#
my $message;
if ($findborrower) {
( $findborrower ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $findborrower );
Koha::Plugins->call( 'patron_barcode_transform', \$findborrower );
my $patron = Koha::Patrons->find( { cardnumber => $findborrower } );
if ( $patron ) {
$borrowernumber = $patron->borrowernumber;

4
members/memberentry.pl
View file

@ -325,7 +325,9 @@ if ($op eq 'save' || $op eq 'insert'){
# If the cardnumber is blank, treat it as null.
$newdata{'cardnumber'} = undef if $newdata{'cardnumber'} =~ /^\s*$/;
my ( $new_barcode ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $newdata{'cardnumber'} );
my $new_barcode = $newdata{'cardnumber'};
Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode );
$newdata{'cardnumber'} = $new_barcode;
if (my $error_code = checkcardnumber( $newdata{cardnumber}, $borrowernumber )){

2
opac/sco/sco-main.pl
View file

@ -120,7 +120,7 @@ if (C4::Context->preference('SelfCheckoutByLogin') && !$patronid) {
my ( $borrower, $patron );
if ( $patronid ) {
( $patronid ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $patronid );
Koha::Plugins->call( 'patron_barcode_transform', \$patronid );
$patron = Koha::Patrons->find( { cardnumber => $patronid } );
$borrower = $patron->unblessed if $patron;
}

22
t/db_dependent/Koha/Plugins/Plugins.t
View file

@ -86,8 +86,8 @@ subtest 'call() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'call_recursive() tests' => sub {
plan tests => 6;
subtest 'more call() tests' => sub {
plan tests => 3;
$schema->storage->txn_begin;
# Temporarily remove any installed plugins data
@ -100,18 +100,18 @@ subtest 'call_recursive() tests' => sub {
$plugin->enable();
}
my (@responses) = Koha::Plugins->call_recursive('test_call_recursive', 1);
is( scalar @responses, 1, "Got back one element" );
is( $responses[0], 11, "Got expected response" );
my $bc = 1;
Koha::Plugins->call('item_barcode_transform', \$bc);
is( $bc, 4, "Got expected response" );
(@responses) = Koha::Plugins->call_recursive('test_call_recursive', 'abcd');
is( scalar @responses, 1, "Got back one element" );
is( $responses[0], 'abcdabcd', "Got expected response" );
my $cn = 'abcd';
Koha::Plugins->call('item_barcode_transform', \$cn);
is( $cn, 'abcd', "Got expected response" );
t::lib::Mocks::mock_config('enable_plugins', 0);
(@responses) = Koha::Plugins->call_recursive('test_call_recursive', 1);
is( scalar @responses, 1, "Got back one element" );
is( $responses[0], 1, "call_recursive should return the original arguments if plugins are disabled" );
$bc = 1;
Koha::Plugins->call('item_barcode_transform', \$bc);
is( $bc, 1, "call should return the original arguments if plugins are disabled" );
$schema->storage->txn_rollback;
};

8
t/lib/Koha/Plugin/Test.pm
View file

@ -95,13 +95,15 @@ sub intranet_js {
sub item_barcode_transform {
my ( $self, $barcode ) = @_;
Koha::Exceptions::Exception->throw("item_barcode_transform called with parameter: $barcode");
my $param = $$barcode;
$$barcode = 4 if "$$barcode" eq 1;
Koha::Exceptions::Exception->throw("item_barcode_transform called with parameter: $param");
}
sub patron_barcode_transform {
my ( $self, $barcode ) = @_;
$barcode //= '';
Koha::Exceptions::Exception->throw("patron_barcode_transform called with parameter: $barcode");
$$barcode //= '';
Koha::Exceptions::Exception->throw("patron_barcode_transform called with parameter: $$barcode");
}
sub configure {