Bug 24900: Checks in MARC mod templates for when from field does not equal conditiona...

When MARC modification template actions are applied, they assume that
the from field is the same as the conditional field. This patch adds
checks for this, as well as tests to confirm the behaviour is correct.

CASE 1: Delete 1st field 020 if 651$z exists
BROKEN BEHAVIOUR (before patch): deletes the 2nd instance of 020 instead
                                 of 1st
EXPECTED BEHAVIOUR (corrected by patch): deletes the 1st instance of 020

CASE 2: Delete 1st field 020 if 651$z matches Berlin. (must include '.')
BROKEN BEHAVIOUR (before patch): deletes the 2nd instance of 020
EXPECTED BEHAVIOUR (corrected by patch): deletes the 1st instance of 020

CASE 3: Delete field 020 if 650$2 does not match fast
BROKEN BEHAVIOUR (before patch): deletes all 020 fields even though
                                 650$2 does match fast
EXPECTED BEHAVIOUR (corrected by patch): does not delete 020 fields

Confirm tests pass: t/db_dependent/MarcModificationTemplates.t

Sponsored-by: Catalyst IT
Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm
index 54846e5..f4a38a7 100644 (file)
--- a/C4/MarcModificationTemplates.pm
+++ b/C4/MarcModificationTemplates.pm
@@ -585,9 +585,15 @@ sub ModifyRecordWithTemplate {
                     }
                 ];
                 $field_numbers = [Koha::MoreUtils::singleton ( @$field_numbers, @$all_fields ) ];
-                $do = $conditional eq 'if'
-                    ? @$field_numbers
-                    : not @$field_numbers;
+                if ( $from_field == $conditional_field ){
+                    $do = $conditional eq 'if'
+                        ? @$field_numbers
+                        : not @$field_numbers;
+                } else {
+                    $do = $conditional eq 'if'
+                        ? not @$field_numbers
+                        : @$field_numbers;
+                }
             }
         }
 
@@ -599,8 +605,23 @@ sub ModifyRecordWithTemplate {
             # A condition has been given
             if ( @$field_numbers > 0 ) {
                 if ( $field_number == 1 ) {
-                    # We want only the first matching
-                    $field_numbers = [ $field_numbers->[0] ];
+                    # We want only the first
+                    if ( $from_field == $conditional_field ){
+                        # want first field matching condition
+                        $field_numbers = [ $field_numbers->[0] ];
+                    } else {
+                        # condition doesn't match, so just want first occurrence of from field
+                        $field_numbers = [ 1 ];
+                    }
+                } else {
+                    unless ( $from_field == $conditional_field ){
+                        # condition doesn't match from fields so need all occurrences of from fields for action
+                        $field_numbers = field_exists({
+                            record => $record,
+                            field => $from_field,
+                            subfield => $from_subfield,
+                        });
+                    }
                 }
             }
             # There was no condition

diff --git a/t/db_dependent/MarcModificationTemplates.t b/t/db_dependent/MarcModificationTemplates.t
index 2d64619..aee89f6 100644 (file)
--- a/t/db_dependent/MarcModificationTemplates.t
+++ b/t/db_dependent/MarcModificationTemplates.t
@@ -1,6 +1,6 @@
 use Modern::Perl;
 
-use Test::More tests => 125;
+use Test::More tests => 126;
 
 use Koha::Database;
 use Koha::SimpleMARC;
@@ -415,6 +415,51 @@ subtest "not_equals" => sub {
     is_deeply( $record, $expected_record, 'None 650 have been moved, no $650$b exists' );
 };
 
+subtest "when conditional field doesn't match the from field" => sub {
+    plan tests => 3;
+    $dbh->do(q|DELETE FROM marc_modification_templates|);
+    my $template_id = AddModificationTemplate("template_name");
+    AddModificationTemplateAction(
+        $template_id, 'delete_field', 0,
+        '650', '9', '', '', '',
+        '', '', '',
+        'if', '245', 'a', 'equals', 'Bad title', '',
+        'Delete fields 650$9 if 245$a == "Bad title"'
+    );
+    my $record = new_record();
+    ModifyRecordWithTemplate( $template_id, $record );
+    my $expected_record = expected_record_3();
+    is_deeply( $record, $expected_record, '650$9 fields have been deleted when 245$a == "Bad title"' );
+
+    $dbh->do(q|DELETE FROM marc_modification_templates|);
+    $template_id = AddModificationTemplate("template_name");
+    AddModificationTemplateAction(
+        $template_id, 'delete_field', 0,
+        '650', '9', '', '', '',
+        '', '', '',
+        'if', '245', 'a', 'exists', '', '',
+        'Delete fields 650$9 if 245$a exists'
+    );
+    $record = new_record();
+    ModifyRecordWithTemplate( $template_id, $record );
+    $expected_record = expected_record_3();
+    is_deeply( $record, $expected_record, '650$9 fields have been deleted because 245$a exists' );
+
+    $dbh->do(q|DELETE FROM marc_modification_templates|);
+    $template_id = AddModificationTemplate("template_name");
+    AddModificationTemplateAction(
+        $template_id, 'delete_field', 1,
+        '650', '', '', '', '',
+        '', '', '',
+        'if', '245', 'a', 'exists', '', '',
+        'Delete 1st field 650 if 245$a exists'
+    );
+    $record = new_record();
+    ModifyRecordWithTemplate( $template_id, $record );
+    $expected_record = expected_record_4();
+    is_deeply( $record, $expected_record, '1st 650 field has been deleted because 245$a exists' );
+};
+
 sub new_record {
     my $record = MARC::Record->new;
     $record->leader('03174nam a2200445 a 4500');
@@ -547,6 +592,81 @@ sub expected_record_2 {
     return $record;
 }
 
+sub expected_record_3 {
+    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(
+            245, '1', '4',
+            a => 'Bad title',
+            c => 'Donald E. Knuth.',
+        ),
+        MARC::Field->new(
+            650, ' ', '0',
+            a => 'Computer programming.',
+        ),
+        MARC::Field->new(
+            650, ' ', '0',
+            a => 'Computer programming.',
+        ),
+        MARC::Field->new(
+            952, ' ', ' ',
+            p => '3010023917',
+            y => 'BK',
+            c => 'GEN',
+            d => '2001-06-25',
+        ),
+    );
+    $record->append_fields(@fields);
+    return $record;
+}
+
+sub expected_record_4 {
+    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(
+            245, '1', '4',
+            a => 'Bad title',
+            c => 'Donald E. Knuth.',
+        ),
+        MARC::Field->new(
+            650, ' ', '0',
+            a => 'Computer programming.',
+            9 => '499',
+        ),
+        MARC::Field->new(
+            952, ' ', ' ',
+            p => '3010023917',
+            y => 'BK',
+            c => 'GEN',
+            d => '2001-06-25',
+        ),
+    );
+    $record->append_fields(@fields);
+    return $record;
+}
+
 # Tests related to use of subfield 0 ($0)
 
 sub new_record_0 {