From 2039c1516fcb7d15a1075e0e53f1f71ca56eb9d6 Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Thu, 2 May 2019 15:34:47 +0300 Subject: [PATCH] Bug 11529: (follow-up) Fix QA issues - Remove SplitKohaField - Avoid using Stash in templates - Improved display of part fields --- C4/Biblio.pm | 20 ------------------- catalogue/detail.pl | 1 + .../prog/en/includes/biblio-title-head.inc | 13 +++++++++++- .../prog/en/includes/biblio-title.inc | 16 ++++++++++----- .../prog/en/modules/catalogue/detail.tt | 5 ++--- .../en/includes/biblio-title-head.inc | 13 +++++++++++- .../bootstrap/en/includes/biblio-title.inc | 16 ++++++++++----- .../bootstrap/en/modules/opac-detail.tt | 7 +++---- svc/checkouts | 3 ++- svc/holds | 4 ++-- t/Biblio.t | 2 +- t/Biblio2.t | 16 --------------- 12 files changed, 57 insertions(+), 59 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 20c270347d..a9806a4cd4 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -66,7 +66,6 @@ BEGIN { TransformHtmlToMarc TransformHtmlToXml prepare_host_field - SplitKohaField ); # Internal functions @@ -3425,25 +3424,6 @@ sub RemoveAllNsb { return $record; } -=head2 SplitKohaField - - $subtitles = SplitKohaField($biblio->subtitle()); - -Splits a Koha field with multiple values to an array. Multiple matches for a -Koha field (according to the bibliographic framework) are concatenated with -' | ', but in many cases it's not optimal for display and an array is -preferred. - -=cut - -sub SplitKohaField { - my $field = shift; - - my @parts = split(/ \| /, $field // '' ); - - return \@parts; -} - 1; diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 631bb299d2..6d8a73e8fe 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -78,6 +78,7 @@ my $biblionumber = $query->param('biblionumber'); $biblionumber = HTML::Entities::encode($biblionumber); my $record = GetMarcBiblio({ biblionumber => $biblionumber }); my $biblio = Koha::Biblios->find( $biblionumber ); +$template->param( 'biblio', $biblio ); if ( not defined $record ) { # biblionumber invalid -> report and exit diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title-head.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title-head.inc index e0c420144d..d7a692aab8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title-head.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title-head.inc @@ -7,4 +7,15 @@ [% FOREACH subtitle IN biblio.subtitle.split(' \| ') %][% IF Koha.Preference('marcflavour')=='UNIMARC' %],[% END %] [% subtitle | html %] [% END %] -[% biblio.part_number | html %] [% biblio.part_name | html %] +[% part_numbers = biblio.part_number.split(' \\| ') %] +[% part_names = biblio.part_name.split(' \\| ') %] +[% i = 0 %] +[% WHILE ( part_numbers.$i.defined || part_names.$i.defined ) %] + [% IF ( part_numbers.$i.defined ) %] + [% part_numbers.$i | html %] + [% END %] + [% IF ( part_names.$i.defined ) %] + [% part_names.$i | html %] + [% END %] + [% i = i + 1 %] +[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title.inc index fd01ecead1..1f070cffcc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/biblio-title.inc @@ -9,9 +9,15 @@ [% FOREACH subtitle IN biblio.subtitle.split(' \\| ') %][% IF Koha.Preference('marcflavour')=='UNIMARC' %],[% END %] [% subtitle | html %] [% END %] -[% IF ( biblio.part_number ) %] - [% biblio.part_number | html %] -[% END %] -[% IF ( biblio.part_name ) %] - [% biblio.part_name | html %] +[% part_numbers = biblio.part_number.split(' \\| ') %] +[% part_names = biblio.part_name.split(' \\| ') %] +[% i = 0 %] +[% WHILE ( part_numbers.$i.defined || part_names.$i.defined ) %] + [% IF ( part_numbers.$i.defined ) %] + [% part_numbers.$i | html %] + [% END %] + [% IF ( part_names.$i.defined ) %] + [% part_names.$i | html %] + [% END %] + [% i = i + 1 %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index bbc998909b..348726b085 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -6,7 +6,6 @@ [% USE Branches %] [% USE Biblio %] [% USE ColumnsSettings %] -[% USE Stash %] [% SET AdlibrisEnabled = Koha.Preference('AdlibrisCoversEnabled') %] [% SET AdlibrisURL = Koha.Preference('AdlibrisCoversURL') %] @@ -35,7 +34,7 @@ [% IF ( unknownbiblionumber ) %] Unknown record [% ELSE %] - Details for [% INCLUDE 'biblio-title-head.inc' biblio=Stash.stash() %] + Details for [% INCLUDE 'biblio-title-head.inc' %] [% END %] [% INCLUDE 'doc-head-close.inc' %] @@ -50,7 +49,7 @@ [% IF ( unknownbiblionumber ) %] Unknown record [% ELSE %] - Details for [% INCLUDE 'biblio-title.inc' biblio=Stash.stash() %] + Details for [% INCLUDE 'biblio-title.inc' %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title-head.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title-head.inc index 70bb297b36..8eebda6bc6 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title-head.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title-head.inc @@ -6,4 +6,15 @@ [% FOREACH subtitle IN biblio.subtitle.split(' \| ') %][% IF Koha.Preference('marcflavour')=='UNIMARC' %],[% END %] [% subtitle | html %] [% END %] -[% biblio.part_number | html %] [% biblio.part_name | html %] +[% part_numbers = biblio.part_number.split(' \\| ') %] +[% part_names = biblio.part_name.split(' \\| ') %] +[% i = 0 %] +[% WHILE ( part_numbers.$i.defined || part_names.$i.defined ) %] + [% IF ( part_numbers.$i.defined ) %] + [% part_numbers.$i | html %] + [% END %] + [% IF ( part_names.$i.defined ) %] + [% part_names.$i | html %] + [% END %] + [% i = i + 1 %] +[% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title.inc index f85eb71c6e..3bf0b488aa 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/biblio-title.inc @@ -6,9 +6,15 @@ [% FOREACH subtitle IN biblio.subtitle.split(' \| ') %][% IF Koha.Preference('marcflavour')=='UNIMARC' %],[% END %] [% subtitle | html %] [% END %] -[% IF ( biblio.part_number ) %] - [% biblio.part_number | html %] -[% END %] -[% IF ( biblio.part_name ) %] - [% biblio.part_name | html %] +[% part_numbers = biblio.part_number.split(' \\| ') %] +[% part_names = biblio.part_name.split(' \\| ') %] +[% i = 0 %] +[% WHILE ( part_numbers.$i.defined || part_names.$i.defined ) %] + [% IF ( part_numbers.$i.defined ) %] + [% part_numbers.$i | html %] + [% END %] + [% IF ( part_names.$i.defined ) %] + [% part_names.$i | html %] + [% END %] + [% i = i + 1 %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt index 9e136be1c2..fcceea5dce 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ -6,7 +6,6 @@ [% USE Branches %] [% USE ColumnsSettings %] [% USE AuthorisedValues %] -[% USE Stash %] [% SET TagsShowEnabled = ( ( Koha.Preference( 'TagsEnabled' ) == 1 ) && TagsShowOnDetail ) %] [% SET TagsInputEnabled = ( ( Koha.Preference( 'opacuserlogin' ) == 1 ) && ( Koha.Preference( 'TagsEnabled' ) == 1 ) && TagsInputOnDetail ) %] [% IF Koha.Preference('AmazonAssocTag') %] @@ -31,7 +30,7 @@ [% END %] [% INCLUDE 'doc-head-open.inc' %] -[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog › Details for: [% INCLUDE 'biblio-title-head.inc' biblio=Stash.stash() %] +[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog › Details for: [% INCLUDE 'biblio-title-head.inc' %] [% INCLUDE 'doc-head-close.inc' %] [% Asset.css("lib/emoji-picker/css/emoji.css") | $raw %] @@ -42,7 +41,7 @@
@@ -113,7 +112,7 @@ [% IF ( OPACXSLTDetailsDisplay ) %] [% XSLTBloc | $raw %] [% ELSE %] -

[% INCLUDE 'biblio-title.inc' biblio=Stash.stash() %]

+

[% INCLUDE 'biblio-title.inc' %]

[% IF ( author ) %]
by [% author | html %]
[% END %] [% UNLESS ( item_level_itypes ) %] diff --git a/svc/checkouts b/svc/checkouts index 3b442307d7..892eab84fe 100755 --- a/svc/checkouts +++ b/svc/checkouts @@ -174,10 +174,11 @@ while ( my $c = $sth->fetchrow_hashref() ) { my $av = Koha::AuthorisedValues->search({ category => 'DAMAGED', authorised_value => $c->{damaged} }); $damaged = $av->count ? $av->next->lib : ''; } + my @subtitles = split(/ \| /, $c->{'subtitle'} // '' ); my $checkout = { DT_RowId => $c->{itemnumber} . '-' . $c->{borrowernumber}, title => $c->{title}, - subtitle => C4::Biblio::SplitKohaField($c->{'subtitle'}), + subtitle => \@subtitles, medium => $c->{medium} // '', part_number => $c->{part_number} // '', part_name => $c->{part_name} // '', diff --git a/svc/holds b/svc/holds index 2368a51e1a..c406c0e4a4 100755 --- a/svc/holds +++ b/svc/holds @@ -23,7 +23,6 @@ use CGI; use JSON qw(to_json); use C4::Auth qw(check_cookie_auth); -use C4::Biblio qw(SplitKohaField); use C4::Charset; use C4::Circulation qw(GetTransfers); use C4::Context; @@ -86,11 +85,12 @@ while ( my $h = $holds_rs->next() ) { } my $biblio = $h->biblio(); + my @subtitles = split(/ \| /, $biblio->subtitle() // ''); my $hold = { DT_RowId => $h->reserve_id(), biblionumber => $biblionumber, title => $biblio->title(), - subtitle => C4::Biblio::SplitKohaField($biblio->subtitle()), + subtitle => \@subtitles, medium => $biblio->medium() // '', part_number => $biblio->part_number() // '', part_name => $biblio->part_name() // '', diff --git a/t/Biblio.t b/t/Biblio.t index 9ce3d5de29..0550ce7826 100755 --- a/t/Biblio.t +++ b/t/Biblio.t @@ -21,7 +21,7 @@ use Test::More; use Test::MockModule; use Test::Warn; -plan tests => 45; +plan tests => 43; use_ok('C4::Biblio'); diff --git a/t/Biblio2.t b/t/Biblio2.t index bf78de9732..3cb0cb051c 100644 --- a/t/Biblio2.t +++ b/t/Biblio2.t @@ -52,20 +52,4 @@ sub _koha_marc_update_bib_ids_control { is($r->field('004')->data(), 20, 'Biblioitemnumber to control field'); } -subtest 'SplitKohaField' => sub { - plan tests => 4; - - my $res = C4::Biblio::SplitKohaField(undef); - is_deeply($res, [], 'undef returned as an array'); - - $res = C4::Biblio::SplitKohaField(''); - is_deeply($res, [], 'Empty string returned as an array'); - - $res = C4::Biblio::SplitKohaField('Single'); - is_deeply($res, ['Single'], 'Single subtitle returned as an array'); - - $res = C4::Biblio::SplitKohaField('First | Second'); - is_deeply($res, ['First', 'Second'], 'Two subtitles returned as an array'); -}; - done_testing(); -- 2.17.1