Bugzilla – Attachment 73777 Details for
Bug 14769
Authorities merge: Set correct indicators in biblio field
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14769: Put ControlledIndicators to work
Bug-14769-Put-ControlledIndicators-to-work.patch (text/plain), 14.75 KB, created by
Nick Clemens (kidclamp)
on 2018-04-06 13:06:02 UTC
(
hide
)
Description:
Bug 14769: Put ControlledIndicators to work
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2018-04-06 13:06:02 UTC
Size:
14.75 KB
patch
obsolete
>From 8104661d930f5a9ddadec2ec8f1aa3777a05a2b7 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 26 Jan 2018 10:17:33 +0100 >Subject: [PATCH] Bug 14769: Put ControlledIndicators to work > >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> >--- > 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 321a0a9..8027bdc 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14769
:
42197
|
69343
|
70948
|
70949
|
70950
|
70951
|
71338
|
71339
|
71340
|
71341
|
71342
|
71343
|
71351
|
71352
|
71353
|
71354
|
71355
|
71366
|
71374
|
71375
|
71376
|
71377
|
71378
|
71379
|
71380
|
71381
|
72398
|
72399
|
72400
|
72401
|
72402
|
72403
|
72404
|
73389
|
73773
|
73774
|
73775
|
73776
| 73777 |
73778
|
73779
|
74052