Bugzilla – Attachment 31139 Details for
Bug 12167
datetime type casting issue regarding bug 7567
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12167: Invalid results check for opac_news test
Bug-12167-Invalid-results-check-for-opacnews-test.patch (text/plain), 7.90 KB, created by
Mark Tompsett
on 2014-08-25 13:55:27 UTC
(
hide
)
Description:
Bug 12167: Invalid results check for opac_news test
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2014-08-25 13:55:27 UTC
Size:
7.90 KB
patch
obsolete
>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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12167
:
27825
|
27826
|
30171
|
30172
|
30173
|
30174
|
30175
|
30176
|
30599
|
30600
|
30601
|
30642
|
30663
|
30882
|
30883
|
30884
|
30885
|
31139
|
38721
|
38722
|
38723
|
38724