From e35ee7c008436b86a9ae23cd4203c6e4a1bd5a9d Mon Sep 17 00:00:00 2001
From: Josef Moravec <josef.moravec@gmail.com>
Date: Tue, 19 Mar 2019 10:17:31 +0000
Subject: [PATCH] Bug 22544: Move get_opac_news to Koha namespace

Signed-off-by: David Nind <david@davidnind.com>
---
 C4/NewsChannels.pm                                 | 42 +---------------------
 Koha/NewsItem.pm                                   | 35 ++++++++++++++++++
 .../prog/en/modules/tools/koha-news.tt             | 16 ++++-----
 t/db_dependent/NewsChannels.t                      | 41 ++-------------------
 tools/koha-news.pl                                 | 22 +++++-------
 5 files changed, 54 insertions(+), 102 deletions(-)

diff --git a/C4/NewsChannels.pm b/C4/NewsChannels.pm
index 071f054c22..8ff9419606 100644
--- a/C4/NewsChannels.pm
+++ b/C4/NewsChannels.pm
@@ -28,7 +28,7 @@ BEGIN {
     @ISA = qw(Exporter);
     @EXPORT = qw(
         &GetNewsToDisplay
-        &add_opac_new &upd_opac_new &del_opac_new &get_opac_news
+        &add_opac_new &upd_opac_new &del_opac_new
     );
 }
 
@@ -123,46 +123,6 @@ sub del_opac_new {
     }
 }
 
-sub get_opac_news {
-    my ($limit, $lang, $branchcode) = @_;
-    my @values;
-    my $dbh = C4::Context->dbh;
-    my $query = q{
-                  SELECT opac_news.*, branches.branchname,
-                         published_on AS newdate,
-                         borrowers.title AS author_title,
-                         borrowers.firstname AS author_firstname,
-                         borrowers.surname AS author_surname
-                  FROM opac_news LEFT JOIN branches
-                      ON opac_news.branchcode=branches.branchcode
-                  LEFT JOIN borrowers on borrowers.borrowernumber = opac_news.borrowernumber
-                };
-    $query .= ' WHERE 1';
-    if ($lang) {
-        $query .= " AND (opac_news.lang='' OR opac_news.lang=?)";
-        push @values,$lang;
-    }
-    if ($branchcode) {
-        $query .= ' AND (opac_news.branchcode IS NULL OR opac_news.branchcode=?)';
-        push @values,$branchcode;
-    }
-    $query.= ' ORDER BY published_on DESC ';
-    #if ($limit) {
-    #    $query.= 'LIMIT 0, ' . $limit;
-    #}
-    my $sth = $dbh->prepare($query);
-    $sth->execute(@values);
-    my @opac_news;
-    my $count = 0;
-    while (my $row = $sth->fetchrow_hashref) {
-        if ((($limit) && ($count < $limit)) || (!$limit)) {
-            push @opac_news, $row;
-        }
-        $count++;
-    }
-    return ($count, \@opac_news);
-}
-
 =head2 GetNewsToDisplay
 
     $news = &GetNewsToDisplay($lang,$branch);
diff --git a/Koha/NewsItem.pm b/Koha/NewsItem.pm
index d2fbcc05fe..aa694f4132 100644
--- a/Koha/NewsItem.pm
+++ b/Koha/NewsItem.pm
@@ -22,6 +22,8 @@ use Modern::Perl;
 use Carp;
 
 use Koha::Database;
+use Koha::DateUtils;
+use Koha::Libraries;
 use Koha::Patrons;
 
 use base qw(Koha::Object);
@@ -51,6 +53,39 @@ sub author {
     return Koha::Patron->_new_from_dbic($self->_result->borrowernumber);
 }
 
+=head3 is_expired
+
+my $is_expired = $news_item->is_expired;
+
+Returns 1 if the news item is expired or 0;
+
+=cut
+
+sub is_expired {
+    my ( $self ) = @_;
+
+    return 0 unless $self->expirationdate;
+    return 1 if dt_from_string( $self->expirationdate ) < dt_from_string->truncate( to => 'day' );
+    return 0;
+}
+
+=head3 library
+
+my $library = $news_item->library;
+
+Returns Koha::Library object or undef
+
+=cut
+
+sub library {
+    my ( $self ) = @_;
+
+    my $library_rs = $self->_result->branchcode;
+    return unless $library_rs;
+    return Koha::Library->_new_from_dbic( $library_rs );
+}
+
+
 =head3 _type
 
 =cut
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt
index 2ad87f82a6..a94e78c4cd 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt
@@ -7,8 +7,6 @@
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Tools &rsaquo; News</title>
 [% INCLUDE 'doc-head-close.inc' %]
-[% IF ( opac_news_count ) %]
-[% END %]
 [% UNLESS ( wysiwyg ) %]
     [% Asset.css("lib/codemirror/codemirror.css") | $raw %]
     [% Asset.css("lib/codemirror/lint.min.css") | $raw %]
@@ -111,7 +109,7 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                 <fieldset class="action"><input class="button" type="submit" value="Submit" /> <a class="cancel" href="/cgi-bin/koha/tools/koha-news.pl">Cancel</a></fieldset>
         </form>
     [% ELSE %]
-        [% IF ( opac_news_count ) %]
+        [% IF ( opac_news.count ) %]
         <form id="del_form" method="post" action="/cgi-bin/koha/tools/koha-news.pl">
                 <table id="newst">
                    <thead> <tr>
@@ -127,7 +125,7 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                         <th class="NoSort">Actions</th>
                     </tr></thead>
                     <tbody>[% FOREACH opac_new IN opac_news %]
-                         [% IF ( opac_new.expired ) %]
+                         [% IF ( opac_new.is_expired ) %]
                             <tr class="expired">
                             [% ELSE %]
                             <tr>
@@ -148,13 +146,13 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                              </td>
                             <td>[% IF ( opac_new.branchcode == "" ) -%]
                                 All libraries
-                                [% ELSE %][% opac_new.branchname | html %]
+                                [% ELSE %][% opac_new.library.branchname | html %]
                                 [% END %]</td>
                             <td>[% opac_new.number | html %]</td>
-                            <td><span title="[% opac_new.newdate | html %]">[% opac_new.newdate | $KohaDates %]</span></td>
-                            <td><span title="[% opac_new.expirationdate | html %]">[% opac_new.expirationdate | $KohaDates %] [% IF ( opac_new.expired ) %](<span class="expired">expired</span>)[% END %]</span></td>
+                            <td><span title="[% opac_new.timestamp | html %]">[% opac_new.timestamp | $KohaDates %]</span></td>
+                            <td><span title="[% opac_new.expirationdate | html %]">[% opac_new.expirationdate | $KohaDates %] [% IF ( opac_new.is_expired ) %](<span class="expired">expired</span>)[% END %]</span></td>
                             <td>[% opac_new.title | html %]</td>
-                            <td>[% opac_new.author_title | html %] [% opac_new.author_firstname | html %] [% opac_new.author_surname | html %]</td>
+                            <td>[% IF ( opac_new.author) %][% INCLUDE 'patron-title.inc' patron=opac_new.author hide_patron_infos_if_needed=1 %][% END %]</td>
                             <td>
                                 <div class="btn-group">
                                     <a class="preview_news btn btn-default btn-xs" data-number="[% loop.count | html %]"><i class="fa fa-eye" aria-hidden="true"></i> Preview content</a>
@@ -255,7 +253,7 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
 [% MACRO jsinclude BLOCK %]
     [% INCLUDE 'calendar.inc' %]
     [% Asset.js("js/tools-menu.js") | $raw %]
-    [% IF ( opac_news_count ) %]
+    [% IF ( opac_news.count ) %]
         [% INCLUDE 'datatables.inc' %]
         <script>
             $('.preview_news').click( function() {
diff --git a/t/db_dependent/NewsChannels.t b/t/db_dependent/NewsChannels.t
index 3f3c8bdea4..60ec4fb961 100755
--- a/t/db_dependent/NewsChannels.t
+++ b/t/db_dependent/NewsChannels.t
@@ -4,8 +4,9 @@ use Modern::Perl;
 use Koha::Database;
 use Koha::DateUtils;
 use Koha::Libraries;
+use Koha::News;
 
-use Test::More tests => 11;
+use Test::More tests => 9;
 
 BEGIN {
     use_ok('C4::NewsChannels');
@@ -134,43 +135,7 @@ $href_entry2->{idnew} = $idnew2;
 $rv                   = upd_opac_new($href_entry2);
 is( $rv, 1, 'Successfully updated second dummy news item!' );
 
-# Test get_opac_news (multiple news items)
-my ( $opac_news_count, $arrayref_opac_news ) = get_opac_news( 0, q{}, 'LIB1' );
-
-# using >= 2, because someone may have LIB1 news already.
-ok( $opac_news_count >= 2, 'Successfully tested get_opac_news for LIB1!' );
-
 # Test GetNewsToDisplay
-( $opac_news_count, $arrayref_opac_news ) = GetNewsToDisplay( q{}, 'LIB1' );
+my ( $opac_news_count, $arrayref_opac_news ) = GetNewsToDisplay( q{}, 'LIB1' );
 ok( $opac_news_count >= 2, 'Successfully tested GetNewsToDisplay for LIB1!' );
 
-# Regression test 14248 -- make sure author_title, author_firstname, and
-# author_surname exist.
-
-subtest 'Regression tests on author title, firstname, and surname.', sub {
-    my ( $opac_news_count, $opac_news ) = get_opac_news( 0, q{}, 'LIB1' );
-    my $check = 0; # bitwise flag to confirm NULL and not NULL borrowernumber.
-    ok($opac_news_count>0,'Data exists for regression testing');
-    foreach my $news_item (@$opac_news) {
-        ok(exists $news_item->{author_title},    'Author title exists');
-        ok(exists $news_item->{author_firstname},'Author first name exists');
-        ok(exists $news_item->{author_surname},  'Author surname exists');
-        if ($news_item->{borrowernumber}) {
-            ok(defined $news_item->{author_title} ||
-               defined $news_item->{author_firstname} ||
-               defined $news_item->{author_surname},  'Author data defined');
-            $check = $check | 2; # bitwise flag;
-        }
-        else {
-            ok(!defined $news_item->{author_title},
-               'Author title undefined as expected');
-            ok(!defined $news_item->{author_firstname},
-               'Author first name undefined as expected');
-            ok(!defined $news_item->{author_surname},
-               'Author surname undefined as expected');
-            $check = $check | 1; # bitwise flag;
-        }
-    }
-    ok($check==3,'Both with and without author data tested');
-    done_testing();
-};
diff --git a/tools/koha-news.pl b/tools/koha-news.pl
index c57f1d6b81..4f912f5ecb 100755
--- a/tools/koha-news.pl
+++ b/tools/koha-news.pl
@@ -149,21 +149,15 @@ elsif ( $op eq 'del' ) {
 }
 
 else {
-
-    my ( $opac_news_count, $opac_news ) = &get_opac_news( undef, undef, undef );
-    
-    foreach my $new ( @$opac_news ) {
-        next unless $new->{'expirationdate'};
-        my @date = split (/-/,$new->{'expirationdate'});
-        if ($date[0]*$date[1]*$date[2]>0 && Date_to_Days( @date ) < Date_to_Days(&Today) ){
-			$new->{'expired'} = 1;
+    my $params;
+    $params->{lang} = $lang if $lang;
+    my $opac_news = Koha::News->search(
+        $params,
+        {
+            order_by => { -desc =>  'timestamp' },
         }
-    }
-    
-    $template->param(
-        opac_news       => $opac_news,
-        opac_news_count => $opac_news_count,
-		);
+    );
+    $template->param( opac_news => $opac_news );
 }
 $template->param(
     lang => $lang,
-- 
2.11.0