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

Passing timezone aware DateTime to setStartDate results in wrong time values #254

Open
frisi opened this issue Dec 16, 2016 · 2 comments
Open

Comments

@frisi
Copy link
Contributor

frisi commented Dec 16, 2016

p.a.event 1.1.X in plone 4.3 for archetypes

the way the DateTime values are treated in https://github.com/plone/plone.app.event/blob/1.1.8/plone/app/event/at/content.py#L425 results in the following behaviour:

base_edit works

the current implementation is geared towards the edit form:
users shall decide which time the event starts and also which timezone shall be applied to the event.
so although the system timezone is europe/vienna (in my case) creating an event with
start and enddtime 09:00 and timezone europe/london shall result in 09:00 in the chosen timezone

the form passes:

# for the startDate
>>> value
DateTime('2016/12/16 09:00:00 GMT+1')
>>> value.timezoneNaive()
True
>>> value.ISO()
'2016-12-16T09:00:00'

#for the endDate
>>> value
DateTime('2017/08/16 09:00:00 GMT+2')
>>> value.timezoneNaive()
True
>>> value.ISO()
'2017-08-16T09:00:00'
  • event.timezone: europe/london

result:

>>> event.start()
DateTime('2016/12/16 09:00:00 Europe/London')
# 2016-12-16T09:00:00+00:00
>>> event.end()
DateTime('2017/04/07 09:00:00 Europe/London')
# 2017-04-07T09:00:00+01:00

setting dates in python code leads to unexpected results

>>> event.timezone
u'Europe/London'
>>> start=DateTime('2008/09/04 07:00:00')
>>> start
DateTime('2008/09/04 07:00:00 GMT+2')
>>> start.timezoneNaive()
True
>>> start.ISO()
'2008-09-04T07:00:00'


>>> event.setStartDate(start)

# accessing the field directly leads to the expected result:
>>> event.getField('startDate').get(event)
DateTime('2008/09/04 07:00:00 GMT+0')
>>> event.getField('startDate').get(event).ISO()
'2008-09-04T07:00:00'

# using the custom accessor that appends the timezone
>>> event.start()
DateTime('2008/09/04 08:00:00 Europe/London')
>>> event.start().ISO()
'2008-09-04T08:00:00+01:00'

i'd at least expect DateTime('2008/09/04 07:00:00 Europe/London') ('2008-09-04T07:00:00+01:00') to be consistent with the add/edit form behaviour.

from a programmer's point of view, i'd expect the following behaviour:

A) timezone naive values (DateTime().timezoneNaive()==True):

use the time and simply append event.timezone

'2008-09-04T07:00:00' -> '2008-09-04T07:00:00+01:00' (for timezone london)

B) timezone aware values:

localize the date for the timezone of the event

'2008-09-04T08:00:00+02:00' -> '2008-09-04T07:00:00+01:00' (dt.toZone('Europe/London'))

@frisi
Copy link
Contributor Author

frisi commented Dec 16, 2016

i got fooled by inconsitent behaviour of zope DateTime's toZone method that is used in the _dt_getter method (https://github.com/plone/plone.app.event/blob/1.1.8/plone/app/event/at/content.py#L409)

Zope DateTime

timezone aware DateTime objects are correctly localized to a different timezone

>>> aware = DateTime('2016-06-15T07:00:00+02:00')
>>> aware.timezoneNaive()
False
>>> aware.toZone('UTC').ISO()
'2016-06-15T05:00:00+00:00

timezone naive DateTime objects are treated correct (!sometimes!)

'2016-06-15T07:00:00' works

>>> unaware = DateTime('2016-06-15T07:00:00')
>>> unaware.timezoneNaive()
True
>>> unaware.toZone('UTC').ISO()
'2016-06-15T07:00:00+00:00'

but sometimes not:

'2016/06/15 07:00:00' does not work

here we use a slightly different constructor.

.ISO() returns the same value.

but toZone results in 05:00 instead of 07:00!

>>> unaware2=DateTime('2016/06/15 07:00:00')
>>> unaware2.ISO()
'2016-06-15T07:00:00'
>>> unaware.ISO() == unaware2.ISO()
True
>>> unaware2.toZone('UTC').ISO()
'2016-06-15T05:00:00+00:00'

in contrast: python datetime behaves as expected

for timezone naive values, the time is kept as is

>>> from datetime import datetime
>>> import pytz

>>> dt_unaware = datetime(2016,6,15,7,0)
>>> dt_unaware.isoformat()
'2016-06-15T07:00:00'


>>> pytz.utc.localize(dt_unaware).isoformat()
'2016-06-15T07:00:00+00:00'

only timezone aware values are localized

>>> dt_aware = pytz.timezone('Europe/Vienna').localize(datetime(2016,6,15,7,0))
>>> dt_aware.isoformat()
'2016-06-15T07:00:00+02:00'

>>> dt_aware.astimezone(pytz.utc).isoformat()
'2016-06-15T05:00:00+00:00'

@frisi
Copy link
Contributor Author

frisi commented Dec 17, 2016

to achieve the behaviour suggested above (respect timezones if tz-aware datetime or DateTime values are passed to the setter method) we could change the setter

  • do not use plone.app.event.base.DT since it adds timezone info to tz-naive datetime objects
  • if DateTime objects are not timezoneNaive, use timezone information and localize them to UTC
    security.declarePrivate('_dt_setter')
    def _dt_setter(self, fieldtoset, value, **kwargs):
        """Always set the date in UTC, saving the timezone in another field.
        But since the timezone value isn't known at the time of saving the
        form, we have to save it timezone-naive first and let
        timezone_handler convert it to the target zone afterwards.
        """
        if not isinstance(value, DateTime):
            # value = DT(value)
            # DT adds timezone info to naive datetime objects
            # https://github.com/plone/plone.app.event/issues/254
            value = DateTime(value)

        if value.timezoneNaive():
            # This way, we set DateTime timezoneNaive
            value = DateTime(
                '%04d-%02d-%02dT%02d:%02d:%02d' % (
                    value.year(),
                    value.month(),
                    value.day(),
                    value.hour(),
                    value.minute(),
                    int(value.second())  # No microseconds
                )
            )
        else:
            value = value.toZone('UTC')
        self.getField(fieldtoset).set(self, value, **kwargs)

@thet what do you think about this suggestion?

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

No branches or pull requests

1 participant