Bug 8995: (follow-up) Add tests, move open_url/coins routines to Koha namespace

Test plan:
1) Ensure the COinS span tag is still included on this pages. You need
to look into html source and search for span tag with class 'Z3988',
   which has COinS string in title.
   Staff client:
       catalogue -> ISBDdetail
       catalogue -> MARCdetail
       catalogue -> detail
       virtualshelves -> shelves
    OPAC (you should have COinSinOPACResults system preference enabled):
        opac detail
        opac search
        opac shelves
2) Run tests:
prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested with all 9 current patches. Works as advertised, including
OPACURLOpenInNewWindow. If a record has no items, no OpenURL link
is displayed. All the suggested tests pass. I did not test with
XSLT turned off.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Josef Moravec 2019-03-11 21:27:52 +00:00 committed by Nick Clemens
parent 29c687bff7
commit d99d32d033
13 changed files with 254 additions and 218 deletions

View File

@ -43,7 +43,6 @@ BEGIN {
GetMarcUrls
GetUsedMarcStructure
GetXmlBiblio
GetCOinSBiblio
GetMarcPrice
MungeMarcPrice
GetMarcQuantity
@ -88,8 +87,6 @@ use MARC::File::USMARC;
use MARC::File::XML;
use POSIX qw(strftime);
use Module::Load::Conditional qw(can_load);
use URI;
use URI::Escape; # GetCOinSBiblio
use C4::Koha;
use C4::Log; # logaction
@ -1232,194 +1229,6 @@ sub GetXmlBiblio {
return $marcxml;
}
=head2 GetCOinSBiblio
my $coins = GetCOinSBiblio($record);
Returns the COinS (a span) which can be included in a biblio record
=cut
sub GetCOinSBiblio {
my $record = shift;
# get the coin format
if ( ! $record ) {
carp 'GetCOinSBiblio called with undefined record';
return;
}
my $pos7 = substr $record->leader(), 7, 1;
my $pos6 = substr $record->leader(), 6, 1;
my $mtx;
my $genre;
my ( $aulast, $aufirst ) = ( '', '' );
my @authors;
my $title;
my $hosttitle;
my $pubyear = '';
my $isbn = '';
my $issn = '';
my $publisher = '';
my $pages = '';
my $titletype = '';
# For the purposes of generating COinS metadata, LDR/06-07 can be
# considered the same for UNIMARC and MARC21
my $fmts6 = {
'a' => 'book',
'b' => 'manuscript',
'c' => 'book',
'd' => 'manuscript',
'e' => 'map',
'f' => 'map',
'g' => 'film',
'i' => 'audioRecording',
'j' => 'audioRecording',
'k' => 'artwork',
'l' => 'document',
'm' => 'computerProgram',
'o' => 'document',
'r' => 'document',
};
my $fmts7 = {
'a' => 'journalArticle',
's' => 'journal',
};
$genre = $fmts6->{$pos6} ? $fmts6->{$pos6} : 'book';
if ( $genre eq 'book' ) {
$genre = $fmts7->{$pos7} if $fmts7->{$pos7};
}
##### We must transform mtx to a valable mtx and document type ####
if ( $genre eq 'book' ) {
$mtx = 'book';
$titletype = 'b';
} elsif ( $genre eq 'journal' ) {
$mtx = 'journal';
$titletype = 'j';
} elsif ( $genre eq 'journalArticle' ) {
$mtx = 'journal';
$genre = 'article';
$titletype = 'a';
} else {
$mtx = 'dc';
}
if ( C4::Context->preference("marcflavour") eq "UNIMARC" ) {
# Setting datas
$aulast = $record->subfield( '700', 'a' ) || '';
$aufirst = $record->subfield( '700', 'b' ) || '';
push @authors, "$aufirst $aulast" if ($aufirst or $aulast);
# others authors
if ( $record->field('200') ) {
for my $au ( $record->field('200')->subfield('g') ) {
push @authors, $au;
}
}
$title = $record->subfield( '200', 'a' );
my $subfield_210d = $record->subfield('210', 'd');
if ($subfield_210d and $subfield_210d =~ /(\d{4})/) {
$pubyear = $1;
}
$publisher = $record->subfield( '210', 'c' ) || '';
$isbn = $record->subfield( '010', 'a' ) || '';
$issn = $record->subfield( '011', 'a' ) || '';
} else {
# MARC21 need some improve
# Setting datas
if ( $record->field('100') ) {
push @authors, $record->subfield( '100', 'a' );
}
# others authors
if ( $record->field('700') ) {
for my $au ( $record->field('700')->subfield('a') ) {
push @authors, $au;
}
}
$title = $record->subfield( '245', 'a' ) . $record->subfield( '245', 'b' );
if ($titletype eq 'a') {
$pubyear = $record->field('008') || '';
$pubyear = substr($pubyear->data(), 7, 4) if $pubyear;
$isbn = $record->subfield( '773', 'z' ) || '';
$issn = $record->subfield( '773', 'x' ) || '';
$hosttitle = $record->subfield( '773', 't' ) || $record->subfield( '773', 'a') || q{};
my @rels = $record->subfield( '773', 'g' );
$pages = join(', ', @rels);
} else {
$pubyear = $record->subfield( '260', 'c' ) || '';
$publisher = $record->subfield( '260', 'b' ) || '';
$isbn = $record->subfield( '020', 'a' ) || '';
$issn = $record->subfield( '022', 'a' ) || '';
}
}
my @params = (
[ 'ctx_ver', 'Z39.88-2004' ],
[ 'rft_val_fmt', "info:ofi/fmt:kev:mtx:$mtx" ],
[ ($mtx eq 'dc' ? 'rft.type' : 'rft.genre'), $genre ],
[ "rft.${titletype}title", $title ],
);
# rft.title is authorized only once, so by checking $titletype
# we ensure that rft.title is not already in the list.
if ($hosttitle and $titletype) {
push @params, [ 'rft.title', $hosttitle ];
}
push @params, (
[ 'rft.isbn', $isbn ],
[ 'rft.issn', $issn ],
);
# If it's a subscription, these informations have no meaning.
if ($genre ne 'journal') {
push @params, (
[ 'rft.aulast', $aulast ],
[ 'rft.aufirst', $aufirst ],
(map { [ 'rft.au', $_ ] } @authors),
[ 'rft.pub', $publisher ],
[ 'rft.date', $pubyear ],
[ 'rft.pages', $pages ],
);
}
my $coins_value = join( '&amp;',
map { $$_[1] ? $$_[0] . '=' . uri_escape_utf8( $$_[1] ) : () } @params );
return $coins_value;
}
sub GetOpenURLResolverURL {
my ($record) = @_;
my $coins = GetCOinSBiblio($record);
my $OpenURLResolverURL = C4::Context->preference('OpenURLResolverURL');
if ($OpenURLResolverURL) {
my $uri = URI->new($OpenURLResolverURL);
if (not defined $uri->query) {
$OpenURLResolverURL .= '?';
} else {
$OpenURLResolverURL .= '&amp;';
}
$OpenURLResolverURL .= $coins;
}
return $OpenURLResolverURL;
}
=head2 GetMarcPrice
return the prices in accordance with the Marc format.

View File

@ -247,11 +247,18 @@ sub XSLTParse4Display {
$variables ||= {};
if (C4::Context->preference('OPACShowOpenURL')) {
my ($biblio) = GetBiblioItemByBiblioNumber($biblionumber);
my @biblio_itemtypes;
my $biblio = Koha::Biblios->find($biblionumber);
if (C4::Context->preference('item-level_itypes')) {
@biblio_itemtypes = $biblio->items->get_column("itype");
} else {
push @biblio_itemtypes, $biblio->biblioitem->itemtype;
}
my @itypes = split( /\s/, C4::Context->preference('OPACOpenURLItemTypes') );
if (grep /^$biblio->{itemtype}$/, @itypes) {
$variables->{OpenURLResolverURL} =
C4::Biblio::GetOpenURLResolverURL($orig_record);
my %original = ();
map { $original{$_} = 1 } @biblio_itemtypes;
if ( grep { $original{$_} } @itypes ) {
$variables->{OpenURLResolverURL} = $biblio->get_openurl;
}
}
my $varxml = "<variables>\n";

View File

@ -21,6 +21,8 @@ use Modern::Perl;
use Carp;
use List::MoreUtils qw(any);
use URI;
use URI::Escape;
use C4::Biblio qw();
@ -464,6 +466,196 @@ sub has_items_waiting_or_intransit {
return 0;
}
=head2 get_coins
my $coins = $biblio->get_coins;
Returns the COinS (a span) which can be included in a biblio record
=cut
sub get_coins {
my ( $self ) = @_;
my $record = $self->metadata->record;
my $pos7 = substr $record->leader(), 7, 1;
my $pos6 = substr $record->leader(), 6, 1;
my $mtx;
my $genre;
my ( $aulast, $aufirst ) = ( '', '' );
my @authors;
my $title;
my $hosttitle;
my $pubyear = '';
my $isbn = '';
my $issn = '';
my $publisher = '';
my $pages = '';
my $titletype = '';
# For the purposes of generating COinS metadata, LDR/06-07 can be
# considered the same for UNIMARC and MARC21
my $fmts6 = {
'a' => 'book',
'b' => 'manuscript',
'c' => 'book',
'd' => 'manuscript',
'e' => 'map',
'f' => 'map',
'g' => 'film',
'i' => 'audioRecording',
'j' => 'audioRecording',
'k' => 'artwork',
'l' => 'document',
'm' => 'computerProgram',
'o' => 'document',
'r' => 'document',
};
my $fmts7 = {
'a' => 'journalArticle',
's' => 'journal',
};
$genre = $fmts6->{$pos6} ? $fmts6->{$pos6} : 'book';
if ( $genre eq 'book' ) {
$genre = $fmts7->{$pos7} if $fmts7->{$pos7};
}
##### We must transform mtx to a valable mtx and document type ####
if ( $genre eq 'book' ) {
$mtx = 'book';
$titletype = 'b';
} elsif ( $genre eq 'journal' ) {
$mtx = 'journal';
$titletype = 'j';
} elsif ( $genre eq 'journalArticle' ) {
$mtx = 'journal';
$genre = 'article';
$titletype = 'a';
} else {
$mtx = 'dc';
}
if ( C4::Context->preference("marcflavour") eq "UNIMARC" ) {
# Setting datas
$aulast = $record->subfield( '700', 'a' ) || '';
$aufirst = $record->subfield( '700', 'b' ) || '';
push @authors, "$aufirst $aulast" if ($aufirst or $aulast);
# others authors
if ( $record->field('200') ) {
for my $au ( $record->field('200')->subfield('g') ) {
push @authors, $au;
}
}
$title = $record->subfield( '200', 'a' );
my $subfield_210d = $record->subfield('210', 'd');
if ($subfield_210d and $subfield_210d =~ /(\d{4})/) {
$pubyear = $1;
}
$publisher = $record->subfield( '210', 'c' ) || '';
$isbn = $record->subfield( '010', 'a' ) || '';
$issn = $record->subfield( '011', 'a' ) || '';
} else {
# MARC21 need some improve
# Setting datas
if ( $record->field('100') ) {
push @authors, $record->subfield( '100', 'a' );
}
# others authors
if ( $record->field('700') ) {
for my $au ( $record->field('700')->subfield('a') ) {
push @authors, $au;
}
}
$title = $record->subfield( '245', 'a' ) . $record->subfield( '245', 'b' );
if ($titletype eq 'a') {
$pubyear = $record->field('008') || '';
$pubyear = substr($pubyear->data(), 7, 4) if $pubyear;
$isbn = $record->subfield( '773', 'z' ) || '';
$issn = $record->subfield( '773', 'x' ) || '';
$hosttitle = $record->subfield( '773', 't' ) || $record->subfield( '773', 'a') || q{};
my @rels = $record->subfield( '773', 'g' );
$pages = join(', ', @rels);
} else {
$pubyear = $record->subfield( '260', 'c' ) || '';
$publisher = $record->subfield( '260', 'b' ) || '';
$isbn = $record->subfield( '020', 'a' ) || '';
$issn = $record->subfield( '022', 'a' ) || '';
}
}
my @params = (
[ 'ctx_ver', 'Z39.88-2004' ],
[ 'rft_val_fmt', "info:ofi/fmt:kev:mtx:$mtx" ],
[ ($mtx eq 'dc' ? 'rft.type' : 'rft.genre'), $genre ],
[ "rft.${titletype}title", $title ],
);
# rft.title is authorized only once, so by checking $titletype
# we ensure that rft.title is not already in the list.
if ($hosttitle and $titletype) {
push @params, [ 'rft.title', $hosttitle ];
}
push @params, (
[ 'rft.isbn', $isbn ],
[ 'rft.issn', $issn ],
);
# If it's a subscription, these informations have no meaning.
if ($genre ne 'journal') {
push @params, (
[ 'rft.aulast', $aulast ],
[ 'rft.aufirst', $aufirst ],
(map { [ 'rft.au', $_ ] } @authors),
[ 'rft.pub', $publisher ],
[ 'rft.date', $pubyear ],
[ 'rft.pages', $pages ],
);
}
my $coins_value = join( '&amp;',
map { $$_[1] ? $$_[0] . '=' . uri_escape_utf8( $$_[1] ) : () } @params );
return $coins_value;
}
=head2 get_openurl
my $url = $biblio->get_openurl;
Returns url for OpenURL resolver set in OpenURLResolverURL system preference
=cut
sub get_openurl {
my ( $self ) = @_;
my $OpenURLResolverURL = C4::Context->preference('OpenURLResolverURL');
if ($OpenURLResolverURL) {
my $uri = URI->new($OpenURLResolverURL);
if (not defined $uri->query) {
$OpenURLResolverURL .= '?';
} else {
$OpenURLResolverURL .= '&amp;';
}
$OpenURLResolverURL .= $self->get_coins;
}
return $OpenURLResolverURL;
}
=head3 type
=cut

View File

@ -137,7 +137,7 @@ $template->param (
biblionumber => $biblionumber,
isbdview => 1,
z3950_search_params => C4::Search::z3950_search_args(GetBiblioData($biblionumber)),
ocoins => GetCOinSBiblio($record),
ocoins => $biblio->get_coins,
C4::Search::enabled_staff_search_views,
searchid => scalar $query->param('searchid'),
);

View File

@ -99,8 +99,6 @@ if ( not defined $record ) {
exit;
}
$template->param( ocoins => GetCOinSBiblio($record) );
my $biblio_object = Koha::Biblios->find( $biblionumber ); # FIXME Should replace $biblio
my $tagslib = &GetMarcStructure(1,$frameworkcode);
my $biblio = GetBiblioData($biblionumber);
@ -115,6 +113,8 @@ if($query->cookie("holdfor")){
);
}
$template->param( ocoins => $biblio_object->get_coins );
#count of item linked
my $itemcount = $biblio_object->items->count;
$template->param( count => $itemcount,

View File

@ -77,6 +77,7 @@ if ( C4::Context->preference('UseKohaPlugins') &&
my $biblionumber = $query->param('biblionumber');
$biblionumber = HTML::Entities::encode($biblionumber);
my $record = GetMarcBiblio({ biblionumber => $biblionumber });
my $biblio = Koha::Biblios->find( $biblionumber );
if ( not defined $record ) {
# biblionumber invalid -> report and exit
@ -117,7 +118,7 @@ if ( $xslfile ) {
}
$template->param( 'SpineLabelShowPrintOnBibDetails' => C4::Context->preference("SpineLabelShowPrintOnBibDetails") );
$template->param( ocoins => GetCOinSBiblio($record) );
$template->param( ocoins => $biblio->get_coins );
# some useful variables for enhanced content;
# in each case, we're grabbing the first value we find in
@ -489,7 +490,6 @@ if (C4::Context->preference('TagsEnabled') and $tag_quantity = C4::Context->pref
}
#we only need to pass the number of holds to the template
my $biblio = Koha::Biblios->find( $biblionumber );
my $holds = $biblio->holds;
$template->param( holdcount => $holds->count );

View File

@ -847,9 +847,7 @@ $template->param(
);
# COinS format FIXME: for books Only
$template->param(
ocoins => GetCOinSBiblio($record),
);
$template->param( ocoins => $biblio->get_coins );
my ( $loggedincommenter, $reviews );
if ( C4::Context->preference('reviewson') ) {

View File

@ -682,8 +682,8 @@ for (my $i=0;$i<@servers;$i++) {
}
if (C4::Context->preference('COinSinOPACResults')) {
my $record = GetMarcBiblio({ biblionumber => $res->{'biblionumber'} });
$res->{coins} = GetCOinSBiblio($record);
my $biblio = Koha::Biblios->find( $res->{'biblionumber'} );
$res->{coins} = $biblio->get_coins;
}
if ( C4::Context->preference( "Babeltheque" ) and $res->{normalized_isbn} ) {
if( my $isbn = Business::ISBN->new( $res->{normalized_isbn} ) ) {

View File

@ -294,7 +294,7 @@ if ( $op eq 'view' ) {
$this_item->{description} = $itemtype->description; #FIXME Should not it be translated_description?
$this_item->{notforloan} = $itemtype->notforloan;
}
$this_item->{'coins'} = GetCOinSBiblio($record);
$this_item->{'coins'} = $biblio->get_coins;
$this_item->{'subtitle'} = GetRecordValue( 'subtitle', $record, GetFrameworkCode( $biblionumber ) );
$this_item->{'normalized_upc'} = GetNormalizedUPC( $record, $marcflavour );
$this_item->{'normalized_ean'} = GetNormalizedEAN( $record, $marcflavour );

View File

@ -21,7 +21,7 @@ use Test::More;
use Test::MockModule;
use Test::Warn;
plan tests => 47;
plan tests => 45;
use_ok('C4::Biblio');
@ -63,12 +63,6 @@ warning_is { @arr = LinkBibHeadingsToAuthorities(q{}, q{}) }
is($arr[0], 0, 'LinkBibHeadingsToAuthorities correct error return');
warning_is { $ret = GetCOinSBiblio() }
{ carped => 'GetCOinSBiblio called with undefined record'},
"GetCOinSBiblio returns carped warning on undef record";
ok( !defined $ret, 'GetCOinSBiblio returns undef if not passed rec');
warning_is { $ret = GetMarcPrice(undef, 'MARC21') }
{ carped => 'GetMarcPrice called on undefined record'},
"GetMarcPrice returns carped warning on undef record";

View File

@ -17,7 +17,7 @@
use Modern::Perl;
use Test::More tests => 12;
use Test::More tests => 11;
use Test::MockModule;
use List::MoreUtils qw( uniq );
use MARC::Record;

View File

@ -17,13 +17,14 @@
use Modern::Perl;
use Test::More tests => 5;
use t::lib::TestBuilder;
use Test::More tests => 6;
use C4::Biblio;
use Koha::Database;
use t::lib::TestBuilder;
use t::lib::Mocks;
BEGIN {
use_ok('Koha::Biblio');
use_ok('Koha::Biblios');
@ -107,3 +108,38 @@ subtest 'items() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'get_coins and get_openurl' => sub {
plan tests => 3;
$schema->storage->txn_begin;
my $builder = t::lib::TestBuilder->new;
my $biblio = $builder->build_sample_biblio({
title => 'Title 1',
author => 'Author 1'
});
is(
$biblio->get_coins,
'ctx_ver=Z39.88-2004&amp;rft_val_fmt=info%3Aofi%2Ffmt%3Akev%3Amtx%3Abook&amp;rft.genre=book&amp;rft.btitle=Title%201&amp;rft.au=Author%201',
'GetCOinsBiblio returned right metadata'
);
t::lib::Mocks::mock_preference("OpenURLResolverURL", "https://koha.example.com/");
is(
$biblio->get_openurl,
'https://koha.example.com/?ctx_ver=Z39.88-2004&amp;rft_val_fmt=info%3Aofi%2Ffmt%3Akev%3Amtx%3Abook&amp;rft.genre=book&amp;rft.btitle=Title%201&amp;rft.au=Author%201',
'Koha::Biblio->get_openurl returned right URL'
);
t::lib::Mocks::mock_preference("OpenURLResolverURL", "https://koha.example.com/?client_id=ci1");
is(
$biblio->get_openurl,
'https://koha.example.com/?client_id=ci1&amp;ctx_ver=Z39.88-2004&amp;rft_val_fmt=info%3Aofi%2Ffmt%3Akev%3Amtx%3Abook&amp;rft.genre=book&amp;rft.btitle=Title%201&amp;rft.au=Author%201',
'Koha::Biblio->get_openurl returned right URL'
);
$schema->storage->txn_rollback;
};

View File

@ -279,7 +279,7 @@ if ( $op eq 'view' ) {
$this_item->{imageurl} = $itemtype ? C4::Koha::getitemtypeimagelocation( 'intranet', $itemtype->imageurl ) : q{};
$this_item->{description} = $itemtype ? $itemtype->description : q{}; #FIXME Should this be translated_description ?
$this_item->{notforloan} = $itemtype->notforloan if $itemtype;
$this_item->{'coins'} = GetCOinSBiblio($record);
$this_item->{'coins'} = $biblio->get_coins;
$this_item->{'subtitle'} = GetRecordValue( 'subtitle', $record, GetFrameworkCode( $biblionumber ) );
$this_item->{'normalized_upc'} = GetNormalizedUPC( $record, $marcflavour );
$this_item->{'normalized_ean'} = GetNormalizedEAN( $record, $marcflavour );