From 6f64a3366a5fdb9c91c5479aa546c402cfabd22d 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 Signed-off-by: Tomas Cohen Arazi Signed-off-by: Josef Moravec Signed-off-by: Jonathan Druart (cherry picked from commit 1ea55acd0c997b96039a2a1a4a9f9941d2558bda) Signed-off-by: Fridolin Somers --- 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 4d335e2c23..0e17862d20 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -756,14 +756,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; } @@ -773,6 +776,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 fc6745156f..55f13b75d8 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 4e0d27de9e..1f0d336a73 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 34b27c0ccd..26d3db9f8a 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 c25db59397..bb2cc72410 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -294,7 +294,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.39.5