https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24608 --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Andrew Isherwood from comment #29)
(In reply to Jonathan Druart from comment #26)
Hi Jonathan
When I edit I pick 23:59 but 22:59 is saved. Then edit again, save without modifying anything, 21:59 is saved, and so on.
I can't replicate this. So, you're selecting a date, default time is 23:59, but when you save, 22:59 is saved? Is that the value sent by the API call, or the value that's returned by the API? As I say, I can't replicate this, it works fine for me.
This is sent when I pick 23:59 {"date_due":"2020-03-18T22:59:00.000Z"} My host and the server do not have the same timezone, that's why I see something different. What your patch does (and so the JS lib from bug 24455 I guess) is the expected behavior, in an ideal world. But it cannot be the only place in Koha where it works like that. If such behavior is really needed, we need to redesign how we store and display date/datetime.
If I edit the date and remove the time page, save, it's loading endlessly. Which means 1. errors are not handled correctly, and 2. date only formatted dates are not taken into account.
The "loading endlessly" is actually the JS falling over because the call to DateTime_from_syspref is failing because DateTime_from_syspref doesn't check what it's got before trying to split it. So it splits the received string by " ", then tries to split the second part of that by ":" which of course fails because the second part is null.
Having considered this for a while, I'm not clear of the best way forward, it potentially opens a can of worms. We could say that DateTime_from_syspref should validate what it's got before working with it, and if it fails validation, it should error. We have date validation with accompanying errors in the form of calendar.inc's validate_date, so it would be reasonable to say we need similar time validation. However, if we're doing that, should we not be adding that to the upcoming moment.js work in Bug 24455 since that uses moment.js for date formatting, it should probably be leveraging moment.js' date & time validation. But if we do that with the time component, should we not also switch the date validation that's currently in calendar.inc to using moment.js.
Maybe I'm overcomplicating, but that was my thought process. I don't want to just put a hack in, but at the same time, doing it "properly" could be a pain in the ass.
Any thoughts?
Looks like a quick change to DateTime_from_syspref could do the trick. If the time part does not exist then add it with "23:59". -- You are receiving this mail because: You are watching all bug changes.