From 6058d4a70b02da099aae400901d82c56d235df91 Mon Sep 17 00:00:00 2001 From: Baptiste Wojtkowski Date: Tue, 20 Aug 2024 12:38:38 +0200 Subject: [PATCH] Bug 37684: Direct link to expired news are broken When using a direct link to a news (koha.url/cgi-bin/koha/opac-main.pl?news_id=XXX), the link is broken if the news is expired. Formerly, a using AdditionalContent->get( id => "my_news_id") on an expired news was returning a news, and calling get without id was returning all news but the expired ones. This patch adds tests to check this behaviour by adding one expired news and performing following new tests: 1 - It may not be returned by AdditionalContent.get() 2 - It must be returned by AdditionalContent.get() using its id This patch fixes the behaviour by addind the new behaviour or AdditionalContent.get: 1 - Any news must be returned by AdditionalContent.get() using its id; TEST PLAN: 1 - Apply patch 2 - Remove the changes made to Koha/AdditionalContents.pm 3 - Run tests -> one test must fail 4 - Create a news with a expired expiration date, notice the id of the news in the url of the modification panel 5 - Go to "opac.url/cgi-bin/koha/opac-main.pl?news_id=MY_ID" -> notice it does not work 6 - Reapply the whole patch 7 - Run test -> all test must pass 8 - Go to "opac.url/cgi-bin/koha/opac-main.pl?news_id=MY_ID" -> notice it does work now Signed-off-by: Olivier V --- Koha/AdditionalContents.pm | 10 +++--- .../Template/Plugin/AdditionalContents.t | 31 ++++++++++++++++++- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Koha/AdditionalContents.pm b/Koha/AdditionalContents.pm index 215e6247fc..f434fba1a0 100644 --- a/Koha/AdditionalContents.pm +++ b/Koha/AdditionalContents.pm @@ -84,11 +84,11 @@ sub search_for_display { $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 ] ] + $search_params->{published_on} = { '<=' => \'CAST(NOW() AS DATE)' } unless $params->{id}; + $search_params->{expirationdate} = [ '-or', { '>=' => \'CAST(NOW() AS DATE)' }, undef ] unless $params->{id}; + $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/t/db_dependent/Template/Plugin/AdditionalContents.t b/t/db_dependent/Template/Plugin/AdditionalContents.t index 0a3226dc86..95c519f0f0 100755 --- a/t/db_dependent/Template/Plugin/AdditionalContents.t +++ b/t/db_dependent/Template/Plugin/AdditionalContents.t @@ -33,7 +33,7 @@ my $builder = t::lib::TestBuilder->new; $schema->storage->txn_begin; subtest 'get' => sub { - plan tests => 4; + plan tests => 6; my $additional_contents = Koha::Template::Plugin::AdditionalContents->get( { category => 'news', location => [ 'opac_only', 'staff_and_opac' ], lang => 'default', library => '%' } ); @@ -61,6 +61,35 @@ subtest 'get' => sub { { 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" ); + my $expired_additional_content = $builder->build_object( + { + class => 'Koha::AdditionalContents', + value => { + category => 'news', + location => 'opac_only', + branchcode => undef, + expirationdate => '0001-01-01', + } + } + ); + $builder->build_object( + { + class => 'Koha::AdditionalContentsLocalizations', + value => { + additional_content_id => $expired_additional_content->id, + lang => 'default', + } + } + ); + $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 do not get the expired content" ); + + $additional_contents = Koha::Template::Plugin::AdditionalContents->get( + { category => 'news', location => [ 'opac_only', 'staff_and_opac' ], id => $expired_additional_content->id } ); + is( $additional_contents->{content}->count, 1, "We still get expired news through direct link" ); + $additional_contents = Koha::Template::Plugin::AdditionalContents->get( { category => 'news', location => [ 'opac_only', 'staff_and_opac' ], lang => 'default', -- 2.34.1