llazzaro / django-scheduler

A calendaring app for Django.
BSD 3-Clause "New" or "Revised" License
1.27k stars 391 forks source link

views: _api_occurrences: Support handling of timezone-aware datetime … #553

Open daym opened 1 year ago

daym commented 1 year ago

…strings (2022-06-07T02:03:04+05:00).

Previously, such strings would fail with ValueError unconverted data remains: +01:00.

This patch changes it so it uses dateutil.parser.parse. The latter can parse all of the following and return a non-surprising result:

>>> dateutil.parser.parse('2020-01-01')
datetime.datetime(2020, 1, 1, 0, 0)
>>> dateutil.parser.parse('2020-01-01 05:42')
datetime.datetime(2020, 1, 1, 5, 42)
>>> dateutil.parser.parse('2020-01-01T05:42')
datetime.datetime(2020, 1, 1, 5, 42)
>>> dateutil.parser.parse('2020-01-01T05:42:00')
datetime.datetime(2020, 1, 1, 5, 42)
>>> dateutil.parser.parse('2020-01-01T05:42:00+01:00')
datetime.datetime(2020, 1, 1, 5, 42, tzinfo=tzoffset(None, 3600))

Additionally, since now it's possible for the convert function to return a datetime with tzinfo that is set, make sure not to try to add another (conflicting) tzinfo in that case.

dwasyl commented 1 year ago

@daym Have you been using this with recurring events at all? I had applied a similar change, and whenever Event.get_occurrences runs, it fails when generating new occurrences because the tz generated by .parse() is tzoffset(None, 3600) (or whichever TZ it's in) rather than a specific timezone.

o_start = pytz.timezone(str(tzinfo)).localize(o_start) 

Generates an UnknownTimeZone exception as tzinfo is tzoffset(None, 3600) rather than an actual timezone.

Just for anyone in the future, I ended up dealing with it by using the offset to calculate UTC and then replacing the tzinfo with UTC.

So the code in the PR looks like this in my environment:

d = dateutil.parser.parse(ddatetime)
d = d.replace(tzinfo=pytz.UTC) - d.utcoffset()
return d
daym commented 1 year ago

@daym Have you been using this with recurring events at all? I had applied a similar change, and whenever Event.get_occurrences runs, it fails when generating new occurrences because the tz generated by .parse() is tzoffset(None, 3600) (or whichever TZ it's in) rather than a specific timezone.

By now I have a lot more changes in https://github.com/daym/django-scheduler --and that I do use with recurrent events in production (using fullcalendar in Javascript).

The ideas neccessary to fix it are the following (already used by django-scheduler but I had to find out):

I'll eventually add more PRs here about it.

More details about timezones and datetime: https://blog.ganssle.io/articles/2018/03/pytz-fastest-footgun.html - all these things are not at all incorporated into this PR (it would require Python >= 3.6 if we did).

o_start = pytz.timezone(str(tzinfo)).localize(o_start) 

Generates an UnknownTimeZone exception as tzinfo is tzoffset(None, 3600) rather than an actual timezone.

Yeah :(

See above.