Summary: | filtering overdue report by due date can fail if TimeFormat is 12hr | ||
---|---|---|---|
Product: | Koha | Reporter: | Galen Charlton <gmcharlt> |
Component: | Circulation | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | f.demians, gmcharlt, katrin.fischer, kyle.m.hall, kyle |
Version: | master | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Bug 17001: fix due date filter on the overdue report
Bug 17001: fix due date filter on the overdue report k [PASSED QA] Bug 17001: fix due date filter on the overdue report |
Description
Galen Charlton
2016-07-29 20:38:34 UTC
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. |