Bugzilla – Attachment 91692 Details for
Bug 11529
Add subtitle, medium and part fields to biblio table
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11529: (follow-up) Fix QA issues
Bug-11529-follow-up-Fix-QA-issues.patch (text/plain), 12.52 KB, created by
Ere Maijala
on 2019-07-22 12:58:36 UTC
(
hide
)
Description:
Bug 11529: (follow-up) Fix QA issues
Filename:
MIME Type:
Creator:
Ere Maijala
Created:
2019-07-22 12:58:36 UTC
Size:
12.52 KB
patch
obsolete
>From 09c9e6ea64c2c2e795f61fd407fa4c1502506e53 Mon Sep 17 00:00:00 2001 >From: Ere Maijala <ere.maijala@helsinki.fi> >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 0e009ee914..55a2c01890 100644 >--- 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 b041c829ae..b4d9bae646 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 %] > <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 c81b8685d2..3819628274 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 %] > </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 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 %] > <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 b95dc8be7b..8afe1b76a5 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' %] >-<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog › Details for: [% INCLUDE 'biblio-title-head.inc' biblio=Stash.stash() %]</title> >+<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog › 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">›</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 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11529
:
81930
|
81931
|
82024
|
82025
|
82026
|
82027
|
82028
|
82029
|
82088
|
82089
|
82090
|
82428
|
82429
|
82430
|
82431
|
82538
|
82539
|
82540
|
82541
|
82542
|
83103
|
83104
|
83105
|
83106
|
83107
|
83108
|
83961
|
83962
|
83963
|
83964
|
83965
|
83966
|
85587
|
85588
|
85589
|
85590
|
85591
|
85592
|
86677
|
86678
|
86679
|
86680
|
86681
|
86682
|
87248
|
87249
|
87250
|
87251
|
87252
|
87253
|
87256
|
87257
|
87258
|
87259
|
87260
|
87261
|
87911
|
87912
|
87913
|
87914
|
87915
|
87916
|
87917
|
87918
|
87919
|
88247
|
88248
|
88249
|
88250
|
88251
|
88252
|
88253
|
88254
|
88255
|
88256
|
88406
|
88407
|
88408
|
88409
|
88410
|
88411
|
88412
|
88413
|
88414
|
88415
|
88416
|
88433
|
88434
|
88435
|
88436
|
88437
|
88438
|
88439
|
88440
|
88441
|
88442
|
88443
|
88762
|
88763
|
88764
|
88765
|
88766
|
88767
|
88768
|
88769
|
88770
|
88771
|
88772
|
88851
|
88852
|
88853
|
88854
|
88855
|
88856
|
88857
|
88858
|
88859
|
88860
|
88861
|
88862
|
89096
|
89097
|
89098
|
89099
|
89100
|
89101
|
89102
|
89103
|
89104
|
89105
|
89106
|
89107
|
89234
|
89235
|
89236
|
89237
|
89238
|
89239
|
89240
|
89241
|
89242
|
89243
|
89244
|
89245
|
89246
|
91680
|
91681
|
91682
|
91683
|
91684
|
91685
|
91686
|
91687
|
91688
|
91689
|
91690
|
91691
|
91692
|
91824
|
91825
|
91826
|
91827
|
91828
|
91829
|
91830
|
91831
|
91832
|
91833
|
91834
|
91835
|
91836
|
91837
|
92016
|
92019
|
92020
|
105667
|
105672