Bug 37643 - Check for NaN instead of truthiness if calendar.inc accepts_time
Summary: Check for NaN instead of truthiness if calendar.inc accepts_time
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Paul Derscheid
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 33703
Blocks:
  Show dependency treegraph
 
Reported: 2024-08-14 09:21 UTC by Paul Derscheid
Modified: 2024-10-21 12:31 UTC (History)
4 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:
24.11.00,24.05.04
Circulation function:


Attachments
Bug 37643: Check for NaN instead of truthiness if calendar.inc accepts_time (1.35 KB, patch)
2024-08-14 09:32 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37643: Check for NaN instead of truthiness if calendar.inc accepts_time (1.40 KB, patch)
2024-08-14 11:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37643: Check for NaN instead of truthiness if calendar.inc accepts_time (1.49 KB, patch)
2024-08-16 07:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2024-08-14 09:21:42 UTC
The check for truthiness in case calendar.inc accepts time (accepts_time = true) has one flaw: we can't set the time to 00:00.

Let's fix it.
Comment 1 Paul Derscheid 2024-08-14 09:32:30 UTC
Created attachment 170312 [details] [review]
Bug 37643: Check for NaN instead of truthiness if calendar.inc accepts_time

To test:
1) Go to circulation.pl (checking out to a patron)
2) Click the settings in the barcode input
3) Set any due date with the time set to 00:00
4) Note that the picker resets the time to 23:59
5) Apply the patch
6) Repeat 1-3
7) Note that the time (00:00) stays put
8) Sign off
Comment 2 Owen Leonard 2024-08-14 11:45:33 UTC
Created attachment 170315 [details] [review]
Bug 37643: Check for NaN instead of truthiness if calendar.inc accepts_time

To test:
1) Go to circulation.pl (checking out to a patron)
2) Click the settings in the barcode input
3) Set any due date with the time set to 00:00
4) Note that the picker resets the time to 23:59
5) Apply the patch
6) Repeat 1-3
7) Note that the time (00:00) stays put
8) Sign off

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Marcel de Rooy 2024-08-16 07:18:23 UTC
Created attachment 170430 [details] [review]
Bug 37643: Check for NaN instead of truthiness if calendar.inc accepts_time

To test:
1) Go to circulation.pl (checking out to a patron)
2) Click the settings in the barcode input
3) Set any due date with the time set to 00:00
4) Note that the picker resets the time to 23:59
5) Apply the patch
6) Repeat 1-3
7) Note that the time (00:00) stays put
8) Sign off

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Katrin Fischer 2024-08-19 15:21:48 UTC
The description reads very "developer" :) - I wonder if we should update it to the problem described in the test plan?
Comment 5 Katrin Fischer 2024-08-19 15:31:16 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 6 Lucas Gass (lukeg) 2024-09-12 22:08:11 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 7 Jonathan Druart 2024-09-18 12:00:42 UTC
Not sure it's directly related to this but I found an issue in this area:

Go to the circ page, open the settings
Open the calendar and notice the time part is available
Click the red cross to blank the input (bug exists if input is empty or not)
Notice the error in the console:
  Uncaught TypeError: parsedDate is undefined

Open the calendar again
=> The time part is no longer available
Comment 8 Jonathan Druart 2024-09-18 12:01:45 UTC
I guess the error existed prior to this patch as we were already using parsedDate without testing if it was defined.
Comment 9 Fridolin Somers 2024-10-21 12:31:45 UTC
Depends on Bug 33703 not in 23.11.x