Bugzilla – Attachment 21916 Details for
Bug 8015
Add MARC Modifications Templates
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 8015: Get rid of the eval in ModifyRecordWithTemplate
Bug-8015-Get-rid-of-the-eval-in-ModifyRecordWithTe.patch (text/plain), 13.95 KB, created by
Bernardo Gonzalez Kriegel
on 2013-10-09 14:27:32 UTC
(
hide
)
Description:
Bug 8015: Get rid of the eval in ModifyRecordWithTemplate
Filename:
MIME Type:
Creator:
Bernardo Gonzalez Kriegel
Created:
2013-10-09 14:27:32 UTC
Size:
13.95 KB
patch
obsolete
>From 33d1d0a74ac26dba61a86f68550c357645ec0d5d Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >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 <bgkriegel@gmail.com> >--- > 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
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 8015
:
9313
|
9329
|
9330
|
9331
|
10089
|
11579
|
11607
|
11608
|
12280
|
12288
|
12569
|
12570
|
12571
|
12572
|
12839
|
12856
|
12857
|
12858
|
12859
|
12860
|
13121
|
13138
|
13495
|
14030
|
14031
|
14032
|
14033
|
14034
|
14035
|
14040
|
16330
|
16331
|
16332
|
16333
|
16334
|
16335
|
16363
|
16376
|
16377
|
16378
|
16379
|
16413
|
16415
|
16423
|
16455
|
16468
|
16470
|
16471
|
16472
|
16473
|
16475
|
16476
|
16477
|
16478
|
16479
|
16480
|
17036
|
17037
|
17038
|
18372
|
18441
|
21024
|
21025
|
21026
|
21027
|
21028
|
21029
|
21030
|
21031
|
21032
|
21033
|
21034
|
21035
|
21036
|
21037
|
21038
|
21371
|
21484
|
21485
|
21486
|
21487
|
21808
|
21809
|
21876
|
21901
|
21902
|
21903
|
21904
|
21905
|
21906
|
21907
|
21908
|
21909
|
21910
|
21911
|
21912
|
21913
|
21914
|
21915
|
21916
|
21917
|
21918
|
21919
|
21920
|
22185
|
22186
|
22187
|
22188
|
22189
|
22190
|
22284
|
22285
|
22290
|
22293
|
22392
|
22393
|
22394
|
22395
|
22396
|
22397
|
22398
|
22399
|
22400
|
22401