Bug 25418

Summary: Backdated check out date loses time
Product: Koha Reporter: Nick Clemens <nick>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Nick Clemens <nick>
Severity: major    
Priority: P5 - low CC: 1joynelson, gmcharlt, jonathan.druart, kyle.m.hall, lucas, martin.renvoize, victor
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06, 19.05.11
Bug Depends on: 25133    
Bug Blocks:    
Attachments: Bug 25418: (bug 25133 follow-up) Display due date with hours
Bug 25418: (bug 25133 follow-up) Display due date with hours
Bug 25418: (bug 25133 follow-up) Display due date with hours
Bug 25418: (QA follow-up) Raise size of input box

Description Nick Clemens 2020-05-07 14:36:15 UTC
To recreate:
 1 - Select a date before today at checkout, leaving time at 23:59
 2 - Note the alert box has only the date
 3 - Confirm the checkout
 4 - Item is due at 00:00
Comment 1 Jonathan Druart 2020-05-07 15:19:50 UTC
Created attachment 104521 [details] [review]
Bug 25418: (bug 25133 follow-up) Display due date with hours

And not as due date, it's not a display-only info, but an input.

This patch is a follow-up of Bug 25133: Fix time part of due date for 12hr
Comment 2 Jonathan Druart 2020-05-07 15:20:45 UTC
Please test the different combinations of:
Backdate and not
12 and 24hr timeformat
23:59 and others
Comment 3 Victor Grousset/tuxayo 2020-05-10 17:08:49 UTC
1. Did I understood well the params for the combinations?
2. Is the other value of the time relevant? (11:59) should there be other values of the time?

== Draft of test plan ==
- tomorrow 23:59 12h timeformat syspref
- backdate 23:59 12h timeformat syspref
- tomorrow 11:59 12h timeformat syspref
- backdate 11:59 12h timeformat syspref
- tomorrow 23:59 24h timeformat syspref
- backdate 23:59 24h timeformat syspref
- tomorrow 23:59 24h timeformat syspref
- backdate 11:59 24h timeformat syspref
Comment 4 Jonathan Druart 2020-05-11 08:48:15 UTC
and minutes != 59 (to make the time part != 23:59)
Comment 5 Victor Grousset/tuxayo 2020-05-11 13:54:02 UTC
Created attachment 104705 [details] [review]
Bug 25418: (bug 25133 follow-up) Display due date with hours

And not as due date, it's not a display-only info, but an input.

This patch is a follow-up of Bug 25133: Fix time part of due date for 12hr

== Test plan ==
1. create checkouts with the following params and results
2. tomorrow 11:59 PM timeformat syspref: 12h
   - item is due without a specific time, only date (after-checkout green box)
3. backdate 11:59 PM timeformat syspref: 12h
   - note the yellow confirm box has only the date
   - item is due at 12:00 PM (after-checkout green box)
4. tomorrow 11:23 AM timeformat syspref: 12h
   - item is due at 11:23 AM (after-checkout green box)
5. backdate 11:23 AM timeformat syspref: 12h
   - note the yellow confirm box has the date and time
   - item is due 11:23 AM (after-checkout green box)
6. tomorrow 23:59 timeformat syspref: 24h
   - item is due without a specific time, only date (after-checkout green box)
7. backdate 23:59 timeformat syspref: 24h
   - note the yellow confirm box has only the date
   - item is due at 00:00 (after-checkout green box)
8. tomorrow 11:23 timeformat syspref: 24h
   - item is due at 11:23 (after-checkout green box)
9. backdate 11:23 timeformat syspref: 24h
   - note the yellow confirm box has the date and time
   - item is due 11:23 (after-checkout green box)
10. apply patch
11. create checkouts with the following params and results
12. tomorrow 11:59 PM timeformat syspref: 12h
    - item is due without a specific time, only date (after-checkout green box)
13. backdate 11:59 PM timeformat syspref: 12h
    - note the yellow confirm box has the date and time
      - CHANGE COMPARED TO BEFORE THE PATCH
    - item is due without a specific time, only date (after-checkout green box)
      - CHANGE COMPARED TO BEFORE THE PATCH
14. tomorrow 11:23 AM timeformat syspref: 12h
    - item is due 11:23 AM (after-checkout green box)
15. backdate 11:23 AM timeformat syspref: 12h
    - note the yellow confirm box has the date and time
    - item is due 11:23 AM (after-checkout green box)
16. tomorrow 23:59 timeformat syspref: 24h
    - item is due without a specific time, only date (after-checkout green box)
17. backdate 23:59 timeformat syspref: 24h
    - note the yellow confirm box has the date and time
      - CHANGE COMPARED TO BEFORE THE PATCH
    - item is due without a specific time, only date (after-checkout green box)
      - CHANGE COMPARED TO BEFORE THE PATCH
18. tomorrow 11:23 timeformat syspref: 24h
    - item is due at 11:23 (after-checkout green box)
19. backdate 11:23 timeformat syspref: 24h
    - note the yellow confirm box has the date and time
    - item is due 11:23 (after-checkout green box)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2020-05-11 14:01:45 UTC
I've noted (in caps in the test plan) 4 changes compared to before and after the patch. Looks good to me.
Though I'm not sure if that's enough. Like if the after-checkout green box should always display the time. And if other times than 23:59 and 11:23 should be tested.

Jonathan or QA: do you confirm that the results show well the issues and that the patch addresses them successfully?

Signing off because there is no doubt it fixed the issues described in comment 0
Comment 7 Jonathan Druart 2020-05-11 14:20:22 UTC
Victor, we store a "date" without the time part with "23:59". If a time part is passed, then we store it.

When we display a "due date", we display it without 23:59 if the time part is "23:59", which means the user has to check the item in that specific day (without hour constraint). This is done using the TT plugin KohaDates, with the as_due_date flag turned on.

The 23:59 is only removed for display purpose, if the due date is displayed in an input field (like the green confirmation box), then the time part has to be visible (for technical purpose only actually, to make the JS plugin work correctly).

Hope it clarifies why it's sometimes visible and sometimes not.
Comment 8 Victor Grousset/tuxayo 2020-05-11 14:52:08 UTC
Yep, thanks :)
Comment 9 Nick Clemens 2020-05-11 17:10:05 UTC
Created attachment 104709 [details] [review]
Bug 25418: (bug 25133 follow-up) Display due date with hours

And not as due date, it's not a display-only info, but an input.

This patch is a follow-up of Bug 25133: Fix time part of due date for 12hr

== Test plan ==
1. create checkouts with the following params and results
2. tomorrow 11:59 PM timeformat syspref: 12h
   - item is due without a specific time, only date (after-checkout green box)
3. backdate 11:59 PM timeformat syspref: 12h
   - note the yellow confirm box has only the date
   - item is due at 12:00 PM (after-checkout green box)
4. tomorrow 11:23 AM timeformat syspref: 12h
   - item is due at 11:23 AM (after-checkout green box)
5. backdate 11:23 AM timeformat syspref: 12h
   - note the yellow confirm box has the date and time
   - item is due 11:23 AM (after-checkout green box)
6. tomorrow 23:59 timeformat syspref: 24h
   - item is due without a specific time, only date (after-checkout green box)
7. backdate 23:59 timeformat syspref: 24h
   - note the yellow confirm box has only the date
   - item is due at 00:00 (after-checkout green box)
8. tomorrow 11:23 timeformat syspref: 24h
   - item is due at 11:23 (after-checkout green box)
9. backdate 11:23 timeformat syspref: 24h
   - note the yellow confirm box has the date and time
   - item is due 11:23 (after-checkout green box)
10. apply patch
11. create checkouts with the following params and results
12. tomorrow 11:59 PM timeformat syspref: 12h
    - item is due without a specific time, only date (after-checkout green box)
13. backdate 11:59 PM timeformat syspref: 12h
    - note the yellow confirm box has the date and time
      - CHANGE COMPARED TO BEFORE THE PATCH
    - item is due without a specific time, only date (after-checkout green box)
      - CHANGE COMPARED TO BEFORE THE PATCH
14. tomorrow 11:23 AM timeformat syspref: 12h
    - item is due 11:23 AM (after-checkout green box)
15. backdate 11:23 AM timeformat syspref: 12h
    - note the yellow confirm box has the date and time
    - item is due 11:23 AM (after-checkout green box)
16. tomorrow 23:59 timeformat syspref: 24h
    - item is due without a specific time, only date (after-checkout green box)
17. backdate 23:59 timeformat syspref: 24h
    - note the yellow confirm box has the date and time
      - CHANGE COMPARED TO BEFORE THE PATCH
    - item is due without a specific time, only date (after-checkout green box)
      - CHANGE COMPARED TO BEFORE THE PATCH
18. tomorrow 11:23 timeformat syspref: 24h
    - item is due at 11:23 (after-checkout green box)
19. backdate 11:23 timeformat syspref: 24h
    - note the yellow confirm box has the date and time
    - item is due 11:23 (after-checkout green box)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens 2020-05-11 17:10:08 UTC
Created attachment 104710 [details] [review]
Bug 25418: (QA follow-up) Raise size of input box

In twelve hour format the length of the string is 20 characters
we need the box to fit all of them

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Martin Renvoize 2020-05-12 10:24:58 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Joy Nelson 2020-05-13 19:20:31 UTC
pushed to 19.11.x for 19.11.06
Comment 13 Lucas Gass 2020-05-15 22:49:55 UTC
backported to 19.05.x for 19.05.11