Bug 11319: Koha::SimpleMARC should take a hashref for parameters
authorJonathan Druart <jonathan.druart@biblibre.com>
Wed, 11 Dec 2013 13:22:03 +0000 (14:22 +0100)
committerTomas Cohen Arazi <tomascohen@gmail.com>
Wed, 12 Nov 2014 15:04:12 +0000 (12:04 -0300)
In order to avoid a long list of parameters, it should be better to
pass all of them into a hashref.

This patch does not add or modify a behavior.

Test plan:
Verify the unit tests still pass
- prove t/SimpleMARC.t
- prove t/db_dependent/MarcModificationTemplates.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
C4/MarcModificationTemplates.pm
Koha/SimpleMARC.pm
t/SimpleMARC.t

index 65b5292..52b28fe 100644 (file)
@@ -523,83 +523,101 @@ sub ModifyRecordWithTemplate {
             $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, { search => $to_regex_search, replace => $to_regex_replace, modifiers => $to_regex_modifiers} )
-                    : () ),
-                ( $field_number
-                    ? $field_number
-                    : () )
-        );
-
         my $do = 1;
-        if ($conditional) {
+        if ( $conditional ) {
             if ( $conditional_comparison eq 'exists' ) {
-                my $exists = field_exists( $record, $conditional_field,
-                    $conditional_subfield );
-                $do =
-                    $conditional eq 'if'
-                  ? $exists
-                  : not $exists;
+                my $exists = field_exists({
+                        record =>$record,
+                        field => $conditional_field,
+                        subfield => $conditional_subfield,
+                    });
+                $do = $conditional eq 'if'
+                    ? $exists
+                    : not $exists;
             }
             elsif ( $conditional_comparison eq 'not_exists' ) {
-                my $exists = field_exists( $record, $conditional_field,
-                    $conditional_subfield );
-                $do =
-                  $conditional eq 'if'
-                  ? not $exists
-                  : $exists;
+                my $exists = field_exists({
+                        record => $record,
+                        field => $conditional_field,
+                        subfield => $conditional_subfield
+                    });
+                $do = $conditional eq 'if'
+                    ? not $exists
+                    : $exists;
             }
             elsif ( $conditional_comparison eq 'equals' ) {
-                my $equals = field_equals(
-                    $record,            $conditional_value,
-                    $conditional_field, $conditional_subfield,
-                    $conditional_regex
-                );
-                $do =
-                    $conditional eq 'if'
-                  ? $equals
-                  : not $equals;
+                my $equals = field_equals({
+                    record => $record,
+                    value => $conditional_value,
+                    field => $conditional_field,
+                    subfield => $conditional_subfield,
+                    regex => $conditional_regex
+                });
+                $do = $conditional eq 'if'
+                    ? $equals
+                    : not $equals;
             }
             elsif ( $conditional_comparison eq 'not_equals' ) {
-                my $equals = field_equals(
-                    $record,            $conditional_value,
-                    $conditional_field, $conditional_subfield,
-                    $conditional_regex
-                );
-                $do =
-                  $conditional eq 'if'
-                  ? not $equals
-                  : $equals;
+                my $equals = field_equals({
+                    record => $record,
+                    value => $conditional_value,
+                    field => $conditional_field,
+                    subfield => $conditional_subfield,
+                    regex => $conditional_regex
+                });
+                $do = $conditional eq 'if'
+                    ? not $equals
+                    : $equals;
             }
         }
 
-        if ($do) {
+        if ( $do ) {
             if ( $action eq 'copy_field' ) {
-                copy_field(@params);
+                copy_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
+                    },
+                    n => $field_number,
+                });
             }
+
             elsif ( $action eq 'update_field' ) {
-                update_field(@params);
+                update_field({
+                    record => $record,
+                    field => $from_field,
+                    subfield => $from_subfield,
+                    values => [ $field_value ],
+                });
             }
             elsif ( $action eq 'move_field' ) {
-                move_field(@params);
+                move_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
+                    },
+                    n => $field_number,
+                });
             }
             elsif ( $action eq 'delete_field' ) {
-                delete_field(@params);
+                delete_field({
+                    record => $record,
+                    field => $from_field,
+                    subfield => $from_subfield,
+                    n => $field_number,
+                });
             }
         }
 
index 3ffe768..f5728d1 100644 (file)
@@ -73,11 +73,19 @@ at your option, any later version of Perl 5 you may have available.
 =cut
 
 sub copy_field {
-  my ( $record, $fromFieldName, $fromSubfieldName, $toFieldName, $toSubfieldName, $regex, $n, $dont_erase ) = @_;
+  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 $n = $params->{n};
+  my $dont_erase = $params->{dont_erase};
 
   if ( ! ( $record && $fromFieldName && $toFieldName ) ) { return; }
 
-  my @values = read_field( $record, $fromFieldName, $fromSubfieldName );
+  my @values = read_field({ record => $record, field => $fromFieldName, subfield => $fromSubfieldName });
   @values = ( $values[$n-1] ) if ( $n );
 
   if ( $regex and $regex->{search} ) {
@@ -103,7 +111,7 @@ sub copy_field {
         }
     }
   }
-  update_field( $record, $toFieldName, $toSubfieldName, $dont_erase, @values );
+  update_field({ record => $record, field => $toFieldName, subfield => $toSubfieldName, dont_erase => $dont_erase, values => \@values });
 }
 
 =head2 update_field
@@ -121,7 +129,12 @@ sub copy_field {
 =cut
 
 sub update_field {
-  my ( $record, $fieldName, $subfieldName, $dont_erase, @values ) = @_;
+  my ( $params ) = @_;
+  my $record = $params->{record};
+  my $fieldName = $params->{field};
+  my $subfieldName = $params->{subfield};
+  my $dont_erase = $params->{dont_erase};
+  my @values = @{ $params->{values} };
 
   if ( ! ( $record && $fieldName ) ) { return; }
 
@@ -179,7 +192,11 @@ sub update_field {
 =cut
 
 sub read_field {
-  my ( $record, $fieldName, $subfieldName, $n ) = @_;
+  my ( $params ) = @_;
+  my $record = $params->{record};
+  my $fieldName = $params->{field};
+  my $subfieldName = $params->{subfield};
+  my $n = $params->{n};
 
   my @fields = $record->field( $fieldName );
 
@@ -207,7 +224,10 @@ sub read_field {
 =cut
 
 sub field_exists {
-  my ( $record, $fieldName, $subfieldName ) = @_;
+  my ( $params ) = @_;
+  my $record = $params->{record};
+  my $fieldName = $params->{field};
+  my $subfieldName = $params->{subfield};
 
   if ( ! $record ) { return; }
 
@@ -236,12 +256,18 @@ sub field_exists {
 =cut
 
 sub field_equals {
-  my ( $record, $value, $fieldName, $subfieldName, $regex, $n ) = @_;
+  my ( $params ) = @_;
+  my $record = $params->{record};
+  my $value = $params->{value};
+  my $fieldName = $params->{field};
+  my $subfieldName = $params->{subfield};
+  my $regex = $params->{regex};
+  my $n = $params->{n};
   $n = 1 unless ( $n ); ## $n defaults to first field of a repeatable field series
 
   if ( ! $record ) { return; }
 
-  my @field_values = read_field( $record, $fieldName, $subfieldName, $n );
+  my @field_values = read_field({ record => $record, field => $fieldName, subfield => $subfieldName, n => $n });
   my $field_value = $field_values[$n-1];
 
   if ( $regex ) {
@@ -265,9 +291,17 @@ sub field_equals {
 =cut
 
 sub move_field {
-  my ( $record, $fromFieldName, $fromSubfieldName, $toFieldName, $toSubfieldName, $regex, $n ) = @_;
-  copy_field( $record, $fromFieldName, $fromSubfieldName, $toFieldName, $toSubfieldName, $regex, $n , 'dont_erase' );
-  delete_field( $record, $fromFieldName, $fromSubfieldName, $n );
+  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 $n = $params->{n};
+
+  copy_field({ record => $record, from_field => $fromFieldName, from_subfield => $fromSubfieldName, to_field => $toFieldName, to_subfield => $toSubfieldName, regex => $regex, n => $n , dont_erase => 1 });
+  delete_field({ record => $record, field => $fromFieldName, subfield => $fromSubfieldName, n => $n });
 }
 
 =head2 delete_field
@@ -282,7 +316,11 @@ sub move_field {
 =cut
 
 sub delete_field {
-  my ( $record, $fieldName, $subfieldName, $n ) = @_;
+  my ( $params ) = @_;
+  my $record = $params->{record};
+  my $fieldName = $params->{field};
+  my $subfieldName = $params->{subfield};
+  my $n = $params->{n};
 
   my @fields = $record->field( $fieldName );
 
index 2969bd2..c867807 100644 (file)
@@ -40,8 +40,8 @@ sub new_record {
 my $record = new_record;
 
 # field_exists
-is( field_exists( $record, '650', 'a'), 'Computer programming.', '650$a exists' );
-is( field_exists( $record, '650', 'b'), undef, '650$b does not exist' );
+is( field_exists({ record => $record, field => '650', subfield => 'a' }), 'Computer programming.', '650$a exists' );
+is( field_exists({ record => $record, field => '650', subfield => 'b' }), undef, '650$b does not exist' );
 
 $record->append_fields(
     MARC::Field->new(
@@ -51,61 +51,59 @@ $record->append_fields(
     )
 );
 
-is( field_exists( $record, '650', 'a'), 'Computer programming.', '650$a exists, field_exists returns the first one' );
+is( field_exists({ record => $record, field => '650', subfield => 'a' }), 'Computer programming.', '650$a exists, field_exists returns the first one' );
 
 # read_field
-my @fields_650a = read_field( $record, '650', 'a');
+my @fields_650a = read_field({ record => $record, field => '650', subfield => 'a' });
 is( $fields_650a[0], 'Computer programming.', 'first 650$a' );
 is( $fields_650a[1], 'Computer algorithms.', 'second 650$a' );
-is( read_field( $record, '650', 'a', 1 ), 'Computer programming.', 'first 650$a bis' );
-is( read_field( $record, '650', 'a', 2 ), 'Computer algorithms.', 'second 650$a bis' );
-is( read_field( $record, '650', 'a', 3 ), undef, 'There is no 3 650$a' );
+is( read_field({ record => $record, field => '650', subfield => 'a', n => 1 }), 'Computer programming.', 'first 650$a bis' );
+is( read_field({ record => $record, field => '650', subfield => 'a', n => 2 }), 'Computer algorithms.', 'second 650$a bis' );
+is( read_field({ record => $record, field => '650', subfield => 'a', n => 3 }), undef, 'There is no 3 650$a' );
 
 # copy_field
-copy_field( $record, '245', 'a', '246', 'a' );
-is_deeply( read_field( $record, '245', 'a' ), 'The art of computer programming', 'After copy 245$a still exists' );
-is_deeply( read_field( $record, '246', 'a' ), 'The art of computer programming', '246$a is a new field' );
-delete_field( $record, '246' );
-is( field_exists( $record, '246', 'a', '246$a does not exist anymore' ), undef );
-
-copy_field( $record, '650', 'a', '651', 'a' );
-my @fields_651a = read_field( $record, '651', 'a' );
+copy_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', 'After copy 245$a still exists' );
+is_deeply( read_field({ record => $record, field => '246', subfield => 'a' }), 'The art of computer programming', '246$a is a new field' );
+delete_field({ record => $record, field => '246' });
+is( field_exists({ record => $record, field => '246', subfield => 'a' }), undef, '246$a does not exist anymore' );
+
+copy_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 multivalued field' );
-delete_field( $record, '651' );
+delete_field({ record => $record, field => '651' });
 
-copy_field( $record, '650', 'a', '651', 'a', undef, 1 );
-@fields_651a = read_field( $record, '651', 'a' );
-is_deeply( read_field( $record, '651', 'a' ), 'Computer programming.', 'Copy first field 650$a' );
-delete_field( $record, '651' );
+copy_field({ record => $record, from_field => '650', from_subfield => 'a', to_field => '651', to_subfield => 'a', n => 1 });
+is_deeply( read_field({record => $record, field => '651', subfield => 'a' }), 'Computer programming.', 'Copy first field 650$a' );
+delete_field({ record => $record, field => '652' });
 
-copy_field( $record, '650', 'a', '651', 'a', undef, 2 );
-@fields_651a = read_field( $record, '651', 'a' );
-is_deeply( read_field( $record, '651', 'a' ), 'Computer algorithms.', 'Copy second field 650$a' );
-delete_field( $record, '651' );
+copy_field({ record => $record, from_field => '650', from_subfield => 'a', to_field => '651', to_subfield => 'a', n => 2 });
+is_deeply( read_field({ record => $record, field => '651', subfield => 'a' }), 'Computer algorithms.', 'Copy second field 650$a' );
+delete_field({ record => $record, field => '651' });
 
-copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The art of' } );
-@fields_651a = read_field( $record, '651', 'a' );
+copy_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 field using regex' );
 
-copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The mistake of' } );
-@fields_651a = read_field( $record, '651', 'a' );
+copy_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 fields using regex on existing fields' );
-delete_field( $record, '651' );
+delete_field({ record => $record, field => '651' });
 
-copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The art of' } );
-copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The mistake of' }, 1, "dont_erase" );
-@fields_651a = read_field( $record, '651', 'a' );
+copy_field({ record => $record, from_field => '650', from_subfield => 'a', to_field => '651', to_subfield => 'a', regex => { search => 'Computer', replace => 'The art of' } } );
+copy_field({ record => $record, from_field => '650', from_subfield => 'a', to_field => '651', to_subfield => 'a', regex => { search => 'Computer', replace => 'The mistake of' }, n => 1, dont_erase => 1 } );
+@fields_651a = read_field({ record => $record, field => '651', subfield => 'a' });
 is_deeply( \@fields_651a, [
     'The art of programming.',
     'The mistake of programming.',
     'The art of algorithms.',
     'The mistake of programming.'
 ], 'Copy first field using regex on existing fields without erase existing values' );
-delete_field( $record, '651' );
+delete_field({ record => $record, field => '651' });
 
-copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The art of' } );
-copy_field( $record, '650', 'a', '651', 'a', { search => 'Computer', replace => 'The mistake of' }, undef , "dont_erase" );
-@fields_651a = read_field( $record, '651', 'a' );
+copy_field({ record => $record, from_field => '650', from_subfield => 'a', to_field => '651', to_subfield => 'a', regex => { search => 'Computer', replace => 'The art of' }  });
+copy_field({ record => $record, from_field => '650', from_subfield => 'a', to_field => '651', to_subfield => 'a', regex => { search => 'Computer', replace => 'The mistake of' }, dont_erase => 1 });
+@fields_651a = read_field({ record =>  $record, field => '651', subfield => 'a' });
 is_deeply( \@fields_651a, [
     'The art of programming.',
     'The mistake of programming.',
@@ -114,33 +112,33 @@ is_deeply( \@fields_651a, [
     'The mistake of programming.',
     'The mistake of algorithms.'
 ], 'Copy fields using regex on existing fields without erase existing values' );
-delete_field( $record, '651' );
+delete_field({ record => $record, field => '651' });
 
 # Copy with regex modifiers
-copy_field( $record, '650', 'a', '652', 'a', { search => 'o', replace => 'foo' } );
-my @fields_652a = read_field( $record, '652', 'a' );
+copy_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 field using regex' );
 
-copy_field( $record, '650', 'a', '653', 'a', { search => 'o', replace => 'foo', modifiers => 'g' } );
-my @fields_653a = read_field( $record, '653', 'a' );
+copy_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 field using regex' );
 
-copy_field( $record, '650', 'a', '654', 'a', { search => 'O', replace => 'foo', modifiers => 'i' } );
-my @fields_654a = read_field( $record, '654', 'a' );
+copy_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 field using regex' );
 
-copy_field( $record, '650', 'a', '655', 'a', { search => 'O', replace => 'foo', modifiers => 'gi' } );
-my @fields_655a = read_field( $record, '655', 'a' );
+copy_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 field using regex' );
 
 # update_field
-update_field( $record, '952', 'p', undef, '3010023918' );
-is_deeply( read_field( $record, '952', 'p' ), '3010023918', 'update existing subfield 952$p' );
-delete_field( $record, '952' );
-update_field( $record, '952', 'p', undef, '3010023918' );
-update_field( $record, '952', 'y', undef, 'BK' );
-is_deeply( read_field( $record, '952', 'p' ), '3010023918', 'create subfield 952$p' );
-is_deeply( read_field( $record, '952', 'y' ), 'BK', 'create subfield 952$k on existing 952 field' );
+update_field({ record => $record, field => '952', subfield => 'p', values => ['3010023918'] });
+is_deeply( read_field({ record => $record, field => '952', subfield => 'p' }), '3010023918', 'update existing subfield 952$p' );
+delete_field({ record => $record, field => '952' });
+update_field({ record => $record, field => '952', subfield => 'p', values => ['3010023918'] });
+update_field({ record => $record, field => '952', subfield => 'y', values => ['BK'] });
+is_deeply( read_field({ record => $record, field => '952', subfield => 'p' }), '3010023918', 'create subfield 952$p' );
+is_deeply( read_field({ record => $record, field => '952', subfield => 'y' }), 'BK', 'create subfield 952$k on existing 952 field' );
 $record->append_fields(
     MARC::Field->new(
         952, ' ', ' ',
@@ -148,12 +146,12 @@ $record->append_fields(
         y => 'BK',
     ),
 );
-update_field( $record, '952', 'p', undef, '3010023919' );
-my @fields_952p = read_field( $record, '952', 'p' );
+update_field({ record => $record, field => '952', subfield => 'p', values => ['3010023919'] });
+my @fields_952p = read_field({ record =>  $record, field => '952', subfield => 'p' });
 is_deeply( \@fields_952p, ['3010023919', '3010023919'], 'update all subfields 952$p with the same value' );
 
-update_field( $record, '952', 'p', undef, ('3010023917', '3010023918') );
-@fields_952p = read_field( $record, '952', 'p' );
+update_field({ record => $record, field => '952', subfield => 'p', values => ['3010023917', '3010023918'] });
+@fields_952p = read_field({ record => $record, field => '952', subfield => 'p' });
 is_deeply( \@fields_952p, ['3010023917', '3010023918'], 'update all subfields 952$p with the different values' );
 
 # move_field
@@ -166,19 +164,19 @@ $record->append_fields(
         y => 'BK',
     ),
 );
-copy_field( $record, '952', 'd', '952', 'd' );
-@fields_952d = read_field( $record, '952', 'd' );
+copy_field({ record => $record, from_field => '952', from_subfield => 'd', to_field => '952', to_subfield => 'd' });
+@fields_952d = read_field({ record =>  $record, field => '952', subfield => 'd' });
 is_deeply( \@fields_952d, ['2001-06-25', '2001-06-25'], 'copy 952$d into others 952 field' );
 
-move_field( $record, '952', 'c', '954', 'c' );
-@fields_952c = read_field( $record, '952', 'c' );
-@fields_954c = read_field( $record, '954', 'c' );
+move_field({ record => $record, from_field => '952', from_subfield => 'c', to_field => '954', to_subfield => 'c' });
+@fields_952c = read_field({ record =>  $record, field => '952', subfield => 'c' });
+@fields_954c = read_field({ record =>  $record, field => '954', subfield => 'c' });
 is_deeply( \@fields_952c, [], 'The 952$c has moved' );
 is_deeply( \@fields_954c, ['GEN'], 'Now 954$c exists' );
 
-move_field( $record, '952', 'p', '954', 'p', undef, 1 ); # Move the first field
-@fields_952p = read_field( $record, '952', 'p' );
-@fields_954p = read_field( $record, '954', 'p' );
+move_field({ record => $record, from_field => '952', from_subfield => 'p', to_field => '954', to_subfield => 'p', n => 1 }); # Move the first field
+@fields_952p = read_field({ record =>  $record, field => '952', subfield => 'p' });
+@fields_954p = read_field({ record =>  $record, field => '954', subfield => 'p' });
 is_deeply( \@fields_952p, ['3010023917'], 'One of 952$p has moved' );
 is_deeply( \@fields_954p, ['3010023917'], 'Now 954$p exists' );
 
@@ -191,9 +189,9 @@ $record->append_fields(
     ),
 );
 
-move_field( $record, '952', 'p', '954', 'p' ); # Move all field
-@fields_952p = read_field( $record, '952', 'p' );
-@fields_954p = read_field( $record, '954', 'p' );
+move_field({ record => $record, from_field => '952', from_subfield => 'p', to_field => '954', to_subfield => 'p' }); # Move all field
+@fields_952p = read_field({ record =>  $record, field => '952', subfield => 'p' });
+@fields_954p = read_field({ record =>  $record, field => '954', subfield => 'p' });
 is_deeply( \@fields_952p, [], 'All 952$p have moved' );
 is_deeply( \@fields_954p, ['3010023917', '3010023917'], 'Now 2 954$p exist' );