Bugzilla – Attachment 102121 Details for
Bug 24900
Fix 'MARC modification templates' to not assume that 'from field' will match 'conditional field'
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24900: Checks in MARC mod templates for when from field does not equal conditional field
Bug-24900-Checks-in-MARC-mod-templates-for-when-fr.patch (text/plain), 8.22 KB, created by
Jonathan Druart
on 2020-03-31 11:06:44 UTC
(
hide
)
Description:
Bug 24900: Checks in MARC mod templates for when from field does not equal conditional field
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-31 11:06:44 UTC
Size:
8.22 KB
patch
obsolete
>From 573d1d7c88087894a19c6c010e374e5e845c242e Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Thu, 19 Mar 2020 04:37:34 +0000 >Subject: [PATCH] Bug 24900: Checks in MARC mod templates for when from field > does not equal conditional field > >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> >--- > C4/MarcModificationTemplates.pm | 31 +++++- > t/db_dependent/MarcModificationTemplates.t | 122 ++++++++++++++++++++- > 2 files changed, 147 insertions(+), 6 deletions(-) > >diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm >index 54846e5956..f4a38a702e 100644 >--- 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 2d646190f6..aee89f6198 100644 >--- 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 { >-- >2.20.1
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 24900
:
100992
|
100993
|
101665
|
101666
|
101970
| 102121