From d753ed6f8fb1eb04fb268e09884a65f0bce6b064 Mon Sep 17 00:00:00 2001 From: Josef Moravec Date: Wed, 20 Mar 2019 11:07:22 +0000 Subject: [PATCH] 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 Signed-off-by: Martin Renvoize --- C4/Members.pm | 13 ++--- Koha/News.pm | 50 ++++++++++++++++++- Koha/Template/Plugin/KohaNews.pm | 25 +++------- .../prog/en/modules/intranet-main.tt | 4 +- .../bootstrap/en/modules/opac-main.tt | 4 +- mainpage.pl | 11 ++-- opac/opac-main.pl | 13 ++--- opac/opac-news-rss.pl | 13 ++--- 8 files changed, 74 insertions(+), 59 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 367fe41e18f..c0fa4516c91 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -591,15 +591,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; diff --git a/Koha/News.pm b/Koha/News.pm index c7d93faa0ab..f0106a78525 100644 --- a/Koha/News.pm +++ b/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 diff --git a/Koha/Template/Plugin/KohaNews.pm b/Koha/Template/Plugin/KohaNews.pm index d6026462344..a82461466e4 100644 --- a/Koha/Template/Plugin/KohaNews.pm +++ b/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, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt index b13d4660f46..189425fd0ea 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt @@ -27,9 +27,7 @@ [% IF ( koha_news.count ) %]

News

- [% 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 %]

[% koha_new.title | html %]

[% koha_new.content | $raw %]
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 56a559c716f..a8b41bf63e9 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt @@ -95,9 +95,7 @@ [% ELSE %]
- [% 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 %]

diff --git a/mainpage.pl b/mainpage.pl index 6d2c79783c0..4c46811c774 100755 --- a/mainpage.pl +++ b/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, diff --git a/opac/opac-main.pl b/opac/opac-main.pl index f11aecdf016..40e9466a884 100755 --- a/opac/opac-main.pl +++ b/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, }); } diff --git a/opac/opac-news-rss.pl b/opac/opac-news-rss.pl index ff00cdfe24d..29b0bef5b97 100755 --- a/opac/opac-news-rss.pl +++ b/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 ); -- 2.20.1