Libki / libki-server

Libki Server
Other
55 stars 28 forks source link

Start and end time for reservation #223

Closed JSahlberg closed 3 years ago

JSahlberg commented 3 years ago

The start and end time for reservation wasn't set. They were only set with an undef value. I set them and removed the letter T from the string.

kylemhall commented 3 years ago

@JSahlberg I just pushed a patch to master that I think fixes the same bug. Would you be able to test it and let me know? Just pull the latest master and you should see the commit below. Thanks!

commit 4492fbb40bef660807ff563bca0dcc8a5b7044e8 (HEAD, origin/master, origin/HEAD)
Author: Kyle M Hall <kyle@bywatersolutions.com>
Date:   Fri Oct 22 06:41:51 2021 -0400

    Start and end time for reservation #223
JSahlberg commented 3 years ago

Just tested it now and it works! :D BUT... I think you forgot to remove the letter T which occures on both start and end time between the date and time. :/

kylemhall commented 3 years ago

Just tested it now and it works! :D BUT... I think you forgot to remove the letter T which occures on both start and end time between the date and time. :/

That threw me off too, but then I realized that, where you are seeing that T is in the stringified DateTime object. When you warn $begin it's converting the DateTime to a string in that format that contains the T, so when you apply that regular expression to remove the T, it's not really doing anything because $begin is actually a blessed hashref. You can see this by using warn Data::Dumper::Dumper( $begin ) to print out the real contents of the object!

kylemhall commented 3 years ago

Or am I completely off base and you are seeing the T in the rendered template?

JSahlberg commented 3 years ago

I dunno :P But I can see on other places involving the same collection of DateTime from the db that the regular expression is used frequently... so, I did the same ;)

kylemhall commented 3 years ago

Ah, I see. I looked at those other instances. In those ones, Bin Wen is converting the DateTime object to a string using stringify instead of using DateTime::Format::MySQL, which is why it is needed there. I'm sure that code could be rewritten to use the DateTime objects, but I'm reluctant to mess with the reservations code ;)

kylemhall commented 3 years ago

@JSahlberg thanks for catching this bug!

JSahlberg commented 3 years ago

image

No problem =)

But, still is the T there... so what's the solution for that?

kylemhall commented 3 years ago

Give me a few minutes and I'll have something for you to try.

JSahlberg commented 3 years ago

Ah sorry! Didn't know you were working on it :P

kylemhall commented 3 years ago

Sorry that took so long! You should see a new setting "Date display format" under "Time Settings" in the settings. Set that to your preferred date format and you should be good to go!

I found a couple different variations on date formatting on the backend. I've consolidated it all under a single helper method. Over time we can catch any non-formatted dates and datetimes.

JSahlberg commented 3 years ago

Awesome Kyle! =)

Still there is one place that doesn't seem to be affected by the DateTimeFormat. If you look at /libki-server/root/dynamic/includes/index/clients.tt and the row:

$("td:eq(4)", nRow).html( "[% c.loc("First reservation by") %]" + " " + "<i>" + aData[5] + "</i>" + " [% c.loc('starting') %] " + moment(aData[6]).format('lll') );

Shouldn't that be corrected in someway to show the correct format, now it seems to only show US standard. I tried manually changing it to only aData[6] with out the "moment" and ".format('lll')" and then it shows the correct format.

kylemhall commented 3 years ago

Good find @JSahlberg ! If you check the latest master, I've updated the code so the date format will also be passed to the javascript on the front-end and used for formatting.

If you see more places where this needs fixed, file a bug report and I'll get them taken care of!