Bugzilla – Attachment 127688 Details for
Bug 28709
Fetching biblio objects for custom covers is inefficient
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28709: Refactor Koha::Biblio->custom_cover_image_url to allow passing parameters
Bug-28709-Refactor-KohaBiblio-customcoverimageurl-.patch (text/plain), 10.70 KB, created by
Nick Clemens (kidclamp)
on 2021-11-16 12:10:45 UTC
(
hide
)
Description:
Bug 28709: Refactor Koha::Biblio->custom_cover_image_url to allow passing parameters
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2021-11-16 12:10:45 UTC
Size:
10.70 KB
patch
obsolete
>From 8242852320046120de1867d0a0fcab2dea646aef Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 13 Jul 2021 18:18:34 +0000 >Subject: [PATCH] Bug 28709: Refactor Koha::Biblio->custom_cover_image_url to > allow passing parameters > >Currently the search result template receive a Koha::Biblio object and call the method. > >The searchResults routine already has the isbn, issn, and normalized_isbn calculated. >Rather than fetching the object and passing to the template we can pass the parameters, >generate the url, and pass only the URL to the template > >To test: >1 - Set the system preferences: > CustomCoverImages : Display > OPACCustomCoverImages: Display > CustomCoverImagesURL: https://images-na.ssl-images-amazon.com/images/P/{normalized_isbn}.01.LZZZZZZZ.jpg >2 - Search on staff and opac for a term that returns titles with covers, e.g. 'shuffle' >3 - Confirm the covers show >4 - Apply patch >5 - Restart all >6 - Confirm covers still show >7 - prove -v t/db_dependent/Koha/Biblios.t >--- > C4/Search.pm | 19 +++++++++ > Koha/Biblio.pm | 12 +++--- > .../prog/en/modules/catalogue/results.tt | 2 +- > .../bootstrap/en/modules/opac-opensearch.tt | 2 +- > .../bootstrap/en/modules/opac-results.tt | 2 +- > t/db_dependent/Koha/Biblios.t | 41 ++++++++++++++++--- > 6 files changed, 65 insertions(+), 13 deletions(-) > >diff --git a/C4/Search.pm b/C4/Search.pm >index 0bf8c6d209..29aae21088 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -2045,6 +2045,25 @@ sub searchResults { > $oldbiblio->{'alternateholdings_count'} = $alternateholdingscount; > } > >+ if ( >+ ( >+ C4::Context->preference('CustomCoverImages') >+ || C4::Context->preference('OPACCustomCoverIMages') >+ ) >+ && C4::Context->preference('CustomCoverImagesURL') >+ ) >+ { >+ $oldbiblio->{custom_cover_image_url} = >+ Koha::Biblio->custom_cover_image_url( >+ { >+ isbn => $oldbiblio->{isbn} // "", >+ normalized_isbn => $oldbiblio->{normalized_isbn} // "", >+ issn => $oldbiblio->{issn} // "", >+ record => $marcrecord >+ } >+ ); >+ } >+ > push( @newresults, $oldbiblio ); > } > >diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm >index c4c12128af..75409d1ca2 100644 >--- a/Koha/Biblio.pm >+++ b/Koha/Biblio.pm >@@ -859,20 +859,22 @@ It is built regaring the value of the system preference CustomCoverImagesURL > =cut > > sub custom_cover_image_url { >- my ( $self ) = @_; >+ my ( $self, $params ) = @_; >+ return unless (ref $self || $params); >+ > my $url = C4::Context->preference('CustomCoverImagesURL'); > if ( $url =~ m|{isbn}| ) { >- my $isbn = $self->biblioitem->isbn; >+ my $isbn = $params->{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); >+ my $normalized_isbn = $params->{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; >+ my $issn = $params->{issn} // $self->biblioitem->issn; > return unless $issn; > $url =~ s|{issn}|$issn|g; > } >@@ -881,7 +883,7 @@ sub custom_cover_image_url { > if ( $url =~ $re ) { > my $field = $+{field}; > my $subfield = $+{subfield}; >- my $marc_record = $self->metadata->record; >+ my $marc_record = $params->{record} // $self->metadata->record; > my $value; > if ( $subfield ) { > $value = $marc_record->subfield( $field, $subfield ); >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 59def93d62..91b3b451b0 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt >@@ -467,7 +467,7 @@ > [% END %] > > [% IF Koha.Preference('CustomCoverImages') && Koha.Preference('CustomCoverImagesURL') %] >- [% SET custom_cover_image_url = SEARCH_RESULT.biblio_object.custom_cover_image_url %] >+ [% SET custom_cover_image_url = SEARCH_RESULT.custom_cover_image_url %] > [% IF custom_cover_image_url %] > <div id="custom-coverimg-[% SEARCH_RESULT.biblionumber | html %]" class="cover-image custom-coverimg"> > <a class="custom_cover_image" href="[% custom_cover_image_url | url %]"> >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 de69346dc4..29317a5b74 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt >@@ -61,7 +61,7 @@ > [% IF ( BakerTaylorEnabled ) %][% IF bt_id %]<a href="https://[% BakerTaylorBookstoreURL |url %][% bt_id | uri %]"><img alt="See Baker & Taylor" src="[% BakerTaylorImageURL |url %][% bt_id | uri %]" /></a>[% END %][% END %] > > [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] >- [% SET custom_cover_image_url = SEARCH_RESULT.biblio_object.custom_cover_image_url %] >+ [% SET custom_cover_image_url = SEARCH_RESULT.custom_cover_image_url %] > [% IF custom_cover_image_url %] > <img alt="Cover image" src="[% custom_cover_image_url | url %]" /> > [% 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 28b4bdef6c..df5c7b7280 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt >@@ -423,7 +423,7 @@ > [% END %] > > [% IF Koha.Preference('OPACCustomCoverImages') AND Koha.Preference('CustomCoverImagesURL') %] >- [% SET custom_cover_image_url = SEARCH_RESULT.biblio_object.custom_cover_image_url %] >+ [% SET custom_cover_image_url = SEARCH_RESULT.custom_cover_image_url %] > [% IF custom_cover_image_url %] > [% IF ( OPACURLOpenInNewWindow ) %] > <a class="custom_cover_image" href="[% custom_cover_image_url | url %]" target="_blank" rel="noreferrer"><img alt="Cover image" src="[% custom_cover_image_url | url %]" /></a> >diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t >index b93efebb21..f120bc9504 100755 >--- a/t/db_dependent/Koha/Biblios.t >+++ b/t/db_dependent/Koha/Biblios.t >@@ -193,34 +193,65 @@ subtest 'can_be_transferred' => sub { > }; > > subtest 'custom_cover_image_url' => sub { >- plan tests => 6; >+ plan tests => 10; > > t::lib::Mocks::mock_preference( 'CustomCoverImagesURL', 'https://my_url/{isbn}_{issn}.png' ); > > my $isbn = '0553573403 | 9780553573404 (pbk.).png'; >+ my $normalized_isbn = C4::Koha::GetNormalizedISBN($isbn); > my $issn = 'my_issn'; > my $cf_value = 'from_control_field'; > my $marc_record = MARC::Record->new; > my ( $biblionumber, undef ) = C4::Biblio::AddBiblio($marc_record, ''); > >+ my $custom_cover_image_url; >+ > my $biblio = Koha::Biblios->find( $biblionumber ); > my $biblioitem = $biblio->biblioitem->set( > { isbn => $isbn, issn => $issn }); >- is( $biblio->custom_cover_image_url, "https://my_url/${isbn}_${issn}.png" ); >+ is( $biblio->custom_cover_image_url, "https://my_url/${isbn}_${issn}.png", "URL is correctly generated for isbn and issbn when called as object" ); >+ $custom_cover_image_url = Koha::Biblio->custom_cover_image_url({ >+ isbn => $isbn, >+ issn => $issn, >+ record => $marc_record, >+ normalized_isbn => $normalized_isbn >+ }); >+ is( $custom_cover_image_url, "https://my_url/${isbn}_${issn}.png", "URL is correctly generated for isbn and issn when values passed as parameters" ); >+ > > my $marc_024a = '710347104926'; > $marc_record->append_fields( MARC::Field->new( '024', '', '', a => $marc_024a ) ); > C4::Biblio::ModBiblio( $marc_record, $biblio->biblionumber ); > > t::lib::Mocks::mock_preference( 'CustomCoverImagesURL', 'https://my_url/{024$a}.png' ); >- is( $biblio->custom_cover_image_url, "https://my_url/$marc_024a.png" ); >+ is( $biblio->custom_cover_image_url, "https://my_url/$marc_024a.png", "URL is correctly generated using a custom field when called as object method" ); >+ $custom_cover_image_url = Koha::Biblio->custom_cover_image_url({ >+ isbn => $isbn, >+ issn => $issn, >+ record => $marc_record, >+ normalized_isbn => $normalized_isbn >+ }); >+ is( $custom_cover_image_url, "https://my_url/$marc_024a.png", "URL is correctly generated using a custom fieldwhen parameters passed" ); > > 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" ); >+ is( $biblio->custom_cover_image_url, "https://my_url/$normalized_isbn.png", "URL correctly generated using normalized ISBN when called as object method" ); >+ $custom_cover_image_url = Koha::Biblio->custom_cover_image_url({ >+ isbn => $isbn, >+ issn => $issn, >+ record => $marc_record, >+ normalized_isbn => $normalized_isbn >+ }); >+ is( $custom_cover_image_url, "https://my_url/$normalized_isbn.png", "URL correctly generated using normalized ISBN when passed parameters" ); > > $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" ); >+ $custom_cover_image_url = Koha::Biblio->custom_cover_image_url({ >+ isbn => $isbn, >+ issn => $issn, >+ record => $marc_record, >+ normalized_isbn => '' >+ }); >+ is( $custom_cover_image_url, undef, "Don't generate the url if the value needed to generate it is not passed" ); > > t::lib::Mocks::mock_preference( 'CustomCoverImagesURL', 'https://my_url/{001}.png' ); > is( $biblio->custom_cover_image_url, undef, 'Record does not have 001' ); >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=127688">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28709
:
122832
|
122833
|
122835
|
122962
|
127657
| 127688