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

Timezone Parse Issue (Newfoundland, Australia) #2101

Closed
DGeoWils opened this issue Jun 4, 2020 · 8 comments
Closed

Timezone Parse Issue (Newfoundland, Australia) #2101

DGeoWils opened this issue Jun 4, 2020 · 8 comments
Assignees
Milestone

Comments

@DGeoWils
Copy link

DGeoWils commented Jun 4, 2020

Hello,

My app receives scheduling requests from a third party system (Calendly). Calendly handles the scheduling aspect, but we need to store the scheduled time in our system with the correct timezones to handle notifications, and other things.

When using certain timezones (Newfoundland UTC-2:30 for example) Carbon throws an error when attempting to pull the timezone name out for user display.

Calendly sends the time stamp over in this format: "2020-06-11T12:30:00-02:30"

I encountered an issue with the following code:

$parsedCarbon = Carbon::parse("2020-06-11T12:30:00-02:30");
$timezone = new CarbonTimeZone($parsedCarbon->timezoneName);
$scheduled_timezone = $timezone->toRegionName();
$scheduled_timezone_display = $timezone->toRegionTimeZone()->getAbbreviatedName($parsedCarbon->isDST());

This code works with a timestamp like "‌2020-06-05T14:30:00-04:00"

Carbon version: 2.33.0

PHP version: v7.2.26

I expected to get:

nst (Newfoundland Standard Time)

But I actually get:

InvalidArgumentException: Unknown timezone for offset -9000 seconds.  (vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php:216) 

Thank You!

@kylekatarnls
Copy link
Collaborator

Hello, before going any further into this error message. I want to warn you: timezone offset => timezone name is never a 1:1 conversion. For instance CEDT (ex. Paris) and CET (ex. Tunis) has the same offset during winter: GMT+1 but a different one in summer (GMT+2 for CEDT). So when you get +01:00 you can't determine if it's CEDT or CET, and you can't group those two different timezones. Else you will have timezone problems on daylight time changes and the display will not be relevant for the user.

So the very priority is to get the real timezone from Calendly: the best one is the city: Europe/Paris, Europe/Tunis. This gives you the very precise location because timezone of cities/countries can change in the future. If you can't have as much precision, getting CEDT, CET, CT; etc. is still better than using offset.

Now for the error, it is thrown by the PHP function we uses: timezone_name_from_abbr

As you can see:

var_dump(timezone_name_from_abbr("", 2*3600, 0));
var_dump(timezone_name_from_abbr("", 2.5*3600, 0));
var_dump(timezone_name_from_abbr("", 3*3600, 0));

It "works" (gives a match) for 2:00 and 3:00, but false for 2:30

I'm afraid, if the offset is not supported by timezone_name_from_abbr(), you will have to iterate over the whole list to try to find the match. https://www.php.net/manual/fr/datetimezone.listidentifiers.php

But still as explained, the offset is not enough to guess the timezone name for a proper display.

@kylekatarnls
Copy link
Collaborator

On the Calendly documentation I saw this sample:

"timezone": "UTC",
"created_at": "2018-03-14T00:00:00Z",

This is exactly what you should use: date in UTC (Z means GMT) and the timezone property will probably contains the Newfoundland timezone identifier.

So you can easily find the right time to display using:

Carbon::parse($created_at)->tz($timezone);

// Example:
$created_at = "2018-03-14T00:00:00Z";
$timezone = "America/Chicago";
echo Carbon::parse($created_at)->tz($timezone); // 2018-03-13 19:00:00

@kylekatarnls
Copy link
Collaborator

See this:
https://help.calendly.com/hc/en-us/articles/223195488-Webhooks

The timezone is in invitee and this is correct because, N people can attend 1 digital meeting from different places in the world.

@kylekatarnls kylekatarnls reopened this Jun 4, 2020
@kylekatarnls kylekatarnls self-assigned this Jun 4, 2020
@kylekatarnls
Copy link
Collaborator

Here is how to find the matching for America/St_Johns:

$date = Carbon::parse('2020-06-11T12:30:00-02:30');
$offset = $date->getOffset();
$timezone = null;

foreach (timezone_identifiers_list() as $tz) {
  if (Carbon::parse('2020-06-11T12:30:00-02:30')->tz($tz)->getOffset() === $offset) {
    $timezone = $tz;

    break;
  }
}

echo $timezone;

I could add it in the toRegionName calculation as a fallback.

@kylekatarnls kylekatarnls added this to the 2.36.0 milestone Jun 4, 2020
@DGeoWils
Copy link
Author

DGeoWils commented Jun 4, 2020

See this:
https://help.calendly.com/hc/en-us/articles/223195488-Webhooks

The timezone is in invitee and this is correct because, N people can attend 1 digital meeting from different places in the world.

Thank you for the help.

We are actually using the redirect feature which passes this info in the query params:

 'assigned_to' => '...',
  'event_type_uuid' => '...',
  'event_type_name' => 'Test Survey Event ',
  'event_start_time' => '2020-06-05T15:30:00-02:30',
  'event_end_time' => '2020-06-05T16:00:00-02:30',
  'invitee_uuid' => '...',
  'invitee_full_name' => '...',
  'invitee_email' => '...',
  ...

I can see why only having the offset does not allow us to necessarily map to a specific region time zone. This of course can be affected by DST (which is why before implementing this feature we store the timestamp and a timezone identifier id 'America\New_York').

I will get in touch with Calendly, perhaps they can pass the selected timezone identifier along with their request.

kylekatarnls added a commit that referenced this issue Jun 4, 2020
…mezone

Fix #2101 iterate over every timezone to find it
@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Jun 4, 2020

In this case you don't need the timezone, you should store date-time as GMT (UTC) and once you have the UTC moment, get rid of -02:30 and use the id 'America\New_York' you have.

You can test the new feature requiring "nesbot/carbon": "dev-master" but it's still a poor work-around.

The main principle is: 2 systems (Calendly and your app) should exchange date-time information in the standard way, this standard is UTC (based on GMT+0 offset). Then the timezone is a location information not related to the moment that you should only use when you contextualize a date in a place (display to the user).

Please read this for more details:
https://medium.com/@kylekatarnls/always-use-utc-dates-and-times-8a8200ca3164

@DGeoWils
Copy link
Author

DGeoWils commented Jun 5, 2020

Normally I'd agree with you - I am storing the date in UTC - but we have 3 parties who could be in 3 different timezones.

  1. An internal team (America\New_York)
  2. A scheduling agent (can set up their timezone in their profile)
  3. A party that is not in our system, but receives notifications in the timezone where the event was scheduled. The only thing tying this individual to a timezone is the event creation so I store the timezone identifier of the scheduled event.

I've submitted a feature request to Calendly to send over the timezone identifier.

Thank you for your time!

Edit:
FWIW -- the fix in dev-master put enough of a bandaid on my issue that I should be able to get it out the door. Thanks again!

@kylekatarnls
Copy link
Collaborator

When doing it, I realized toRegion* methods are really not good. They just don't handle the DST at all when it comes to offset, and "finding the first one" using the PHP functions getOffset/timezone_name_from_abbr/timezone_identifiers_list may be way arbitrary.

I will probably completely rethink the way of doing this in Carbon 3, an objective feature would be a Carbon::parse('2020-06-11T12:30:00-02:30')->getPossibleRegions() that would return an array with 0 to N timezones that could have this offset for this given date.

And that's the same thing you should try to do. With 1 offset (like -02:30) you should look for 1 timezone name, but N possible timezone names. And the actual reliable data is -02:30, if you can't have a better identifier, you should keep the offset as is and not taking the risk to loosing it in guessing the location it meant to represent.

Last thing: considering the offset in a date string was necessarily generated by a timezone is a business assertion (you can only asserting considering you know how they are generated). Else you actually shouldn't even assume that and just consider the offset as an interval to GMT.

See '2000-01-01 00:00:00+23:00' is a valid date string even if there is no timezone in the world with a +23 shift. The same way +01:23 is a valid offset string too.

I keep thinking using toRegionName or toRegionTimeZone will cause you trouble some day. You should consider by receiving "2020-06-11T12:30:00-02:30" you get no concrete information about the location in which this moment should be contextualized.

Also remember: if you store the original data (-02:30) you'll still be able to do any conversion later.

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

No branches or pull requests

2 participants