When using the overdue report circ/overdue.pl, if the TimeFormat system preference is set to 12-hour, setting a filter on the due date range can result in zero overdue loans being displayed, even if there are ones that fall in the range. This happens because the date/time that gets passed to the SQL is formatted incorrectly; rather than '2016-07-29 23:59', it gets passed as '2016-07-29 11:59 PM'; on many MySQL and MariaDB versions, that becomes NULL when cast to a datetime.
Created attachment 53814 [details] [review] Bug 17001: fix due date filter on the overdue report When the TimeFormat system preference is set to "12 hour", setting a filter on the due date can result in: - no overdue loans being reported, even if there are some that meet the criteria OR - overdue loans being omitted from the report if they are due on the "until" date in the filter This patch fixes this by replacing output_pref() with DateTime::Format::MySQL to format the date filter values to pass to the SQL query. To test ------- [1] Run the overdue report (circ/overdue.pl) and set a filter on due date, using values that should bring up one or more overdue loans. [2] Note that zero overdue loans are returned (if using MySQL 5.5, 5.6, or 5.7 or MariaDB 5) or that loans due on the "until" date are omitted (if using MarioDB 10). [3] Apply the patch and repeat step 1. This time, the correct set of overdue loans should be reported. Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
Created attachment 54077 [details] [review] Bug 17001: fix due date filter on the overdue report When the TimeFormat system preference is set to "12 hour", setting a filter on the due date can result in: - no overdue loans being reported, even if there are some that meet the criteria OR - overdue loans being omitted from the report if they are due on the "until" date in the filter This patch fixes this by replacing output_pref() with DateTime::Format::MySQL to format the date filter values to pass to the SQL query. To test ------- [1] Run the overdue report (circ/overdue.pl) and set a filter on due date, using values that should bring up one or more overdue loans. [2] Note that zero overdue loans are returned (if using MySQL 5.5, 5.6, or 5.7 or MariaDB 5) or that loans due on the "until" date are omitted (if using MarioDB 10). [3] Apply the patch and repeat step 1. This time, the correct set of overdue loans should be reported. Signed-off-by: Galen Charlton <gmcharlt@gmail.com> Signed-off-by: Your Name <jrobb@sekls.org>
Created attachment 54119 [details] [review] k
Created attachment 54120 [details] [review] [PASSED QA] Bug 17001: fix due date filter on the overdue report When the TimeFormat system preference is set to "12 hour", setting a filter on the due date can result in: - no overdue loans being reported, even if there are some that meet the criteria OR - overdue loans being omitted from the report if they are due on the "until" date in the filter This patch fixes this by replacing output_pref() with DateTime::Format::MySQL to format the date filter values to pass to the SQL query. To test ------- [1] Run the overdue report (circ/overdue.pl) and set a filter on due date, using values that should bring up one or more overdue loans. [2] Note that zero overdue loans are returned (if using MySQL 5.5, 5.6, or 5.7 or MariaDB 5) or that loans due on the "until" date are omitted (if using MarioDB 10). [3] Apply the patch and repeat step 1. This time, the correct set of overdue loans should be reported. Signed-off-by: Galen Charlton <gmcharlt@gmail.com> Signed-off-by: Jason Robb <jrobb@sekls.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Hi Jason, fixed your sign-off line to have your name - you might want to fix your git config :)
Would it not be simpler to pass the parameter timeformat => '24hr' to output_pref instead of using an external lib?
(In reply to Kyle M Hall from comment #6) > Would it not be simpler to pass the parameter timeformat => '24hr' to > output_pref instead of using an external lib? I note that output_pref(), if we were to take its name literally, is for formatting date and time *output*. DateTime::Format::MySQL is already used in other places and its name makes it clear that a DateTime passed through is meant to be fed to an SQL query. I think it better that we keep the concerns of formatting timestamps for human display separate from formatting them for computer consumption. I am reseting the status of this bug back to "Passed QA".
That is sufficient to me! Pushed to master for 16.11! (In reply to Galen Charlton from comment #7) > (In reply to Kyle M Hall from comment #6) > > Would it not be simpler to pass the parameter timeformat => '24hr' to > > output_pref instead of using an external lib? > > I note that output_pref(), if we were to take its name literally, is for > formatting date and time *output*. DateTime::Format::MySQL is already used > in other places and its name makes it clear that a DateTime passed through > is meant to be fed to an SQL query. > > I think it better that we keep the concerns of formatting timestamps for > human display separate from formatting them for computer consumption. > > I am reseting the status of this bug back to "Passed QA".
Pushed in 16.05. Will be in 16.05.03.