From 6f85d81d6d85ceeaf30a2c5c8867e659586f9376 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 --- C4/Members.pm | 13 ++---- Koha/News.pm | 50 +++++++++++++++++++++- Koha/Template/Plugin/KohaNews.pm | 24 +++-------- .../intranet-tmpl/prog/en/modules/intranet-main.tt | 4 +- .../opac-tmpl/bootstrap/en/modules/opac-main.tt | 4 +- mainpage.pl | 11 ++--- opac/opac-main.pl | 13 ++---- opac/opac-news-rss.pl | 13 ++---- 8 files changed, 73 insertions(+), 59 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 22b5b3b6b3..5f43a4305e 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -580,15 +580,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 4a3d759278..0d1632e5af 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 0d6e7681de..95a5d6dd3c 100644 --- a/Koha/Template/Plugin/KohaNews.pm +++ b/Koha/Template/Plugin/KohaNews.pm @@ -34,24 +34,12 @@ sub get { my $display_location = $params->{location}; my $lang = $params->{lang}; my $library = $params->{library}; - my $news_lang; - - if( !$display_location ){ - $news_lang = $lang; - } else { - $news_lang = $display_location."_".$lang; - } - - 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' - }); + + my $content = Koha::News->search_for_display({ + type => $display_location, + lang => $lang, + library_id => $library, + }); return $content; } 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 ab01dee0e8..50f4e66f9e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt @@ -20,9 +20,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 ba8c80e521..7bf922bea0 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt @@ -80,9 +80,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 85559afc07..459c3a38e6 100755 --- a/mainpage.pl +++ b/mainpage.pl @@ -48,13 +48,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 3de41c4832..a614483985 100755 --- a/opac/opac-main.pl +++ b/opac/opac-main.pl @@ -73,15 +73,10 @@ if (defined $news_id){ $template->param( single_news_error => 1 ); } } else { - my $params; - $params->{lang} = [ $news_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 => $news_lang, + library_id => $homebranch, }); } diff --git a/opac/opac-news-rss.pl b/opac/opac-news-rss.pl index ea9893bfaa..7d0f22d750 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.11.0