From 658d7e7c4c9a2073ba1717ebd52dddbd604f1d3f 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 --- 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 215e624..f434fba 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 0a3226d..95c519f 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.43.0