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 <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t
index 9b81115..123577e 100755 (executable)
--- 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;