Bugzilla – Attachment 39585 Details for
Bug 14098
Regression in Marc Modification Templates
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14098: Implement the copy_and_replace action for MTT
Bug-14098-Implement-the-copyandreplace-action-for-.patch (text/plain), 25.42 KB, created by
Jonathan Druart
on 2015-05-27 12:16:14 UTC
(
hide
)
Description:
Bug 14098: Implement the copy_and_replace action for MTT
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2015-05-27 12:16:14 UTC
Size:
25.42 KB
patch
obsolete
>From 27ed22cae60b59f4283d752780d6499cac46f9e3 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@koha-community.org> >Date: Wed, 27 May 2015 14:02:25 +0200 >Subject: [PATCH] Bug 14098: Implement the copy_and_replace action for MTT > >This patch implements the copy and replace action for the marc >modification templates. >Instead of copying a field/subfield, it will erase the destination >fields/subfields. > >Test plan: >Find it yourself. >Compare the differences between the copy and the copy_and_replace >actions. >The easier way to test is to 1/ create a complete record, 2/create some >modification templates and 3/ use the batch record modification with the >"preview" function. > >QA note: I kept the same tests as "copy" and, if no change were >expected, I noted them "(same as copy)", to be sure this new action won't >introduce regression on these tests. >--- > C4/MarcModificationTemplates.pm | 15 ++ > Koha/SimpleMARC.pm | 64 ++++- > t/SimpleMARC.t | 578 +++++++++++++++++++++++++++++++++++++++- > 3 files changed, 648 insertions(+), 9 deletions(-) > >diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm >index 3af0013..203f445 100644 >--- a/C4/MarcModificationTemplates.pm >+++ b/C4/MarcModificationTemplates.pm >@@ -612,6 +612,21 @@ sub ModifyRecordWithTemplate { > field_numbers => $field_numbers, > }); > } >+ elsif ( $action eq 'copy_and_replace_field' ) { >+ copy_and_replace_field({ >+ record => $record, >+ from_field => $from_field, >+ from_subfield => $from_subfield, >+ to_field => $to_field, >+ to_subfield => $to_subfield, >+ regex => { >+ search => $to_regex_search, >+ replace => $to_regex_replace, >+ modifiers => $to_regex_modifiers >+ }, >+ field_numbers => $field_numbers, >+ }); >+ } > elsif ( $action eq 'update_field' ) { > update_field({ > record => $record, >diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm >index c4e4c51..04ff8f9 100644 >--- a/Koha/SimpleMARC.pm >+++ b/Koha/SimpleMARC.pm >@@ -19,6 +19,7 @@ our @EXPORT = qw( > read_field > update_field > copy_field >+ copy_and_replace_field > move_field > delete_field > field_exists >@@ -113,6 +114,46 @@ sub copy_field { > } > } > >+sub copy_and_replace_field { >+ my ( $params ) = @_; >+ my $record = $params->{record}; >+ my $fromFieldName = $params->{from_field}; >+ my $fromSubfieldName = $params->{from_subfield}; >+ my $toFieldName = $params->{to_field}; >+ my $toSubfieldName = $params->{to_subfield}; >+ my $regex = $params->{regex}; >+ my $field_numbers = $params->{field_numbers} // []; >+ >+ if ( ! ( $record && $fromFieldName && $toFieldName ) ) { return; } >+ >+ >+ if ( not $fromSubfieldName or $fromSubfieldName eq '' >+ or not $toSubfieldName or $toSubfieldName eq '' >+ ) { >+ _copy_move_field( >+ { record => $record, >+ from_field => $fromFieldName, >+ to_field => $toFieldName, >+ regex => $regex, >+ field_numbers => $field_numbers, >+ action => 'replace', >+ } >+ ); >+ } else { >+ _copy_move_subfield( >+ { record => $record, >+ from_field => $fromFieldName, >+ from_subfield => $fromSubfieldName, >+ to_field => $toFieldName, >+ to_subfield => $toSubfieldName, >+ regex => $regex, >+ field_numbers => $field_numbers, >+ action => 'replace', >+ } >+ ); >+ } >+} >+ > sub update_field { > my ( $params ) = @_; > my $record = $params->{record}; >@@ -477,13 +518,14 @@ sub _copy_move_field { > my $field_numbers = $params->{field_numbers} // []; > my $action = $params->{action} || 'copy'; > >- my @fields = $record->field( $fromFieldName ); >+ my @from_fields = $record->field( $fromFieldName ); > if ( @$field_numbers ) { >- @fields = map { $_ <= @fields ? $fields[ $_ - 1 ] : () } @$field_numbers; >+ @from_fields = map { $_ <= @from_fields ? $from_fields[ $_ - 1 ] : () } @$field_numbers; > } > >- for my $field ( @fields ) { >- my $new_field = $field->clone; >+ my @new_fields; >+ for my $from_field ( @from_fields ) { >+ my $new_field = $from_field->clone; > $new_field->{_tag} = $toFieldName; # Should be replaced by set_tag, introduced by MARC::Field 2.0.4 > if ( $regex and $regex->{search} ) { > for my $subfield ( $new_field->subfields ) { >@@ -492,10 +534,18 @@ sub _copy_move_field { > $new_field->update( $subfield->[0], $value ); > } > } >- $record->append_fields( $new_field ); >- $record->delete_field( $field ) >- if $action eq 'move'; >+ if ( $action eq 'move' ) { >+ $record->delete_field( $from_field ) >+ } >+ elsif ( $action eq 'replace' ) { >+ my @to_fields = $record->field( $toFieldName ); >+ if ( @to_fields ) { >+ $record->delete_field( $to_fields[0] ); >+ } >+ } >+ push @new_fields, $new_field; > } >+ $record->append_fields( @new_fields ); > } > > sub _copy_move_subfield { >diff --git a/t/SimpleMARC.t b/t/SimpleMARC.t >index 880653b..5958508 100644 >--- a/t/SimpleMARC.t >+++ b/t/SimpleMARC.t >@@ -1,6 +1,6 @@ > use Modern::Perl; > >-use Test::More tests => 9; >+use Test::More tests => 10; > > use_ok("MARC::Field"); > use_ok("MARC::Record"); >@@ -804,7 +804,7 @@ subtest 'copy_field' => sub { > ) > ], > [ '4242423917', 'BK', 'GEN', '2001-06-25' ], >- "copy all wirh regex: first original fields has been copied" >+ "copy all with regex: first original fields has been copied" > ); > is_deeply( > [ >@@ -857,6 +857,580 @@ subtest 'copy_field' => sub { > }; > }; > >+# copy_and_replace_field - subfield >+subtest 'copy_and_replace_field' => sub { >+ plan tests => 2; >+ subtest 'copy and replace subfield' => sub { >+ plan tests => 19; >+ my $record = new_record; >+ $record->append_fields( >+ MARC::Field->new( >+ 650, ' ', '0', >+ a => 'Computer algorithms.', >+ 9 => '463', >+ ) >+ ); >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '245', >+ from_subfield => 'a', >+ to_field => '246', >+ to_subfield => 'a' >+ } >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '245', subfield => 'a' } >+ ) >+ ], >+ ['The art of computer programming'], >+ 'Copy and replace should not have modify original subfield 245$a (same as copy)' >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '246', subfield => 'a' } >+ ) >+ ], >+ ['The art of computer programming'], >+ 'Copy and replace should create a new 246$a (same as copy)' >+ ); >+ >+ $record = new_record; >+ $record->append_fields( >+ MARC::Field->new( >+ 650, ' ', '0', >+ a => 'Computer algorithms.', >+ 9 => '463', >+ ) >+ ); >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a' >+ } >+ ); >+ my @fields_651a = >+ read_field( { record => $record, field => '651', subfield => 'a' } ); >+ is_deeply( >+ \@fields_651a, >+ [ 'Computer programming.', 'Computer algorithms.' ], >+ 'Copy and replace multivalued field (same as copy)' >+ ); >+ delete_field( { record => $record, field => '651' } ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a', >+ field_numbers => [1] >+ } >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '651', subfield => 'a' } >+ ) >+ ], >+ ['Computer programming.'], >+ 'Copy and replace first field 650$a should only copy the 1st (same as copy)' >+ ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a', >+ field_numbers => [2] >+ } >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '651', subfield => 'a' } >+ ) >+ ], >+ ['Computer algorithms.'], >+ 'Copy and replace second field 650$a should erase 651$a' >+ ); >+ delete_field( { record => $record, field => '651' } ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a', >+ regex => { search => 'Computer', replace => 'The art of' } >+ } >+ ); >+ @fields_651a = >+ read_field( { record => $record, field => '651', subfield => 'a' } ); >+ is_deeply( >+ \@fields_651a, >+ [ 'The art of programming.', 'The art of algorithms.' ], >+ 'Copy and replace field using regex (same as copy)' >+ ); >+ delete_field( { record => $record, field => '651' } ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a', >+ regex => { search => 'Computer', replace => 'The mistake of' } >+ } >+ ); >+ @fields_651a = >+ read_field( { record => $record, field => '651', subfield => 'a' } ); >+ is_deeply( >+ \@fields_651a, >+ [ 'The mistake of programming.', 'The mistake of algorithms.' ], >+ 'Copy and replace fields using regex on existing fields (same as copy)' >+ ); >+ delete_field( { record => $record, field => '651' } ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a', >+ regex => { search => 'Computer', replace => 'The art of' } >+ } >+ ); >+ @fields_651a = >+ read_field( { record => $record, field => '651', subfield => 'a' } ); >+ is_deeply( >+ \@fields_651a, >+ [ 'The art of programming.', 'The art of algorithms.', ], >+ 'Copy and replace all fields using regex (same as copy)' >+ ); >+ delete_field( { record => $record, field => '651' } ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '651', >+ to_subfield => 'a', >+ regex => { search => 'Computer', replace => 'The art of' }, >+ field_numbers => [1] >+ } >+ ); >+ @fields_651a = >+ read_field( { record => $record, field => '651', subfield => 'a' } ); >+ is_deeply( >+ \@fields_651a, >+ [ 'The art of programming.', ], >+ 'Copy and replace first field using regex (same as copy)' >+ ); >+ delete_field( { record => $record, field => '651' } ); >+ >+ # Copy and replace with regex modifiers >+ $record = new_record; >+ $record->append_fields( >+ MARC::Field->new( >+ 650, ' ', '0', >+ a => 'Computer algorithms.', >+ 9 => '463', >+ ) >+ ); >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '652', >+ to_subfield => 'a', >+ regex => { search => 'o', replace => 'foo' } >+ } >+ ); >+ my @fields_652a = >+ read_field( { record => $record, field => '652', subfield => 'a' } ); >+ is_deeply( >+ \@fields_652a, >+ [ 'Cfoomputer programming.', 'Cfoomputer algorithms.' ], >+ 'Copy and replace field using regex (same as copy)' >+ ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '653', >+ to_subfield => 'a', >+ regex => { search => 'o', replace => 'foo', modifiers => 'g' } >+ } >+ ); >+ my @fields_653a = >+ read_field( { record => $record, field => '653', subfield => 'a' } ); >+ is_deeply( >+ \@fields_653a, >+ [ 'Cfoomputer prfoogramming.', 'Cfoomputer algfoorithms.' ], >+ 'Copy and replace field using regex (same as copy)' >+ ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '654', >+ to_subfield => 'a', >+ regex => { search => 'O', replace => 'foo', modifiers => 'i' } >+ } >+ ); >+ my @fields_654a = >+ read_field( { record => $record, field => '654', subfield => 'a' } ); >+ is_deeply( >+ \@fields_654a, >+ [ 'Cfoomputer programming.', 'Cfoomputer algorithms.' ], >+ 'Copy and replace field using regex (same as copy)' >+ ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '650', >+ from_subfield => 'a', >+ to_field => '655', >+ to_subfield => 'a', >+ regex => { search => 'O', replace => 'foo', modifiers => 'gi' } >+ } >+ ); >+ my @fields_655a = >+ read_field( { record => $record, field => '655', subfield => 'a' } ); >+ is_deeply( >+ \@fields_655a, >+ [ 'Cfoomputer prfoogramming.', 'Cfoomputer algfoorithms.' ], >+ 'Copy and replace field using regex (same as copy)' >+ ); >+ >+ $record = new_record; >+ $record->append_fields( >+ MARC::Field->new( >+ 952, ' ', ' ', >+ p => '3010023917', >+ y => 'BK', >+ ), >+ ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '952', >+ from_subfield => 'd', >+ to_field => '952', >+ to_subfield => 'd' >+ } >+ ); >+ my @fields_952d = >+ read_field( { record => $record, field => '952', subfield => 'd' } ); >+ is_deeply( >+ \@fields_952d, >+ [ '2001-06-25', '2001-06-25' ], >+ 'copy and replace 952$d into others 952 field' >+ ); >+ >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '111', >+ from_subfield => '1', >+ to_field => '999', >+ to_subfield => '9' >+ } >+ ); >+ my @fields_9999 = >+ read_field( { record => $record, field => '999', subfield => '9' } ); >+ is_deeply( \@fields_9999, [], >+ 'copy and replace a nonexistent subfield does not create a new one (same as copy)' ); >+ >+ $record = new_record; >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => 245, >+ from_subfield => 'a', >+ to_field => 245, >+ to_subfield => 'a', >+ regex => { search => '^', replace => 'BEGIN ' } >+ } >+ ); >+ # This is the same as update the subfield >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '245', subfield => 'a' } >+ ) >+ ], >+ ['BEGIN The art of computer programming'], >+ 'Copy and replace - Update a subfield: add a string at the beginning' >+ ); >+ >+ $record = new_record; >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => 245, >+ from_subfield => 'a', >+ to_field => 245, >+ to_subfield => 'a', >+ regex => { search => '$', replace => ' END' } >+ } >+ ); >+ # This is the same as update the subfield >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '245', subfield => 'a' } >+ ) >+ ], >+ ['The art of computer programming END'], >+ 'Copy and replace - Update a subfield: add a string at the end' >+ ); >+ >+ $record = new_record; >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => 245, >+ from_subfield => 'c', >+ to_field => 650, >+ to_subfield => 'c', >+ } >+ ); >+ >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '650' } >+ ) >+ ], >+ [ 'Computer programming.', '462', 'Donald E. Knuth.' ], >+ 'Copy and replace a subfield to an existent field but inexistent subfield (same as copy)' >+ ); >+ >+ $record = new_record; >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => 245, >+ from_subfield => 'c', >+ to_field => 650, >+ to_subfield => '9', >+ } >+ ); >+ >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '650' } >+ ) >+ ], >+ [ 'Computer programming.', 'Donald E. Knuth.' ], >+ 'Copy and replace a subfield to an existent field / subfield, the origin subfield is replaced' >+ ); >+ }; >+ >+ subtest 'copy and replace field' => sub { >+ plan tests => 14; >+ my $record = new_record; >+ $record->append_fields( >+ MARC::Field->new( >+ 952, ' ', ' ', >+ p => '3010023918', >+ y => 'CD', >+ ), >+ ); >+ >+ #- copy all fields >+ copy_and_replace_field( >+ { record => $record, from_field => '952', to_field => '953' } ); >+ my @fields_952 = read_field( { record => $record, field => '952' } ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '952', field_numbers => [1] } >+ ) >+ ], >+ [ '3010023917', 'BK', 'GEN', '2001-06-25' ], >+ "copy all: original first field still exists (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '952', field_numbers => [2] } >+ ) >+ ], >+ [ '3010023918', 'CD' ], >+ "copy all: original second field still exists (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '953', field_numbers => [1] } >+ ) >+ ], >+ [ '3010023917', 'BK', 'GEN', '2001-06-25' ], >+ "copy all: first original fields has been copied (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '953', field_numbers => [2] } >+ ) >+ ], >+ [ '3010023918', 'CD' ], >+ "copy all: second original fields has been copied (same as copy)" >+ ); >+ >+ #- copy only the first field >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '953', >+ to_field => '954', >+ field_numbers => [1] >+ } >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '953', field_numbers => [1] } >+ ) >+ ], >+ [ '3010023917', 'BK', 'GEN', '2001-06-25' ], >+ "copy and replace first: first original fields has been copied (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '953', field_numbers => [2] } >+ ) >+ ], >+ [ '3010023918', 'CD' ], >+ "copy and replace first: second original fields has been copied (same as copy)" >+ ); >+ is_deeply( >+ [ read_field( { record => $record, field => '954' } ) ], >+ [ '3010023917', 'BK', 'GEN', '2001-06-25' ], >+ "copy and replace first: only first, first 953 has been copied (same as copy)" >+ ); >+ >+ $record = new_record; >+ $record->append_fields( >+ MARC::Field->new( >+ 952, ' ', ' ', >+ p => '3010023918', >+ y => 'CD', >+ ), >+ ); >+ >+ #- copy and replace all fields and modify values using a regex >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '952', >+ to_field => '953', >+ regex => { search => '30100', replace => '42424' } >+ } >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '952', field_numbers => [1] } >+ ) >+ ], >+ [ '3010023917', 'BK', 'GEN', '2001-06-25' ], >+ "copy and replace all with regex: original first field still exists (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '952', field_numbers => [2] } >+ ) >+ ], >+ [ '3010023918', 'CD' ], >+ "copy and replace all with regex: original second field still exists (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '953', field_numbers => [1] } >+ ) >+ ], >+ [ '4242423917', 'BK', 'GEN', '2001-06-25' ], >+ "copy and replace all with regex: first original fields has been copied (same as copy)" >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '953', field_numbers => [2] } >+ ) >+ ], >+ [ '4242423918', 'CD' ], >+ "copy and replace all with regex: second original fields has been copied (same as copy)" >+ ); >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => '111', >+ to_field => '999', >+ } >+ ); >+ my @fields_9999 = >+ read_field( { record => $record, field => '999', subfield => '9' } ); >+ is_deeply( \@fields_9999, [], >+ 'copy and replace a nonexistent field does not create a new one (same as copy)' ); >+ >+ $record = new_record; >+ copy_and_replace_field( >+ { >+ record => $record, >+ from_field => 245, >+ to_field => 650, >+ } >+ ); >+ >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '650', field_numbers => [1] } >+ ) >+ ], >+ [ 'The art of computer programming', 'Donald E. Knuth.' ], >+ 'Copy and replace to an existent field should erase the original field' >+ ); >+ is_deeply( >+ [ >+ read_field( >+ { record => $record, field => '650', field_numbers => [2] } >+ ) >+ ], >+ [], >+ 'Copy and replace to an existent field should not create a new field' >+ ); >+ }; >+}; >+ > # move_field - subfields > subtest 'move_field' => sub { > plan tests => 2; >-- >2.1.0
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 14098
:
39582
|
39583
|
39584
|
39585
|
39610
|
39611
|
39612
|
39613
|
42282
|
42283
|
42284
|
42285