From 6aa93127366c3559ba7ceb90a2ea89938f7de909 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 --- 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 29c7d1edb47..3aed9c7ee95 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 1f9d0241dc4..861ec3cc8f9 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 b8f19814eae..ebb64f26cee 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 396470d977e..84f2b3082c0 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 4e0d27de9ec..1f0d336a737 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 4a12cdeb84d..a423f8f5c38 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 34b27c0ccd0..26d3db9f8aa 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 aa108764ad2..4ee6813fc73 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 12e92aa3c5b..bb432de1375 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 cdfe84a4ace..010a1766c62 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 548af38171b..c623e6d98fb 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 fcba56dcd77..82229f1f353 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.29.2