Bug 14769: Put ControlledIndicators to work
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Fri, 26 Jan 2018 09:17:33 +0000 (10:17 +0100)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 12 Apr 2018 13:50:35 +0000 (10:50 -0300)
This patch does:
[1] Adds Koha::Authority->controlled_indicators (with a test).
[2] Adds a call to controlled_indicators in AuthoritiesMarc::merge.
    Unit test Merge.t is extended too.
[3] Simplifies the code in authorities/blinddetail-biblio-search.pl by
    calling controlled_indicators.

Test plan:
[1] Run t/db_dependent/Koha/Authorities.t
[2] Run t/db_dependent/Authority/Merge.t
[3] Steps 3 to 7 for MARC21:
    Create a PERSO_NAME authority with 008/11=r and ind1=3
[4] Edit a biblio and add a 600 linked to the new authority.
[5] Verify that the biblio has ind1==3 and ind2==7 and $2==aat.
    (If $2 is not visible, check the metadata in biblio_metadata.)
[6] Edit the PERSO_NAME authority and change 008/11 to '|' (bar).
[7] Verify that merge updated your biblio record: $ind2==4 and $2 gone.
[8] UNIMARC: Follow the pattern from steps 3 to 7.
    Create authority, link it in a biblio, check indicators (they should
    be copied both). Edit authority, change indicators and verify the
    merge results in the biblio record.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested PERSO_NAME and UNIF_TITLE.
For UNIF_TITLE the second authority indicator is copied to ind1 or ind2,
depending on the biblio tag involved.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/AuthoritiesMarc.pm
Koha/Authority.pm
authorities/blinddetail-biblio-search.pl
t/db_dependent/Authority/Merge.t
t/db_dependent/Koha/Authorities.t

index 165ef36..f441c60 100644 (file)
@@ -1471,10 +1471,11 @@ sub merge {
                 my $newtag = $tags_new && @$tags_new
                   ? _merge_newtag( $tag, $tags_new )
                   : $tag;
+                my $controlled_ind = Koha::Authority->new({ authtypecode => $authtypeto ? $authtypeto->authtypecode : undef })->controlled_indicators({ record => $MARCto, biblio_tag => $newtag }); #FIXME Replace this tric with new when refactoring
                 my $field_to = MARC::Field->new(
                     $newtag,
-                    $field->indicator(1),
-                    $field->indicator(2),
+                    $controlled_ind->{ind1} // $field->indicator(1),
+                    $controlled_ind->{ind2} // $field->indicator(2),
                     9 => $mergeto, # Needed to create field, will be moved
                 );
                 my ( @prefix, @postfix );
@@ -1500,6 +1501,15 @@ sub merge {
                 foreach my $subfield ( @prefix, @record_to, @postfix ) {
                     $field_to->add_subfields($subfield->[0] => $subfield->[1]);
                 }
+                if( exists $controlled_ind->{sub2} ) { # thesaurus info
+                    if( defined $controlled_ind->{sub2} ) {
+                        # Add or replace
+                        $field_to->update( 2 => $controlled_ind->{sub2} );
+                    } else {
+                        # Key alerts us here to remove $2
+                        $field_to->delete_subfield( code => '2' );
+                    }
+                }
                 # Move $9 to the end
                 $field_to->delete_subfield( code => '9' );
                 $field_to->add_subfields( 9 => $mergeto );
index ac2b25c..b427d7f 100644 (file)
@@ -20,6 +20,8 @@ package Koha::Authority;
 use Modern::Perl;
 
 use base qw(Koha::Object);
+
+use Koha::Authority::ControlledIndicators;
 use Koha::SearchEngine::Search;
 
 =head1 NAME
@@ -59,6 +61,55 @@ sub linked_biblionumbers {
     return Koha::Authorities->linked_biblionumbers( $params );
 }
 
+=head3 controlled_indicators
+
+    Some authority types control the indicators of some corresponding
+    biblio fields (especially in MARC21).
+    For example, if you have a PERSO_NAME authority (report tag 100), the
+    first indicator of biblio field 600 directly comes from the authority,
+    and the second indicator depends on thesaurus settings in the authority
+    record. Use this method to obtain such controlled values. In this example
+    you should pass 600 in the biblio_tag parameter.
+
+    my $result = $self->controlled_indicators({
+        record => $auth_marc, biblio_tag => $bib_tag
+    });
+    my $ind1 = $result->{ind1};
+    my $ind2 = $result->{ind2};
+    my $subfield_2 = $result->{sub2}; # Optional subfield 2 when ind==7
+
+    If an indicator is not controlled, the result hash does not contain a key
+    for its value. (Same for the sub2 key for an optional subfield $2.)
+
+    Note: The record parameter is a temporary bypass in order to prevent
+    needless conversion of $self->marcxml.
+
+=cut
+
+sub controlled_indicators {
+    my ( $self, $params ) = @_;
+    my $tag = $params->{biblio_tag} // q{};
+    my $record = $params->{record};
+
+    my $flavour = C4::Context->preference('marcflavour') eq 'UNIMARC'
+        ? 'UNIMARCAUTH'
+        : 'MARC21';
+    if( !$record ) {
+        $record = MARC::Record->new_from_xml(
+            $self->marcxml, 'UTF-8', $flavour );
+    }
+
+    my $authtype = Koha::Authority::Types->find( $self->authtypecode );
+    return {} if !$authtype;
+
+    return Koha::Authority::ControlledIndicators->new->get({
+        auth_record => $record,
+        report_tag  => $authtype->auth_tag_to_report,
+        biblio_tag  => $tag,
+        flavour     => $flavour,
+    });
+}
+
 =head2 Class Methods
 
 =head3 type
index 9e7930d..0146a66 100755 (executable)
@@ -70,11 +70,16 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
+# Extract the tag number from the index
+my $tag_number = $index;
+$tag_number =~ s/^tag_(\d*)_.*$/$1/;
+
 # fill arrays
 my @subfield_loop;
 my ($indicator1, $indicator2);
 if ($authid) {
-    my $authtypecode = Koha::Authorities->find($authid)->authtypecode;
+    my $auth = Koha::Authorities->find( $authid );
+    my $authtypecode = $auth ? $auth->authtypecode : q{};
     my $auth_type = Koha::Authority::Types->find($authtypecode);
     my $record = GetAuthority($authid);
     my @fields = $record->field( $auth_type->auth_tag_to_report );
@@ -98,56 +103,19 @@ if ($authid) {
     }
 
     push( @subfield_loop, { marc_subfield => 'w', marc_values => $relationship } ) if ( $relationship );
-    if (C4::Context->preference('marcflavour') eq 'UNIMARC') {
-        $indicator1 = $field->indicator('1');
-        $indicator2 = $field->indicator('2');
-    } elsif (C4::Context->preference('marcflavour') eq 'MARC21') {
-        my $tag_from = $auth_type->auth_tag_to_report;
-        my $tag_to = $index;
-        $tag_to =~ s/^tag_(\d*)_.*$/$1/;
-        if ($tag_to =~ /^6/) {  # subject heading
-            my %thes_mapping = qw / a 0
-                                    b 1
-                                    c 2
-                                    d 3
-                                    k 5
-                                    n 4
-                                    r 7
-                                    s 7
-                                    v 6
-                                    z 7
-                                    | 4 /;
-            my $thes_008_11 = '';
-            $thes_008_11 = substr($record->field('008')->data(), 11, 1) if $record->field('008')->data();
-            $indicator2 = defined $thes_mapping{$thes_008_11} ? $thes_mapping{$thes_008_11} : $thes_008_11;
-            if ($indicator2 eq '7') {
-                if ($thes_008_11 eq 'r') {
-                    push @subfield_loop, { marc_subfield => '2', marc_values => [ 'aat' ] };
-                } elsif ($thes_008_11 eq 's') {
-                    push @subfield_loop, { marc_subfield => '2', marc_values => [ 'sears' ] };
-                }
-            }
-        }
-        if ($tag_from eq '130') {  # unified title -- the special case
-            if ($tag_to eq '830' || $tag_to eq '240') {
-                $indicator2 = $field->indicator('2');
-            } else {
-                $indicator1 = $field->indicator('2');
-            }
-        } else {
-            $indicator1 = $field->indicator('1');
-        }
+
+    my $controlled_ind = $auth->controlled_indicators({ record => $record, biblio_tag => $tag_number });
+    $indicator1 = $controlled_ind->{ind1} // q{};
+    $indicator2 = $controlled_ind->{ind2} // q{};
+    if( defined $controlled_ind->{sub2} ) {
+        my $v = $controlled_ind->{sub2};
+        push @subfield_loop, { marc_subfield => '2', marc_values => [ $v ] };
     }
-}
-else {
+} else {
     # authid is empty => the user want to empty the entry.
     $template->param( "clear" => 1 );
 }
 
-# Extract the tag number from the index
-my $tag_number = $index;
-$tag_number =~ s/^tag_(\d*)_.*$/$1/;
-
 # Remove spaces in indicators
 $indicator1 =~ s/\s//g;
 $indicator2 =~ s/\s//g;
@@ -163,4 +131,3 @@ $template->param(
 );
 
 output_html_with_http_headers $query, $cookie, $template->output;
-
index d987b83..3634250 100755 (executable)
@@ -4,7 +4,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 9;
+use Test::More tests => 10;
 
 use Getopt::Long;
 use MARC::Record;
@@ -15,6 +15,7 @@ use t::lib::TestBuilder;
 
 use C4::Biblio;
 use Koha::Authorities;
+use Koha::Authority::ControlledIndicators;
 use Koha::Authority::MergeRequests;
 use Koha::Database;
 
@@ -385,6 +386,42 @@ subtest 'merge should not reorder too much' => sub {
     is_deeply( $subfields, [ 'i', 'a', 'b', 'c', '9' ], 'Order kept' );
 };
 
+subtest 'Test how merge handles controlled indicators' => sub {
+    plan tests => 4;
+
+    # Note: See more detailed tests in t/Koha/Authority/ControlledIndicators.t
+
+    # Testing MARC21 because thesaurus code makes it more interesting
+    t::lib::Mocks::mock_preference( 'marcflavour', 'MARC21' );
+    t::lib::Mocks::mock_preference('AuthorityControlledIndicators', q|marc21,*,ind1:auth1,ind2:thesaurus|);
+
+    my $authmarc = MARC::Record->new;
+    $authmarc->append_fields(
+        MARC::Field->new( '008', (' 'x11).'r' ), # thesaurus code
+        MARC::Field->new( '109', '7', '', 'a' => 'a' ),
+    );
+    my $id = AddAuthority( $authmarc, undef, $authtype1 );
+    my $biblio = MARC::Record->new;
+    $biblio->append_fields(
+        MARC::Field->new( '609', '8', '4', a => 'a', 2 => '2', 9 => $id ),
+    );
+    my ( $biblionumber ) = C4::Biblio::AddBiblio( $biblio, '' );
+
+    # Merge and check indicators and $2
+    merge({ mergefrom => $id, MARCfrom => $authmarc, mergeto => $id, MARCto => $authmarc, biblionumbers => [ $biblionumber ] });
+    my $biblio2 = C4::Biblio::GetMarcBiblio({ biblionumber => $biblionumber });
+    is( $biblio2->field('609')->indicator(1), '7', 'Indicator1 OK' );
+    is( $biblio2->field('609')->indicator(2), '7', 'Indicator2 OK' );
+    is( $biblio2->subfield('609', '2'), 'aat', 'Subfield $2 OK' );
+
+    # Test $2 removal now
+    $authmarc->field('008')->update( (' 'x11).'a' ); # LOC, no $2 needed
+    AddAuthority( $authmarc, $id, $authtype1 ); # modify
+    merge({ mergefrom => $id, MARCfrom => $authmarc, mergeto => $id, MARCto => $authmarc, biblionumbers => [ $biblionumber ] });
+    $biblio2 = C4::Biblio::GetMarcBiblio({ biblionumber => $biblionumber });
+    is( $biblio2->subfield('609', '2'), undef, 'No subfield $2 left' );
+};
+
 sub set_mocks {
     # After we removed the Zebra code from merge, we only need to mock
     # get_usage_count and linked_biblionumbers here.
index 321a0a9..8027bdc 100644 (file)
@@ -19,7 +19,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 6;
+use Test::More tests => 7;
 use MARC::Field;
 use MARC::File::XML;
 use MARC::Record;
@@ -29,7 +29,9 @@ use Test::MockObject;
 use Test::Warn;
 
 use C4::Context;
+use C4::AuthoritiesMarc;
 use Koha::Authority;
+use Koha::Authority::ControlledIndicators;
 use Koha::Authorities;
 use Koha::Authority::MergeRequest;
 use Koha::Authority::Type;
@@ -164,6 +166,37 @@ subtest 'Trivial tests for get_usage_count and linked_biblionumbers' => sub {
     t::lib::Mocks::mock_preference('SearchEngine', 'Elasticsearch');
     cmp_deeply( [ $auth1->linked_biblionumbers ], [ 2001 ],
         'linked_biblionumbers with Elasticsearch' );
+    t::lib::Mocks::mock_preference('SearchEngine', 'Zebra');
+};
+
+subtest 'Simple test for controlled_indicators' => sub {
+    plan tests => 4;
+
+    # NOTE: See more detailed tests in t/Koha/Authority/ControlledIndicators.t
+
+    # Mock pref so that authority indicators are swapped for marc21/unimarc
+    # The biblio tag is actually made irrelevant here
+    t::lib::Mocks::mock_preference('AuthorityControlledIndicators', q|marc21,*,ind1:auth2,ind2:auth1
+unimarc,*,ind1:auth2,ind2:auth1|);
+    t::lib::Mocks::mock_preference( 'marcflavour', 'MARC21' );
+
+    my $record = MARC::Record->new;
+    $record->append_fields( MARC::Field->new( '100', '1', '2', a => 'Name' ) );
+    my $type = $builder->build({ source => 'AuthType', value => { auth_tag_to_report => '100'} });
+    my $authid = C4::AuthoritiesMarc::AddAuthority( $record, undef, $type->{authtypecode} );
+    my $auth = Koha::Authorities->find( $authid );
+    is( $auth->controlled_indicators({ biblio_tag => '123' })->{ind1}, '2', 'MARC21: Swapped ind2' );
+    is( $auth->controlled_indicators({ biblio_tag => '234' })->{ind2}, '1', 'MARC21: Swapped ind1' );
+
+    # try UNIMARC too
+    t::lib::Mocks::mock_preference( 'marcflavour', 'UNIMARC' );
+    $record = MARC::Record->new;
+    $record->append_fields( MARC::Field->new( '210', '1', '2', a => 'Name' ) );
+    $type = $builder->build({ source => 'AuthType', value => { auth_tag_to_report => '210'} });
+    $authid = C4::AuthoritiesMarc::AddAuthority( $record, undef, $type->{authtypecode} );
+    $auth = Koha::Authorities->find( $authid );
+    is( $auth->controlled_indicators({ biblio_tag => '345' })->{ind1}, '2', 'UNIMARC: Swapped ind2' );
+    is( $auth->controlled_indicators({ biblio_tag => '456' })->{ind2}, '1', 'UNIMARC: Swapped ind1' );
 };
 
 sub simple_search_compat {