From bc65c39b94571004aa05852d56ca5e78cb8c763e Mon Sep 17 00:00:00 2001
From: Mark Tompsett <mtompset@hotmail.com>
Date: Sun, 27 Jul 2014 21:01:29 -0400
Subject: [PATCH] Bug 12167: Invalid results check for opac_news test

C4/NewsChannels.pm has a function GetNewsToDisplay at the end
of the file. It returns an array reference (i.e. one result).
t/db_dependent/NewsChannels.t has a test which expects two
values. This is likely a cut and paste error on my part.
Notice get_opac_news returns an array of two things.

TEST PLAN
---------
1) prove -v t/db_dependent/NewsChannels.t
   -- magically the wrong condition still passes.
2) apply this first patch only.
3) prove -v t/db_dependent/NewsChannels.t
   -- notice the second result value is UNDEFINED.
   -- notice the first result is an array reference.
   -- and the last test passes, because unless somehow the
      array reference was less than 2, it will pass.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 12167: Bad expected results and condition check for GetNewsToDisplay

This corrected the results expected to a single array reference.
It then determines that it is defined and an array reference.
And lastly, correct the logic that was supposed to check for
valid results.

TEST PLAN
---------
1) Apply patch 1 and 2.
2) prove -v t/db_dependent/NewsChannels.t
   -- all tests should pass
   -- note the test changes to verify correctly what should
      be tested for.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 12167: Clean up GetNewsToDisplay

This bug started out as a result of busting while in transitions
of MySQL were happening in Ubuntu. However, I figured this fix
should remove backtick MySQLisms as per
http://wiki.koha-community.org/wiki/Coding_Guidelines#SQL6:_Backticks
And clean up GetNewsToDisplay a bit.

TEST PLAN
---------
1) News should function fine before applying any patches.
2) Apply all patches.
3) prove -v t/db_dependent/NewsChannels.t
   -- expecting all happy results.
   -- note the removal of backticks and the assumed typecasting
      of a timestamp to date.
   -- specified opac_news.timestamp to make sure that it would
      more likely parse as a fieldname and not as a keyword.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

All patches applied, test pass, cleaner code, no regressions found,
no koha-qa errors

Bug 12167 - Made SQL even more ANSI SQL.

The comparison against '00-00-0000' is not possible under
PostgreSQL. By providing a database upgrade which replaces all
'0000-00-00' dates with NULL, this portion of the SQL query can
be removed.

Additionally, MySQL can handle CURRENT_DATE(), but PostgreSQL
barfs horribly. By removing the ()'s, it functions in both.

The NewsChannels.t specifically added an explanationdate IS NULL
case to catch what was not tested before.

TEST PLAN
---------
1) Apply patch.
2) ./installer/data/mysql/updatedatabase.pl
3) prove -v t/db_dependent/NewsChannels.t
   -- This will confirm that the modified GetNewsToDisplay
      function has not broken.
---
 C4/NewsChannels.pm                     |   11 +++++-----
 installer/data/mysql/updatedatabase.pl |    8 ++++++++
 t/db_dependent/NewsChannels.t          |   35 ++++++++++++++++++++++++++++----
 3 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/C4/NewsChannels.pm b/C4/NewsChannels.pm
index 8694c1b..b6c7f00 100644
--- a/C4/NewsChannels.pm
+++ b/C4/NewsChannels.pm
@@ -188,27 +188,26 @@ sub get_opac_news {
 sub GetNewsToDisplay {
     my ($lang,$branch) = @_;
     my $dbh = C4::Context->dbh;
-    # SELECT *,DATE_FORMAT(timestamp, '%d/%m/%Y') AS newdate
     my $query = q{
-     SELECT *,timestamp AS newdate
+     SELECT *,opac_news.timestamp AS newdate
      FROM   opac_news
      WHERE   (
         expirationdate >= CURRENT_DATE()
         OR    expirationdate IS NULL
-        OR    expirationdate = '00-00-0000'
      )
-     AND   DATE(timestamp) < DATE_ADD(CURDATE(), INTERVAL 1 DAY)
+     AND   CAST( opac_news.timestamp AS DATE ) <= CURRENT_DATE
      AND   (lang = '' OR lang = ?)
      AND   (branchcode IS NULL OR branchcode = ?)
      ORDER BY number
     }; # expirationdate field is NOT in ISO format?
-       # timestamp has HH:mm:ss, CURRENT_DATE generates 00:00:00
-       #           by adding 1, that captures today correctly.
+       # casting timestamp to date truncates HH:mm:ss
+       # changing to <= CURRENT_DATE, today is captured.
     my $sth = $dbh->prepare($query);
     $lang = $lang // q{};
     $sth->execute($lang,$branch);
     my @results;
     while ( my $row = $sth->fetchrow_hashref ){
+        # this will type cast and respect system preferences.
         $row->{newdate} = format_date($row->{newdate});
         push @results, $row;
     }
diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index de77bf3..f2ff950 100755
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -8613,6 +8613,14 @@ if ( CheckVersion($DBversion) ) {
     SetVersion($DBversion);
 }
 
+$DBversion = "3.17.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("UPDATE opac_news SET expirationdate=NULL WHERE expirationdate='0000-00-00';");
+    print "Upgrade to $DBversion done (Bug 12167: Clean up date and time type casting issues)\n";
+    SetVersion ($DBversion);
+}
+
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)
diff --git a/t/db_dependent/NewsChannels.t b/t/db_dependent/NewsChannels.t
index f0a86e0..6a817e0 100644
--- a/t/db_dependent/NewsChannels.t
+++ b/t/db_dependent/NewsChannels.t
@@ -3,7 +3,7 @@
 use Modern::Perl;
 use C4::Dates qw(format_date);
 use C4::Branch qw(GetBranchName);
-use Test::More tests => 10;
+use Test::More tests => 14;
 
 BEGIN {
     use_ok('C4::NewsChannels');
@@ -27,7 +27,7 @@ my $rv = add_opac_new();    # intentionally bad
 ok( $rv == 0, 'Correctly failed on no parameter!' );
 
 my $timestamp = '2000-01-01';
-my ( $timestamp1, $timestamp2 ) = ( $timestamp, $timestamp );
+my ( $timestamp1, $timestamp2, $timestamp3 ) = ( $timestamp, $timestamp, $timestamp );
 my ( $title1, $new1, $lang1, $expirationdate1, $number1 ) =
   ( 'News Title', '<p>We have some exciting news!</p>', q{}, '2999-12-30', 1 );
 my $href_entry1 = {
@@ -57,6 +57,20 @@ my $href_entry2 = {
 $rv = add_opac_new($href_entry2);
 ok( $rv == 1, 'Successfully added the second dummy news item!' );
 
+my ( $title3, $new3, $lang3, $expirationdate3, $number3 ) =
+  ( 'News Title3', '<p>We make news exciting!</p>', q{}, undef, 1 );
+my $href_entry3 = {
+    title          => $title3,
+    new            => $new3,
+    lang           => $lang3,
+    expirationdate => $expirationdate3,
+    timestamp      => $timestamp3,
+    number         => $number3,
+    branchcode     => 'LIB1',
+};
+$rv = add_opac_new($href_entry3);
+ok( $rv == 1, 'Successfully added the third dummy news item!' );
+
 # We need to determine the idnew in a non-MySQLism way.
 # This should be good enough.
 my $query =
@@ -132,7 +146,20 @@ my ( $opac_news_count, $arrayref_opac_news ) = get_opac_news( 0, q{}, 'LIB1' );
 ok( $opac_news_count >= 2, 'Successfully tested get_opac_news for LIB1!' );
 
 # Test GetNewsToDisplay
-( $opac_news_count, $arrayref_opac_news ) = GetNewsToDisplay( q{}, 'LIB1' );
-ok( $opac_news_count >= 2, 'Successfully tested GetNewsToDisplay for LIB1!' );
+( $arrayref_opac_news ) = GetNewsToDisplay( q{}, 'LIB1' );
+ok( $arrayref_opac_news, '$arrayref_opac_news is ' .
+                          ( defined($arrayref_opac_news) ?
+                              ('Defined') : 'UNDEFINED')
+  );
+ok( ref $arrayref_opac_news eq 'ARRAY',
+    '$arrayref_opac_news is an array reference.' );
+ok( (scalar @$arrayref_opac_news) >= 2, 'Successfully tested GetNewsToDisplay for LIB1!' );
+
+$query =
+q{ SELECT idnew from opac_news WHERE timestamp='2000-01-01' AND expirationdate IS NULL; };
+$sth = $dbh->prepare($query);
+$sth->execute();
+my $nullcheck = $sth->fetchrow();
+ok ($nullcheck && $nullcheck>=0, 'Successfully found third dummy newsitem.');
 
 $dbh->rollback;
-- 
1.7.9.5