ICategorization(context).subjects vs. context.subject #118
Labels
Comments
|
@davisagli @tisto, is there a solution to this API inconsitency? |
|
It's stored on |
thet
added a commit
to plone/buildout.coredev
that referenced
this issue
Jan 29, 2015
Branch: refs/heads/master Date: 2015-01-29T15:28:42+01:00 Author: Johannes Raggam (thet) <raggam-nl@adm.at> Commit: plone/plone.app.event@c12bec5 Remove data_postprocessing Remove ``data_postprocessing`` logic, which was handling ``open_end`` and ``whole_day`` events and was manipulating the object on form submission. Instead, just adapt start/end dates on indexing and when accessing them via ``IEventAccessor``. Files changed: M CHANGES.rst M docs/development.rst M plone/app/event/dx/behaviors.py M plone/app/event/dx/configure.zcml M plone/app/event/tests/base_setup.py M plone/app/event/tests/test_base_module.py M plone/app/event/tests/test_dx_behaviors.py diff --git a/CHANGES.rst b/CHANGES.rst index 761e5de..544e68d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,12 @@ Changelog 2.0.0 (unreleased) ------------------ +- Remove ``data_postprocessing`` logic, which was handling ``open_end`` and + ``whole_day`` events and was manipulating the object on form submission. + Instead, just adapt start/end dates on indexing and when accessing them via + ``IEventAccessor``. + [thet] + - No need to return DateTime objects for the indexer. Products.DateRecurringIndex works with Python datetime objects. [thet] diff --git a/docs/development.rst b/docs/development.rst index e3607d1..03d1f94 100644 --- a/docs/development.rst +++ b/docs/development.rst @@ -136,35 +136,11 @@ tzinfo object directly on the datetime object like `datetime(2010, 10, 10, 12, your timezone! -2) If you set `whole_day` or `open_end`, wether set the start and end times -accordingly or use the provided `data_postprocessing` or -`data_postprocessing_context` conveininence functions. - -Setting a `whole_day` event:: - - event.start = tz.localize(datetime(2010, 10, 10, 0, 0, 0)) - event.end = tz.localize(datetime(2010, 10, 10, 23, 59, 59)) - event.whole_day = True - -Setting a `open_end` event:: - - event.start = tz.localize(datetime(2010, 10, 10, 12, 12)) - event.end = tz.localize(datetime(2010, 10, 10, 23, 59, 59)) - event.open_day = True - -Using `data_postprocessing`:: - - event.start, event.end, event.whole_day, event.open_end =\ - data_postprocessing( - event.start, event.end, event.whole_day, event.open_end) - -Using `data_postprocessing_context`:: - - data_postprocessing_context(context) - - -If you set the values through a z3c.form, an event handler listening to -`z3c.form.events.DataExtractedEvent` will take care of this. +2) Since plone.app.event 2.0b1, there is no need to call the +``data_postprocessing`` function to manipulate the object accordingly to the +value of the ``whole_day`` or ``end_date`` attributes. The start and end dates +are only converted to the beginning respectively to the end of the day for +indexing and when accessing the dates via the IEventAccessor. Accessing event objects via an unified accessor object diff --git a/plone/app/event/dx/behaviors.py b/plone/app/event/dx/behaviors.py index 8a4b047..5e3cc09 100644 --- a/plone/app/event/dx/behaviors.py +++ b/plone/app/event/dx/behaviors.py @@ -285,89 +285,19 @@ class IEventContact(model.Schema): alsoProvides(IEventContact, IFormFieldProvider) -def data_postprocessing(start, end, whole_day, open_end): - """Adjust start and end according to whole_day and open_end setting. - """ - - def _fix_dt(dt, tz): - """Fix datetime: Apply missing timezones, remove microseconds. - """ - if dt.tzinfo is None: - dt = tz.localize(dt) - - return dt.replace(microsecond=0) - - tz_default = default_timezone(as_tzinfo=True) - - tz_start = getattr(start, 'tzinfo', None) or tz_default - tz_end = getattr(end, 'tzinfo', None) or tz_default - start = _fix_dt(start, tz_start) - end = _fix_dt(end, tz_end) - - # Adapt for whole day - if whole_day: - start = dt_start_of_day(start) - if open_end: - end = start # Open end events end on same day - if open_end or whole_day: - end = dt_end_of_day(end) - - # TODO: - """ - if not obj.sync_uid: - # sync_uid has to be set for icalendar data exchange. - uid = IUUID(obj) - # We don't want to fail when getRequest() returns None, e.g when - # creating an event during test layer setup time. - request = getRequest() or {} - domain = request.get('HTTP_HOST') - obj.sync_uid = '%s%s' % ( - uid, - domain and '@%s' % domain or '' - ) - """ - - return start, end, whole_day, open_end - - -def data_postprocessing_handler(event): - """Event handler called after extractData step of z3c.form to adjust form - data. - """ - data = event.data - - if not 'IEventBasic.start' in data: - # is not a IEventBasic form - return - if data.get('processed', False): - # data was already manipulated - return - - # TODO: e.g. on open_end events, there is no IEventBasic.end data in the - # data. In that case, we have to add it. - start = data['IEventBasic.start'] - end = data.get('IEventBasic.end') or start # end can be missing - whole_day = data['IEventBasic.whole_day'] - open_end = data['IEventBasic.open_end'] - - start, end, whole_day, open_end = data_postprocessing( - start, end, whole_day, open_end) - - data['IEventBasic.start'] = start - if data.get('IEventBasic.end'): # end can be missing - data['IEventBasic.end'] = end - data['IEventBasic.whole_day'] = whole_day - data['IEventBasic.open_end'] = open_end - - data['processed'] = True - - -def data_postprocessing_context(context): - """Convenience method to adjust data on the context. - """ - context.start, context.end, context.whole_day, context.open_end =\ - data_postprocessing( - context.start, context.end, context.whole_day, context.open_end) +""" +if not obj.sync_uid: + # sync_uid has to be set for icalendar data exchange. + uid = IUUID(obj) + # We don't want to fail when getRequest() returns None, e.g when + # creating an event during test layer setup time. + request = getRequest() or {} + domain = request.get('HTTP_HOST') + obj.sync_uid = '%s%s' % ( + uid, + domain and '@%s' % domain or '' + ) +""" ## Attribute indexer @@ -375,13 +305,23 @@ def data_postprocessing_context(context): # Start indexer @indexer(IDXEvent) def start_indexer(obj): - return IEventBasic(obj).start + acc = IEventBasic(obj) + start = acc.start + if acc.whole_day: + start = dt_start_of_day(start) + return start # End indexer @indexer(IDXEvent) def end_indexer(obj): - return IEventBasic(obj).end + acc = IEventBasic(obj) + end = acc.end + if acc.open_end: + end = acc.start # Open end events end on same day + if acc.open_end or acc.whole_day: + end = dt_end_of_day(end) + return end # Location indexer @@ -496,16 +436,40 @@ def duration(self): return self.end - self.start @Property + def start(self): + start = IEventBasic(self.context).start + if self.whole_day: + start = dt_start_of_day(start) + return start + + @start.setter + def start(self, value): + return setattr(self, 'start', value) + + @Property + def end(self): + end = IEventBasic(self.context).end + if self.open_end: + end = IEventBasic(self.context).start + if self.open_end or self.whole_day: + end = dt_end_of_day(end) + return end + + @end.setter + def end(self, value): + return setattr(self, 'end', value) + + @Property def timezone(self): """Returns the timezone name for the event. If the start timezone differs from the end timezone, it returns a tuple with (START_TIMEZONENAME, END_TIMEZONENAME). """ tz_start = tz_end = None - tz = getattr(self.start, 'tzinfo', None) + tz = getattr(IEventBasic(self.context).start, 'tzinfo', None) if tz: tz_start = tz.zone - tz = getattr(self.end, 'tzinfo', None) + tz = getattr(IEventBasic(self.context).end, 'tzinfo', None) if tz: tz_end = tz.zone return tz_start if tz_start == tz_end else (tz_start, tz_end) diff --git a/plone/app/event/dx/configure.zcml b/plone/app/event/dx/configure.zcml index 539824d..33e1f6e 100644 --- a/plone/app/event/dx/configure.zcml +++ b/plone/app/event/dx/configure.zcml @@ -31,9 +31,6 @@ <adapter name="SearchableText" factory=".behaviors.searchable_text_indexer" /> <adapter name="sync_uid" factory=".behaviors.sync_uid_indexer" /> - <subscriber for="z3c.form.interfaces.IDataExtractedEvent" - handler=".behaviors.data_postprocessing_handler" /> - <plone:behavior title="Event Basic" description="Basic Event schema." diff --git a/plone/app/event/tests/base_setup.py b/plone/app/event/tests/base_setup.py index 6ef4135..255b272 100644 --- a/plone/app/event/tests/base_setup.py +++ b/plone/app/event/tests/base_setup.py @@ -2,7 +2,6 @@ from datetime import datetime from datetime import timedelta from plone.app.event.dx import behaviors -from plone.app.event.dx.behaviors import data_postprocessing_context from plone.app.event.testing import set_browserlayer from plone.app.event.testing import set_timezone from plone.app.testing import TEST_USER_ID @@ -76,7 +75,6 @@ def setUp(self): recurrence='RRULE:FREQ=DAILY;COUNT=3') workflow.doActionFor(self.past_event, 'publish') # adjust start and end according to whole_day and open_end - data_postprocessing_context(self.past_event) self.past_event.reindexObject() self.now_event = factory( @@ -98,7 +96,6 @@ def setUp(self): # plone/plone.dexterity#18 # plone/plone.app.dexterity#118 workflow.doActionFor(self.now_event, 'publish') - data_postprocessing_context(self.now_event) self.now_event.reindexObject() self.future_event = factory( @@ -109,7 +106,6 @@ def setUp(self): end=future + duration, location=u'Graz') workflow.doActionFor(self.future_event, 'publish') - data_postprocessing_context(self.future_event) self.future_event.reindexObject() self.portal.invokeFactory('Folder', 'sub', title=u'sub') @@ -121,7 +117,6 @@ def setUp(self): end=far, location=u'Schaftal') workflow.doActionFor(self.long_event, 'publish') - data_postprocessing_context(self.long_event) self.long_event.reindexObject() # For AT based tests, this is a plone.app.collection ICollection type diff --git a/plone/app/event/tests/test_base_module.py b/plone/app/event/tests/test_base_module.py index 3fe2d86..81e88c3 100644 --- a/plone/app/event/tests/test_base_module.py +++ b/plone/app/event/tests/test_base_module.py @@ -19,7 +19,6 @@ from plone.app.event.base import find_site from plone.app.event.base import get_events from plone.app.event.base import localized_now -from plone.app.event.dx.behaviors import data_postprocessing_context from plone.app.event.testing import PAEventDX_INTEGRATION_TESTING from plone.app.event.testing import PAEvent_INTEGRATION_TESTING from plone.app.event.testing import set_env_timezone @@ -556,8 +555,6 @@ def test_get_event_limit(self): location=u"Dornbirn", recurrence='RRULE:FREQ=WEEKLY;COUNT=4', ) - # data_postprocessing normalization is not needed, as we values are set - # correctly in the first place. tomorrow = factory( container=self.portal, @@ -568,9 +565,6 @@ def test_get_event_limit(self): open_end=True, location=u"Dornbirn", ) - # Normalize values and reindex, what normally the form would do - # (especially, end time isn't set like open_end settings requests to. - data_postprocessing_context(tomorrow) tomorrow.reindexObject() limit = get_events(self.portal, start=self.now, expand=True, @@ -655,8 +649,6 @@ def setUp(self): location=u"Dornbirn", recurrence='RRULE:FREQ=WEEKLY;COUNT=4', ) - # data_postprocessing normalization is not needed, as we values are set - # correctly in the first place. tomorrow = factory( container=self.portal, @@ -667,9 +659,6 @@ def setUp(self): open_end=True, location=u"Dornbirn", ) - # Normalize values and reindex, what normally the form would do - # (especially, end time isn't set like open_end settings requests to. - data_postprocessing_context(tomorrow) tomorrow.reindexObject() self.occ = [ diff --git a/plone/app/event/tests/test_dx_behaviors.py b/plone/app/event/tests/test_dx_behaviors.py index 899c6d8..4adb2c7 100644 --- a/plone/app/event/tests/test_dx_behaviors.py +++ b/plone/app/event/tests/test_dx_behaviors.py @@ -1,5 +1,4 @@ # -*- coding: utf-8 -*- -from DateTime import DateTime from OFS.SimpleItem import SimpleItem from datetime import datetime, timedelta from plone.app.event import base @@ -8,7 +7,6 @@ from plone.app.event.dx.behaviors import IEventBasic from plone.app.event.dx.behaviors import IEventRecurrence from plone.app.event.dx.behaviors import StartBeforeEnd -from plone.app.event.dx.behaviors import data_postprocessing_context from plone.app.event.dx.behaviors import default_end from plone.app.event.dx.behaviors import default_start from plone.app.event.dx.interfaces import IDXEvent @@ -221,7 +219,7 @@ def test_edit_context(self): self.assertTrue('23:59' in self.browser.contents) -class TestDataPostprocessing(unittest.TestCase): +class TestEventAccessor(unittest.TestCase): layer = PAEventDX_INTEGRATION_TESTING def setUp(self): @@ -230,7 +228,28 @@ def setUp(self): set_browserlayer(self.request) setRoles(self.portal, TEST_USER_ID, ['Manager']) - def test_data_postprocessing(self): + def test_event_accessor(self): + tz = pytz.timezone("Europe/Vienna") + e1 = createContentInContainer( + self.portal, + 'plone.app.event.dx.event', + title='event1', + start=tz.localize(datetime(2011, 11, 11, 11, 0)), + end=tz.localize(datetime(2011, 11, 11, 12, 0)), + ) + + # setting attributes via the accessor + acc = IEventAccessor(e1) + new_end = tz.localize(datetime(2011, 11, 13, 10, 0)) + acc.end = new_end + + # context's end should be set to new_end + self.assertEqual(e1.end, new_end) + + # accessor's and context datetime should be the same + self.assertEqual(acc.end, e1.end) + + def test_event_accessor_whole_day__open_end(self): at = pytz.timezone("Europe/Vienna") @@ -247,25 +266,25 @@ def test_data_postprocessing(self): start=start, end=end ) + acc = IEventAccessor(e1) - # See, if start isn't moved by timezone offset. Addressing issue #62 - self.assertEqual(e1.start, start) - self.assertEqual(e1.end, end) - data_postprocessing_context(e1) + # check set self.assertEqual(e1.start, start) self.assertEqual(e1.end, end) # Setting open end e1.open_end = True - data_postprocessing_context(e1) self.assertEqual(e1.start, start) - self.assertEqual(e1.end, end_end) + self.assertEqual(e1.end, end) + self.assertEqual(acc.start, start) + self.assertEqual(acc.end, end_end) # Setting whole day e1.whole_day = True - data_postprocessing_context(e1) - self.assertEqual(e1.start, start_start) - self.assertEqual(e1.end, end_end) + self.assertEqual(e1.start, start) + self.assertEqual(e1.end, end) + self.assertEqual(acc.start, start_start) + self.assertEqual(acc.end, end_end) class TestDXIntegration(unittest.TestCase): @@ -344,27 +363,6 @@ def test_recurrence_indexing(self): ) self.assertEqual(len(result), 4) - def test_event_accessor(self): - tz = pytz.timezone("Europe/Vienna") - e1 = createContentInContainer( - self.portal, - 'plone.app.event.dx.event', - title='event1', - start=tz.localize(datetime(2011, 11, 11, 11, 0)), - end=tz.localize(datetime(2011, 11, 11, 12, 0)), - ) - - # setting attributes via the accessor - acc = IEventAccessor(e1) - new_end = tz.localize(datetime(2011, 11, 13, 10, 0)) - acc.end = new_end - - # context's end should be set to new_end - self.assertEqual(e1.end, new_end) - - # accessor's and context datetime should be the same - self.assertEqual(acc.end, e1.end) - class TestDXEventRecurrence(unittest.TestCase):
|
I think the goal should be to deprecate "subjects" in favour of "subject" in ICategorization. Simply because "subjects" is not DC metadata and thus our Dexterity DC behavior implementation can be considered broken. Capitalized or not. |
|
Stale issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
the ICategorization behavior has a
subjects(plural) field, which is stored on thesubject(singular) attribute on the context. This is quite confusing and made me scratch my head for quite some time.If I recall correctly, this is done because the Dublin Core defines a
Subject(plural, capitalization) field.I fear, we need this confusing situation to not break the catalog metadata, CMFCore and whatever API. But isn't there a better way - like providing a
Subjectindexer but storing the attribute on context.subjects?The text was updated successfully, but these errors were encountered: