datetime converter breaks dexterity content types with datetime fields

[frisi]

the converter introduced in this commit: e8a169b
breaks dexterity content with datetime fields: (ie from the plone.app.dexterity.behaviors.metadata.IPublication behaviour)

z3c.form checks if the widget value has been changed, and the converter installed here returns a timezone-aware datetime object and the datamanager returns the attribute stored for this behaviour without a timezone.

TypeError: can't compare offset-naive and offset-aware datetimes
> /home/frisi/.buildout/eggs/z3c.form-3.2.9-py2.7.egg/z3c/form/util.py(180)changedField()
    179 # Or it is an Object, in case we'll never know
    181 if (not dm.canAccess() or dm.query() != value):

ipdb> dm.query() # attribute 
datetime.datetime(2016, 10, 7, 17, 31, 25)
ipdb> value  # obtained by using the converter: interfaces.IDataConverter(widget).toFieldValue(raw)
datetime.datetime(2016, 10, 7, 17, 31, tzinfo=tzfile('/etc/localtime'))

[frisi]

@vladimir-iliev @vipod
using this converter for all datetimewidgets seems to break other use-cases

what problem did you solve by adding this converter?
is there another way to add timezone info to the notifications (ie. the getter-method or a customer indexer)?
another option might be to register it only for the widgets used in this package

[vipod]

@frisi I think @vladimir-iliev got an answer here.

But I'm not sure if he is still active here. So if we do not get reply in couple of days - feel free to make you decision.

[frisi]

adding timezone information makes sense. tests pass without the overrides.zcml (it was not even loaded for the test environment) but saving notices lead to an error (catalog only allows dates with timezone info)
i decided to remove the converter (that adds tzinfos to the data passed to the factory by the z3c.form) and add timezone information to the object properties if they get datetime objects w/o timezone info.

@vipod would be great if you can review the PR so i can make a release (if that's ok with you)

[vipod]

@frisi It looks like we changed datetime field value types we store in database. Don't we need also to add some kind of migration profile for this change? Would existing collective.notices installations still work after upgrade?

[frisi]

hey @vipod - thanks for you feedback!

actually there is no migration needed.
before this pull request, the custom converter registered for datetimewidgets added a timezone value to the date so it was stored with timezone.
now there is no converter and we make sure a timezone is added on class level.

old entries already have timezone info, new entries will have them automatically too.

after adding my changes there is no need to manually add the timezone for test-content (see changes in the test.py)

from my point of view existing installations will still work fine.

[vipod]

@frisi ok, thank you for explanation!

[frisi]

you're welcome.
will you make a new release @vipod ?

[vipod]

Can you do this? I guess you got all the necessary permissions here and at pypy?

[frisi]

i guess so, too! will do!

[frisi]

done https://pypi.python.org/pypi/collective.notices/1.2