From 33d1d0a74ac26dba61a86f68550c357645ec0d5d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 26 Sep 2013 12:39:30 +0200 Subject: [PATCH] Bug 8015: Get rid of the eval in ModifyRecordWithTemplate This patch removes the use of eval in the C4::MarcModificationTemplates::ModifyRecordWithTemplate routine. Now this routine call the wanted modification routine with the list of parameters. This call is done only if the condition is respected. Signed-off-by: Bernardo Gonzalez Kriegel --- C4/MarcModificationTemplates.pm | 179 ++++++++++++++++------------ Koha/SimpleMARC.pm | 2 +- t/SimpleMARC.t | 7 +- t/db_dependent/MarcModificationTemplates.t | 140 +++++++++++++++++++++- 4 files changed, 245 insertions(+), 83 deletions(-) diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm index 2c7bb97..ef48c3d 100644 --- a/C4/MarcModificationTemplates.pm +++ b/C4/MarcModificationTemplates.pm @@ -492,82 +492,111 @@ sub ModifyRecordsWithTemplate { =cut sub ModifyRecordWithTemplate { - my ( $template_id, $record ) = @_; - C4::Koha::Log( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG; - warn( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG; - C4::Koha::Log( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10; - warn( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10; - - my $current_date = DateTime->now()->ymd(); - my $branchcode = C4::Context->userenv->{branch}; - - my @actions = GetModificationTemplateActions( $template_id ); - - foreach my $a ( @actions ) { - my $action = $a->{'action'}; - my $field_number = $a->{'field_number'}; - my $from_field = $a->{'from_field'}; - my $from_subfield = $a->{'from_subfield'}; - my $field_value = $a->{'field_value'}; - my $to_field = $a->{'to_field'}; - my $to_subfield = $a->{'to_subfield'}; - my $to_regex = $a->{'to_regex'}; - my $conditional = $a->{'conditional'}; - my $conditional_field = $a->{'conditional_field'}; - my $conditional_subfield = $a->{'conditional_subfield'}; - my $conditional_comparison = $a->{'conditional_comparison'}; - my $conditional_value = $a->{'conditional_value'}; - my $conditional_regex = $a->{'conditional_regex'}; - - my $eval = "$action( \$record, '$from_field', '$from_subfield', "; - - if ( $field_value ) { - C4::Koha::Log( "Field value before replacements: $field_value" ) if ( DEBUG >= 3 ); - warn( "Field value before replacements: $field_value" ) if ( DEBUG >= 3 ); - - $field_value =~ s/__CURRENTDATE__/$current_date/g; - $field_value =~ s/__BRANCHCODE__/$branchcode/g; - - $eval .= " undef, " if ( $action eq 'update_field' ); - $eval .= " '$field_value' "; - - C4::Koha::Log( "Field value after replacements: $field_value" ) if ( DEBUG >= 3 ); - warn( "Field value after replacements: $field_value" ) if ( DEBUG >= 3 ); - } elsif ( $to_field ) { - $eval .= " '$to_field', '$to_subfield', '$to_regex' "; - } - - $eval .= ", '$field_number' " if ( $field_number ); - $eval .= ') '; - - if ( $conditional ) { - $eval .= " $conditional ( "; - - if ( $conditional_comparison eq 'exists' ) { - $eval .= "field_exists( \$record, '$conditional_field', '$conditional_subfield' )"; - - } elsif ( $conditional_comparison eq 'not_exists' ) { - $eval .= "!field_exists( \$record, '$conditional_field', '$conditional_subfield' )"; - - } elsif ( $conditional_comparison eq 'equals' ) { - $eval .= "field_equals( \$record, '$conditional_value', '$conditional_field', '$conditional_subfield', '$conditional_regex' ) "; - - } elsif ( $conditional_comparison eq 'not_equals' ) { - $eval .= "!field_equals( \$record, '$conditional_value', '$conditional_field', '$conditional_subfield', '$conditional_regex' ) "; - } - - $eval .= " )"; + my ( $template_id, $record ) = @_; + C4::Koha::Log( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG; + warn( "C4::MarcModificationTemplates::ModifyRecordWithTemplate( $template_id, $record )" ) if DEBUG; + C4::Koha::Log( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10; + warn( "Unmodified Record:\n" . $record->as_formatted() ) if DEBUG >= 10; + + my $current_date = DateTime->now()->ymd(); + my $branchcode = C4::Context->userenv->{branch}; + + my @actions = GetModificationTemplateActions( $template_id ); + + foreach my $a ( @actions ) { + my $action = $a->{'action'}; + my $field_number = $a->{'field_number'}; + my $from_field = $a->{'from_field'}; + my $from_subfield = $a->{'from_subfield'}; + my $field_value = $a->{'field_value'}; + my $to_field = $a->{'to_field'}; + my $to_subfield = $a->{'to_subfield'}; + my $to_regex = $a->{'to_regex'}; + my $conditional = $a->{'conditional'}; + my $conditional_field = $a->{'conditional_field'}; + my $conditional_subfield = $a->{'conditional_subfield'}; + my $conditional_comparison = $a->{'conditional_comparison'}; + my $conditional_value = $a->{'conditional_value'}; + my $conditional_regex = $a->{'conditional_regex'}; + + if ( $field_value ) { + $field_value =~ s/__CURRENTDATE__/$current_date/g; + $field_value =~ s/__BRANCHCODE__/$branchcode/g; + } + + my @params = ( $record, $from_field, $from_subfield ); + if ( $action eq 'update_field' ) { + push @params, + ( $field_value + ? ( undef, $field_value ) + : () + ); + } else { + push @params, + ( $field_value + ? $field_value + : () + ); + } + push @params, ( + ( ( not $field_value and $to_field ) + ? ( $to_field, $to_subfield, $to_regex ) + : () ), + ( $field_number + ? $field_number + : () ) + ); + + my $do = 1; + if ( $conditional ) { + for ( $conditional_comparison ) { + when ( /^exists$/ ) { + my $exists = field_exists( $record, $conditional_field, $conditional_subfield ); + $do = $conditional eq 'if' + ? $exists + : not $exists; + } + when ( /^not_exists$/ ) { + my $exists = field_exists( $record, $conditional_field, $conditional_subfield ); + $do = $conditional eq 'if' + ? not $exists + : $exists; + } + when ( /^equals$/ ) { + my $equals = field_equals( $record, $conditional_value, $conditional_field, $conditional_subfield, $conditional_regex ); + $do = $conditional eq 'if' + ? $equals + : not $equals; + } + when ( /^not_equals$/ ) { + my $equals = field_equals( $record, $conditional_value, $conditional_field, $conditional_subfield, $conditional_regex ); + $do = $conditional eq 'if' + ? not $equals + : $equals; + } + } + } + + if ( $do ) { + for ( $action ) { + when ( /^copy_field$/ ) { + copy_field( @params ); + } + when ( /^update_field$/ ) { + update_field( @params ); + } + when ( /^move_field$/ ) { + move_field( @params ); + } + when ( /^delete_field$/ ) { + delete_field( @params ); + } + } + } + + C4::Koha::Log( $record->as_formatted() ) if DEBUG >= 10; + warn( $record->as_formatted() ) if DEBUG >= 10; } - - $eval .= ";"; - - C4::Koha::Log("eval $eval") if DEBUG >= 2; - warn("eval $eval") if DEBUG >= 2; - eval {$eval}; - C4::Koha::Log( $record->as_formatted() ) if DEBUG >= 10; - warn( $record->as_formatted() ) if DEBUG >= 10; - - } } 1; __END__ diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm index ef40694..e78fc92 100644 --- a/Koha/SimpleMARC.pm +++ b/Koha/SimpleMARC.pm @@ -125,7 +125,7 @@ sub update_field { $field->update( "$subfieldName" => $values[$i++] ); } } - if ( $i <= scalar @values - 1 ) { + if ( $i <= scalar ( @values ) - 1 ) { foreach my $field ( @fields ) { foreach my $j ( $i .. scalar( @values ) - 1) { $field->add_subfields( "$subfieldName" => $values[$j] ); diff --git a/t/SimpleMARC.t b/t/SimpleMARC.t index cfba615..def2edb 100644 --- a/t/SimpleMARC.t +++ b/t/SimpleMARC.t @@ -1,14 +1,11 @@ use Modern::Perl; -use Test::More;#tests => 1; - -use Data::Dumper; +use Test::More tests => 33; use_ok("MARC::Field"); use_ok("MARC::Record"); use_ok("Koha::SimpleMARC"); - sub new_record { my $record = MARC::Record->new; $record->leader('03174nam a2200445 a 4500'); @@ -184,5 +181,3 @@ move_field( $record, '952', 'p', '954', 'p' ); # Move all field is_deeply( \@fields_952p, [], 'All 952$p have moved' ); is_deeply( \@fields_954p, ['3010023917', '3010023917'], 'Now 2 954$p exist' ); - -done_testing; diff --git a/t/db_dependent/MarcModificationTemplates.t b/t/db_dependent/MarcModificationTemplates.t index d253b82..c31983e 100644 --- a/t/db_dependent/MarcModificationTemplates.t +++ b/t/db_dependent/MarcModificationTemplates.t @@ -1,9 +1,18 @@ use Modern::Perl; -use Test::More tests => 66; +use Test::More tests => 74; +use_ok("MARC::Field"); +use_ok("MARC::Record"); use_ok("C4::MarcModificationTemplates"); +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +$dbh->do(q|DELETE FROM marc_modification_template_actions|); +$dbh->do(q|DELETE FROM marc_modification_templates|); + # Creation my $template_id = AddModificationTemplate("template_name"); like( $template_id, qr|^\d+$|, "new template returns an id" ); @@ -123,3 +132,132 @@ is( GetModificationTemplateAction( $actions[1]->{mmta_id} ), undef, "second acti is( GetModificationTemplateAction( $actions[2]->{mmta_id} ), undef, "third action does not exist anymore" ); is( GetModificationTemplateActions( $template_id ), 0, "There is no action for deleted template" ); + +# ModifyRecordWithTemplate +my @USERENV = ( + 1, + 'test', + 'MASTERTEST', + 'Test', + 'Test', + 't', + 'Test', + 0, +); +C4::Context->_new_userenv ('DUMMY_SESSION_ID'); +C4::Context->set_userenv ( @USERENV ); + +$template_id = AddModificationTemplate("template_name"); +like( $template_id, qr|^\d+$|, "new template returns an id" ); + +is( AddModificationTemplateAction( + $template_id, 'copy_field', 0, + '245', 'a', '', + '246', 'a', '', + '', '', '', '', '', '', '', + 'copy field 245$a to 246$a' +), 1, 'Add first action: copy 245$a to 246$a'); + +is( AddModificationTemplateAction( + $template_id, 'delete_field', 0, + '650', 'a', '', + '', '', '', + 'if', '650', '9', 'equals', '462', '', + 'Delete field 650$a if 650$9=462' +), 1, 'Add second action: delete field 650$a if 650$9=462'); + +is( AddModificationTemplateAction( + $template_id, 'update_field', 0, + '952', 'p', '3010023917_updated', + '', '', '', + 'unless', '650', '9', 'equals', '42', '', + 'Update field 952$p with "3010023917_updated" if 650$9 != 42' +), 1, 'Add third action: update field 952$p with "3010023917_updated" if 650$9 != 42'); + +is( AddModificationTemplateAction( + $template_id, 'move_field', 0, + '952', 'd', '', + '952', 'e', '', + 'if', '952', 'c', 'equals', '/^GEN/', '1', + 'Move field 952$d to 952$e if 952$c =~ /^GE/' +), 1, 'Add fourth action: move field 952$d to 952$e if 952$c =~ /^GE/'); + +my $record = new_record(); + +ModifyRecordWithTemplate( $template_id, $record ); + +my $expected_record = expected_record(); +is_deeply( $record, $expected_record ); + +done_testing; + +sub new_record { + my $record = MARC::Record->new; + $record->leader('03174nam a2200445 a 4500'); + my @fields = ( + MARC::Field->new( + 100, '1', ' ', + a => 'Knuth, Donald Ervin', + d => '1938', + ), + MARC::Field->new( + 245, '1', '4', + a => 'The art of computer programming', + c => 'Donald E. Knuth.', + ), + MARC::Field->new( + 650, ' ', '0', + a => 'Computer programming.', + 9 => '462', + ), + MARC::Field->new( + 952, ' ', ' ', + p => '3010023917', + y => 'BK', + c => 'GEN', + d => '2001-06-25', + ), + ); + $record->append_fields(@fields); + return $record; +} + +sub expected_record { + my $record = MARC::Record->new; + $record->leader('03174nam a2200445 a 4500'); + my @fields = ( + MARC::Field->new( + 100, '1', ' ', + a => 'Knuth, Donald Ervin', + d => '1938', + ), + MARC::Field->new( + 245, '1', '4', + a => 'The art of computer programming', + c => 'Donald E. Knuth.', + ), + MARC::Field->new( + 650, ' ', '0', + 9 => '462', + ), + MARC::Field->new( + 952, ' ', ' ', + p => '3010023917_updated', + y => 'BK', + c => 'GEN', + e => '2001-06-25', + ), + MARC::Field->new( + 246, '', ' ', + a => 'The art of computer programming', + ), + ); + $record->append_fields(@fields); + return $record; +} + + +# C4::Context->userenv +sub Mock_userenv { + return { branchcode => 'CPL' }; +} -- 1.7.9.5