Skip to content

Provide 'event_view' view on occurences for better consistency #123

Merged
merged 4 commits into from Feb 12, 2014

2 participants

@tdesvenain
Plone Foundation member

By example it fixes a bug on Solgema.fullcalendar, wich needs to display 'event_view' on all items

@tdesvenain
Plone Foundation member

Hi, can you review this PLIP ?

@thet thet commented on an outdated diff Jan 4, 2014
@@ -43,7 +43,7 @@
'Products.statusmessages',
'Zope2',
'collective.elephantvocabulary',
- 'icalendar',
+ 'icalendar>=3.0',
@thet
Plone Foundation member
thet added a note Jan 4, 2014

did you run into an issue by not declaring this minimum version of icalendar?
i'm not sure, if 3.0 would still work with plone.app.event. also, there is no downloadable 3.0 version on pypi (see: https://pypi.python.org/simple/icalendar/ )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thet thet commented on an outdated diff Jan 4, 2014
plone/app/event/browser/configure.zcml
@@ -65,6 +65,16 @@
permission="zope2.View"
layer="..interfaces.IBrowserLayer"
/>
+
+ <browser:page
+ for="plone.event.interfaces.IOccurrence"
+ name="event_view"
+ class=".event_view.EventView"
+ template="event_view.pt"
+ permission="zope2.View"
+ layer="..interfaces.IBrowserLayer"
+ />
+
@thet
Plone Foundation member
thet added a note Jan 4, 2014

looks good! only, i'd remove the index_html view declaration above and modify the defaultview declaration below to use the event_view name.
then, if all tests run through and the event_view is still used as default when visiting an occurrence, this would be ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thet
Plone Foundation member
thet commented Jan 4, 2014

thanks for the pull-request! see my comments on the diff.

@tdesvenain
Plone Foundation member

Hi, I updated the pull request
Thanks

@thet thet added a commit that referenced this pull request Feb 12, 2014
@thet thet changelog note for #123 225878a
@thet thet merged commit c5d7fff into plone:master Feb 12, 2014
@tdesvenain
Plone Foundation member

Hi, will this be merged into 1.x branch ?

@thet
Plone Foundation member
thet commented Feb 12, 2014

actually that change breaks some code in other projects. i have two bigger deployments, where we overload the index_html view. that was the reason not merging it into 1.x.

i could easily change the code for my projects, but others may be surprised.

what about using the master branch or just providing these config statements in your integration project?

@tdesvenain
Plone Foundation member
@thet
Plone Foundation member
thet commented Feb 12, 2014

what i meant is, changing the 'index_html' to 'event_view' name isn't absolutly backwards compatible, therefore i'm more comfortable with having this change in the current master branch, which will make the next 1.1 release.
is this ok for you?

@tdesvenain
Plone Foundation member
@jaroel jaroel added a commit that referenced this pull request Feb 12, 2014
@jaroel jaroel Merge branch 'master' into plip13476-mockup
* master: (41 commits)
  update test to expect correct default value (introduced in my last commit 483f91d)
  Provide tuple as default value for Tuple field
  Replace RecurrenceField with plain Text field in the dx recurrence behavior. This reverts the change from 1.0rc2. We don't use form schema hints but an adapter to configure the widget. Closes #137, Fixes #131.
  Use attribute storage instead of annotation storage in all Dexterity behaviors. Closes #136, #95, Refs #20.
  update Changes for #131
  Refs #131 actually we can simply replace the field that stores the recurrence. No upgrade step necessary, because existing data is kept
  also move the EventSummary (text) from Annotation to Attribute storage
  changelog note for #123
  we can simplify the indexer for SearchableText. We don't need to apply the IEventSummary behavior, since the data (text) is already provided by the EventAccessor
  In the upgrade step, if we modified content, we should actually throw the ObjectModified event
  Remove the 3 deprecated interfaces for the behaviors. They were only used in tests
  nicer descriptions in the tests
  add a test that checks that already existing values don't get overwritten by the storage migration
  Added a test for the annotation to attribute storage migration
  added upgrade step to migrate the annotation storage from behavior fields
  make sure to actually include the upgrades for dx
  more version fixes
  back to development
  prepare release
  prepare release
  ...

Conflicts:
	plone/app/event/dx/behaviors.py
dc88b3f
@thet
Plone Foundation member
thet commented Feb 17, 2014

yes, the 1.1 release is Plone 4.2/4.3 compatible. just follow the instructions in https://github.com/plone/plone.app.event/blob/master/docs/installation.rst and note what has changed in https://github.com/plone/plone.app.event/blob/master/CHANGES.rst

for the 1.x branch, you could re-add the event_view without removing index_html as you said.

for the reference, this would be an alternative for an integration addon:
<configure package="plone.app.event.browser"><!-- all relative paths are relative to p.a.event.browser -->
<browser:page
for="plone.event.interfaces.IOccurrence"
name="event_view"
class=".event_view.EventView"
template="event_view.pt"
permission="zope2.View"
layer="..interfaces.IBrowserLayer"
/>
</configure>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.