Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Journeys: Time zone issue in journey milestone date. #2199

Closed
ziggabyte opened this issue Sep 28, 2024 · 9 comments · Fixed by #2403
Closed

Journeys: Time zone issue in journey milestone date. #2199

ziggabyte opened this issue Sep 28, 2024 · 9 comments · Fixed by #2403
Assignees
Labels
🐜 bug Something isn't working 🚪 entry-level Good for newcomers

Comments

@ziggabyte
Copy link
Contributor

ziggabyte commented Sep 28, 2024

Description.

When I set the date of a milestone of a journey, the representation of that date in relative time in the "Next milestone deadline" column in the journeys list is not correct. This (i think) is because that when the milestone date is set, it's set at the time of 00:00:00 on the selected date. So in the journeys list this is the time that comes from the server, and then is converted to local time of the browser (for me currently in Sweden, that means it adds 2 hours).

Steps to reproduce

  1. Go to a journey
  2. Go to the "milestones" tab
  3. Set the deadline of a to the date that is two days from the current date
  4. Go to the over view of all journeys, organize/{orgId}/journeys
  5. Look in the "Next milestone deadline" column

Expected Behaviour

It correctly says that the next milestone deadline is two days from now

Actual Behaviour

It says that the next milestone deadline is tomorrow

Screenshots (if you have any)

In this screenshot, Sept 30 is correct, that is the date that i chose for the milestone - but "tomorrow" is incorrect, as you see the date of the day i did this is Sept 28th.
bild

@ziggabyte ziggabyte added 🐜 bug Something isn't working 🚪 entry-level Good for newcomers labels Sep 28, 2024
@troldmand troldmand self-assigned this Sep 29, 2024
@troldmand
Copy link
Collaborator

It's maybe one of those tricky issues, that's only an issue temporarily in those odd 'gap hours', because my first re-production didn't yield the same issue:
Screenshot 2024-09-29 at 10 49 43
Time of writing is 29th of september, at 10:50, so 1st of september is in "two days".

Will try and reproduce by overwriting the data payloads in network tab of developer tools :)

@troldmand
Copy link
Collaborator

I was able to reproduce the error, by using a chrome extension for 'time traveling' to late in a day, and if today is 22:57, then the milestone that's technically two days later will show as 'tomorrow'!

Screenshot 2024-09-29 at 10 56 44

Getting closer.

@troldmand
Copy link
Collaborator

Oh, it's not a timezone thing.

Screenshot 2024-09-29 at 11 10 44

If there's 24 hours or more, then it will say two days. If it's 23 hours and 59 minutes, it will say 1 day.

To be continued ...

@troldmand
Copy link
Collaborator

{
      field: 'nextMilestoneDeadline',
      renderCell: (params) =>
        params.value ? (
          <ZUIRelativeTime datetime={'00:00 10-01-2024' as string} />
        ) : null,
        

This is showing as tomorrow, while 00:01 is in two days oOOoo

@troldmand
Copy link
Collaborator

@Idkirsch and I tried, but failed :'(

But we had fun hehehehehe

@troldmand troldmand removed their assignment Sep 29, 2024
@Herover
Copy link
Contributor

Herover commented Sep 29, 2024

I think it's something in the react-intl library think that the best format for all times between now and 23 hours and 29 minutes and 59 seconds into the future should be formatted as ex. "in 23 hours", but higher until 1 day and 11:59 later become "tomorrow", then until 2 days and 11:59 it says "in 2 days" etc. (based on some tests, didn't check their code.)

In my mind it would make more sense to also consider the current time, ex. if the time is 20:00 then anything higher than 4 hours is "tomorrow" OR anything between 24 and 28 hours is tomorrow, etc. I don't see any obvious way to do this in their docs, but I think they use a (poly filled?) version of Intl.RelativeTimeFormat. Maybe <ZUIRelativeTime/> could be implemented using this directly without react-intl? Unless there's somewhere else where their idea of formatted relative time makes more sense.

I'm open for spending some time later this week if you this sounds reasonable.

(I also spent enough time on Saturday to get annoyed that it didn't get a solution, so looked some more this evening.)

@ziggabyte
Copy link
Contributor Author

@Herover @Idkirsch @troldmand I love all the explorations you have done of this! So frustrating that you didn't find a solution over the weekend, but as you probably know, every time you research or attempt to solve time related bugs a magical koala baby gets its wings 🐨 🦄 🪽

@Herover go for it! I think your ideas sound cool!

@troldmand
Copy link
Collaborator

Thanks @ziggabyte, and go nuts @Herover. Maybe @Idkirsch and I would also look into another fix on Thursday ☮️ we will post her before picking up any work, hope you will the same @Herover 🤙

@troldmand
Copy link
Collaborator

We have a patch for this now! :) But it could be improved, see the review comment on the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment