From a59f156fae54977daae8cbde003d4d4a3a3be2e9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 5 Nov 2020 16:02:14 +0100 Subject: [PATCH] Bug 26921: Don't generate an invalid custom cover image if the url cannot be generated In case a custom cover image url is used to generate the cover image of bibliographic records, we should not build one if the record does not have the necessary data. For instance if you have CustomCoverImagesURL set to https://covers.openlibrary.org/b/isbn/{isbn}-M.jpb and a biblio does not have the isbn defined, we should not generate and empty image (empty or invalid src) Test plan: 0. Set CustomCoverImagesURL to https://covers.openlibrary.org/b/isbn/{isbn}-M.jpb Enable CustomCoverImages and OPACCustomCoverImages To highlight the issue you should disable LocalCoverImages and OPACLocalCoverImages. 1. Make sure you have some of your bibliographic records with a valid isbn 2. Make sure you have at least 1 bibliographic record without an isbn set 3. Visit the search result and detail views (OPAC and staff interfaces) => Without this patch you should see a "Cover image" link, and an empty block/div on the detail page => With this patch applied you should only see images when the url can be generated Note that the problem will persist if the isbn is not valid (ie. no image is generated) Sponsored-by: Orex Digital --- Koha/Biblio.pm | 4 ++++ .../prog/en/modules/catalogue/detail.tt | 15 +++++++++------ .../prog/en/modules/catalogue/results.tt | 5 ++++- .../bootstrap/en/includes/shelfbrowser.inc | 9 ++++++--- .../bootstrap/en/modules/opac-detail.tt | 11 +++++++---- .../bootstrap/en/modules/opac-opensearch.tt | 5 ++++- .../bootstrap/en/modules/opac-results.tt | 11 +++++++---- .../bootstrap/en/modules/opac-shelves.tt | 17 ++++++++++------- .../en/modules/opac-showreviews-rss.tt | 5 ++++- .../bootstrap/en/modules/opac-showreviews.tt | 5 ++++- .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 5 ++++- t/db_dependent/Koha/Biblios.t | 6 +++++- 12 files changed, 68 insertions(+), 30 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 29c7d1edb4..3aed9c7ee9 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -757,14 +757,17 @@ sub custom_cover_image_url { my $url = C4::Context->preference('CustomCoverImagesURL'); if ( $url =~ m|{isbn}| ) { my $isbn = $self->biblioitem->isbn; + return unless $isbn; $url =~ s|{isbn}|$isbn|g; } if ( $url =~ m|{normalized_isbn}| ) { my $normalized_isbn = C4::Koha::GetNormalizedISBN($self->biblioitem->isbn); + return unless $normalized_isbn; $url =~ s|{normalized_isbn}|$normalized_isbn|g; } if ( $url =~ m|{issn}| ) { my $issn = $self->biblioitem->issn; + return unless $issn; $url =~ s|{issn}|$issn|g; } @@ -774,6 +777,7 @@ sub custom_cover_image_url { my $subfield = $+{subfield}; my $marc_record = $self->metadata->record; my $value = $marc_record->subfield($field, $subfield); + return unless $value; $url =~ s|$re|$value|; } 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 1f9d0241dc..861ec3cc8f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -213,12 +213,15 @@ [% END %] [% IF Koha.Preference('CustomCoverImages') && Koha.Preference('CustomCoverImagesURL') %] -
- - Custom cover image - -
Custom cover image
-
+ [% SET custom_cover_image_url = biblio.custom_cover_image_url %] + [% IF custom_cover_image_url %] +
+ + Custom cover image + +
Custom cover image
+
+ [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt index b8f19814ea..ebb64f26ce 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt @@ -453,7 +453,10 @@ [% END %] [% IF Koha.Preference('CustomCoverImages') && Koha.Preference('CustomCoverImagesURL') %] - Cover image + [% SET custom_cover_image_url = SEARCH_RESULT.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + Cover image + [% END %] [% END %] [% END # /IF( AmazonCoverImages || LocalCoverImages || AdlibrisEnabled || IntranetCoce )%] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/shelfbrowser.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/shelfbrowser.inc index 396470d977..84f2b3082c 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/shelfbrowser.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/shelfbrowser.inc @@ -71,9 +71,12 @@ [% END %] [% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - - Cover image - + [% SET custom_cover_image_url = item.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + + Cover image + + [% END %] [% 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 a88a7caf0b..d3f7f88359 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ -113,10 +113,13 @@ [% END %] [% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - [% IF ( OPACURLOpenInNewWindow ) %] - Cover image - [% ELSE %] - Cover image + [% SET custom_cover_image_url = biblio.custom_cover_image_url %] + [% IF custom_cover_image_url %] + [% IF ( OPACURLOpenInNewWindow ) %] + Cover image + [% ELSE %] + Cover image + [% END %] [% END %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt index 4a12cdeb84..a423f8f5c3 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt @@ -66,7 +66,10 @@ [% IF ( BakerTaylorEnabled ) %][% IF bt_id %]See Baker & Taylor[% END %][% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - Cover image + [% SET custom_cover_image_url = SEARCH_RESULT.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + Cover image + [% END %] [% END %]

[% IF ( SEARCH_RESULT.author ) %]By [% SEARCH_RESULT.author | html %]. [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt index 1fd9010ef9..6dd514e9e3 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt @@ -371,10 +371,13 @@ [% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - [% IF ( OPACURLOpenInNewWindow ) %] - Cover image - [% ELSE %] - Cover image + [% SET custom_cover_image_url = SEARCH_RESULT.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + [% IF ( OPACURLOpenInNewWindow ) %] + Cover image + [% ELSE %] + Cover image + [% END %] [% END %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt index aa108764ad..4ee6813fc7 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt @@ -453,16 +453,19 @@ [% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - [% IF ( itemsloo.BiblioDefaultViewmarc ) %] - - [% ELSE %] - [% IF ( itemsloo.BiblioDefaultViewisbd ) %] - + [% SET custom_cover_image_url = itemsloo.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + [% IF ( itemsloo.BiblioDefaultViewmarc ) %] + [% ELSE %] - + [% IF ( itemsloo.BiblioDefaultViewisbd ) %] + + [% ELSE %] + + [% END %] [% END %] + Cover image [% END %] - Cover image [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews-rss.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews-rss.tt index 12e92aa3c5..bb432de137 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews-rss.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews-rss.tt @@ -25,7 +25,10 @@ [% IF ( BakerTaylorEnabled && bt_id ) %]See Baker & Taylor[% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - Cover image + [% SET custom_cover_image_url = review.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + Cover image + [% END %] [% END %] [% IF ( review.author ) %]

By [% review.author | html %].

[% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews.tt index cdfe84a4ac..010a1766c6 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews.tt @@ -158,7 +158,10 @@ [% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - Cover image + [% SET custom_cover_image_url = review.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + Cover image + [% END %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt index 548af38171..c623e6d98f 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -292,7 +292,10 @@ [% END %] [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] - Cover image + [% SET custom_cover_image_url = ISSUE.biblio_object.custom_cover_image_url %] + [% IF custom_cover_image_url %] + Cover image + [% END %] [% END %] [% IF ( SyndeticsEnabled && SyndeticsCoverImages ) %] diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index fcba56dcd7..82229f1f35 100755 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -189,7 +189,7 @@ subtest 'can_be_transferred' => sub { }; subtest 'custom_cover_image_url' => sub { - plan tests => 3; + plan tests => 4; t::lib::Mocks::mock_preference( 'CustomCoverImagesURL', 'https://my_url/{isbn}_{issn}.png' ); @@ -213,6 +213,10 @@ subtest 'custom_cover_image_url' => sub { t::lib::Mocks::mock_preference( 'CustomCoverImagesURL', 'https://my_url/{normalized_isbn}.png' ); my $normalized_isbn = C4::Koha::GetNormalizedISBN($isbn); is( $biblio->custom_cover_image_url, "https://my_url/$normalized_isbn.png" ); + + $biblio->biblioitem->isbn('')->store; + is( $biblio->custom_cover_image_url, undef, "Don't generate the url if the biblio does not have the value needed to generate it" ); + }; $schema->storage->txn_rollback; -- 2.20.1