From 9d2d125ec7fc8f5fb235f116e6d689ca3a017508 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 24 Oct 2023 15:12:44 +0200 Subject: [PATCH] Bug 37628: (follow-up) Remove get_opac_news_by_id Keep the same behaviour, but wondering why we don't return 'content' when there is no match. ie. + is( $additional_contents, undef ); should certainly be + is( $additional_contents->{content}->count, 0 ); WNC amended patch - tidied Signed-off-by: Nick Clemens --- Koha/AdditionalContents.pm | 15 +++--- Koha/Template/Plugin/AdditionalContents.pm | 7 +-- .../bootstrap/en/modules/opac-main.tt | 2 +- .../Template/Plugin/AdditionalContents.t | 52 ++++++++++++++----- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/Koha/AdditionalContents.pm b/Koha/AdditionalContents.pm index bbad486..215e624 100644 --- a/Koha/AdditionalContents.pm +++ b/Koha/AdditionalContents.pm @@ -81,13 +81,14 @@ sub search_for_display { qq|(SELECT COUNT(*) FROM additional_contents_localizations WHERE lang='$lang' AND additional_content_id=me.additional_content_id)=0|; my $search_params; - $search_params->{location} = $params->{location}; - $search_params->{branchcode} = $params->{library_id} ? [ $params->{library_id}, undef ] : undef; - $search_params->{published_on} = { '<=' => \'CAST(NOW() AS DATE)' }; - $search_params->{expirationdate} = [ '-or', { '>=' => \'CAST(NOW() AS DATE)' }, undef ]; - $search_params->{category} = $params->{category} if $params->{category}; - $search_params->{lang} = 'default' if !$lang || $lang eq 'default'; - $search_params->{-or} = [ { 'lang' => $lang }, '-and' => [ 'lang', 'default', \$subquery ] ] + $search_params->{'additional_content.id'} = $params->{id} if $params->{id}; + $search_params->{location} = $params->{location}; + $search_params->{branchcode} = $params->{library_id} ? [ $params->{library_id}, undef ] : undef; + $search_params->{published_on} = { '<=' => \'CAST(NOW() AS DATE)' }; + $search_params->{expirationdate} = [ '-or', { '>=' => \'CAST(NOW() AS DATE)' }, undef ]; + $search_params->{category} = $params->{category} if $params->{category}; + $search_params->{lang} = 'default' if !$lang || $lang eq 'default'; + $search_params->{-or} = [ { 'lang' => $lang }, '-and' => [ 'lang', 'default', \$subquery ] ] if !$search_params->{lang}; my $attribs = { prefetch => 'additional_content', order_by => 'additional_content.number' }; diff --git a/Koha/Template/Plugin/AdditionalContents.pm b/Koha/Template/Plugin/AdditionalContents.pm index a44de96..b919ecf 100644 --- a/Koha/Template/Plugin/AdditionalContents.pm +++ b/Koha/Template/Plugin/AdditionalContents.pm @@ -40,10 +40,11 @@ sub get { my $content = Koha::AdditionalContents->search_for_display( { - category => $category, - location => $location, - lang => $lang, + category => $category, + location => $location, + lang => $lang, ( $library ? ( library_id => $library ) : () ), + ( $id ? ( id => $id ) : () ), } ); diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt index a8fdf2c..22a995b 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt @@ -82,7 +82,7 @@ [% END %] [% IF news_id %] - [% SET koha_news = AdditionalContents.get( category => 'news', location => ['opac_only', 'staff_and_opac'], lang => lang, news_id => news_id ) %] + [% SET koha_news = AdditionalContents.get( category => 'news', location => ['opac_only', 'staff_and_opac'], lang => lang, id => news_id ) %] [% ELSE %] [% SET koha_news = AdditionalContents.get( category => 'news', location => ['opac_only', 'staff_and_opac'], lang => lang, library => branchcode ) %] [% END %] diff --git a/t/db_dependent/Template/Plugin/AdditionalContents.t b/t/db_dependent/Template/Plugin/AdditionalContents.t index cf30973..0a3226d 100755 --- a/t/db_dependent/Template/Plugin/AdditionalContents.t +++ b/t/db_dependent/Template/Plugin/AdditionalContents.t @@ -33,18 +33,21 @@ my $builder = t::lib::TestBuilder->new; $schema->storage->txn_begin; subtest 'get' => sub { - plan tests => 2; + plan tests => 4; - my $additional_contents = Koha::Template::Plugin::AdditionalContents->get({ category => 'news', location => ['opac_only', 'staff_and_opac'], lang => 'default', library => '%' }); - my $before_count = $additional_contents ? $additional_contents->{content}->count() : 0; - my $additional_content = $builder->build_object({ - class => 'Koha::AdditionalContents', - value => { - category => 'news', - location => 'opac_only', - branchcode => undef + my $additional_contents = Koha::Template::Plugin::AdditionalContents->get( + { category => 'news', location => [ 'opac_only', 'staff_and_opac' ], lang => 'default', library => '%' } ); + my $before_count = $additional_contents ? $additional_contents->{content}->count() : 0; + my $additional_content = $builder->build_object( + { + class => 'Koha::AdditionalContents', + value => { + category => 'news', + location => 'opac_only', + branchcode => undef + } } - }); + ); $builder->build_object( { class => 'Koha::AdditionalContentsLocalizations', @@ -54,13 +57,34 @@ subtest 'get' => sub { } } ); - $additional_contents = Koha::Template::Plugin::AdditionalContents->get({ category => 'news', location => ['opac_only', 'staff_and_opac'], lang => 'default' }); - is( $additional_contents->{content}->count, $before_count + 1, "We get the additional one we added"); + $additional_contents = Koha::Template::Plugin::AdditionalContents->get( + { category => 'news', location => [ 'opac_only', 'staff_and_opac' ], lang => 'default' } ); + is( $additional_contents->{content}->count, $before_count + 1, "We get the additional one we added" ); + + $additional_contents = Koha::Template::Plugin::AdditionalContents->get( + { + category => 'news', location => [ 'opac_only', 'staff_and_opac' ], lang => 'default', + blocktitle => 'blockhead' + } + ); - $additional_contents = Koha::Template::Plugin::AdditionalContents->get({ category => 'news', location => ['opac_only', 'staff_and_opac'], lang => 'default', blocktitle => 'blockhead' }); + is( $additional_contents->{blocktitle}, 'blockhead', "Block title is passed through" ); - is( $additional_contents->{blocktitle}, 'blockhead', "Block title is passed through"); + $additional_contents = Koha::Template::Plugin::AdditionalContents->get( + { + id => $additional_content->id, category => 'news', location => [ 'opac_only', 'staff_and_opac' ], + lang => 'default' + } + ); + is( $additional_contents->{content}->count, 1 ); + $additional_contents = Koha::Template::Plugin::AdditionalContents->get( + { + id => $additional_content->id, category => 'html_customizations', + location => [ 'opac_only', 'staff_and_opac' ], lang => 'default' + } + ); + is( $additional_contents, undef ); }; -- 2.43.0