Liturgical-Calendar / LiturgicalCalendarFrontend

Presentation of the Liturgical Calendar Project, using bootstrap theming
Apache License 2.0
0 stars 1 forks source link

"Liturgy of any day" page is off by 1 day #59

Closed hudon closed 2 years ago

hudon commented 2 years ago

On the "Liturgy of any day" page, entering in a Day returns results for the day preceding the input day, see screenshot:

Screen Shot 2022-04-15 at 13 35 23

code (but the bug may be in the backend): https://github.com/Liturgical-Calendar/LiturgicalCalendarFrontend/blob/main/liturgyOfAnyDay.php#L81

JohnRDOrazio commented 2 years ago

I'll try to investigate this further. It seems to me that it was working alright, but maybe daylight savings has had an effect.

I'm also seeing that the calendar for Italy is currently not working for that example, I'll have to look into that.

I have tried to use UTC timestamps consistently throughout the backend, to avoid timezone issues. On the frontend javascript is generally tainted with a timezone, so it probably has something to do with making sure a UTC value is sent to the backend in the initial request, otherwise translating between javascript timestamps and PHP UTC timestamps can easily wind up with day off values.

JohnRDOrazio commented 2 years ago

Italian calendar is working again, I just needed to do a git pull in the api/v3 directory to pull in the latest updates from the development branch.

JohnRDOrazio commented 2 years ago

@hudon could you please test against this page:

https://litcal-staging.johnromanodorazio.com/liturgyOfAnyDay.php

and let me know what you're seeing there? You might have to try at different times of the day, to see if the problem is there. If you remember what time it was when you did the last test, perhaps try again around that time of day...

hudon commented 2 years ago
Screen Shot 2022-04-18 at 10 36 35

same issue. I open the page and it says 18 in the input form but 17 in the output. It's reproducible on latest Safari and Edge, macOS 12.4.

JohnRDOrazio commented 2 years ago

Problem can't be on the backend, because the backend only sends data for an entire year. There is no backend communication for a single day, requests are sent to the backend only if the year changes or if the calendar changes. Must have to do with how javascript dates are being handled.

In the PHP backend, all dates are handled as UTC, and the timestamp associated with any given liturgical "event" is therefore in UTC.

In the frontend, the request for a single day is processed by simply filtering through all possible liturgical events for those that have the same UTC timestamp as the day requested:

https://github.com/Liturgical-Calendar/LiturgicalCalendarFrontend/blob/ec240c11623a1986d94784b3c5b1f2e219ef8dc4/assets/js/liturgyOfAnyDay.js#L93-L97

https://github.com/Liturgical-Calendar/LiturgicalCalendarFrontend/blob/ec240c11623a1986d94784b3c5b1f2e219ef8dc4/assets/js/liturgyOfAnyDay.js#L130-L131

hudon commented 2 years ago
 newDate = new Date(Date.UTC(year, month - 1, day, 0, 0, 0)); 
 let timestamp = newDate.getTime() / 1000; 

Is this redundant? I think you can do let timestamp = Date.UTC(year, month - 1, day, 0, 0, 0) / 1000 and thus not introduce a Date object

JohnRDOrazio commented 2 years ago

Looking at the example you posted, I see the liturgical event that is returned is correct:

Nativity of Saint John the Baptist is in fact on June 23rd as requested.

The string at the top of the box that is supposed to show the date requested in a localized form, is showing June 22nd even though June 23rd was requested, and results for June 23rd are being shown. So it has to do with the localization of the date:

https://github.com/Liturgical-Calendar/LiturgicalCalendarFrontend/blob/ec240c11623a1986d94784b3c5b1f2e219ef8dc4/assets/js/liturgyOfAnyDay.js#L192

hudon commented 2 years ago

Oh I see. Yea it should be localized format without localizing the date/time itself

JohnRDOrazio commented 2 years ago

I think this commit might fix the issue: a30723b0f37f9256dc8f1cd2bd34513838e8b883

Can you test again on the staging page and see if it's fixed?

hudon commented 2 years ago
Screen Shot 2022-04-18 at 12 48 28

same thing. Can you repro?

JohnRDOrazio commented 2 years ago

Probably being so close to UTC time here in Rome, it's not easy to reproduce. I'll have to try changing my system time, or try somewhere near midnight.Il 18 Apr 2022 21:48, James Hudon @.***> ha scritto:

same thing. Can you repro?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

JohnRDOrazio commented 2 years ago

I tried setting my system clock to all different timezones, and I am able to reproduce on https://litcal.johnromanodorazio.com/liturgyOfAnyDay.php, I am not able to reproduce instead on https://litcal-staging.johnromanodorazio.com/liturgyOfAnyDay.php after the last commit. Can you please verify that you are testing against https://litcal-staging.johnromanodorazio.com/liturgyOfAnyDay.php and not https://litcal.johnromanodorazio.com/liturgyOfAnyDay.php ?

hudon commented 2 years ago

Verified:

Screen Shot 2022-04-18 at 14 19 48
hudon commented 2 years ago

I'm in Pacific Daylight Time (PDT), if it helps.

JohnRDOrazio commented 2 years ago

can you make sure you have force refreshed the page / emptied the cache? Since it's a fix on the javascript side, if the javascript is still cached it won't show as fixed... In Chrome the only surefire way I have of doing this (and Edge is chromium based now), is by opening developer tool settings and checking Disable cache (while Dev Tools is open), then refreshing the page. Sometimes Ctrl+Shift+R will do the trick, but if not I use the dev tools checkbox, and keep it open while I force refresh the page. Either that, or open an Incognito window...

hudon commented 2 years ago

yup, JS was cached. Looks good now 👍🏻

JohnRDOrazio commented 2 years ago

Fixed in commit https://github.com/Liturgical-Calendar/LiturgicalCalendarFrontend/commit/a30723b0f37f9256dc8f1cd2bd34513838e8b883