Attachment 170495 Details for Bug 37684 – Bug 37684: Direct link to expired news are broken

[patch] Bug 37684: Direct link to expired news are broken

Bug-37684-Direct-link-to-expired-news-are-broken.patch (text/plain), 5.40 KB, created by Baptiste Wojtkowski (bwoj) on 2024-08-20 12:43:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Baptiste Wojtkowski (bwoj)

Created: 2024-08-20 12:43:30 UTC

Size: 5.40 KB

patch

obsolete

>From 658d7e7c4c9a2073ba1717ebd52dddbd604f1d3f Mon Sep 17 00:00:00 2001
>From: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 37684: 170495 | 171147 | 173867