Bug 17913: We always need some housekeeping

Remove some commented warnings
Remove the commented old code at the end of sub merge
Explicitly set merge mode in the first subtest
Move the return to loose mode from the second subtest to the third

Test plan:
Run t/db_dependent/Authorities/Merge.t

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/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm
index 7517419..e920c01 100644 (file)
--- a/C4/AuthoritiesMarc.pm
+++ b/C4/AuthoritiesMarc.pm
@@ -1455,7 +1455,6 @@ sub merge {
         $z++;
     }
     $oResult->destroy();
-    #warn scalar(@reccache)." biblios to update";
     # Get All candidate Tags for the change 
     # (This will reduce the search scope in marc records).
     my $sql = "SELECT DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode=?";
@@ -1464,8 +1463,7 @@ sub merge {
     if ($authtypeto->authtypecode ne $authtypefrom->authtypecode){
         $tags_new = $dbh->selectcol_arrayref( $sql, undef, ( $authtypeto->authtypecode ));
     }  
-    # BulkEdit marc records
-    # May be used as a template for a bulkedit field  
+
     my $overwrite = C4::Context->preference( 'AuthorityMergeMode' ) eq 'strict';
     my $skip_subfields = $overwrite
         # This hash contains all subfields from the authority report fields
@@ -1476,11 +1474,9 @@ sub merge {
     foreach my $marcrecord(@reccache){
         my $update = 0;
         foreach my $tagfield (@$tags_using_authtype){
-#             warn "tagfield : $tagfield ";
             my $countfrom = 0; # used in strict mode to remove duplicates
             foreach my $field ($marcrecord->field($tagfield)){
-                # biblio is linked to authority with $9 subfield containing authid
-                my $auth_number=$field->subfield("9");
+                my $auth_number = $field->subfield("9"); # link to authority
                 my $tag=$field->tag();
                 next if !defined($auth_number) || $auth_number ne $mergefrom;
                 $countfrom++;
@@ -1516,8 +1512,8 @@ sub merge {
                 $field->replace_with($field_to);
             }
                 $update=1;
-            }#for each tag
-        }#foreach tagfield
+            }
+        }
         my ($bibliotag,$bibliosubf) = GetMarcFromKohaField("biblio.biblionumber","") ;
         my $biblionumber;
         if ($bibliotag<10){
@@ -1535,57 +1531,9 @@ sub merge {
             $counteditedbiblio++;
             warn $counteditedbiblio if (($counteditedbiblio % 10) and $ENV{DEBUG});
         }    
-    }#foreach $marc
+    }
     return $counteditedbiblio;  
-  # now, find every other authority linked with this authority
-  # now, find every other authority linked with this authority
-#   my $oConnection=C4::Context->Zconn("authorityserver");
-#   my $query;
-# # att 9210               Auth-Internal-authtype
-# # att 9220               Auth-Internal-LN
-# # ccl.properties to add for authorities
-#   $query= "= ".$mergefrom;
-#   my $oResult = $oConnection->search(new ZOOM::Query::CCL2RPN( $query, $oConnection ));
-#   my $count=$oResult->size() if  ($oResult);
-#   my @reccache;
-#   my $z=0;
-#   while ( $z<$count ) {
-#   my $rec;
-#           $rec=$oResult->record($z);
-#       my $marcdata = $rec->raw();
-#   push @reccache, $marcdata;
-#   $z++;
-#   }
-#   $oResult->destroy();
-#   foreach my $marc(@reccache){
-#     my $update;
-#     my $marcrecord;
-#     $marcrecord = MARC::File::USMARC::decode($marc);
-#     foreach my $tagfield (@tags_using_authtype){
-#       $tagfield=substr($tagfield,0,3);
-#       my @tags = $marcrecord->field($tagfield);
-#       foreach my $tag (@tags){
-#         my $tagsubs=$tag->subfield("9");
-#     #warn "$tagfield:$tagsubs:$mergefrom";
-#         if ($tagsubs== $mergefrom) {
-#           $tag->update("9" =>$mergeto);
-#           foreach my $subfield (@record_to) {
-#     #        warn "$subfield,$subfield->[0],$subfield->[1]";
-#             $tag->update($subfield->[0] =>$subfield->[1]);
-#           }#for $subfield
-#         }
-#         $marcrecord->delete_field($tag);
-#         $marcrecord->add_fields($tag);
-#         $update=1;
-#       }#for each tag
-#     }#foreach tagfield
-#     my $authoritynumber = TransformMarcToKoha($marcrecord,"") ;
-#     if ($update==1){
-#       &ModAuthority($marcrecord,$authoritynumber,GetAuthTypeCode($authoritynumber)) ;
-#     }
-# 
-#   }#foreach $marc
-}#sub
+}
 
 sub _merge_newtag {
 # Routine is only called for an (exceptional) authtypecode change

diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t
index 2bed211..c318f16 100755 (executable)
--- a/t/db_dependent/Authorities/Merge.t
+++ b/t/db_dependent/Authorities/Merge.t
@@ -41,6 +41,9 @@ subtest 'Test merge A1 to A2 (within same authtype)' => sub {
 # Tests originate from bug 11700
     plan tests => 9;
 
+    # Start in loose mode, although it actually does not matter here
+    t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose');
+
     # Add two authority records
     my $auth1 = MARC::Record->new;
     $auth1->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'George Orwell' ));
@@ -80,7 +83,7 @@ subtest 'Test merge A1 to A2 (within same authtype)' => sub {
         'Check biblio2 609$a' );
 };
 
-subtest 'Test merge A1 to modified A1' => sub {
+subtest 'Test merge A1 to modified A1, test strict mode' => sub {
 # Tests originate from bug 11700
     plan tests => 11;
 
@@ -103,7 +106,7 @@ subtest 'Test merge A1 to modified A1' => sub {
     my ( $biblionumber1 ) = AddBiblio( $MARC1, '');
     my ( $biblionumber2 ) = AddBiblio( $MARC2, '');
 
-    # Time to merge
+    # Time to merge in loose mode first
     @zebrarecords = ( $MARC1, $MARC2 );
     $index = 0;
     t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose');
@@ -137,15 +140,16 @@ subtest 'Test merge A1 to modified A1' => sub {
         scalar($auth1new->field('109')->subfields) + 1,
         'Check number of subfields in strict mode for the remaining 609' );
         # Note: the +1 comes from the added subfield $9 in the biblio
-    t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose');
 };
 
 subtest 'Test merge A1 to B1 (changing authtype)' => sub {
 # Tests were aimed for bug 9988, moved to 17909 in adjusted form
 # Would not encourage this type of merge, but we should test what we offer
-# The merge routine still needs the fixes on bug 17913
     plan tests => 13;
 
+    # Get back to loose mode now
+    t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose');
+
     # 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' ));