From bc65c39b94571004aa05852d56ca5e78cb8c763e Mon Sep 17 00:00:00 2001 From: Mark Tompsett 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 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 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 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', '

We have some exciting news!

', 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', '

We make news exciting!

', 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