Bug 27442 - Overdues report uses 23:59:00 on To date filter
Summary: Overdues report uses 23:59:00 on To date filter
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-15 18:21 UTC by Kelly McElligott
Modified: 2023-06-08 22:26 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 27442: Assume 23:59:59 for date due end date (1.05 KB, patch)
2021-01-19 20:30 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27442: Assume 23:59:59 for date due end date (1.10 KB, patch)
2021-04-17 18:56 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kelly McElligott 2021-01-15 18:21:38 UTC
I believe the SQL is wrong in the hardcoded report "Overdue" under the Circulation Module.  When a due date range is added when running the report, it is not showing the items that are overdue for a particular date, a library will need to run a range of 2 days to find out the day's overdue.

Steps to Recreate:
1. Run Overdue report
2. Use the filter on the left to enter the date due to range of 10/12- 10/12.
3. No results are found with the actual date due to 10/12

4 Run the same report using the filter set at 10/12 - 10/13.
5 Koha will provide items that are due on 10/12.

The SQL is looking at date_due rather than date(date_due) which is not giving accurate results.
Comment 1 Andrew Fuerste-Henry 2021-01-15 19:58:20 UTC
I've been unable to recreate this in my testing docker on master, 20.05, or 19.11. Here's what I tried:
To test:
1 - check an item out with a due date of yesterday, 23:59:59
2 - go to overdue.pl, observe that the defauly search for all overdues includes your checkout
3 - perform a new search on overdue.pl, entering yesterday's date for both the FROM and TO limits on Date due
4 - your checkout still shows in the new search

Per your description, I'd expect to not see the checkout in step 4
Comment 2 Katrin Fischer 2021-01-16 19:54:36 UTC
Kelly, could you please retest?
Comment 3 Kelly McElligott 2021-01-19 20:09:49 UTC
Katrin,
This can be closed!  The due dates of these items were updated incorrectly and therefore not getting captured!
Comment 4 Andrew Fuerste-Henry 2021-01-19 20:20:51 UTC
On second thought, I think there is still a bug here.
When one sets a date range on this report, Koha appends a time onto the later date in order to account for the fact that our date_due values include a time. However, the time it appends is 23:59:00. That's mostly ok, since Koha generally uses 23:59:00 as its end-of-day date_due time. However, if events conspire such that one ends up with items due between 23:59:01 and 23:59:59, those items will be left out of the report.

We should just make Koha append 23:59:59 instead. 
If someone wants to get fancy in a later bug, we could also add a time-picker to the interface and let one limit the search by time.
Comment 5 Andrew Fuerste-Henry 2021-01-19 20:30:10 UTC
Created attachment 115351 [details] [review]
Bug 27442: Assume 23:59:59 for date due end date

To test:
1 - have a checkout due yesterday at 23:59:59
2 - go to overdue.pl, search with yesterday's date in the From and To dates
3 - confirm your checkout is not listed
4 - change your To date to today
5 - confirm your check is listed
6 - apply patch, restart service
7 - repeat step 2
8 - your checkout is there!
Comment 6 Katrin Fischer 2021-04-17 18:56:26 UTC
Created attachment 119801 [details] [review]
Bug 27442: Assume 23:59:59 for date due end date

To test:
1 - have a checkout due yesterday at 23:59:59
2 - go to overdue.pl, search with yesterday's date in the From and To dates
3 - confirm your checkout is not listed
4 - change your To date to today
5 - confirm your check is listed
6 - apply patch, restart service
7 - repeat step 2
8 - your checkout is there!

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2021-04-17 18:58:09 UTC
Really small change, going to set this directly to PQA.
Comment 8 Jonathan Druart 2021-04-21 12:48:45 UTC
How do you checkout an item with a due date containing 59 in the second part?
Comment 9 Katrin Fischer 2021-04-21 12:57:41 UTC
(In reply to Jonathan Druart from comment #8)
> How do you checkout an item with a due date containing 59 in the second part?

Hm, I thought you could not with the widget, but manually enter it - I was mistaken. I wonder if a process in Koha would generate such cases or if it was a migration issue initially? I think it would not hurt here as we are using a field that stores seconds as well.
Comment 10 Jonathan Druart 2021-04-22 08:00:45 UTC
We have other places in the code where we ->set_minute(59) and does not care about the seconds. I don't think this patch should be pushed as it without knowing why there are possibly checkouts created with 59 in the seconds part.

Also, one place in C4::Circ is doing:
3704         $hardduedate->truncate( to => 'minute' );
3705         $hardduedate->set_hour(23);
3706         $hardduedate->set_minute(59);


Kelly, Andrew, can you give us more info please?
Comment 11 Katrin Fischer 2022-07-08 11:43:42 UTC
> 
> Kelly, Andrew, can you give us more info please?

Ping!
Comment 12 Andrew Fuerste-Henry 2022-07-08 14:25:47 UTC
In all honesty, at this point I remember that we filed this because we ran into a Koha system that had due dates with 23:59:59 due dates, but I can no longer recall what that system was or how those checkouts were created. It's possible they were migrated in that way and it's not actually possible to set a due date with seconds on a checkout performed in Koha.
Comment 13 Katrin Fischer 2022-07-08 15:58:20 UTC
I am trying to see how we could resolve it - would closing it and fixing the data if you run into it again be an option?
Comment 14 Andrew Fuerste-Henry 2022-07-08 16:01:44 UTC
Works for me!