[Bug 12167] New: datetime type casting issue regarding bug 7567
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bug ID: 12167 Summary: datetime type casting issue regarding bug 7567 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Tools Assignee: gmcharlt@gmail.com Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org I just upgraded the mysql server on our live and test systems running Ubuntu, and bug 7567 stopped displaying news in the staff client. I was able to track it down to a type casting issue. A patch follows. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |7567 Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 27825 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27825&action=edit Bug 12167 - Correct faulty test logic GetNewsToDisplay returns a reference to an array of hashes. Of course the hash >= 2. This patch properly checks the scalar size of the array. TEST PLAN --------- 1) prove -v t/db_dependent/NewsChannels.t -- should "work" 2) apply the patch 3) prove -v t/db_dependent/NewsChannels.t -- should "fail" This is in preparation for the next patch. NOTE: I believe this may only work under Ubuntu 12.04 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 27826 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27826&action=edit Bug 12167 - Patch disappearing news problem This doesn't duplicate on Debian, but it does under Ubuntu. TEST PLAN --------- 1) Update your git master to current. 2) Log into your staff client. 3) The news may have disappeared. -- Make sure to have news entered. The problem portion is the timestamp and date+1 logic don't seem to function properly. 4) Apply the first patch. 5) prove -v t/db_dependent/NewsChannels.t -- It should "fail". 6) Apply the second patch. 7) The news should reappear. -- There is no apparent change under Debian. 8) prove -v t/db_dependent/NewsChannels.t -- It should now "work". -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #3 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Tested on Ubuntu 12.04 and 14.04 Patch 1: for me is correct, but it pass before AND after. Why it should fail? Patch 2: why CAST? Usually I use date(timestamp) for this. Anyway, on current master (and both Ubuntu vers) my news display normally. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Patch 1 - patch 1 the logic is wrong, this is a required fixed. It's broken logic that works without really testing what it should be. Patch 2 - Since MySQL has updated again, it has since corrected itself on Ubuntu, but I haven't retested recently. So it may not be necessary. I'll retest this later. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #5 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to M. Tompsett from comment #4)
Patch 1 - patch 1 the logic is wrong, this is a required fixed. It's broken logic that works without really testing what it should be.
I agree, and your solution is correct. My only point from your test plan is that it can't fail
Patch 2 - Since MySQL has updated again, it has since corrected itself on Ubuntu, but I haven't retested recently. So it may not be necessary. I'll retest this later.
Again, I think is correct, or better than current code I only mention that my news are displayed, and curious about the use of CAST. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- CAST is ANSI SQL. I was trying to make sure to avoid MySQLisms. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #7 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Ok then. Would you mind to change test plan on both patches? I can sign them, but only as they change for good some code. But I see nothing as described in current test plan. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30171 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30171&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27825|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30172 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30172&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27826|0 |1 is obsolete| | --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30173 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30173&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30171|0 |1 is obsolete| | --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30174 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30174&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30172|0 |1 is obsolete| | --- Comment #12 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30175 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30175&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30173|0 |1 is obsolete| | --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30176 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30176&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30174|0 |1 is obsolete| | --- Comment #14 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30599 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30599&action=edit [SIGNED-OFF] 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30175|0 |1 is obsolete| | --- Comment #15 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30600 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30600&action=edit [SIGNED-OFF] 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30176|0 |1 is obsolete| | --- Comment #16 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30601 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30601&action=edit [SIGNED-OFF] 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Ludwin Hernández Vásquez <alduvi11@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alduvi11@gmail.com --- Comment #17 from Ludwin Hernández Vásquez <alduvi11@gmail.com> --- It seems to be that the function your working with not avoid mysqlism. I propose a bit of code that work with mysql and postgresql: sub GetNewsToDisplay { my ($lang,$branch) = @_; my $dbh = C4::Context->dbh; # SELECT *,DATE_FORMAT(timestamp, '%d/%m/%Y') AS newdate my $query = q{ SELECT *,opac_news.timestamp AS newdate FROM opac_news WHERE ( expirationdate >= current_date OR expirationdate IS NULL #OR expirationdate = '00-00-0000' not proper for postgresql ) 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. my $sth = $dbh->prepare($query); $lang = $lang // q{}; $sth->execute($lang,$branch); my @results; while ( my $row = $sth->fetchrow_hashref ){ $row->{newdate} = format_date($row->{newdate}); push @results, $row; } return \@results; } functions to substitute or remove *OR expirationdate = '00-00-0000' not proper for postgresql *CURRENT_DATE() equivalent in postgresql current_date kind regards! Ludwin Hernández Vásquez Universidad de El Salvador -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167
M. Tompsett <mtompset@hotmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Signed Off |ASSIGNED
--- Comment #18 from M. Tompsett <mtompset@hotmail.com> ---
(In reply to Ludwin Hernández Vásquez from comment #17)
> It seems to be that the function your working with not avoid mysqlism.
>
> I propose a bit of code that work with mysql and postgresql:
[SNIP]
> SELECT *,opac_news.timestamp AS newdate
> FROM opac_news
> WHERE (
> expirationdate >= current_date
> OR expirationdate IS NULL
> #OR expirationdate = '00-00-0000' not proper for postgresql
>
> )
> 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.
[SNIP and REORDER]
> functions to substitute or remove
> *CURRENT_DATE() equivalent in postgresql current_date
1) current_date and CURRENT_DATE are equivalent in postgresql, as far as I can
tell (http://www.postgresql.org/docs/8.1/static/functions-datetime.html -- See
Table 9-26), though perhaps some SQLs may be case sensitive. It functions in
postgresql just fine, as far as I can tell. Though, it does barf on
CURRENT_DATE() with the ()'s, so I'll remove that.
> *OR expirationdate = '00-00-0000' not proper for postgresql
Okay, after an hour or two of trying to get postgresql installed and trying to
mimick this type of query, I see the problem.
This would potentially need another patch to actually correct any historical
mysql news by updating expirationdate=NULL where expirationdate='0000-00-00' in
order to safely remove this. Though, I am surprised it functioned with
'00-00-0000'. Anyways, I guess I'll add a follow up patch.
--
You are receiving this mail because:
You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #19 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30642 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30642&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30642|0 |1 is obsolete| | --- Comment #20 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30663 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30663&action=edit [SIGNED-oFF] 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. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Update database runs without problem, test pass, no koha-qa errors. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |katrin.fischer@bsz-bw.de Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30600|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30601|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30663|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30599|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30882 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30882&action=edit [PASSED QA] 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> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested news expiration and publication dates: - publication date today, expiration date tomorrow: shows - publication date tomorrow, expiration date later: doesn't show - publication date yesterday, expiration date today: shows - publication date some time ago, expiration date yesterday: doesn't show - publication date today, expiration date empty: shows Passes all tests and QA script, no problems found. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30883 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30883&action=edit [PASSED QA] 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> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30884 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30884&action=edit [PASSED QA] 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 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30885 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30885&action=edit [PASSED QA] 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. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Update database runs without problem, test pass, no koha-qa errors. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply CC| |tomascohen@gmail.com --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi, this patches don't apply anymore. On rebasing, could we squash them, and explain better its purpose? It seems to me that it is a refactoring patch, so I expect to have regression tests for all stuff that gets touched. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #26 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 31139 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31139&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #27 from M. Tompsett <mtompset@hotmail.com> --- Are per Tomas' suggestion, I fought a learning curve, and think I have DBIx code for GetNewsToDisplay. Still testing my code. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31139|0 |1 is obsolete| | --- Comment #28 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38721 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38721&action=edit 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> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested news expiration and publication dates: - publication date today, expiration date tomorrow: shows - publication date tomorrow, expiration date later: doesn't show - publication date yesterday, expiration date today: shows - publication date some time ago, expiration date yesterday: doesn't show - publication date today, expiration date empty: shows Passes all tests and QA script, no problems found. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #29 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38722 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38722&action=edit 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> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #30 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38723 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38723&action=edit 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 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #31 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38724 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38724&action=edit 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. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Update database runs without problem, test pass, no koha-qa errors. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30882|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30883|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30884|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30885|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12167 --- Comment #32 from M. Tompsett <mtompset@hotmail.com> --- I rebased this, even though I am still working on something based on comment #27. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org