From 21dc2228ca6cb18b74050f39f06f3004511c566d Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 26 Jan 2018 10:17:33 +0100 Subject: [PATCH] Bug 14769: Put ControlledIndicators to work Content-Type: text/plain; charset=utf-8 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 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. --- C4/AuthoritiesMarc.pm | 14 ++++++-- Koha/Authority.pm | 51 ++++++++++++++++++++++++++ authorities/blinddetail-biblio-search.pl | 61 ++++++++------------------------ t/db_dependent/Authority/Merge.t | 39 +++++++++++++++++++- t/db_dependent/Koha/Authorities.t | 35 +++++++++++++++++- 5 files changed, 149 insertions(+), 51 deletions(-) diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index 165ef36..f441c60 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -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 ); diff --git a/Koha/Authority.pm b/Koha/Authority.pm index ac2b25c..b427d7f 100644 --- a/Koha/Authority.pm +++ b/Koha/Authority.pm @@ -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 diff --git a/authorities/blinddetail-biblio-search.pl b/authorities/blinddetail-biblio-search.pl index 9e7930d..0146a66 100755 --- a/authorities/blinddetail-biblio-search.pl +++ b/authorities/blinddetail-biblio-search.pl @@ -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; - diff --git a/t/db_dependent/Authority/Merge.t b/t/db_dependent/Authority/Merge.t index d987b83..3634250 100755 --- a/t/db_dependent/Authority/Merge.t +++ b/t/db_dependent/Authority/Merge.t @@ -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. diff --git a/t/db_dependent/Koha/Authorities.t b/t/db_dependent/Koha/Authorities.t index 9fa6c73..27c97b1 100644 --- a/t/db_dependent/Koha/Authorities.t +++ b/t/db_dependent/Koha/Authorities.t @@ -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 { -- 2.1.4