From ed44281c4bdd31a26daacdfb69cb67032182a504 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 16 Jan 2017 12:15:56 +0100 Subject: [PATCH] Bug 17909: Additional polishing No spectacular things: [1] Move the framework modifications to a sub. Use same style to create auth types and linked fields. [2] Change some new Object occurrences to Object->new. [3] Add tests for field count and field order in the first two subsets. [4] Few whitespace changes (sorry) and comment lines. Signed-off-by: Marcel de Rooy Signed-off-by: Josef Moravec Signed-off-by: Julian Maurice Signed-off-by: Kyle M Hall (cherry picked from commit 4457d2e64c30f95fc79e306023e271ee7274b740) Signed-off-by: Katrin Fischer --- t/db_dependent/Authorities/Merge.t | 185 +++++++++++++++++------------ 1 file changed, 108 insertions(+), 77 deletions(-) diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t index 9b811151a4..123577e381 100755 --- a/t/db_dependent/Authorities/Merge.t +++ b/t/db_dependent/Authorities/Merge.t @@ -35,85 +35,86 @@ my $zoom_obj = Test::MockObject->new; my $zoom_record_obj = Test::MockObject->new; set_mocks(); +# Framework operations +my ( $authtype1, $authtype2 ) = modify_framework(); + subtest 'Test merge A1 to A2 (withing same authtype)' => sub { # Tests originate from bug 11700 - plan tests => 5; - - # Create authority type TEST_PERSO - $dbh->do("INSERT INTO auth_types(authtypecode, authtypetext, auth_tag_to_report, summary) VALUES('TEST_PERSO', 'Personal Name', '109', 'Personal Names');"); - $dbh->do("INSERT INTO auth_tag_structure (authtypecode, tagfield, liblibrarian, libopac, repeatable, mandatory, authorised_value) VALUES('TEST_PERSO', '109', 'HEADING--PERSONAL NAME', 'HEADING--PERSONAL NAME', 0, 0, NULL)"); - $dbh->do("INSERT INTO auth_subfield_structure (authtypecode, tagfield, tagsubfield, liblibrarian, libopac, repeatable, mandatory, tab, authorised_value, value_builder, seealso, isurl, hidden, linkid, kohafield, frameworkcode) VALUES ('TEST_PERSO', '109', 'a', 'Personal name', 'Personal name', 0, 0, 1, NULL, NULL, '', 0, 0, '', '', '')"); - - my $auth1 = new MARC::Record; - $auth1->append_fields(new MARC::Field('109', '0', '0', 'a' => 'George Orwell')); - my $authid1 = AddAuthority($auth1, undef, 'TEST_PERSO'); - my $auth2 = new MARC::Record; - $auth2->append_fields(new MARC::Field('109', '0', '0', 'a' => 'G. Orwell')); - my $authid2 = AddAuthority($auth2, undef, 'TEST_PERSO'); - - $dbh->do("INSERT IGNORE INTO marc_subfield_structure(tagfield, tagsubfield, liblibrarian, libopac, repeatable, mandatory, kohafield, tab, authorised_value, authtypecode, value_builder, isurl, hidden, frameworkcode, seealso, link, defaultvalue) VALUES('609', 'a', 'Personal name', 'Personal name', 0, 0, '', 6, '', 'TEST_PERSO', '', NULL, 0, '', '', '', NULL)"); - $dbh->do("UPDATE marc_subfield_structure SET authtypecode = 'TEST_PERSO' WHERE tagfield='609' AND tagsubfield='a' AND frameworkcode='';"); - my $tagfields = $dbh->selectcol_arrayref("select distinct tagfield from marc_subfield_structure where authtypecode='TEST_PERSO'"); - my $biblio1 = new MARC::Record; - $biblio1->append_fields( - new MARC::Field('609', '0', '0', '9' => $authid1, 'a' => 'George Orwell') - ); + plan tests => 9; + + # Add two authority records + my $auth1 = MARC::Record->new; + $auth1->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'George Orwell' )); + my $authid1 = AddAuthority( $auth1, undef, $authtype1 ); + my $auth2 = MARC::Record->new; + $auth2->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'G. Orwell' )); + my $authid2 = AddAuthority( $auth2, undef, $authtype1 ); + + # Add two biblio records + my $biblio1 = MARC::Record->new; + $biblio1->append_fields( MARC::Field->new( '609', '0', '0', '9' => $authid1, 'a' => 'George Orwell' )); my ( $biblionumber1 ) = AddBiblio($biblio1, ''); - my $biblio2 = new MARC::Record; - $biblio2->append_fields( - new MARC::Field('609', '0', '0', '9' => $authid2, 'a' => 'G. Orwell') - ); + my $biblio2 = MARC::Record->new; + $biblio2->append_fields( MARC::Field->new( '609', '0', '0', '9' => $authid2, 'a' => 'G. Orwell' )); my ( $biblionumber2 ) = AddBiblio($biblio2, ''); + # Time to merge @zebrarecords = ( $biblio1, $biblio2 ); $index = 0; my $rv = C4::AuthoritiesMarc::merge( $authid2, $auth2, $authid1, $auth1 ); is( $rv, 1, 'We expect one biblio record (out of two) to be updated' ); - $biblio1 = GetMarcBiblio($biblionumber1); - is($biblio1->subfield('609', '9'), $authid1, 'Check biblio1 609$9' ); - is($biblio1->subfield('609', 'a'), 'George Orwell', + # Check the results + my $newbiblio1 = GetMarcBiblio($biblionumber1); + compare_field_count( $biblio1, $newbiblio1, 1 ); + compare_field_order( $biblio1, $newbiblio1, 1 ); + is( $newbiblio1->subfield('609', '9'), $authid1, 'Check biblio1 609$9' ); + is( $newbiblio1->subfield('609', 'a'), 'George Orwell', 'Check biblio1 609$a' ); - $biblio2 = GetMarcBiblio($biblionumber2); - is($biblio2->subfield('609', '9'), $authid1, 'Check biblio2 609$9' ); - is($biblio2->subfield('609', 'a'), 'George Orwell', + my $newbiblio2 = GetMarcBiblio($biblionumber2); + compare_field_count( $biblio2, $newbiblio2, 1 ); + compare_field_order( $biblio2, $newbiblio2, 1 ); + is( $newbiblio2->subfield('609', '9'), $authid1, 'Check biblio2 609$9' ); + is( $newbiblio2->subfield('609', 'a'), 'George Orwell', 'Check biblio2 609$a' ); }; subtest 'Test merge A1 to modified A1' => sub { # Tests originate from bug 11700 - plan tests => 4; - - $dbh->do("INSERT IGNORE INTO marc_subfield_structure(tagfield, tagsubfield, liblibrarian, libopac, repeatable, mandatory, kohafield, tab, authorised_value, authtypecode, value_builder, isurl, hidden, frameworkcode, seealso, link, defaultvalue) VALUES('109', 'a', 'Personal name', 'Personal name', 0, 0, '', 6, '', 'TEST_PERSO', '', NULL, 0, '', '', '', NULL)"); - $dbh->do("UPDATE marc_subfield_structure SET authtypecode = 'TEST_PERSO' WHERE tagfield='109' AND tagsubfield='a' AND frameworkcode='';"); + plan tests => 8; + # Simulate modifying an authority from auth1old to auth1new my $auth1old = MARC::Record->new; $auth1old->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'Bruce Wayne' )); my $auth1new = $auth1old->clone; $auth1new->field('109')->update( a => 'Batman' ); - my $authid1 = AddAuthority( $auth1new, undef, 'TEST_PERSO' ); + my $authid1 = AddAuthority( $auth1new, undef, $authtype1 ); - my $MARC1 = MARC::Record->new(); - $MARC1->append_fields( MARC::Field->new( '245', '', '', 'a' => 'From the depths' )); + # Add two biblio records + my $MARC1 = MARC::Record->new; $MARC1->append_fields( MARC::Field->new( '109', '', '', 'a' => 'Bruce Wayne', 'b' => '2014', '9' => $authid1 )); - my $MARC2 = MARC::Record->new(); - $MARC2->append_fields( MARC::Field->new( '245', '', '', 'a' => 'All the way to heaven' )); + $MARC1->append_fields( MARC::Field->new( '245', '', '', 'a' => 'From the depths' )); + my $MARC2 = MARC::Record->new; $MARC2->append_fields( MARC::Field->new( '109', '', '', 'a' => 'Batman', '9' => $authid1 )); + $MARC2->append_fields( MARC::Field->new( '245', '', '', 'a' => 'All the way to heaven' )); my ( $biblionumber1 ) = AddBiblio( $MARC1, ''); my ( $biblionumber2 ) = AddBiblio( $MARC2, ''); + # Time to merge @zebrarecords = ( $MARC1, $MARC2 ); $index = 0; - my $rv = C4::AuthoritiesMarc::merge( $authid1, $auth1old, $authid1, $auth1new ); is( $rv, 2, 'Both records are updated now' ); + #Check the results my $biblio1 = GetMarcBiblio($biblionumber1); + compare_field_count( $MARC1, $biblio1, 1 ); + compare_field_order( $MARC1, $biblio1, 1 ); + is( $auth1new->field(109)->subfield('a'), $biblio1->field(109)->subfield('a'), 'Record1 values updated correctly' ); my $biblio2 = GetMarcBiblio($biblionumber1); - - my $auth_field = $auth1new->field(109)->subfield('a'); - is( $auth_field, $biblio1->field(109)->subfield('a'), 'Record1 values updated correctly' ); - is( $auth_field, $biblio2->field(109)->subfield('a'), 'Record2 values updated correctly' ); + compare_field_count( $MARC2, $biblio2, 1 ); + compare_field_order( $MARC2, $biblio2, 1 ); + is( $auth1new->field(109)->subfield('a'), $biblio2->field(109)->subfield('a'), 'Record2 values updated correctly' ); # TODO Following test will change when we improve merge # Will depend on a preference @@ -126,42 +127,13 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { # The merge routine still needs the fixes on bug 17913 plan tests => 8; - # create another authtype - my $authtype2 = $builder->build({ - source => 'AuthType', - value => { - auth_tag_to_report => '112', - }, - }); - # create two fields linked to this auth type - $schema->resultset('MarcSubfieldStructure')->search({ tagfield => [ '112', '712' ] })->delete; - $builder->build({ - source => 'MarcSubfieldStructure', - value => { - tagfield => '112', - tagsubfield => 'a', - authtypecode => $authtype2->{authtypecode}, - frameworkcode => '', - }, - }); - $builder->build({ - source => 'MarcSubfieldStructure', - value => { - tagfield => '712', - tagsubfield => 'a', - authtypecode => $authtype2->{authtypecode}, - frameworkcode => '', - }, - }); - - # create auth1 (from the earlier type) + # create two auth recs of different type my $auth1 = MARC::Record->new; $auth1->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'George Orwell', b => 'bb' )); - my $authid1 = AddAuthority($auth1, undef, 'TEST_PERSO'); - # create auth2 (new type) + my $authid1 = AddAuthority( $auth1, undef, $authtype1 ); my $auth2 = MARC::Record->new; $auth2->append_fields( MARC::Field->new( '112', '0', '0', 'a' => 'Batman', c => 'cc' )); - my $authid2 = AddAuthority($auth1, undef, $authtype2->{authtypecode} ); + my $authid2 = AddAuthority($auth1, undef, $authtype2 ); # create a biblio with one 109 and two 609s to be touched # seems exceptional see bug 13760 comment10 @@ -178,6 +150,7 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { my ( $biblionumber ) = C4::Biblio::AddBiblio( $marc, '' ); my $oldbiblio = C4::Biblio::GetMarcBiblio( $biblionumber ); + # Time to merge @zebrarecords = ( $marc ); $index = 0; my $retval = C4::AuthoritiesMarc::merge( $authid1, $auth1, $authid2, $auth2 ); @@ -225,6 +198,64 @@ sub set_mocks { $zoom_record_obj->mock( 'raw', sub {} ); } +sub modify_framework { + # create two auth types + my $authtype1 = $builder->build({ + source => 'AuthType', + value => { + auth_tag_to_report => '109', + }, + }); + my $authtype2 = $builder->build({ + source => 'AuthType', + value => { + auth_tag_to_report => '112', + }, + }); + + # Link 109/609 to the first authtype + $builder->build({ + source => 'MarcSubfieldStructure', + value => { + tagfield => '109', + tagsubfield => 'a', + authtypecode => $authtype1->{authtypecode}, + frameworkcode => '', + }, + }); + $builder->build({ + source => 'MarcSubfieldStructure', + value => { + tagfield => '609', + tagsubfield => 'a', + authtypecode => $authtype1->{authtypecode}, + frameworkcode => '', + }, + }); + + # Link 112/712 to the second authtype + $builder->build({ + source => 'MarcSubfieldStructure', + value => { + tagfield => '112', + tagsubfield => 'a', + authtypecode => $authtype2->{authtypecode}, + frameworkcode => '', + }, + }); + $builder->build({ + source => 'MarcSubfieldStructure', + value => { + tagfield => '712', + tagsubfield => 'a', + authtypecode => $authtype2->{authtypecode}, + frameworkcode => '', + }, + }); + + return ( $authtype1->{authtypecode}, $authtype2->{authtypecode} ); +} + sub compare_field_count { my ( $oldmarc, $newmarc, $pass ) = @_; my $t; -- 2.39.5