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

fix: Incorrect Times/Timezones when fetching events #1060

Merged
merged 19 commits into from
Dec 13, 2024

Conversation

techmannih
Copy link
Contributor

What kind of change does this PR introduce?
Fixing a Bug, Incorrect Times/Timezones when fetching events from the Community calendar #1036
Issue Number:
Issue #1036

Screenshots/videos:
image

@techmannih techmannih requested a review from a team as a code owner October 24, 2024 07:07
@techmannih
Copy link
Contributor Author

@DhairyaMajmudar @benjagm please review

@DhairyaMajmudar
Copy link
Member

@techmannih the build and lint workflows are failing, pls. fix them

Copy link

github-actions bot commented Oct 24, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
website ✅ Ready (View Log) Visit Preview 4b493c5

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (718dab5) to head (4b493c5).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1060   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          373       373           
  Branches        94        94           
=========================================
  Hits           373       373           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@techmannih
Copy link
Contributor Author

@techmannih the build and lint workflows are failing, pls. fix them

sure, please check now

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @techmannih.

The times are not correct. They should be:

  • JSON Schema Open Community Working Meetings should be at 19:00 UTC
  • JSON Schema Office Hours (APAC/Americas) should be at 10:00 UTC
  • JSON Schema Office Hours (Europe/Americas) should be at 14:00 UTC

You need to check what is the timezone of the meeting in the calendar and later convert to UTC:

  • JSON Schema Open Community Working Meetings is stored in Europe/London timezone
  • JSON Schema Office Hours (APAC/Americas) should be at 10:00 UTC is stored in America/Los_Angeles timezone.
  • JSON Schema Office Hours (Europe/Americas) should be at 14:00 UTC is stored in Europe/London timezone.

@techmannih
Copy link
Contributor Author

techmannih commented Oct 26, 2024

@benjagm but in the calender these are different, Please clarify whether the time on the calendar is in the local time zone or in UTCimage

@DhairyaMajmudar DhairyaMajmudar added the Hacktoberfest-accepted Pull requests accepted for Hacktoberfest'24 label Oct 26, 2024
@benjagm
Copy link
Collaborator

benjagm commented Nov 5, 2024

Please clarify whether the time on the calendar is in the local time zone or in UTC

  • JSON Schema Open Community Working Meetings is stored int the calendar in Europe/London timezone
  • JSON Schema Office Hours (APAC/Americas) is stored in America/Los_Angeles timezone.
  • JSON Schema Office Hours (Europe/Americas) is stored in Europe/London timezone.

We have 2 alternatives:

  • Show the times and timezones as they are stored in the calendar.
  • Convert all of them to UTC.

Whatever we decide we need the times the be consistent.

@DarhkVoyd
Copy link
Member

DarhkVoyd commented Nov 6, 2024

IMO, the schedule would be better displayed in 24-hour UTC, its easier and clearer to understand for most.

@gregsdennis
Copy link
Member

Just display both user-local and UTC.

@benjagm
Copy link
Collaborator

benjagm commented Nov 8, 2024

@techmannih, any update on this one? We'd love to fix this asap.

@techmannih
Copy link
Contributor Author

Hey @benjagm, on fetching the data it's show on local time zone and for America/los_angles timezone makes difficulties to converting in utc

@benjagm
Copy link
Collaborator

benjagm commented Nov 8, 2024

Hey @benjagm, on fetching the data it's show on local time zone and for America/los_angles timezone makes difficulties to converting in utc

Forget about local timezones. The calendar data has the original timezones associated to the events. You need to find that original timezones (I shared the details in previous comments) and then just convert to UTC accordingly.

Again, forget about local timezones.

@techmannih
Copy link
Contributor Author

techmannih commented Nov 8, 2024

Hey @benjagm

1. JSON Schema Open Community Working Meetings is stored int the calendar in Europe/London timezone which is also in UTC

  • Local Time (Europe/London Time Zone): 8:00 PM – 9:00 PM

  • UTC Time:
    - During Winter (when London is on GMT, i.e., UTC): 8:00 PM – 9:00 PM UTC
    - During Summer (when London is on BST, i.e., UTC+1): 7:00 PM – 8:00 PM UTC

(London is in UTC during the winter months (October to March), and during the summer months (April to September), London is in UTC+1 (British Summer Time), so the time difference is 1 hour.)

2. JSON Schema Office Hours (Europe/Americas) is stored in Europe/London timezone which is also in UTC

  • Local Time (Europe/London Time Zone): 3:00 PM – 4:00 PM

  • UTC Time:
    - During Winter (when London is on GMT, i.e., UTC): 3:00 PM – 4:00 PM UTC
    - During Summer (when London is on BST, i.e., UTC+1): 2:00 PM – 3:00 PM UTC

(London is in UTC during the winter months (October to March), and during the summer months (April to September), London is in UTC+1 (British Summer Time), so the time difference is 1 hour.)

3. JSON Schema Office Hours (APAC/Americas) (America/Los_Angeles Time Zone)

  • Local Time (America/Los_Angeles Time Zone): 11:00 PM – 12:00 AM

  • UTC Time:
    - During Winter (when Los Angeles is on PST, i.e., UTC-8): Next Day, 7:00 AM – 8:00 AM UTC
    - During Summer (when Los Angeles is on PDT, i.e., UTC-7): Next Day, 6:00 AM – 7:00 AM UTC

(Los Angeles follows Pacific Standard Time (PST) during the winter months (typically November to March), which is UTC -8:00. During the summer months (April to October), Los Angeles follows Pacific Daylight Time (PDT), which is UTC -7:00.)

We can fix the time for Europe/London timezone in UTC, but for the America/Los_Angeles timezone, we need to determine the correct local time based on your needs, as the time difference will vary depending on the season (whether it's Standard Time or Daylight Saving Time).

@benjagm
Copy link
Collaborator

benjagm commented Nov 8, 2024

We can fix the time for Europe/London timezone in UTC, but for the America/Los_Angeles timezone, we need to determine the correct local time based on your needs, as the time difference will vary depending on the season (whether it's Standard Time or Daylight Saving Time).

Great progress!!! Congrats.

We'll use Daylight Saving Time.

@techmannih
Copy link
Contributor Author

11:00 PM PDT in Los Angeles would be 6:00 AM UTC (the next day), is it correct ?

@benjagm
Copy link
Collaborator

benjagm commented Nov 10, 2024

11:00 PM PDT in Los Angeles would be 6:00 AM UTC (the next day), is it correct ?

Please note that Pacific Daylight Time (PDT) is a North American time zone in use only between the second Sunday in March to the first Sunday in November during Daylight Saving Time (DST). The rest of the year the timezone is PST Pacific Standard Timezone (PC), therefore now they are in PST.

  • NOW 11pm PST Pacific Standard timezone is 7am UTC
  • During Daylight 11pm PDT will be 6am UTC

@benjagm
Copy link
Collaborator

benjagm commented Nov 10, 2024

Sidenote about Office Hours (APAC/Americas) (America/Los_Angeles Time Zone) because here you provided some details that are not correct:

  1. JSON Schema Office Hours (APAC/Americas) (America/Los_Angeles Time Zone)
    Local Time (America/Los_Angeles Time Zone): 11:00 PM – 12:00 AM

The correct time in the calendar is 3 pm Pacific Time / PT

See screenshot:

Screenshot 2024-11-10 at 08 41 15

@techmannih
Copy link
Contributor Author

techmannih commented Nov 10, 2024

Sidenote about Office Hours (APAC/Americas) (America/Los_Angeles Time Zone) because here you provided some details that are not correct:

The correct time in the calendar is 3 pm Pacific Time / PT

Thanks @benjagm for confirming, But I am getting these times in the calendar for the events.
image

If these are incorrect, please give me actual and correct time for all event and UTC time also

@benjagm
Copy link
Collaborator

benjagm commented Nov 10, 2024

Not sure about your data and how it appears in your browser/calendar.

This is what I see (My timezone is CET)

Screenshot 2024-11-10 at 10 32 02

Remember that we are reading the ical file that is a text file from here https://calendar.google.com/calendar/ical/json.schema.community%40gmail.com/public/basic.ics. In this case this is the data available there:

JSON Schema Open Community Working Meeting

BEGIN:VEVENT
DTSTART;TZID=Europe/London:20240916T200000
DTEND;TZID=Europe/London:20240916T210000
SUMMARY:JSON Schema Open Community Working Meeting

JSON Schema Office Hours (Europe/Americas)

BEGIN:VEVENT
DTSTART;TZID=Europe/London:20240604T150000
DTEND;TZID=Europe/London:20240604T160000
SUMMARY:JSON Schema Office Hours (Europe/Americas)
END:VEVENT

JSON Schema Office Hours (APAC/Americas)

BEGIN:VEVENT
DTSTART;TZID=America/Los_Angeles:20240702T150000
DTEND;TZID=America/Los_Angeles:20240702T160000
SUMMARY:JSON Schema Office Hours (APAC/Americas)
END:VEVENT

Don't get confuse about how the data appears in your google calendar. Focus on the ical file, how this is converted to JSON and what JSON you have to work with.

@techmannih
Copy link
Contributor Author

techmannih commented Nov 10, 2024

@benjagm Thanks for sharing this data; it's very helpful for understanding. The difference between CET and UTC is 1 hour, so according to UTC timings:

  • JSON Schema Open Community Working Meetings should be at 20:00 UTC.
  • JSON Schema Office Hours (APAC/Americas) should be at 23:00 UTC.
  • JSON Schema Office Hours (Europe/Americas) should be at 15:00 UTC.

Please confirm if these are correct. If not, kindly provide the correct UTC timings.

@benjagm
Copy link
Collaborator

benjagm commented Nov 10, 2024

Looks good to me!

@techmannih
Copy link
Contributor Author

Thank you, @benjagm, for confirming! I’ve made the updates, and it looks much better now.

image

@benjagm
Copy link
Collaborator

benjagm commented Nov 10, 2024

Yes!! Can you please add the changes to the logic in the community page?

We have some duplicity of code and this will be a great opportunity to do some refactoring to avoid duplicate code, if that is possible.

@techmannih
Copy link
Contributor Author

techmannih commented Nov 10, 2024

Sure @benjagm How's look?
image

I’ve made the updates, and it looks much better now.

@benjagm
Copy link
Collaborator

benjagm commented Nov 10, 2024

and it looks much better now.

Looking good. Are you going to do some code reformatting to avoid duplicity in the calendar parsing logic?

@techmannih
Copy link
Contributor Author

@benjagm please check now

Copy link
Member

@DarhkVoyd DarhkVoyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @techmannih, what benjagm was trying to convey is that the two files share the same code or function, which leads to redundancy and duplication. To address this issue, we should refactor the code to eliminate duplicate code.

@techmannih
Copy link
Contributor Author

techmannih commented Nov 12, 2024

@benjagm @DarhkVoyd please check now

@techmannih
Copy link
Contributor Author

@benjagm Please review this PR

@benjagm benjagm requested a review from DarhkVoyd December 13, 2024 06:59
Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work with this @techmannih Sorry for the delay with the review!! Let's merge it.

@benjagm benjagm merged commit 9b8228f into json-schema-org:main Dec 13, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest-accepted Pull requests accepted for Hacktoberfest'24
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants