This bug is a continuation of bug 20144
Created attachment 71469 [details] [review] Bug 20178: [sql_modes] Fix output_pref with dateformat=iso Fix for: Incorrect date value: '2018-02-05 06:30 PM' for column 'entrydate' TODO - check other occurrences, put to a separate bug report Then `git grep output_pref|grep iso` and make sure no other calls are impacted. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
This patch makes t/DateUtils.t fail # Failed test 'iso output 12hr' # at t/DateUtils.t line 47. # got: '2011-06-16 12:00' # expected: '2011-06-16 12:00 PM' # Looks like you failed 1 test of 67.
Created attachment 71498 [details] [review] Bug 20178: Fix test - iso formatted dates should not use timeformat
*** Bug 20008 has been marked as a duplicate of this bug. ***
Created attachment 71593 [details] [review] Bug 20178: [sql_modes] Fix output_pref with dateformat=iso Fix for: Incorrect date value: '2018-02-05 06:30 PM' for column 'entrydate' TODO - check other occurrences, put to a separate bug report Then `git grep output_pref|grep iso` and make sure no other calls are impacted. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 71594 [details] [review] Bug 20178: Fix test - iso formatted dates should not use timeformat Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
What if I want to display dates in ISO format but time in 12 hours format ? It is actually possible in master, but this patch prevents this.
(In reply to Julian Maurice from comment #7) > What if I want to display dates in ISO format but time in 12 hours format ? > It is actually possible in master, but this patch prevents this. ISO format includes times, and they must be 24 hour. This is why I took a little longer signing off, because I had to confirm that 12 hour times were not valid in ISO formatting. You could custom tweak your system preference to something that triggers the fallback else case. :)
Yes, what Mark said, dateformat='iso' means, actually, a 'sql' format. This is an historical issue (introduced by me when I improved Koha::DateUtils), we should distinguish these 2 formats but we do not have this need for so far. But in both case the date should be formatted with a date in 24h format.
It looks like a misuse of output_pref. Shouldn't the result of this subroutine be used only for display ? Anyway there is a lack of documentation here. It should be documented at least in POD and in description of both sysprefs IMO.
I forgot this value existed in the syspref, so I guess this is not the right fix. I am wondering if this value is really in used, I am expecting several side-effects using it. Maybe we should fix the test by mocking the pref instead.
This need to be fixed globally or at least check all the occurrences to estimate the issue.
I have opened bug 20199 for the Letters.t failure.
I am not sure how to test this - can someone tell if it's still valid?