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

Move filter control panel to z3c.form. #211

Closed
tisto opened this Issue Jun 20, 2014 · 21 comments

Comments

Projects
None yet
4 participants
@tisto
Member

tisto commented Jun 20, 2014

No description provided.

@davisagli davisagli referenced this issue Jun 20, 2014

Closed

Plone 5 #184

76 of 90 tasks complete

@tisto tisto self-assigned this Dec 4, 2014

@tisto

This comment has been minimized.

Member

tisto commented Dec 6, 2014

I started to work on this.

@tisto

This comment has been minimized.

Member

tisto commented Dec 6, 2014

plone.app.controlpanel pull request: plone/plone.app.controlpanel#33

@tisto

This comment has been minimized.

Member

tisto commented Dec 6, 2014

Problem: There is currently no way of changing the filter settings with generic setup (in Plone < 5.0). We can either use plone.autoform (without p.a.registry) and set the filter settings directly on portal_transform (this is how it currently works with formlib), or we store the settings in p.a.registry and make sure that the settings in p.a.registry and in portal_transforms are in sync (this way we would be able to use generic setup). I'm not too familiar with portal_transforms, so I'm not sure where we are heading or how those settings are currently stored internally. If anybody can shed some light into this, please do!

@tisto

This comment has been minimized.

Member

tisto commented Dec 6, 2014

Products.CMFPlone pull request: #325

@thet

This comment has been minimized.

Member

thet commented Jan 21, 2015

maybe this is helpful: i have some code, where portal_transforms settings are get/set:
https://github.com/collective/collective.setuphandlertools/blob/master/collective/setuphandlertools/setuphandlertools.py#L258
https://github.com/collective/collective.setuphandlertools/blob/master/collective/setuphandlertools/setuphandlertools.py#L283

IMO Products.PortalTransforms should be changed to use plone.app.registry instead of it's own obscure storage.

@tisto

This comment has been minimized.

Member

tisto commented Jan 22, 2015

I think for Plone 5 we should re-write the filter control panel with plone.autoform. That should be pretty straight forward. Here is an example of the editing control panel that I wrote with plone.autoform in the z3cform branch:

plone/plone.app.controlpanel@91d0c2f

The filter control panel should also be moved to Products.CMFPlone.

@MrTango @khink Is that something that you guys would want to work on?

@tisto tisto assigned khink and unassigned tisto Jan 23, 2015

@khink

This comment has been minimized.

Member

khink commented Jan 23, 2015

Starting work on this now.

@khink

This comment has been minimized.

Member

khink commented Jan 23, 2015

There's plip10359-filter-controlpanel branches of CMFPlone and controlpanel.

@khink

This comment has been minimized.

Member

khink commented Jan 24, 2015

The stripped_combinations give me an "AttributeError: adapter", commented that out

@khink

This comment has been minimized.

Member

khink commented Jan 25, 2015

Going with thet's suggestion and making Products.Portaltransforms use p.a.registry storage.

@khink

This comment has been minimized.

Member

khink commented Jan 25, 2015

Current status:

  • Form works and stores in registry, except stripped_combinations which gives AttributeError: adapter (so i commented that out)
  • Form edit also stores disable_filtering and nasty_tags on the portal_transforms tool
  • But we're going to replace that storage with Products.PortalTransforms reading its settings from the registry
  • For this, we need to include p.a.registry in the PortalTransforms tests somehow. Currently this gives me ConfigurationError: ('Unknown directive', u'http://namespaces.zope.org/zope', u'includeDependencies')

Branches:
(There's a plip10359-filter-controlpanel branch of coredev.buildout that sets these)
https://github.com/plone/Products.PortalTransforms/tree/plip10359-filter-controlpanel
https://github.com/plone/Products.CMFPlone/tree/plip10359-filter-controlpanel
https://github.com/plone/plone.app.controlpanel/tree/plip10359-filter-controlpanel

@khink

This comment has been minimized.

Member

khink commented Jan 25, 2015

The last bullet from the list above is solved, all tests in the PortalTransforms branch pass. Now we can start querying the registry.

@tisto

This comment has been minimized.

Member

tisto commented Jan 25, 2015

I'm a bit confused. The plone.app.registry-based filter control panel already was ready for merge:

https://github.com/plone/Products.CMFPlone/tree/plip10359-filter-controlpanel-timo

How does my branch differ from the one you created? The main problem is making P.PortalTransform read those settings. I did a quick PortalTransforms code review and my feeling was that it beyond the scope of the initial PLIP to rewrite it as well.

@thet's code just gets/sets the (old) portal transforms settings, so I don't see how that would help us. If we would use that code we would end up with storing the settings in two locations. This is why I recommended to go with plone.autoform and do the plone.app.registry setting at a later point.

@khink

This comment has been minimized.

Member

khink commented Jan 25, 2015

Branches are quite the same indeed. I wanted to replay it to understand what's going on.

PortalTransforms changes will help filter settings take effect. It was either that, or write to the tool's obscure storage on each change (and have no GS update). The current approach is a more complete solution.

If you look at the code, you'll see i didn't do much with thet's code examples, so i agree with you there as well.

@tisto

This comment has been minimized.

Member

tisto commented Mar 4, 2015

@tisto

This comment has been minimized.

@tisto

This comment has been minimized.

Member

tisto commented Mar 21, 2015

@tisto

This comment has been minimized.

Member

tisto commented Mar 21, 2015

@tisto

This comment has been minimized.

Member

tisto commented Mar 21, 2015

Merged. All Jenkins build green.

@tisto tisto closed this Mar 21, 2015

@thet

This comment has been minimized.

Member

thet commented Mar 21, 2015

@khink @tisto just want to know: the settings are still stored in portal_transforms tool and not in the registry, or not?

@tisto

This comment has been minimized.

Member

tisto commented Mar 21, 2015

Still stored in portal_transforms.

@tisto tisto removed the 99 tag: sprint label Jul 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment