Attachment 26473 Details for Bug 11912 – Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API

[patch] Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API

Bug-11912-refactoring-followup-GetMarcISBN-should-.patch (text/plain), 7.46 KB, created by Jonathan Druart on 2014-03-20 10:16:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2014-03-20 10:16:54 UTC

Size: 7.46 KB

patch

obsolete

>From dbf495ace819fc33bf83403b54350f6f2bfa8cbc Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@gmail.com>
>Date: Mon, 10 Mar 2014 11:52:19 -0300
>Subject: [PATCH] Bug 11912: (refactoring followup) GetMarcISBN should
> implement its advertised API
>
>The current implementation of GetMarcISBN contradicts the documented API.
>It currently returns an array of hashes with only one key (marcisbn) which
>doesn't add any value to it.
>
>I chose to fix GetMarcISBN to honour the API instead of changing the docs,
>because it seems a really silly change.
>
>To test:
>- Run:
> prove t/db_dependent/Biblio.t
>=> SUCCESS
>- catalogue/detail.pl should correctly show ISBNs.
>- opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap.
>- opac-opac-sendshelf.pl should correctly show ISBNs in the email.
>- Sign off :-D
>
>Edit: forgot to git add some templates and fixing the unit tests.
>
>Regards
>To+
>
>Sponsored-by: Universidad Nacional de Cordoba
>Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
>---
> C4/Biblio.pm | 5 ++---
> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt | 2 +-
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt | 2 +-
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt | 2 +-
> koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt | 2 +-
> koha-tmpl/opac-tmpl/prog/en/modules/opac-sendshelf.tt | 2 +-
> t/db_dependent/Biblio.t | 5 ++---
> 7 files changed, 9 insertions(+), 11 deletions(-)
>
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index a34e5b6..61e4c3b 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -1689,13 +1689,12 @@ sub GetMarcISBN {
> } else { # assume marc21 if not unimarc
> $scope = '020';
> }
>+
> my @marcisbns;
>- my $marcisbn;
> foreach my $field ( $record->field($scope) ) {
> my $isbn = $field->as_string();
> if ( $isbn ne "" ) {
>- $marcisbn = { marcisbn => $isbn, };
>- push @marcisbns, $marcisbn;
>+ push @marcisbns, $isbn;
> }
> }
> 
>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 1e1fcd2..03e322d 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>@@ -416,7 +416,7 @@ function verify_images() {
> <div class="yui-u" style="margin-top: 1em;">
> <ul>
> [% IF ( MARCISBNS ) %]
>- <li>ISBN:<ul>[% FOREACH MARCISBN IN MARCISBNS %]<li>[% MARCISBN.marcisbn %]</li>[% END %]</ul></li>
>+ <li>ISBN:<ul>[% FOREACH MARCISBN IN MARCISBNS %]<li>[% MARCISBN %]</li>[% END %]</ul></li>
> [% ELSE %]
> [% IF ( normalized_isbn ) %]
> <li>ISBN: [% normalized_isbn %]</li>
>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 6afb240..4abf17e 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>@@ -197,7 +197,7 @@
> [% END %]
> 
> [% IF ( MARCISBNS ) %]
>- ISBN:[% FOREACH MARCISBN IN MARCISBNS %][% MARCISBN.marcisbn %][% IF ( loop.last ) %].[% ELSE %]; [% END %][% END %]
>+ ISBN:[% FOREACH MARCISBN IN MARCISBNS %][% MARCISBN %][% IF ( loop.last ) %].[% ELSE %]; [% END %][% END %]
> [% ELSE %]
> [% IF ( normalized_isbn ) %]
> ISBN: [% normalized_isbn %]
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt
>index c13a659..f3c7ed7 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelf.tt
>@@ -59,7 +59,7 @@ Your list : [% shelfname %]
> [% IF ( BIBLIO_RESULT.ISBN && BIBLIO_RESULT.size > 0 ) %]
> 
> ISBN: [% FOREACH isbn IN BIBLIO_RESULT.ISBN %]
>- [% isbn.marcisbn %]
>+ [% isbn %]
> [% UNLESS ( loop.last ) %]; [% END %]
> [% END %]
> 
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
>index fce873c..7ffbbd0 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
>@@ -651,7 +651,7 @@ YAHOO.util.Event.onContentReady("furtherm", function () {
> [% IF ( pages ) %]Physical details: [% pages %] [% illus %] [% size %][% END %]
> 
> [% IF ( MARCISBNS ) %]
>- ISBN:[% FOREACH MARCISBN IN MARCISBNS %][% MARCISBN.marcisbn %][% IF ( loop.last ) %].[% ELSE %]; [% END %][% END %]
>+ ISBN:[% FOREACH MARCISBN IN MARCISBNS %][% MARCISBN %][% IF ( loop.last ) %].[% ELSE %]; [% END %][% END %]
> [% ELSE %]
> [% IF ( normalized_isbn ) %]
> ISBN: [% normalized_isbn %]
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-sendshelf.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-sendshelf.tt
>index c13a659..f3c7ed7 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-sendshelf.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-sendshelf.tt
>@@ -59,7 +59,7 @@ Your list : [% shelfname %]
> [% IF ( BIBLIO_RESULT.ISBN && BIBLIO_RESULT.size > 0 ) %]
> 
> ISBN: [% FOREACH isbn IN BIBLIO_RESULT.ISBN %]
>- [% isbn.marcisbn %]
>+ [% isbn %]
> [% UNLESS ( loop.last ) %]; [% END %]
> [% END %]
> 
>diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t
>index 2dd8034..b861023 100755
>--- a/t/db_dependent/Biblio.t
>+++ b/t/db_dependent/Biblio.t
>@@ -19,7 +19,6 @@ use Modern::Perl;
> 
> use Test::More tests => 3;
> use Test::MockModule;
>-use Data::Dumper;
> 
> use MARC::Record;
> 
>@@ -166,7 +165,7 @@ sub run_tests {
> $record_for_isbn->append_fields( $isbn_field );
> $isbns = GetMarcISBN( $record_for_isbn, $marcflavour );
> is( scalar @$isbns, 1, '(GetMarcISBN) The record contains one ISBN');
>- is( $isbns->[0]->{ marcisbn }, $isbn, '(GetMarcISBN) The record contains our ISBN');
>+ is( $isbns->[0], $isbn, '(GetMarcISBN) The record contains our ISBN');
> 
> # We add 3 more ISBNs
> $record_for_isbn = MARC::Record->new();
>@@ -178,7 +177,7 @@ sub run_tests {
> $isbns = GetMarcISBN( $record_for_isbn, $marcflavour );
> is( scalar @$isbns, 4, '(GetMarcISBN) The record contains 4 ISBNs');
> for my $i (0 .. $#more_isbns) {
>- is( $isbns->[$i]->{ marcisbn }, $more_isbns[$i],
>+ is( $isbns->[$i], $more_isbns[$i],
> "(GetMarcISBN) Corretly retrieves ISBN #". ($i + 1));
> }
> 
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 11912: 25968 | 25969 | 26018 | 26019 | 26277 | 26286 | 26287 | 26288 | 26471 | 26472 | 26473 | 26474 | 26475