From 87690b285593cf9c6a93360e5179dc04c79a81ac Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Fri, 18 Dec 2020 14:55:48 +1300 Subject: [PATCH] Bug 27268: Move GetMarcNotes to Koha namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch moves C4::Biblio::GetMarcNotes to Koha::Biblio->get_marc_notes. This is so get_marc_notes can be used in templates and notices. To test: 1. Find a record that has a note (3xx for UNIMARC, 5xx for MARC21). Confirm the notes still show as normal under the Descriptions tab. 2. Add the record to the cart. 3. View your cart and click 'more details'. Confirm notes show as normal. 4. Log in to the OPAC. Find the record and add it to the cart 5. View the cart and click 'more details'. Confirm notes show as normal. 6. View the record detail page and confirm the notes show as normal under the Title Notes tab. 7. Confirm tests pass: - t/Biblio.t - t/db_dependent/Koha/Biblio.t Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ) --- C4/Biblio.pm | 55 ---------------------------------- Koha/Biblio.pm | 52 ++++++++++++++++++++++++++++++++ basket/basket.pl | 3 +- catalogue/detail.pl | 2 +- opac/opac-basket.pl | 3 +- opac/opac-detail.pl | 2 +- t/Biblio.t | 8 +---- t/Biblio/GetMarcNotes.t | 70 -------------------------------------------- t/db_dependent/Biblio.t | 15 ++-------- t/db_dependent/Koha/Biblio.t | 61 +++++++++++++++++++++++++++++++++++++- 10 files changed, 122 insertions(+), 149 deletions(-) delete mode 100755 t/Biblio/GetMarcNotes.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 6e53842ee1..41b3029303 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -32,7 +32,6 @@ BEGIN { GetMarcBiblio GetISBDView GetMarcControlnumber - GetMarcNotes GetMarcISBN GetMarcISSN GetMarcSubjects @@ -1551,60 +1550,6 @@ sub GetMarcISSN { return \@marcissns; } # end GetMarcISSN -=head2 GetMarcNotes - - $marcnotesarray = GetMarcNotes( $record, $marcflavour ); - - Get all notes from the MARC record and returns them in an array. - The notes are stored in different fields depending on MARC flavour. - MARC21 5XX $u subfields receive special attention as they are URIs. - -=cut - -sub GetMarcNotes { - my ( $record, $marcflavour, $opac ) = @_; - if (!$record) { - carp 'GetMarcNotes called on undefined record'; - return; - } - - my $scope = $marcflavour eq "UNIMARC"? '3..': '5..'; - my @marcnotes; - - #MARC21 specs indicate some notes should be private if first indicator 0 - my %maybe_private = ( - 541 => 1, - 542 => 1, - 561 => 1, - 583 => 1, - 590 => 1 - ); - - my %hiddenlist = map { $_ => 1 } - split( /,/, C4::Context->preference('NotesToHide')); - foreach my $field ( $record->field($scope) ) { - my $tag = $field->tag(); - next if $hiddenlist{ $tag }; - next if $opac && $maybe_private{$tag} && !$field->indicator(1); - if( $marcflavour ne 'UNIMARC' && $field->subfield('u') ) { - # Field 5XX$u always contains URI - # Examples: 505u, 506u, 510u, 514u, 520u, 530u, 538u, 540u, 542u, 552u, 555u, 561u, 563u, 583u - # We first push the other subfields, then all $u's separately - # Leave further actions to the template (see e.g. opac-detail) - my $othersub = - join '', ( 'a' .. 't', 'v' .. 'z', '0' .. '9' ); # excl 'u' - push @marcnotes, { marcnote => $field->as_string($othersub) }; - foreach my $sub ( $field->subfield('u') ) { - $sub =~ s/^\s+|\s+$//g; # trim - push @marcnotes, { marcnote => $sub }; - } - } else { - push @marcnotes, { marcnote => $field->as_string() }; - } - } - return \@marcnotes; -} - =head2 GetMarcSubjects $marcsubjcts = GetMarcSubjects($record,$marcflavour); diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 4d335e2c23..ad79b515fd 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -793,6 +793,58 @@ sub cover_images { return Koha::CoverImages->_new_from_dbic($cover_images_rs); } +=head3 get_marc_notes + + $marcnotesarray = $biblio->get_marc_notes({ marcflavour => $marcflavour }); + +Get all notes from the MARC record and returns them in an array. +The notes are stored in different fields depending on MARC flavour. +MARC21 5XX $u subfields receive special attention as they are URIs. + +=cut + +sub get_marc_notes { + my ( $self, $params ) = @_; + + my $marcflavour = $params->{marcflavour}; + my $opac = $params->{opac}; + + my $scope = $marcflavour eq "UNIMARC"? '3..': '5..'; + my @marcnotes; + + #MARC21 specs indicate some notes should be private if first indicator 0 + my %maybe_private = ( + 541 => 1, + 542 => 1, + 561 => 1, + 583 => 1, + 590 => 1 + ); + + my %hiddenlist = map { $_ => 1 } + split( /,/, C4::Context->preference('NotesToHide')); + foreach my $field ( $self->metadata->record->field($scope) ) { + my $tag = $field->tag(); + next if $hiddenlist{ $tag }; + next if $opac && $maybe_private{$tag} && !$field->indicator(1); + if( $marcflavour ne 'UNIMARC' && $field->subfield('u') ) { + # Field 5XX$u always contains URI + # Examples: 505u, 506u, 510u, 514u, 520u, 530u, 538u, 540u, 542u, 552u, 555u, 561u, 563u, 583u + # We first push the other subfields, then all $u's separately + # Leave further actions to the template (see e.g. opac-detail) + my $othersub = + join '', ( 'a' .. 't', 'v' .. 'z', '0' .. '9' ); # excl 'u' + push @marcnotes, { marcnote => $field->as_string($othersub) }; + foreach my $sub ( $field->subfield('u') ) { + $sub =~ s/^\s+|\s+$//g; # trim + push @marcnotes, { marcnote => $sub }; + } + } else { + push @marcnotes, { marcnote => $field->as_string() }; + } + } + return \@marcnotes; +} =head3 to_api diff --git a/basket/basket.pl b/basket/basket.pl index aa4cd97dcf..dd8bd136d6 100755 --- a/basket/basket.pl +++ b/basket/basket.pl @@ -61,8 +61,9 @@ foreach my $biblionumber ( @bibs ) { my $dat = &GetBiblioData($biblionumber); next unless $dat; + my $biblio = Koha::Biblios->find( $biblionumber ); my $record = &GetMarcBiblio({ biblionumber => $biblionumber }); - my $marcnotesarray = GetMarcNotes( $record, $marcflavour ); + my $marcnotesarray = $biblio->get_marc_notes({ marcflavour => $marcflavour }); my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); my $marcsubjctsarray = GetMarcSubjects( $record, $marcflavour ); my $marcseriesarray = GetMarcSeries ($record,$marcflavour); diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 7b7f4d2944..85f34aa488 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -195,7 +195,7 @@ $template->param( normalized_isbn => $isbn, ); -my $marcnotesarray = GetMarcNotes( $record, $marcflavour ); +my $marcnotesarray = $biblio->get_marc_notes({ marcflavour => $marcflavour }); my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search->unblessed } }; diff --git a/opac/opac-basket.pl b/opac/opac-basket.pl index 14c44c4bee..d375759f8b 100755 --- a/opac/opac-basket.pl +++ b/opac/opac-basket.pl @@ -70,6 +70,7 @@ foreach my $biblionumber ( @bibs ) { my $dat = &GetBiblioData($biblionumber); next unless $dat; + my $biblio = Koha::Biblios->find( $biblionumber ); # No filtering on the item records needed for the record itself # since the only reason item information is grabbed is because of branchcodes. @@ -81,7 +82,7 @@ foreach my $biblionumber ( @bibs ) { }); $record_processor->process($record); next unless $record; - my $marcnotesarray = GetMarcNotes( $record, $marcflavour, 1 ); + my $marcnotesarray = $biblio->get_marc_notes({ marcflavour => $marcflavour, opac => 1 }); my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); my $marcsubjctsarray = GetMarcSubjects( $record, $marcflavour ); my $marcseriesarray = GetMarcSeries ($record,$marcflavour); diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 8b935af155..1c7371270e 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -816,7 +816,7 @@ if (!C4::Context->preference("OPACXSLTDetailsDisplay") ) { ); } -my $marcnotesarray = GetMarcNotes ($record,$marcflavour,1); +my $marcnotesarray = $biblio->get_marc_notes({ marcflavour => $marcflavour, opac => 1 }); if( C4::Context->preference('ArticleRequests') ) { my $patron = $borrowernumber ? Koha::Patrons->find($borrowernumber) : undef; diff --git a/t/Biblio.t b/t/Biblio.t index 5596b4bf19..f50cfede97 100755 --- a/t/Biblio.t +++ b/t/Biblio.t @@ -21,7 +21,7 @@ use Test::More; use Test::MockModule; use Test::Warn; -plan tests => 41; +plan tests => 39; use_ok('C4::Biblio'); @@ -87,12 +87,6 @@ warning_is { $ret = GetMarcISSN() } ok( !defined $ret, 'GetMarcISSN returns undef if not passed rec'); -warning_is { $ret = GetMarcNotes() } - { carped => 'GetMarcNotes called on undefined record'}, - "GetMarcNotes returns carped warning on undef record"; - -ok( !defined $ret, 'GetMarcNotes returns undef if not passed rec'); - warning_is { $ret = GetMarcSubjects() } { carped => 'GetMarcSubjects called on undefined record'}, "GetMarcSubjects returns carped warning on undef record"; diff --git a/t/Biblio/GetMarcNotes.t b/t/Biblio/GetMarcNotes.t deleted file mode 100755 index ccd368d346..0000000000 --- a/t/Biblio/GetMarcNotes.t +++ /dev/null @@ -1,70 +0,0 @@ -#!/usr/bin/perl - -# This file is part of Koha. -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -use Modern::Perl; - -use Test::More tests => 2; -use t::lib::Mocks; - -use MARC::Field; -use MARC::Record; - -use C4::Biblio; - -subtest 'GetMarcNotes MARC21' => sub { - plan tests => 11; - t::lib::Mocks::mock_preference( 'NotesToHide', '520' ); - - my $record = MARC::Record->new; - $record->append_fields( - MARC::Field->new( '500', '', '', a => 'Note1' ), - MARC::Field->new( '505', '', '', a => 'Note2', u => 'http://someserver.com' ), - MARC::Field->new( '520', '', '', a => 'Note3 skipped' ), - MARC::Field->new( '541', '0', '', a => 'Note4 skipped on opac' ), - MARC::Field->new( '541', '', '', a => 'Note5' ), - ); - my $notes = C4::Biblio::GetMarcNotes( $record, 'MARC21' ); - is( $notes->[0]->{marcnote}, 'Note1', 'First note' ); - is( $notes->[1]->{marcnote}, 'Note2', 'Second note' ); - is( $notes->[2]->{marcnote}, 'http://someserver.com', 'URL separated' ); - is( $notes->[3]->{marcnote}, 'Note4 skipped on opac',"Not shows if not opac" ); - is( $notes->[4]->{marcnote}, 'Note5', 'Fifth note' ); - is( @$notes, 5, 'No more notes' ); - $notes = C4::Biblio::GetMarcNotes( $record, 'MARC21', 1 ); - is( $notes->[0]->{marcnote}, 'Note1', 'First note' ); - is( $notes->[1]->{marcnote}, 'Note2', 'Second note' ); - is( $notes->[2]->{marcnote}, 'http://someserver.com', 'URL separated' ); - is( $notes->[3]->{marcnote}, 'Note5', 'Fifth note shows after fourth skipped' ); - is( @$notes, 4, 'No more notes' ); - -}; - -subtest 'GetMarcNotes UNIMARC' => sub { - plan tests => 3; - t::lib::Mocks::mock_preference( 'NotesToHide', '310' ); - - my $record = MARC::Record->new; - $record->append_fields( - MARC::Field->new( '300', '', '', a => 'Note1' ), - MARC::Field->new( '300', '', '', a => 'Note2' ), - MARC::Field->new( '310', '', '', a => 'Note3 skipped' ), - ); - my $notes = C4::Biblio::GetMarcNotes( $record, 'UNIMARC' ); - is( $notes->[0]->{marcnote}, 'Note1', 'First note' ); - is( $notes->[1]->{marcnote}, 'Note2', 'Second note' ); - is( @$notes, 2, 'No more notes' ); -}; diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index ac6a6a1255..79bfb27439 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -416,15 +416,6 @@ sub run_tests { } is ($newincbiblioitemnumber, $biblioitemnumbertotest, 'Check newincbiblioitemnumber'); - # test for GetMarcNotes - my $a1= GetMarcNotes( $marc_record, $marcflavour ); - my $field2 = MARC::Field->new( $marcflavour eq 'UNIMARC'? 300: 555, 0, '', a=> 'Some text', u=> 'http://url-1.com', u=> 'nohttp://something_else' ); - $marc_record->append_fields( $field2 ); - my $a2= GetMarcNotes( $marc_record, $marcflavour ); - is( ( $marcflavour eq 'UNIMARC' && @$a2 == @$a1 + 1 ) || - ( $marcflavour ne 'UNIMARC' && @$a2 == @$a1 + 3 ), 1, - 'Check the number of returned notes of GetMarcNotes' ); - # test for GetMarcUrls $marc_record->append_fields( MARC::Field->new( '856', '', '', u => ' https://koha-community.org ' ), @@ -586,14 +577,14 @@ sub create_author_field { } subtest 'MARC21' => sub { - plan tests => 48; + plan tests => 47; run_tests('MARC21'); $schema->storage->txn_rollback; $schema->storage->txn_begin; }; subtest 'UNIMARC' => sub { - plan tests => 48; + plan tests => 47; # Mock the auth type data for UNIMARC $dbh->do("UPDATE auth_types SET auth_tag_to_report = '106' WHERE auth_tag_to_report = '100'") or die $dbh->errstr; @@ -604,7 +595,7 @@ subtest 'UNIMARC' => sub { }; subtest 'NORMARC' => sub { - plan tests => 48; + plan tests => 47; run_tests('NORMARC'); $schema->storage->txn_rollback; $schema->storage->txn_begin; diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index 718b69ebf5..c96f5c2d41 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 12; +use Test::More tests => 14; use C4::Biblio; use Koha::Database; @@ -551,3 +551,62 @@ subtest 'subscriptions() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'get_marc_notes() MARC21 tests' => sub { + plan tests => 11; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'NotesToHide', '520' ); + + my $biblio = $builder->build_sample_biblio; + my $record = $biblio->metadata->record; + $record->append_fields( + MARC::Field->new( '500', '', '', a => 'Note1' ), + MARC::Field->new( '505', '', '', a => 'Note2', u => 'http://someserver.com' ), + MARC::Field->new( '520', '', '', a => 'Note3 skipped' ), + MARC::Field->new( '541', '0', '', a => 'Note4 skipped on opac' ), + MARC::Field->new( '541', '', '', a => 'Note5' ), + ); + C4::Biblio::ModBiblio( $record, $biblio->biblionumber ); + $biblio = Koha::Biblios->find( $biblio->biblionumber); + my $notes = $biblio->get_marc_notes({ marcflavour => 'MARC21' }); + is( $notes->[0]->{marcnote}, 'Note1', 'First note' ); + is( $notes->[1]->{marcnote}, 'Note2', 'Second note' ); + is( $notes->[2]->{marcnote}, 'http://someserver.com', 'URL separated' ); + is( $notes->[3]->{marcnote}, 'Note4 skipped on opac',"Not shows if not opac" ); + is( $notes->[4]->{marcnote}, 'Note5', 'Fifth note' ); + is( @$notes, 5, 'No more notes' ); + $notes = $biblio->get_marc_notes({ marcflavour => 'MARC21', opac => 1 }); + is( $notes->[0]->{marcnote}, 'Note1', 'First note' ); + is( $notes->[1]->{marcnote}, 'Note2', 'Second note' ); + is( $notes->[2]->{marcnote}, 'http://someserver.com', 'URL separated' ); + is( $notes->[3]->{marcnote}, 'Note5', 'Fifth note shows after fourth skipped' ); + is( @$notes, 4, 'No more notes' ); + + $schema->storage->txn_rollback; +}; + +subtest 'get_marc_notes() UNIMARC tests' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'NotesToHide', '310' ); + + my $biblio = $builder->build_sample_biblio; + my $record = $biblio->metadata->record; + $record->append_fields( + MARC::Field->new( '300', '', '', a => 'Note1' ), + MARC::Field->new( '300', '', '', a => 'Note2' ), + MARC::Field->new( '310', '', '', a => 'Note3 skipped' ), + ); + C4::Biblio::ModBiblio( $record, $biblio->biblionumber ); + $biblio = Koha::Biblios->find( $biblio->biblionumber); + my $notes = $biblio->get_marc_notes({ marcflavour => 'UNIMARC' }); + is( $notes->[0]->{marcnote}, 'Note1', 'First note' ); + is( $notes->[1]->{marcnote}, 'Note2', 'Second note' ); + is( @$notes, 2, 'No more notes' ); + + $schema->storage->txn_rollback; +}; -- 2.11.0