Bug 22544: Refactor searching of news items

Test plan:
1) Go to tools and define some news
2) Try different parameters, try to edit new items, and delete some
3) Go to all places where news are presented and ensure that there are
the right ones shown:
    opac main page - based on language
    opac righ column (formerly syspref OpacNavRight) - based on language
    opac news rss feed
    circulation slip (not quick slip)
    intranet main page
4) run tests:
prove t/db_dependent/Koha/News.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/Members.pm

@@ -588,15 +588,10 @@ sub IssueSlip {
                 issues      => $all,
             };
         }
-        my $news = Koha::News->search({
-                lang => [ 'slip', '' ],
-                branchcode => [ $branch, undef ],
-                -or => [ expirationdate => { '>=' => \'NOW()' },
-                         expirationdate => undef ]
-            },{
-                order_by => 'number'
-            }
-        );
+        my $news = Koha::News->search_for_display({
+                type => 'slip',
+                library_id => $branch,
+            });
         my @news;
         while ( my $n = $news->next ) {
             my $all = $n->unblessed_all_relateds;

Koha/News.pm

@@ -22,7 +22,7 @@ use Modern::Perl;
 use Carp;
 
 use Koha::Database;
-
+use Koha::Exceptions;
 use Koha::NewsItem;
 
 use base qw(Koha::Objects);
@@ -37,6 +37,54 @@ Koha::News - Koha News object set class
 
 =cut
 
+=head3 search_for_display
+
+my $news = Koha::News->search_for_display({
+    type => 'slip',
+    lang => 'en',
+    library_id => $branchcode
+})
+
+Return Koha::News set for display to user
+
+You can limit the results by type(lang) and library by optional params
+
+library_id should be valid branchcode of defined library
+
+type is one of this:
+- slip - for ISSUESLIP notice
+- koha - for intranet
+- opac - for online catalogue
+- OpanNavRight - Right column in the online catalogue
+
+lang is language code - it is used only when type is opac or OpacNavRight
+
+=cut
+
+sub search_for_display {
+    my ( $self, $params ) = @_;
+
+    my $search_params;
+    if ($params->{type} ) {
+        if ( $params->{type} eq 'slip' || $params->{type} eq 'koha') {
+            $search_params->{lang} = [ $params->{type}, '' ];
+        } elsif ( $params->{type} eq 'opac' && $params->{lang} ) {
+            $search_params->{lang} = [ $params->{lang}, '' ];
+        } elsif ( $params->{type} eq 'OpacNavRight' && $params->{lang} ) {
+            $search_params->{lang} = $params->{type} . '_' . $params->{lang};
+        } else {
+            Koha::Exceptions::BadParameter->throw("The type and lang parameters combination is not valid");
+        }
+    }
+
+    $search_params->{branchcode} = [ $params->{library_id}, undef ] if $params->{library_id};
+    $search_params->{timestamp} = { '<=' => \'NOW()' };
+    $search_params->{-or} = [ expirationdate => { '>=' => \'NOW()' },
+                              expirationdate => undef ];
+
+    return $self->SUPER::search($search_params, { order_by => 'number' });
+}
+
 =head3 type
 
 =cut

Koha/Template/Plugin/KohaNews.pm

@@ -35,26 +35,15 @@ sub get {
     my $blocktitle = $params->{blocktitle};
     my $lang = $params->{lang};
     my $library = $params->{library};
-    my $news_lang;
 
-    if( !$display_location ){
-        $news_lang = $lang;
-    } else {
-        $news_lang = $display_location."_".$lang;
-    }
+    my $content = Koha::News->search_for_display({
+            type => $display_location,
+            lang => $lang,
+            library_id => $library,
+    });
 
-    my $search_params;
-    $search_params->{lang} = $news_lang;
-    $search_params->{branchcode} = [ $library, undef ] if $library;
-    $search_params->{-or} = [ expirationdate => { '>=' => \'NOW()' },
-                              expirationdate => undef ];
-    my $content = Koha::News->search(
-        $search_params,
-        {
-            order_by => 'number'
-        });
 
-    if( @$content ){
+    if( $content ){
         return {
             content => $content,
             location => $display_location,

koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt

@@ -27,9 +27,7 @@
                 [% IF ( koha_news.count ) %]
                     <div id="area-news">
                         <h3><span class="news_title">News</span></h3>
-                        [% SET show_author =
-                            Koha.Preference('NewsAuthorDisplay') == 'staff'
-                            || Koha.Preference('NewsAuthorDisplay') == 'both' %]
+                        [% SET show_author = Koha.Preference('NewsAuthorDisplay') == 'staff' || Koha.Preference('NewsAuthorDisplay') == 'both' %]
                         [% FOREACH koha_new IN koha_news %]
                             <div class="newsitem" id="news[% koha_new.idnew | html %]"><h4>[% koha_new.title | html %]</h4>
                                 <div class="newsbody">[% koha_new.content | $raw %]</div>

koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt

@@ -96,9 +96,7 @@
             [% ELSE %]
 
                 <div id="news" class="newscontainer">
-                    [% SET show_author =
-                        Koha.Preference('NewsAuthorDisplay') == 'opac'
-                        || Koha.Preference('NewsAuthorDisplay') == 'both' %]
+                    [% SET show_author = Koha.Preference('NewsAuthorDisplay') == 'opac' || Koha.Preference('NewsAuthorDisplay') == 'both' %]
                     [% FOREACH koha_new IN koha_news %]
                         <div class="newsitem">
                             <h4 class="newsheader">

mainpage.pl

@@ -49,13 +49,10 @@ my $homebranch;
 if (C4::Context->userenv) {
     $homebranch = C4::Context->userenv->{'branch'};
 }
-my $koha_news = Koha::News->search({
-    lang => 'koha',
-    branchcode => [ $homebranch, undef ]
-},
-{
-    order_by => 'number'
-});
+my $koha_news = Koha::News->search_for_display({
+        type => 'koha',
+        library_id => $homebranch
+    });
 
 $template->param(
     koha_news   => $koha_news,

opac/opac-main.pl

@@ -70,15 +70,10 @@ if (defined $news_id){
         $template->param( single_news_error => 1 );
     }
 } else {
-    my $params;
-    $params->{lang} = [ $template->lang, '' ];
-    $params->{branchcode} = [ $homebranch, undef ] if $homebranch;
-    $params->{-or} = [ expirationdate => { '>=' => \'NOW()' },
-                       expirationdate => undef ];
-    $koha_news = Koha::News->search(
-        $params,
-        {
-            order_by => 'number'
+    $koha_news = Koha::News->search_for_display({
+            type => 'opac',
+            lang => $template->lang,
+            library_id => $homebranch,
         });
 }
 

opac/opac-news-rss.pl

@@ -43,15 +43,10 @@ my ($theme, $news_lang, $availablethemes) = C4::Templates::themelanguage(C4::Con
 
 my $branchcode = $input->param('branchcode');
 
-my $params;
-$params->{lang} = [ $news_lang, '' ];
-$params->{branchcode} = [ $branchcode, undef ] if $branchcode;
-$params->{-or} = [ expirationdate => { '>=' => \'NOW()' },
-                   expirationdate => undef ];
-my $koha_news = Koha::News->search(
-    $params,
-    {
-        order_by => 'number'
+my $koha_news = Koha::News->search_for_display({
+        type => 'opac',
+        lang => $news_lang,
+        library_id => $branchcode,
     });
 
 $template->param( koha_news => $koha_news );