From 3ebaf8f52edc19aa51138b000b2bfbe006333308 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) Signed-off-by: David Nind --- 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 e84110cccc..0d6fb3eb23 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -32,7 +32,6 @@ BEGIN { GetMarcBiblio GetISBDView GetMarcControlnumber - GetMarcNotes GetMarcISBN GetMarcISSN GetMarcSubjects @@ -1565,60 +1564,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 0e17862d20..fc1affe233 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -797,6 +797,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 9cb607b1e1..8049906bac 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 0f32e2fc8c..e4e4a8341d 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 fe1c4c90e4..f6259e905f 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 3f970acdeb..4e69b3e9b5 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