Bug 11529: (follow-up) Fix QA issues

- Remove SplitKohaField
- Avoid using Stash in templates
- Improved display of part fields

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

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index 0e009ee..55a2c01 100644 (file)
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -66,7 +66,6 @@ BEGIN {
         TransformHtmlToMarc
         TransformHtmlToXml
         prepare_host_field
-        SplitKohaField
     );
 
     # Internal functions
@@ -3420,25 +3419,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 b041c82..b4d9bae 100755 (executable)
--- 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 e0c4201..d7a692a 100644 (file)
--- 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 fd01ece..1f070cf 100644 (file)
--- 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 %]
     <span class="subtitle">[% subtitle | html %]</span>
 [% END %]
-[% IF ( biblio.part_number ) %]
-    <span class="part-number">[% biblio.part_number | html %]</span>
-[% END %]
-[% IF ( biblio.part_name ) %]
-    <span class="part-name">[% biblio.part_name | html %]</span>
+[% 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 ) %]
+        <span class="part-number">[% part_numbers.$i | html %]</span>
+    [% END %]
+    [% IF ( part_names.$i.defined ) %]
+        <span class="part-name">[% part_names.$i | html %]</span>
+    [% 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 c81b868..3819628 100644 (file)
--- 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 %]
 </title>
 [% INCLUDE 'doc-head-close.inc' %]
@@ -50,7 +49,7 @@
   [% IF ( unknownbiblionumber ) %]
     Unknown record
   [% ELSE %]
-    Details for <i>[% INCLUDE 'biblio-title.inc' biblio=Stash.stash() %]</i>
+    Details for <i>[% INCLUDE 'biblio-title.inc' %]</i>
   [% END %]
 </div>
 

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 70bb297..8eebda6 100644 (file)
--- 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 f85eb71..3bf0b48 100644 (file)
--- 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 %]
     <span class="subtitle">[% subtitle | html %]</span>
 [% END %]
-[% IF ( biblio.part_number ) %]
-    <span class="part-number">[% biblio.part_number | html %]</span>
-[% END %]
-[% IF ( biblio.part_name ) %]
-    <span class="part-name">[% biblio.part_name | html %]</span>
+[% 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 ) %]
+        <span class="part-number">[% part_numbers.$i | html %]</span>
+    [% END %]
+    [% IF ( part_names.$i.defined ) %]
+        <span class="part-name">[% part_names.$i | html %]</span>
+    [% 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 b95dc8b..8afe1b7 100644 (file)
--- 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' %]
-<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog &rsaquo; Details for: [% INCLUDE 'biblio-title-head.inc' biblio=Stash.stash() %]</title>
+<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog &rsaquo; Details for: [% INCLUDE 'biblio-title-head.inc' %]</title>
 [% INCLUDE 'doc-head-close.inc' %]
     [% Asset.css("lib/emoji-picker/css/emoji.css") | $raw %]
 </head>
@@ -42,7 +41,7 @@
 <div class="main">
     <ul class="breadcrumb">
         <li><a href="/cgi-bin/koha/opac-main.pl">Home</a> <span class="divider">&rsaquo;</span></li>
-        <li><a href="#"><span>Details for: </span>[% INCLUDE 'biblio-title.inc' biblio=Stash.stash() %]</a></li>
+        <li><a href="#"><span>Details for: </span>[% INCLUDE 'biblio-title.inc' %]</a></li>
     </ul>
 
     <div class="container-fluid">
@@ -113,7 +112,7 @@
                     [% IF ( OPACXSLTDetailsDisplay ) %]
                         [% XSLTBloc | $raw %]
                     [% ELSE %]
-                        <h1 class="title">[% INCLUDE 'biblio-title.inc' biblio=Stash.stash() %]</h1>
+                        <h1 class="title">[% INCLUDE 'biblio-title.inc' %]</h1>
                         [% IF ( author ) %]<h5 class="author">by <a href="/cgi-bin/koha/opac-search.pl?q=au:[% author |url %]">[% author | html %]</a></h5>[% END %]
 
                         <span class="results_summary">[% UNLESS ( item_level_itypes ) %]

diff --git a/svc/checkouts b/svc/checkouts
index 3b44230..892eab8 100755 (executable)
--- 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 2368a51..c406c0e 100755 (executable)
--- 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 9ce3d5d..0550ce7 100755 (executable)
--- 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 bf78de9..3cb0cb0 100644 (file)
--- 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();