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

Add timezone when scheduling Webinar/Meeting #115

Closed
JBlakeMCAA opened this issue Mar 8, 2021 · 11 comments
Closed

Add timezone when scheduling Webinar/Meeting #115

JBlakeMCAA opened this issue Mar 8, 2021 · 11 comments
Assignees
Labels
Breaking Change This change causes backward compatibility issue(s)
Milestone

Comments

@JBlakeMCAA
Copy link

data.AddPropertyIfValue("timezone", "UTC");
is it possible to add the ability to change the timezone to the NuGet?

@Jericho Jericho self-assigned this Mar 8, 2021
@Jericho Jericho added the question Someone is asking a question label Mar 8, 2021
@Jericho
Copy link
Owner

Jericho commented Mar 8, 2021

My goal was to simplify and standardize how webinars and meetings are scheduled as it pertains to the date. time and time zone. In the past, I have experienced all kinds of problems in systems I built (not Zoom related) when I didn't take great care to standardize dates management. That's why I try to always deal with the universal "UTC" and that's why when ZoomNet accepts a date, I always convert it .ToUniversalTime() and I specify UTC as the time zone. Does that make sense?

Having said that, I'm keeping an open mind: do you have a scenario in mind that I didn't consider where you absolutely require specifying a different time zone? Also, if we were to add the ability to specify your own time zone, how do you see we would handle the dates provided by developer? e.g.: never convert to UTC, convert to UTC only if time zone is omitted. What about the various 'kind' of dates that can be provided by developer? How would we handle those? Would we assume the date is already in the desired time zone if the kind is 'local'? Should we throw an exception if the developer specifies a date with an 'UTC' kind and a different time zone? These are just some of the considerations I can think of at the moment but am sure we'll find more if we continue analyzing possible scenarios.

As you can see, it can get pretty complicated very quick. That's what I wanted to avoid by only dealing with UTC.

@JBlakeMCAA
Copy link
Author

Hi Jericho,
I completely understand, I've had the same issue with dates before as well, that smart to do it this way.
I'm using: TimeZoneInfo.ConvertTimeToUtc(WebinarStartDate, TimeZoneInfo.FindSystemTimeZoneById("Central Standard Time"));
There are a number of numskulls at my work that will see a different time in Zoom and change it but that's something we cannot stop, they will just have to look at the time zone. They wont have a clue what Universal Time is but that's ok :)

@michael-aj-kennedy
Copy link
Contributor

As much as I absolutely hate anything to do with UTC conversions (particularly where a "helpful" MVC controller automatically parses the date to server-local time) I think it's unavoidable that we'll need some sort of support for specifying a timezone.

In our use-case we're allowing users to create events in a specified timezone (our clients can be spread over multiple timezones with a server in a central location). We're providing a front-end which will take care of all the timezone conversion for us, but we can't replicate all Zoom functionality within our UI (various factors come into this, including time). There will always be users who have to make use of the Zoom UI and won't understand why it reports the event time differently. This will be documented and referenced directly in our interface, but as @JBlakeMCAA states above, there'll always be someone who insists on changing it without understanding what UTC is - unfortunately it's just a matter of time (pun not intended...).

When we initially investigated this, I was annoyed to see that Zoom doesn't provide a list of supported timezones through the API, and that instead if we wanted to do anything in that area it would have to be hard-coded, but setting that to one side, I wonder it the solution here is to add an extra enumerated type to both meetings & webinars which allow the TZ to be set manually.

If the value is left as "UTC" then execute .ToUniversalTime() as currently (if only to avoid breaking any existing code), but otherwise I think the only thing that can be done is to assume that the date being passed in is already in the correct timezone and forgo any handling of DateTimeKind values (etc).

@Jericho
Copy link
Owner

Jericho commented Mar 11, 2021

Part of my thinking was that your custom UI might allow users to schedule a meeting/webinar in an arbitrary time zone and subsequently display the meeting/webinar in the desired timezone but as far as storing the meeting/webinar in the Zoom "repository", we would standardize on UTC. However, the two of you make a good point that my logic does not work so well if users use the Zoom UI because meetings/webinars will be displayed to them in a time zone they may not be familiar with.

As painful as it might be to me, I am slowly coming around and agreeing with you guys that we need to provide a way to specify a different time zone when scheduling an event. Here's what I have in mind:

  • add an optional parameter to all methods that allow scheduling a meeting/webinar
  • if developer omits this parameter when scheduling an event we will assume UTC
  • Zoom has documented the list of 100+ timezones they support. Probably makes sense to create an enum to match this list of timezones.
  • as @michael-aj-kennedy suggested, ZoomNet will convert a given date to universal time when time zone is UTC (to mimic the current behavior) but no conversion will be performed, no matter the 'kind' of date, when a different time zone is specified. It will be assumed that developer has already taken care of converting the date to the desired time zone

@Jericho
Copy link
Owner

Jericho commented Mar 12, 2021

To be more specific, the methods that will get an additional parameter are:

Meetings

  • CreateScheduledMeetingAsync
  • CreateRecurringMeetingAsync
  • UpdateMeetingOccurrenceAsync
  • UpdateScheduledMeetingAsync
  • UpdateRecurringMeetingAsync

Webinars

  • CreateScheduledWebinarAsync
  • CreateRecurringWebinarAsync
  • UpdateWebinarOccurrenceAsync
  • UpdateScheduledWebinarAsync
  • UpdateRecurringWebinarAsync

@michael-aj-kennedy
Copy link
Contributor

This sounds good to me - it would certainly fix our scenario :)

@JBlakeMCAA
Copy link
Author

JBlakeMCAA commented Mar 12, 2021

Thank you, just added a cherry on top!

@Jericho
Copy link
Owner

Jericho commented Mar 13, 2021

I'm having second thoughts about defaulting the time zone to UTC when scheduling an event because of this statement in Zoom's documentation:

To set time using a specific timezone, use yyyy-MM-ddTHH:mm:ss format and specify the timezone ID in the timezone field OR leave it blank and the timezone set on your Zoom account will be used.

In particular, the leave it blank and the timezone set on your Zoom account will be used part leads me to believe that a developer would expect the timezone to default to their account's timezone when they omit it rather than defaulting to UTC.

What do you think?

@Jericho Jericho added Breaking Change This change causes backward compatibility issue(s) and removed question Someone is asking a question labels Mar 13, 2021
@Jericho Jericho changed the title NuGet Add timezone to Get/Create/Update ScheduledWebinarAsync() Add timezone when scheduling Webinar/Meeting Mar 13, 2021
@Jericho Jericho added this to the 0.26.0 milestone Mar 13, 2021
@Jericho
Copy link
Owner

Jericho commented Mar 15, 2021

Ok I think I found a compromise. I suggest declaring the new parameter like so:

TimeZones? timeZone = TimeZones.UTC

This means that UTC is the default but, because this parameter is nullable, developers can specify a null value which means that Zoom will use the timezone configured on their account. I have published a beta here.

Let me know what you think

@michael-aj-kennedy
Copy link
Contributor

This sounds like a fair compromise, preserves the existing behaviour while still allowing the zoom default. There's not much more you can do there without breaking functionality for existing users/devs.

@Jericho
Copy link
Owner

Jericho commented Mar 18, 2021

🎉 This issue has been resolved in version 0.26.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change This change causes backward compatibility issue(s)
Projects
None yet
Development

No branches or pull requests

3 participants